If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Enable some reftests for WebRenderBorderLayer

RESOLVED FIXED in mozilla54

Status

()

Core
Graphics: WebRender
RESOLVED FIXED
9 months ago
7 months ago

People

(Reporter: ethlin, Assigned: ethlin)

Tracking

unspecified
mozilla54
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

9 months ago
Per bug 1322079 comment 18, I will turn perf on for some reftest. And we should also reduce the limitation for using BorderLayer.
(Assignee)

Comment 1

8 months ago
I tried to enable pref by default, but I got many failures[1]. There seems to be some repainting problem when style changing. I'll create another bug for it. For this bug, I'll turn on the pref for some reftests. Almost every reftest has border layers.


[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=7f9960a71f821da59977a507055de96fdede4094&selectedJob=67919141
(Assignee)

Comment 2

8 months ago
Created attachment 8827354 [details] [diff] [review]
enable pref for some reftests

Set pref(layers.advanced.border-layers, true) for some reftests. There are still many reftests failures when enabling it. Turning the pref on for some reftests is just for preventing other patches breaking it.
Attachment #8827354 - Flags: review?(bugmail)
(Assignee)

Updated

8 months ago
See Also: → bug 1331538
Note that setting this pref on individual tests will also set it for non-webrender builds. Did you do a try push to see if it causes those to fail? Your try push in comment 1 only has reftests for the QR builds, not regular linux/mac/win.
Comment on attachment 8827354 [details] [diff] [review]
enable pref for some reftests

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

r=me with comments addressed. Please make sure it doesn't cause non-QR reftests to fail.

::: layout/reftests/box-sizing/reftest.list
@@ +1,2 @@
> +pref(layers.advanced.border-layers,true) == intrinsic-1a.html intrinsic-1-ref.html
> +pref(layers.advanced.border-layers,true) == intrinsic-1b.html intrinsic-1-ref.html

Instead of just modifying these reftests to have the pref, can you run them twice - once with the pref set and once without? That way we don't lose any reftest coverage.

::: layout/tools/reftest/reftest-preferences.js
@@ +130,5 @@
>  
>  user_pref("media.openUnsupportedTypeWithExternalApp", false);
> +
> +// For WebRenderBorderLayer
> +user_pref("layers.advanced.border-layers", false);

I'd rather put this in modules/libpref/init/all.js instead.
Attachment #8827354 - Flags: review?(bugmail) → review+
(Assignee)

Comment 5

8 months ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #4)
> Comment on attachment 8827354 [details] [diff] [review]
> enable pref for some reftests
> 
> Review of attachment 8827354 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me with comments addressed. Please make sure it doesn't cause non-QR
> reftests to fail.
> 
> ::: layout/reftests/box-sizing/reftest.list
> @@ +1,2 @@
> > +pref(layers.advanced.border-layers,true) == intrinsic-1a.html intrinsic-1-ref.html
> > +pref(layers.advanced.border-layers,true) == intrinsic-1b.html intrinsic-1-ref.html
> 
> Instead of just modifying these reftests to have the pref, can you run them
> twice - once with the pref set and once without? That way we don't lose any
> reftest coverage.
> 

I will add another line for running each test twice.   

> ::: layout/tools/reftest/reftest-preferences.js
> @@ +130,5 @@
> >  
> >  user_pref("media.openUnsupportedTypeWithExternalApp", false);
> > +
> > +// For WebRenderBorderLayer
> > +user_pref("layers.advanced.border-layers", false);
> 
> I'd rather put this in modules/libpref/init/all.js instead.

Okay!
(Assignee)

Comment 6

8 months ago
try looks good:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f348b78ebe215df1de31cd98dfc6298b3b0e6d9b

Comment 7

8 months ago
Pushed by ethlin@mozilla.com:
https://hg.mozilla.org/projects/graphics/rev/e18248e42bfc
Turn on layers.advanced.border-layers for some reftests. r=kats
Status: NEW → RESOLVED
Last Resolved: 8 months ago
Resolution: --- → FIXED

Comment 8

8 months ago
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/projects/graphics/rev/e0dc054dad00
Move new pref outside ifdef so it is available to non-QR reftests. r=ethlin?
Backed out for non-QR reftest failures. I take it you didn't check those passed? :)

https://hg.mozilla.org/projects/graphics/rev/8fcc95d1411532306d08b49471214e0f0fcd7a8c
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 10

8 months ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #9)
> Backed out for non-QR reftest failures. I take it you didn't check those
> passed? :)
> 
> https://hg.mozilla.org/projects/graphics/rev/
> 8fcc95d1411532306d08b49471214e0f0fcd7a8c

oops, I see. Thanks!
(Assignee)

Comment 11

8 months ago
Created attachment 8828248 [details] [diff] [review]
enable pref for some reftests

Looks like it crashed when doing layer transaction. I add skip-if(!webrender) to prevent it. Should we support advanced layer for non-QR?
Attachment #8827354 - Attachment is obsolete: true
Attachment #8828248 - Flags: review?(bugmail)
(Assignee)

Comment 12

8 months ago
(In reply to Ethan Lin[:ethlin] from comment #11)
> Created attachment 8828248 [details] [diff] [review]
> enable pref for some reftests
> 
> Looks like it crashed when doing layer transaction. I add
> skip-if(!webrender) to prevent it. Should we support advanced layer for
> non-QR?

If we should, I can have another bug for it. I guess we don't handle some IPC attributes correctly.
Comment on attachment 8828248 [details] [diff] [review]
enable pref for some reftests

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

Yeah I guess this is OK for now. Change the commit message to "... for some reftests when running with webrender."
Attachment #8828248 - Flags: review?(bugmail) → review+

Comment 14

8 months ago
Pushed by ethlin@mozilla.com:
https://hg.mozilla.org/projects/graphics/rev/309035ec7be2
Turn on layers.advanced.border-layers for some reftests with webrender. r=kats
Status: REOPENED → RESOLVED
Last Resolved: 8 months ago8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.