Closed Bug 1389000 Opened 7 years ago Closed 7 years ago

Turn on layers-free by default

Categories

(Core :: Graphics: WebRender, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox56 --- unaffected
firefox57 --- unaffected
firefox58 --- fixed

People

(Reporter: ethlin, Assigned: kats)

References

Details

(Whiteboard: [wr-mvp])

Attachments

(6 files)

I just sent a try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9581a6af3545e5489348b1575b6aa38926e3fbe3&selectedJob=122164473.

It's not surprising that I got hundreds of failures. There is a canvas crash problem. I will fix the crash and send another try.
Depends on: 1387994
Depends on: 1389433
Depends on: 1390437
Depends on: 1390440
Depends on: 1390804
Depends on: 1391139
Depends on: 1391499
Depends on: 1391541
Depends on: 1392502
Depends on: 1392523
Depends on: 1392921
Update the try push with layers-free and with apz disabled: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1df2ac1e4b07e0e1b2607f881e3b3aa4eded801e&selectedJob=125124312

For the try push, I manually merged some patches that may fix tests.
Depends on: 1393376
Depends on: 1393706
Depends on: 1394306
Depends on: 1394308
Depends on: 1394309
Depends on: 1394336
Depends on: 1394695
Depends on: 1394711
Depends on: 1374622
Depends on: 1395501
Status: NEW → ASSIGNED
Priority: P3 → P1
Whiteboard: [wr-mvp]
Target Milestone: --- → mozilla57
Depends on: 1398706
Depends on: 1399043
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7822fd3bb9346ae539a8968ec001b49b45d475c8 is better. There's no need to disable APZ, we can leave it enabled. My try push shows no crashes, just failures. A lot of them are just fuzzing but we'll have to go through them one by one to see if it's a failure or fuzz.
Okay, I should keep APZ on. This is the latest try push[1] with rounding patches, reftest-wait patches, and inherited scale patches. The R2 and R6 are almost green now. It looks like there are many tests have slight text shift problem. I'll check this issue first. 

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=d6f5f3433177600c1f83a2ab08db634a91da752d&selectedJob=130991381
Ethan: happy to help out with this, lmk which things to investigate to avoid duplicating your work. In the meantime I'm checking out what happens to this patch if I also enable text-layers.
(In reply to Alexis Beingessner [:Gankro] from comment #8)
> Ethan: happy to help out with this, lmk which things to investigate to avoid
> duplicating your work. In the meantime I'm checking out what happens to this
> patch if I also enable text-layers.

Thanks. So far as I know we are still solving clipping and transform problems. These two issues cover many testcases. Other failures are almost about fuzzing and we need to figure out how to fix them or just fuzz. I think you probably can start from fuzz failures, like "layout/reftests/backgrounds/background-tiling-zoom-1.html"[1].

[1] https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=https://queue.taskcluster.net/v1/task/QGol_D0jSRGMhtsWxJGAbg/runs/0/artifacts/public/logs/live_backing.log&only_show_unexpected=1
Lots of text failures, gonna be stuck triaging through those for a while. Sorry!
The text fuzz problem is because we push each text item separately and do pixel snapping for each item. So the results may differ when the text item contains different numbers of words. To fix it, we should use moz2D to paint the sub pixel for each text item and push snapped position to WR. As for other types of items, we may keep using the original path. I'll create a bug for it and think if there are other solutions.
I talked to Jeff and Gankro and we agreed that at this point the number of test failures with layers-free are small enough that it's probably a good idea to flip the switch and make it the default. I'm just going to err on the side of caution and mark everything that's currently failing as fails-if and we can sort out the fuzzies vs actual fails afterwards. With the crash in bug 1397407 fixed layers-free should be stable enough to make the default now.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b376e1cbe58141f6a22e74627301a76ef404166a

This might still have some failures, but it should have the assertions fixed. I also removed the failing talos jobs and skipped the failing mochitest-chrome.
That one is looking good. There's still intermittent failures so I've kicked off retriggers to hopefully get a final list of the intermittents and then I'll flag them as random-if(webrender).
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #12)
> I talked to Jeff and Gankro and we agreed that at this point the number of
> test failures with layers-free are small enough that it's probably a good
> idea to flip the switch and make it the default. I'm just going to err on
> the side of caution and mark everything that's currently failing as fails-if
> and we can sort out the fuzzies vs actual fails afterwards. With the crash
> in bug 1397407 fixed layers-free should be stable enough to make the default
> now.

That's great! I'll set assignee to you since you already have much work here. I'll start to look into some refactoring and performance issues.
Assignee: ethlin → bugmail
Kats, I just submitted another try based on your patch in comment 16 and plus my patch in bug 1396471.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=885328853891535a8b7dd6a66c9de6124d8f9044
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3610350dd6dea50b589151dce5c5314a414dba7b

Hopefully final try push, rebased on current autoland (necessary to pick up Peter's patch from bug 1396471).
Had to some more last-minute tweaks based on the above try push, but there were just a few changes needed.
Comment on attachment 8910792 [details]
Bug 1389000 - Turn on webrender layers-free mode by default.

https://reviewboard.mozilla.org/r/182256/#review187616
Attachment #8910792 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8910793 [details]
Bug 1389000 - Update reftest annotations for layers-free results.

https://reviewboard.mozilla.org/r/182258/#review187618
Attachment #8910793 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8910794 [details]
Bug 1389000 - Remove duplicated layers-free tests.

https://reviewboard.mozilla.org/r/182260/#review187620
Attachment #8910794 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8910795 [details]
Bug 1389000 - Skip a mochitest that fails with layers-free enabled.

https://reviewboard.mozilla.org/r/182262/#review187622
Attachment #8910795 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8910796 [details]
Bug 1389000 - Disable talos tests that are failing with layers-free enabled.

https://reviewboard.mozilla.org/r/182264/#review187624
Attachment #8910796 - Flags: review?(jmuizelaar) → review+
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5a442386c585f172778b774ad317bcfe66e049a2 is my last working try push, rebased on autoland. There's a bunch of failures from the position:sticky patches that I landed previously. But even ignoring those there are yet more intermittent failures. This has been the case on every single try push I've done, and I'm not at all confident that I've gotten them all. I'm sure that after landing this patchset we'll keep triggering a high rate of intermittents for the QR reftests. I'm reluctant to do that. An alternative is to just land it with the reftests turned off but I don't think that's a good idea either. I think we need to figure out where this nondeterminism is coming from and fix it for realz before we can consider turning on layers-free by default.
I think the intermittent failures are due to the wr issue 1398[1] (or bug 1370908). We take each fallback display item as an image, so we have the texture coordinate precision problem. Maybe we should add a large fuzzy value for them, but it's annoying to distinguish between fuzzies and real fails.

[1] https://github.com/servo/webrender/issues/1398
Depends on: 1402229
Rebased to master: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7838ab4d4b420723e6332220b8efd606c4e83891

There are a bunch of changes resulting from bug 1398706 which is in m-c now, so I'll update the patches to account for those changes.
Also in an attempt to try and isolate the problem with the intermittent failures, I trimmed the reftest list to just layout/transform-3d/rotatey-1a.html and removed the fuzzy-if annotation on it. This reftest I expect to pass without any fuzz but currently it intermittently has some fuzz (it runs with layers-free enabled, because it's in the transform-3d folder). I did a try push with recording enabled for just this reftest and am retriggering in the hope that I'll be able to reproduce the fuzz and get a recording of it. Then we can compare the recording against the non-fuzzy recording and that should help isolate whether the problem is on the WR side or the gecko side.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fe718a2658f30eafc065d8a6959b16ff68c58332
The reftests were all green above but the crashtests were failing. Here's a try push rebased on current autoland tip with the failing crashtests disabled:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b9e04a5b7df747f6ee743b2592696d5f63ce4deb
Comment on attachment 8912333 [details]
Bug 1389000 - Disable crashtests that are crashing with assertion failures with layers-free enabled.

https://reviewboard.mozilla.org/r/183668/#review188862
Attachment #8912333 - Flags: review?(jmuizelaar) → review+
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/43963e011193
Turn on webrender layers-free mode by default. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/67db64a527a9
Update reftest annotations for layers-free results. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/662caed2c32f
Remove duplicated layers-free tests. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/552237145426
Skip a mochitest that fails with layers-free enabled. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/9b97c8368e6a
Disable talos tests that are failing with layers-free enabled. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/273f8af59697
Disable crashtests that are crashing with assertion failures with layers-free enabled. r=jrmuizel
Depends on: 1501062
You need to log in before you can comment on or make changes to this bug.