Turn on layers-free by default

RESOLVED FIXED in Firefox 58

Status

()

Core
Graphics: WebRender
P1
normal
RESOLVED FIXED
2 months ago
25 days ago

People

(Reporter: ethlin, Assigned: kats)

Tracking

(Depends on: 1 bug, Blocks: 3 bugs)

unspecified
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 unaffected, firefox57 unaffected, firefox58 fixed)

Details

(Whiteboard: [wr-mvp])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(6 attachments)

(Reporter)

Description

2 months ago
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.
(Reporter)

Updated

2 months ago
Depends on: 1387994
(Assignee)

Updated

2 months ago
Blocks: 1387549
(Reporter)

Comment 1

2 months ago
Update the try result with canvas crash fix.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7dff7c1c7112cf2d2b62ee88891b74f7e497d1b5.
(Reporter)

Updated

2 months ago
Depends on: 1389433

Updated

2 months ago
Blocks: 1386665
(Reporter)

Updated

2 months ago
Depends on: 1390437
(Reporter)

Updated

2 months ago
Depends on: 1390440
(Reporter)

Updated

2 months ago
Depends on: 1390804
Depends on: 1391135
Depends on: 1391136
(Reporter)

Updated

2 months ago
Depends on: 1391139
Depends on: 1391202

Updated

2 months ago
Depends on: 1391499
(Reporter)

Updated

2 months ago
Depends on: 1391541
Depends on: 1392200
Here's another try push with layers-free and webrendest turned on: https://treeherder.mozilla.org/#/jobs?repo=try&revision=57d73b4c46abb07fb66196b23935d2fb0c3e250b
(Reporter)

Updated

2 months ago
Depends on: 1392502
(Reporter)

Updated

2 months ago
Depends on: 1392523
(Reporter)

Updated

2 months ago
Depends on: 1392921
(Reporter)

Comment 3

2 months ago
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.
(Reporter)

Updated

2 months ago
Depends on: 1393376
(Reporter)

Updated

2 months ago
Depends on: 1393706
(Reporter)

Comment 4

2 months ago
New try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b184725a7c569701bc30520fb6102864e8da6f23 (no APZ).

Updated

2 months ago
Depends on: 1394306

Updated

2 months ago
Depends on: 1394308

Updated

2 months ago
Depends on: 1394309
(Reporter)

Updated

2 months ago
Depends on: 1394336
Depends on: 1394340

Updated

2 months ago
Depends on: 1394695
(Reporter)

Updated

2 months ago
Depends on: 1394711

Updated

2 months ago
Depends on: 1374622
(Reporter)

Updated

2 months ago
Depends on: 1395501
Priority: -- → P3

Updated

a month ago
Status: NEW → ASSIGNED
Priority: P3 → P1
Whiteboard: [wr-mvp]
Target Milestone: --- → mozilla57
status-firefox56: --- → unaffected
status-firefox57: --- → unaffected
(Reporter)

Updated

a month ago
Depends on: 1398706
(Reporter)

Updated

a month ago
Depends on: 1399043
Depends on: 1399050
(Reporter)

Comment 5

a month ago
Update try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a5474cb18a74e7c501bb44132eb8b801cc4ca963&selectedJob=129177907
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.
(Reporter)

Comment 7

a month ago
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.
(Reporter)

Comment 9

a month ago
(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
Depends on: 1400888
Lots of text failures, gonna be stuck triaging through those for a while. Sorry!
(Reporter)

Comment 11

a month ago
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.
This should have all the changes needed: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d2927dd94d7ee7f53975db3b1883ffc54eb1b28d
Better: https://treeherder.mozilla.org/#/jobs?repo=try&revision=205b6033ee9e8602877f895f99a925d373fbb06f

But it's hitting the assertions from bug 1396471
Depends on: 1396471
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.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c30cf44e64baa7d24b023dc48d17505c3f6e6fd6
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).
(Reporter)

Comment 18

a month ago
(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.
(Reporter)

Updated

a month ago
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).
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Had to some more last-minute tweaks based on the above try push, but there were just a few changes needed.

Comment 27

a month ago
mozreview-review
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 28

a month ago
mozreview-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 29

a month ago
mozreview-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 30

a month ago
mozreview-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 31

a month ago
mozreview-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.
(Reporter)

Comment 33

a month ago
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

Updated

a month ago
Depends on: 1402229
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7a0ef0298f6cc2a10be0dfaf05a7022ddd853f16
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2fe452faac99774fd96556ac741e1d95dde17eed (also includes patches from bug 1402439)
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
With the changes from bug 1398706 accounted for: https://treeherder.mozilla.org/#/jobs?repo=try&revision=98f075680f92ffa8b7400f1884930fca4196b2c4&filter-searchStr=reftest&group_state=expanded
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2d9c4dc51b52b575f41eb2f7130c6116bb2db19f

This one's actually green! Maybe I finally annotated them all...
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
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 hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 57

26 days ago
mozreview-review
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+
Blocks: 1403035
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Duplicate of this bug: 1374326

Comment 65

26 days ago
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

Comment 66

26 days ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/43963e011193
https://hg.mozilla.org/mozilla-central/rev/67db64a527a9
https://hg.mozilla.org/mozilla-central/rev/662caed2c32f
https://hg.mozilla.org/mozilla-central/rev/552237145426
https://hg.mozilla.org/mozilla-central/rev/9b97c8368e6a
https://hg.mozilla.org/mozilla-central/rev/273f8af59697
Status: ASSIGNED → RESOLVED
Last Resolved: 26 days ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Depends on: 1403459
You need to log in before you can comment on or make changes to this bug.