Bug 105997 – Add 'float FloatPoint::slopeAngleRadians()'

• Component: New Bugs
• Status: RESOLVED FIXED

[Steve Block]

Add 'float FloatPoint::theta()'

[Steve Block]

Created attachment 181149 [details]
Patch

[Steve Block]

There are several places where we calculate the angle between a FloatPoint/Size and the x/horizontal axis, so it seems like a helper method would be useful.

FloatPoint seems the more natural location, and it goes well with the existing 'length()' method. Open to suggestions for naming.

[WebKit Review Bot]

Comment on attachment 181149 [details]
Patch

Attachment 181149 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15628806

New failing tests:
fast/frames/removal-before-attach-crash.html

[Stephen Chenney]

Comment on attachment 181149 [details]
Patch

I fine with the change, but not real happy about the name. I see how you came to choose it (radial coordinates) but the cognitive load to figure out what it means seems high. The SVG spec refers to this as the "slope" of a curve, so I think I would prefer "slopeAngleRadians".

[Steve Block]

Created attachment 181266 [details]
Patch

[Steve Block]

Renamed to slopeAngleRadians

[Stephen White]

Comment on attachment 181266 [details]
Patch

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

OK.  r=me

> Source/WebCore/rendering/svg/SVGMarkerData.h:88
> +        double inslope = rad2deg(inslopeChange.slopeAngleRadians());

There is a slight change of precision here (calling atan2f()/rad2deg() on the floats directly instead of promoting the floats to doubles, and calling atan2()/rad2deg() (double flavour), then downcasting to float).  Luckily, it doesn't seem to have an impact on pixel results.

[WebKit Review Bot]

Comment on attachment 181266 [details]
Patch

Clearing flags on attachment: 181266

Committed r138800: <http://trac.webkit.org/changeset/138800>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Dirk Schulze]

I don't understand why it needs to go into FloatPoint for such a rare usage? (just two file IIRC).