Closed
Bug 1337885
Opened 7 years ago
Closed 7 years ago
Invalidation reftests aren't passing
Categories
(Core :: Graphics: WebRender, defect)
Core
Graphics: WebRender
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: rhunt, Assigned: ethlin)
References
(Blocks 1 open bug)
Details
(Whiteboard: [gfx-noted])
Attachments
(2 files)
7.54 KB,
patch
|
rhunt
:
review+
|
Details | Diff | Splinter Review |
10.66 KB,
patch
|
jrmuizel
:
review+
|
Details | Diff | Splinter Review |
Currently a bunch of reftests in reftests/invalidation/ are not passing with 'failed reftest-no-paint'. Which means that we are painting a display item into a layer that should not. Most of them are paintedlayer-recycling-*.html and layer-splitting-*.html. I don't think the invalidation story for webrender is currently clear, so it's not clear to me what the final solution is.
Reporter | ||
Updated•7 years ago
|
Blocks: webrender-reftests
Reporter | ||
Comment 1•7 years ago
|
||
Talking with Markus and Matt on irc we noticed that WebRenderPaintedLayer's paint method is identical to ClientPaintedLayers' paint method, except it is missing one line [1]. So mValidRegion is never added to, so we completely repaint every PaintedLayer, which is causing these reftests to fail. I did a try run with this line added back in [2], and it fixed the invalidation tests, but it looks like it has some failures. Ethan, you added ContentClient to WebRender and removed the mValidRegion line. Do you know why it causes some test failures? [1] https://hg.mozilla.org/projects/graphics/file/e79bf3dc8926/gfx/layers/client/ClientPaintedLayer.cpp#l99 [2] https://treeherder.mozilla.org/#/jobs?repo=try&revision=e16684a0ba655d04c53fc91f9d96387965a3a65d
Flags: needinfo?(ethlin)
See Also: → 1322504
Reporter | ||
Comment 2•7 years ago
|
||
Also I noticed that we don't seem to destroy the ContentClient the same way as in ClientPaintedLayer. [1] I don't know this code very well, so I don't know if that's an issue. [1] https://searchfox.org/mozilla-central/rev/afcf40f3eafd895611a839017730debb58a342a6/gfx/layers/client/ClientPaintedLayer.h#45
Assignee | ||
Comment 3•7 years ago
|
||
(In reply to Ryan Hunt [:rhunt] from comment #1) > Talking with Markus and Matt on irc we noticed that WebRenderPaintedLayer's > paint method is identical to ClientPaintedLayers' paint method, except it is > missing one line [1]. > > So mValidRegion is never added to, so we completely repaint every > PaintedLayer, which is causing these reftests to fail. > > I did a try run with this line added back in [2], and it fixed the > invalidation tests, but it looks like it has some failures. > > Ethan, you added ContentClient to WebRender and removed the mValidRegion > line. Do you know why it causes some test failures? > > [1] > https://hg.mozilla.org/projects/graphics/file/e79bf3dc8926/gfx/layers/client/ > ClientPaintedLayer.cpp#l99 > [2] > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=e16684a0ba655d04c53fc91f9d96387965a3a65d I don't know why the line causes some test failures, but that's why I removed the line when importing the ContentClient. I think the valid region is correct. It looks like we have another rendering problem.
Flags: needinfo?(ethlin)
Assignee | ||
Comment 4•7 years ago
|
||
I think it's because the WebRenderContainerLayer doesn't handle clip region correctly. I'll fix it after WebRender update.
Assignee: nobody → ethlin
Assignee | ||
Comment 5•7 years ago
|
||
After the WR update, the clipping problem is fixed! So I don't need another patch for the clipping. The original problem is that the clipping region was wrong, so the reftest may get wrong image when updating the snapshot.
Attachment #8837933 -
Flags: review?(rhunt)
Reporter | ||
Updated•7 years ago
|
Attachment #8837933 -
Flags: review?(rhunt) → review+
Pushed by ethlin@mozilla.com: https://hg.mozilla.org/projects/graphics/rev/ce81b6b4c8f2 Update the valid region in WebRenderPaintedLayer and mark related tests as passing. r=rhunt
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 7•7 years ago
|
||
It looks like after this landed 'layout/reftests/css-grid/grid-fragmentation-dyn*.html' went intermittent. But the results are nearly identical, it might be best to just fuzz these tests for now.
Attachment #8838714 -
Flags: review?(jmuizelaar)
Updated•7 years ago
|
Attachment #8838714 -
Flags: review?(jmuizelaar) → review+
Pushed by rhunt@eqrion.net: https://hg.mozilla.org/projects/graphics/rev/6489e07b8265 Fuzz css-grid/grid-fragmentation-dyn tests r=jrmuizel
Updated•7 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 9•7 years ago
|
||
It was backed out by Bug 1340798.
Comment 10•7 years ago
|
||
I backed out the follow-up patch (comment 8) as well. If/when this relands please do a try push to see if that patch also needs relanding.
Comment 11•7 years ago
|
||
The failures for "failed reftest-no-paint" in general are going to be WONTFIX because as we layerizes more things for WR (and eventually stops using layers entirely) the invalidation mechanism these tests use becomes less and less correct.
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•