Closed Bug 1675375 Opened 4 years ago Closed 3 years ago

Support sending polygonal clip-paths to WebRender and hit-testing them accurately

Categories

(Core :: Graphics: WebRender, enhancement)

enhancement

Tracking

()

RESOLVED FIXED
89 Branch
Fission Milestone M7a
Tracking Status
firefox89 --- fixed

People

(Reporter: kats, Assigned: bradwerth)

References

(Blocks 2 open bugs)

Details

Attachments

(7 files, 3 obsolete files)

48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
Attached patch clippath+fisison+wr testcase (obsolete) — Splinter Review

Currently if a page has a polygonal clip-path, we return false from SVGIntegrationUtils::UsingSimpleClipPathForFrame and bail out of creating a complex clip region for WR. Instead, we fall back to creating a mask image.

Then, when we try to do a compositor hit-test on that element, it uses the bounding rect of the mask and can therefore result in incorrect hit results.

With fission this means that the target layersId for a hit-test can be wrong, and the input events can end up getting sent to the wrong process. This can be seen using the test page here - load it in a fission-enabled browser, and mouse over the iframe at the top. Any time you're over the iframe, but outside of the green triangle, it should say the mouse cursor is over the iframe. But with fission enabled it says "parent document" on the entire bounding rect of the green triangle instead, which is incorrect.

Based on a discussion with :jrmuizel today - we should support sending the clip-path information to WR and use it to do precise hit-testing. This will involve:

  1. changing this line to return true
  2. Adding a case StyleBasicShape::Tag::Polygon: to this switch statement that encodes the StyleGenericPolygon contents into a new clip type and sending it to WR
  3. Supporting that new clip type throughout WR wherever needed (intentionally vague because I don't know what all this entails)
  4. Specifically updating hit_test.rs to do a precise hit-test against the polygon clip

As a result of step 1, we should never hit this line when WR is enabled. And APZ will get correct results when hit-testing against clip-paths, which will make the attached testcase pass.

Attachment #9185822 - Attachment description: clippath.patch → clippath+fisison+wr testcase

Comment on attachment 9185822 [details] [diff] [review]
clippath+fisison+wr testcase

Replacing raw patch with an equivalent phabricator revision, rebased on top of bug 1675378

Attachment #9185822 - Attachment is obsolete: true
Severity: -- → S3

Tracking this for M7 (rather than M6c) as polygonal clip-paths seem like an edge case that nightly users are unlikely to run into often.

Fission Milestone: --- → M7
Blocks: gfx-triage

I'll work on this.

Assignee: nobody → bwerth
Status: NEW → ASSIGNED
No longer blocks: gfx-triage

Depends on D105396

Brad, there have been patches on this from 18 days ago, but no reviewer is assigned yet. What are the next steps here?
Fission M7 has to be fixed in Fx88 (soft freeze on March 18).

Flags: needinfo?(bwerth)

(In reply to Neha Kochar [:neha] from comment #8)

Brad, there have been patches on this from 18 days ago, but no reviewer is assigned yet. What are the next steps here?
Fission M7 has to be fixed in Fx88 (soft freeze on March 18).

These are WIP patches that got de-prioritized as I focused on other work. I'll come back to the them in the coming days. They aren't ready for review yet.

Flags: needinfo?(bwerth)

Jeff Muizelaar messaged on slack about this bug and said that polygonal clip-paths work, it’s just hit testing them that’s broken and that the graphics team doesn’t know of any content that actually breaks because of this.
Based on this info, moving this out to M7a (Fx89). Brad, please prioritize this for Fission.

Fission Milestone: M7 → M7a

This leaves the actual clipping in draw_clip_batch_list still as a TODO.

Depends on D105548

Attachment #9210505 - Attachment is obsolete: true
Attachment #9203589 - Attachment description: Bug 1675375 Part 2: Add a polygon clip case to CreateSimpleClipRegion. → Bug 1675375 Part 2: Add a polygon clips to image masks.
Attachment #9203588 - Attachment description: Bug 1675375 Part 1: Define a WebRender PolygonClipRegion. → Bug 1675375 Part 1: Define a WebRender SetPoints DisplayItem for polygons.

This performs a winding number calculation on the polygon and matches the
result against the fill rule of the supplied polygon.

Depends on D105548

This test was originally written by Kats, but I can't figure out how to
preserve authorship.

Depends on D109946

Attachment #9186047 - Attachment is obsolete: true
Attachment #9203588 - Attachment description: Bug 1675375 Part 1: Define a WebRender SetPoints DisplayItem for polygons. → WIP: Bug 1675375 Part 1: Define WebRender structures for polygons.
Attachment #9203589 - Attachment description: Bug 1675375 Part 2: Add a polygon clips to image masks. → WIP: Bug 1675375 Part 2: Add a polygon clips to image masks.
Attachment #9203846 - Attachment description: Bug 1675375 Part 3: Stub in polygon clipping in WebRender. → WIP: Bug 1675375 Part 3: Stub in polygon clipping in WebRender.
Attachment #9211857 - Attachment description: Bug 1675375 Part 4: Perform the polygon hit test in WebRender. → WIP: Bug 1675375 Part 4: Perform the polygon hit test in WebRender, and add unit tests.
Attachment #9211858 - Attachment description: Bug 1675375 Part 5: Add a polygon clip test for Fission WebRender. → WIP: Bug 1675375 Part 5: Add a polygon clip test for Fission WebRender.

This test fails in automation, but those failures are difficult to reproduce
in local builds. When attempting to reproduce locally, the whole test sometimes
times out. Isolating specific subtests seems to affect the test outcomes as
well. Splitting the test into pieces seems to be the best way to make failures
in this test more actionable.

Depends on D109947

Attachment #9214295 - Attachment description: WIP: Bug 1675375 Part 7: Update expectationsin helper_hittest_clippath.html. → WIP: Bug 1675375 Part 7: Update expectations in helper_hittest_clippath.html.
Pushed by bwerth@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4d8b6d3a5f02
WIP: Bug 1675375 Part 1: Define WebRender structures for polygons. r=gw
https://hg.mozilla.org/integration/autoland/rev/c542d63b4dd8
WIP: Bug 1675375 Part 2: Add a polygon clips to image masks. r=gw
https://hg.mozilla.org/integration/autoland/rev/1cd395b58b92
WIP: Bug 1675375 Part 3: Stub in polygon clipping in WebRender. r=gw
https://hg.mozilla.org/integration/autoland/rev/319a7979be09
WIP: Bug 1675375 Part 4: Perform the polygon hit test in WebRender, and add unit tests. r=gw
https://hg.mozilla.org/integration/autoland/rev/d96860372ec5
WIP: Bug 1675375 Part 5: Add a polygon clip test for Fission WebRender. r=gw
https://hg.mozilla.org/integration/autoland/rev/119c1be7430c
WIP: Bug 1675375 Part 6: Break test_group_hittest.html into parts. r=botond
https://hg.mozilla.org/integration/autoland/rev/4cf2b6a19950
WIP: Bug 1675375 Part 7: Update expectations in helper_hittest_clippath.html. r=botond
Attachment #9203588 - Attachment description: WIP: Bug 1675375 Part 1: Define WebRender structures for polygons. → Bug 1675375 Part 1: Define WebRender structures for polygons.
Attachment #9203589 - Attachment description: WIP: Bug 1675375 Part 2: Add a polygon clips to image masks. → Bug 1675375 Part 2: Add a polygon clips to image masks.
Attachment #9203846 - Attachment description: WIP: Bug 1675375 Part 3: Stub in polygon clipping in WebRender. → Bug 1675375 Part 3: Stub in polygon clipping in WebRender.
Attachment #9211857 - Attachment description: WIP: Bug 1675375 Part 4: Perform the polygon hit test in WebRender, and add unit tests. → Bug 1675375 Part 4: Perform the polygon hit test in WebRender, and add unit tests.
Attachment #9211858 - Attachment description: WIP: Bug 1675375 Part 5: Add a polygon clip test for Fission WebRender. → Bug 1675375 Part 5: Add a polygon clip test for Fission WebRender.
Attachment #9214213 - Attachment description: WIP: Bug 1675375 Part 6: Break test_group_hittest.html into parts. → Bug 1675375 Part 6: Break test_group_hittest.html into parts.
Attachment #9214295 - Attachment description: WIP: Bug 1675375 Part 7: Update expectations in helper_hittest_clippath.html. → Bug 1675375 Part 7: Update expectations in helper_hittest_clippath.html.
Attachment #9203589 - Attachment description: Bug 1675375 Part 2: Add a polygon clips to image masks. → WIP: Bug 1675375 Part 2: Add a polygon clips to image masks.
Attachment #9203589 - Attachment description: WIP: Bug 1675375 Part 2: Add a polygon clips to image masks. → Bug 1675375 Part 2: Add a polygon clips to image masks.
Pushed by bwerth@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5b83b96c3057
Part 1: Define WebRender structures for polygons. r=gw
https://hg.mozilla.org/integration/autoland/rev/d29bc0506ced
Part 2: Add a polygon clips to image masks. r=gw
https://hg.mozilla.org/integration/autoland/rev/13d98397d39a
Part 3: Stub in polygon clipping in WebRender. r=gw
https://hg.mozilla.org/integration/autoland/rev/842bc646804b
Part 4: Perform the polygon hit test in WebRender, and add unit tests. r=gw
https://hg.mozilla.org/integration/autoland/rev/7085c1fa22a7
Part 5: Add a polygon clip test for Fission WebRender. r=gw
https://hg.mozilla.org/integration/autoland/rev/c8ef6d2e370b
Part 6: Break test_group_hittest.html into parts. r=botond
https://hg.mozilla.org/integration/autoland/rev/153063d9a02a
Part 7: Update expectations in helper_hittest_clippath.html. r=botond
Pushed by bwerth@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0e463f7857ea
Part 1: Define WebRender structures for polygons. r=gw
https://hg.mozilla.org/integration/autoland/rev/4d13e1221a79
Part 2: Add a polygon clips to image masks. r=gw
https://hg.mozilla.org/integration/autoland/rev/5f8ae081f4ee
Part 3: Stub in polygon clipping in WebRender. r=gw
https://hg.mozilla.org/integration/autoland/rev/dd08dfba1dcd
Part 4: Perform the polygon hit test in WebRender, and add unit tests. r=gw
https://hg.mozilla.org/integration/autoland/rev/5dbfc7ce0a85
Part 5: Add a polygon clip test for Fission WebRender. r=gw
https://hg.mozilla.org/integration/autoland/rev/dba2861e4b53
Part 6: Break test_group_hittest.html into parts. r=botond
https://hg.mozilla.org/integration/autoland/rev/e2e27cedd002
Part 7: Update expectations in helper_hittest_clippath.html. r=botond

Backed out 7 changesets (Bug 1675375) for causing mochitest plain failures in test_group_hittest-2.html
Backout link: https://hg.mozilla.org/integration/autoland/rev/3734414bbb3d3c9e53ccc4515f65cced1cab17a0
Push with failures, failure log.

Hey, just a heads up that in bug 1704070 I added a new subtest to test_group_hittest.html, so the Part 6 patch will need to be rebased across that. Apologies for any inconvenience.

Pushed by bwerth@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1a8ae9663eb0
Part 1: Define WebRender structures for polygons. r=gw
https://hg.mozilla.org/integration/autoland/rev/ae13d5fc65ba
Part 2: Add a polygon clips to image masks. r=gw
https://hg.mozilla.org/integration/autoland/rev/053073e36e53
Part 3: Stub in polygon clipping in WebRender. r=gw
https://hg.mozilla.org/integration/autoland/rev/5fbf7f44c382
Part 4: Perform the polygon hit test in WebRender, and add unit tests. r=gw
https://hg.mozilla.org/integration/autoland/rev/a628a5fa5291
Part 5: Add a polygon clip test for Fission WebRender. r=gw
https://hg.mozilla.org/integration/autoland/rev/e1ec44a75cd0
Part 6: Break test_group_hittest.html into parts. r=botond
https://hg.mozilla.org/integration/autoland/rev/1a2cb51e0573
Part 7: Update expectations in helper_hittest_clippath.html. r=botond
Pushed by bwerth@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dc7b605c2911
Part 1: Define WebRender structures for polygons. r=gw
https://hg.mozilla.org/integration/autoland/rev/d4b0a140332c
Part 2: Add a polygon clips to image masks. r=gw
https://hg.mozilla.org/integration/autoland/rev/06c404b94962
Part 3: Stub in polygon clipping in WebRender. r=gw
https://hg.mozilla.org/integration/autoland/rev/fa02c774226a
Part 4: Perform the polygon hit test in WebRender, and add unit tests. r=gw
https://hg.mozilla.org/integration/autoland/rev/fb5f5a643fa6
Part 5: Add a polygon clip test for Fission WebRender. r=gw
https://hg.mozilla.org/integration/autoland/rev/82cf330340dc
Part 6: Break test_group_hittest.html into parts. r=botond
https://hg.mozilla.org/integration/autoland/rev/3da14f29d4d7
Part 7: Update expectations in helper_hittest_clippath.html. r=botond
Flags: needinfo?(bwerth)
No longer regressions: 1705193
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: