1390437 - cannot run "./mach reftest" normally in layers-free mode

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Ethan Lin[:ethlin]]

I use the command on MacOS:
./mach reftest --setpref reftest.ignoreWindowSize=true --setpref gfx.webrender.layers-free=true --setpref gfx.webrender.enabled=true

Firefox gets stuck before loading the testing files.

Comment 1

[Ethan Lin[:ethlin]]

Attachment: Bug 1390437 - Add NotifyInvalidation after EndTransaction for layers-free mode.

For layers-free mode, we still need to call NotifyInvalidation to trigger
FireDOMPaintEvent. It should be fine to pass an empty rect since we always
take full snapshot for WR.

Review commit: https://reviewboard.mozilla.org/r/168674/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/168674/

Comment 2

[Ethan Lin[:ethlin]]

Comment on attachment 8897354 [details]
Bug 1390437 - Add NotifyInvalidation after EndTransaction for layers-free mode.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/168674/diff/1-2/

Comment 3

[Ethan Lin[:ethlin]]

There is a side effect. Cancel the review request first. The problem is we don't have invalidation region in layers-free mode so we don't know when to call NotifyInvalidation. We shouldn't keep calling NotifyInvalidation neither or it will keep painting.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

(In reply to Ethan Lin[:ethlin] from comment #3)
> We shouldn't keep calling NotifyInvalidation
> neither or it will keep painting.

How did you observe this? With the patch you posted running reftests seems to work. Also loading about:newtab works now which is nice. It doesn't seem to be painting infinitely as far as I can tell. If it's a problem during reftests then maybe we can skip calling NotifyInvalidation if aCtx is non-null (i.e. we're painting into a snapshot)?

Comment 5

[Ethan Lin[:ethlin]]

The side effect only happens on the reftests which have 'reftest-wait' (e.g., layout/reftests/invalidation/mask-invalidation-1a.html). I tried to skip calling NotifyInvalidation if aCtx is non-null, but the original problem happened again. An alternative way is to skip calling NotifyInvalidation if it's not a chrome docshell. This way seems to work fine on try server[1]. I'm not very sure if it's a correct way to fix this problem. I'll add review request flag since at least the patch could make try normal.  

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=00b3a0cf436d74ef9e55d0674571b2255c5e3a04

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8897354 [details]
Bug 1390437 - Add NotifyInvalidation after EndTransaction for layers-free mode.

https://reviewboard.mozilla.org/r/168674/#review174620

::: layout/painting/nsDisplayList.cpp:2139
(Diff revision 2)
>      aBuilder->SetIsCompositingCheap(temp);
>      if (document && widgetTransaction) {
>        TriggerPendingAnimations(document, layerManager->GetAnimationReadyTime());
>      }
>  
> +    if (view && presContext->IsChrome()) {

I'm not really sure why the condition here includes "view". At any rate please add a comment here "TODO: make sure this gets fired at the right times" so that we know this needs revisiting later. I'm ok with landing it for now, it seems to work for me locally, even on the mask-invalidation-1a reftest that you mentioned.

Comment 7

[Ethan Lin[:ethlin]]

Comment on attachment 8897354 [details]
Bug 1390437 - Add NotifyInvalidation after EndTransaction for layers-free mode.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/168674/diff/2-3/

Comment 8

[Ethan Lin[:ethlin]]

Looks like the 'reftest-wait' need a mechanism to trigger the NotifyInvalidation at right time. I will land this bug first and Morris will address the issue in bug 1391202.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/adee83b645db
Add NotifyInvalidation after EndTransaction for layers-free mode. r=kats

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/adee83b645db