Bug 240259

Summary: Implement contain flag for ray() in offset path
Product: WebKit Reporter: Nikos Mouchtaris <nmouchtaris>
Component: New BugsAssignee: Nikos Mouchtaris <nmouchtaris>
Status: RESOLVED FIXED    
Severity: Normal CC: changseok, esprehn+autocc, ews-watchlist, glenn, kondapallykalyan, pdr, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 233344    
Bug Blocks: 203847    
Attachments:
Description Flags
Patch simon.fraser: review+, ews-feeder: commit-queue-

Description Nikos Mouchtaris 2022-05-09 14:48:55 PDT
Support contain flag for ray() in offset path
Comment 1 Nikos Mouchtaris 2022-05-09 18:22:55 PDT
Contain implemented here: https://github.com/ewilligers/petrogale-purpureicollis/blob/20b7403d85d664eb943232e2a34bc95b6f4a8b62/ray.py. Need to understand if it is ok to adapt this code.
Comment 2 Nikos Mouchtaris 2022-05-12 18:11:43 PDT
Created attachment 459267 [details]
Patch
Comment 3 Radar WebKit Bug Importer 2022-05-16 14:49:13 PDT
<rdar://problem/93374029>
Comment 4 Simon Fraser (smfr) 2022-05-17 16:22:50 PDT
Comment on attachment 459267 [details]
Patch

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

> Source/WebCore/ChangeLog:23
> +        Implement contain flag for ray(). Contains purpose is to have the entire box
> +        being animated be contained within the path. "Contained within the path" is defined
> +        as having the box remain within a circle with a radius of the path length and positioned at the offset
> +        anchor. The way this problem is tackled is based on the approach here: 
> +        https://github.com/ewilligers/petrogale-purpureicollis/blob/20b7403d85d664eb943232e2a34bc95b6f4a8b62/ray.py.
> +        The way this solution works is that you construct a coordinate system with the origin being the offset anchor.
> +        You then calculate the position of each vertex of the box in this coordinate system. Next, rotate the vertices based
> +        on the angle's difference from the x axis. We rotate based on the x-axis as we will translate the box along the 
> +        x axis (rather than in the direction of the angle of the ray) as this simplifies the calculation. Next, using the
> +        circle equation (x^2 + y^2 = r^2), we want to find an offset such that (x + offset)^2 + y^2 <= r^2, as this will
> +        result in all points being contained in the circle. Solving for this equation, we get the upper and lower bounds 
> +        of such an offset for each vertex: -x - sqrt(r^2 - y^2) <= offset <= -x + sqrt(r^2 - y^2). Finally, we find the 
> +        largest lower bound and smallest upper bound, and choose the larger of the two values to get the clamped offset.
> +        This currently doesn't take into account if it is not possible to fit the box within the path, as this will 
> +        be completed in a separate patch. Currently test 4 is failing due to rounding error (might want to look into fixing/
> +        deleting this test), and test 5 is failing due to the part not implemented yet.

This needs breaking into paragraphs.

> Source/WebCore/platform/graphics/GeometryUtilities.h:96
> +Vector<FloatPoint> verticesForBox(const FloatRect&, const FloatPoint);

The return value should be a FloatQuad (per maybe a std::array<FloatPoint, 4>. Vector<> implies variable size, and it never is.

> Source/WebCore/rendering/PathOperation.cpp:87
> +    Vector<std::pair<double, double>> bounds;

Vector<..., 4> for inline capacity. Or use a std::array<> and track the early exit some other way.

> Source/WebCore/rendering/PathOperation.cpp:125
> +    double length = lengthForPath();

lengthForPath returns a float
Comment 5 Nikos Mouchtaris 2022-05-19 15:31:57 PDT
Pull request: https://github.com/WebKit/WebKit/pull/806
Comment 6 EWS 2022-05-19 18:23:22 PDT
Committed r294520 (250776@main): <https://commits.webkit.org/250776@main>

Reviewed commits have been landed. Closing PR #806 and removing active labels.