Closed Bug 1337885 Opened 7 years ago Closed 7 years ago

Invalidation reftests aren't passing

Categories

(Core :: Graphics: WebRender, defect)

defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: rhunt, Assigned: ethlin)

References

(Blocks 1 open bug)

Details

(Whiteboard: [gfx-noted])

Attachments

(2 files)

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.
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
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
(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)
I think it's because the WebRenderContainerLayer doesn't handle clip region correctly. I'll fix it after WebRender update.
Assignee: nobody → ethlin
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)
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
Attached patch fuzz-grid.patchSplinter Review
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)
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
Depends on: 1340950
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
It was backed out by Bug 1340798.
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.
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 ago7 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.