Filters and clips interact poorly with WR.
Categories
(Core :: Graphics: WebRender, defect, P3)
Tracking
()
People
(Reporter: kats, Assigned: kats)
References
Details
Attachments
(3 files, 2 obsolete files)
+++ This bug was initially created as a clone of Bug #1514383 +++
Cloning from bug 1514383 comment 7. The attached testcase is rendering wrong.
|
Assignee | |
Comment 1•5 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=06e4554a91e5ffc51bd4fbaba22a8d997b276813
Hopefully this one will be green. My previous try push (bug 1514383 comment 12) had one failure due to the clip chain for a sticky item not getting applied to the contents of the sticky after I broke the clip chain parent links. But this new try push has a patch that fixes that problem for me locally, by explicitly setting the clip chain on the stacking context that holds the contents of the sticky item.
|
|
Assignee | |
Updated•5 years ago
|
|
|
Assignee | |
Comment 2•5 years ago
|
||
Patches cleaned up and added reftest: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=744c32596f2b3e7b8bed52288691017f298ace94
|
|
Assignee | |
Comment 3•5 years ago
|
||
Previous try push had a typo. This one is better:
But there's some stupid fuzzing needed. I'll fix some of it via bug 1515310 which has the same problem.
|
|
Assignee | |
Comment 4•5 years ago
|
||
The API to create a sticky spatial node doesn't allow us to set a clip
chain that will apply to all the contents of the sticky node. This means
that when the ClipManager sets up the clip state for the sticky node,
the clip chain for it is dropped. Everything still works currently
because the contents of the sticky node have their own clip chains whose
parent link will include the sticky node's clip chain. However, in the
next patch we're going to sever that parent link to fix other issues,
and so we will break this mechanism. This patch fixes it up by
explicitly applying the sticky node's clip chain to the stacking context
that contains all the sticky contents. This ensures all those items pick
up the clip chain.
|
|
Assignee | |
Comment 5•5 years ago
|
||
It turns out that setting the parent link on a clip chain is no longer
needed (and probably hasn't been since WR started applying a stacking
context's clip to the SC's contents). In fact it can produce incorrect
behaviour in some cases, because it doesn't match the semantics of
Gecko's clip chains. This removes the parent link on the Gecko side and
adds a test for this scenario.
Depends on D18095
|
|
Assignee | |
Comment 6•5 years ago
|
||
The API to create a sticky spatial node doesn't allow us to set a clip
chain that will apply to all the contents of the sticky node. This means
that when the ClipManager sets up the clip state for the sticky node,
the clip chain for it is dropped. Everything still works currently
because the contents of the sticky node have their own clip chains whose
parent link will include the sticky node's clip chain. However, in the
next patch we're going to sever that parent link to fix other issues,
and so we will break this mechanism. This patch fixes it up by
explicitly applying the sticky node's clip chain to the stacking context
that contains all the sticky contents. This ensures all those items pick
up the clip chain.
|
|
Assignee | |
Comment 7•5 years ago
|
||
It turns out that setting the parent link on a clip chain is no longer
needed (and probably hasn't been since WR started applying a stacking
context's clip to the SC's contents). In fact it can produce incorrect
behaviour in some cases, because it doesn't match the semantics of
Gecko's clip chains. This removes the parent link on the Gecko side and
adds a test for this scenario.
Depends on D18100
|
||
Updated•5 years ago
|
|
|
||
Updated•5 years ago
|
|
|
Assignee | |
Comment 8•5 years ago
|
||
Sorry for the phabricator noise, I ran into a moz-phab bug, filed 1523994 for it.
Also https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=b1565844fe3e2ae0e5de6c20ea8dbc888f16420a is the latest try push.
|
|
Assignee | |
Updated•5 years ago
|
Pushed by kgupta@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2aaf3468dd57 Set a clipchain on sticky contents. r=kvark https://hg.mozilla.org/integration/autoland/rev/92684cd249a1 Don't set the parent link when creating a new clip chain. r=kvark
|
||
Comment 10•5 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/2aaf3468dd57
https://hg.mozilla.org/mozilla-central/rev/92684cd249a1
|
|
Assignee | |
Comment 11•5 years ago
|
||
I'm going to back this out for now as the regressions seem to be quite visible and the fix (see bug 1524261) involves fuzzing a lot of reftests which will take time to do properly.
|
|
Assignee | |
Comment 12•5 years ago
•
|
||
|
||
Updated•5 years ago
|
|
||
Comment 13•5 years ago
|
||
https://bugzilla.mozilla.org/show_bug.cgi?id=1524385 has landed now, so I think that this bug can be considered fixed (assuming that patch doesn't get backed out / cause regressions). Is that right?
|
|
Assignee | |
Comment 14•5 years ago
|
||
Yes but I want to reland the reftest that I wrote for this bug. Working on that now.
|
|
Assignee | |
Comment 15•5 years ago
|
||
Try push is green: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=b40b7ab99195c597ff3273007bdf7ec629ddca59
Since :kvark already reviewed the test from the first time I landed it, I'll just reland on inbound. The test is the same as before, including the fuzz numbers.
|
||
Comment 16•5 years ago
|
||
Pushed by kgupta@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f4639ca94940 Add a reftest. r=kvark
|
||
Comment 17•5 years ago
|
||
| bugherder | ||
Description
•