1354463 - Enable 'layers.advanced.canvas-background-color' by default

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement)

Description

[Ethan Lin[:ethlin]]

Current try result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0d060b0e71ab0af490af736af2d4f6f2792aba30&selectedJob=89434187.

Comment 1

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Latest try result:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8646adc9d3fe3442755290d586cc3f808ed7a5a5

Comment 2

[Ethan Lin[:ethlin]]

current try result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=847d2b8969d3edf2dda1449f2b773d35620c668b&selectedJob=100832168

Comment 3

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Attachment: Bug 1354463 - Enable layers.advanced.canvas-background-color if webrender is enabled.

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

Comment 4

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Attachment: Bug 1354463 - Set fails-if for opaque layer check fail.

This is because in original gecko, canvas background color will merge to
painted layer with other display items. When advanced canvas background
color is enabled, the "other display items" won't be merged with canvas
background color. Which means the layers that are generated by those
display items may not be opaque. So set fails-if to those tests.

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

Comment 5

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Attachment: Bug 1354463 - Add fuzzy-if for tests.

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

Comment 6

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6cbe0e10343ee535d02dca933f6a32f19a56bb1f

Comment 7

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

Comment on attachment 8874773 [details]
Bug 1354463 - Add fuzzy-if for tests.

https://reviewboard.mozilla.org/r/146146/#review150126

Please change all of these fuzzy annotations to be ranges, e.g. fuzzy-if(webrender,15-15,1000-1000) so that if the fuzziness level drops we will find out and can tighten the bounds.

Comment 8

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

Comment on attachment 8874772 [details]
Bug 1354463 - Set fails-if for opaque layer check fail.

https://reviewboard.mozilla.org/r/146144/#review150130

Please make sure to update the webrender reftest spreadsheet with these and mark them as WONTFIX since the failures are expected.

Comment 9

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

Comment on attachment 8874771 [details]
Bug 1354463 - Enable layers.advanced.canvas-background-color if webrender is enabled.

https://reviewboard.mozilla.org/r/146142/#review150132

Comment 10

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

Comment on attachment 8874771 [details]
Bug 1354463 - Enable layers.advanced.canvas-background-color if webrender is enabled.

https://reviewboard.mozilla.org/r/146142/#review150134

Typo in commit message: first "is" should be "if"

Comment 11

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Comment on attachment 8874771 [details]
Bug 1354463 - Enable layers.advanced.canvas-background-color if webrender is enabled.

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

Comment 12

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Comment on attachment 8874772 [details]
Bug 1354463 - Set fails-if for opaque layer check fail.

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

Comment 13

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Comment on attachment 8874773 [details]
Bug 1354463 - Add fuzzy-if for tests.

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

Comment 14

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Thanks for review. Try looks good.

Here is try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bf54341ace7793a56239a0500836c7bc073ee343&group_state=expanded

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4ac4ad3ba687
Enable layers.advanced.canvas-background-color if webrender is enabled. r=kats
https://hg.mozilla.org/integration/autoland/rev/16d73b417ed5
Set fails-if for opaque layer check fail. r=kats
https://hg.mozilla.org/integration/autoland/rev/b884346aefbe
Add fuzzy-if for tests. r=kats

Comment 16

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

Is there a reason you used ranges like 950-1000 instead of a more precise range? Also in some cases you had ranges starting at 0, like "fuzzy-if(webrender,0-1,0-600)" - is this an intermittent? The only reason we should have ranges like this is if the tests are nondeterministic, and if that's the case that worries me.

Comment 17

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Yap, I think precise range is better. I'll attach a patch for fixing this. And about fuzzy-if(webrender,0-1,0-600), it seems the different result in different configurations such as e10s v.s. non-e10s, debug v.s. opt...

Comment 18

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Attachment: Use precise range for fuzzy-if.

MozReview-Commit-ID: Cf0d2ZjCMHb

Comment 19

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Latest try. https://treeherder.mozilla.org/#/jobs?repo=try&revision=8d75ed9ee81add1f764bb2e9c7a5c3ce0d1d50c9&selectedJob=105379180

Comment 20

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

leave the bug open until follow-up patch landed.

Comment 21

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

Comment on attachment 8875586 [details] [diff] [review]
Use precise range for fuzzy-if.

Review of attachment 8875586 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, this is an improvement. I guess debug vs opt and e10s vs non-e10s is a valid reason to have a wider range. As long as we don't have a given configuration producing different values on each run (i.e. non-deterministic results)

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4ac4ad3ba687
https://hg.mozilla.org/mozilla-central/rev/16d73b417ed5
https://hg.mozilla.org/mozilla-central/rev/b884346aefbe

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ad1958745d3f
Use precise range for fuzzy-if. r=kats

Comment 24

[Carsten Book [:Tomcat]]

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