[webkit-reviews] review granted: [Bug 217563] [MotionMark] Computing the fast bounding rect of an arc should not materialize a CGPathRef : [Attachment 411028] Fix ChangeLog typo

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Oct 10 21:48:51 PDT 2020


Darin Adler <darin at apple.com> has granted Wenson Hsieh
<wenson_hsieh at apple.com>'s request for review:
Bug 217563: [MotionMark] Computing the fast bounding rect of an arc should not
materialize a CGPathRef
https://bugs.webkit.org/show_bug.cgi?id=217563

Attachment 411028: Fix ChangeLog typo

https://bugs.webkit.org/attachment.cgi?id=411028&action=review




--- Comment #6 from Darin Adler <darin at apple.com> ---
Comment on attachment 411028
  --> https://bugs.webkit.org/attachment.cgi?id=411028
Fix ChangeLog typo

View in context: https://bugs.webkit.org/attachment.cgi?id=411028&action=review

>>> Source/WebCore/platform/graphics/Path.cpp:406
>>> +		 approximateBounds.extend(arc.offset);
>> 
>> This doesn’t look right to me. Why is FloatRect::extend the appropriate
algorithm here? Because extend takes the offset as a point and expands the
rectangle to encompass that point. That does not seem like what an arc’s offset
does. On the other hand, I guess I don’t really know what an arc’s offset does,
so maybe this is correct!
> 
> Ah, so (IIRC) the `offset` member — if hasOffset is true — allows the ArcData
to contain a starting point that will be connected to the arc itself. I suppose
the name ArcData is misleading here — it’s more like “Arc (with an optional
line segment prepended to it)”.

We should rename "offset" then, because ... it’s not an offset from anything!


More information about the webkit-reviews mailing list