Invalidation reftests aren't passing

RESOLVED WONTFIX

Status

()

RESOLVED WONTFIX
2 years ago
a year ago

People

(Reporter: rhunt, Assigned: ethlin)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [gfx-noted])

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
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

2 years ago
Blocks: 1322815
(Reporter)

Comment 1

2 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: → bug 1322504
(Reporter)

Comment 2

2 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

2 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

2 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

2 years ago
Created attachment 8837933 [details] [diff] [review]
update valid region

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

2 years ago
Attachment #8837933 - Flags: review?(rhunt) → review+

Comment 6

2 years ago
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
Last Resolved: 2 years ago
Resolution: --- → FIXED
(Reporter)

Comment 7

2 years ago
Created attachment 8838714 [details] [diff] [review]
fuzz-grid.patch

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+

Comment 8

2 years ago
Pushed by rhunt@eqrion.net:
https://hg.mozilla.org/projects/graphics/rev/6489e07b8265
Fuzz css-grid/grid-fragmentation-dyn tests r=jrmuizel

Updated

2 years ago
Depends on: 1340950

Updated

2 years ago
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
Last Resolved: 2 years agoa year ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.