Closed Bug 1390437 Opened 3 years ago Closed 3 years ago

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

Categories

(Core :: Graphics: WebRender, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: ethlin, Assigned: ethlin)

References

Details

Attachments

(1 file)

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.
Blocks: layers-free
Attachment #8897354 - Flags: review?(bugmail)
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.
(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)?
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
Attachment #8897354 - Flags: review?(bugmail)
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.
Attachment #8897354 - Flags: review?(bugmail) → review+
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.
Pushed by ethlin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/adee83b645db
Add NotifyInvalidation after EndTransaction for layers-free mode. r=kats
https://hg.mozilla.org/mozilla-central/rev/adee83b645db
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.