Closed Bug 1349949 Opened 4 years ago Closed 4 years ago

Allow having a single pref flip turn webrender on or off

Categories

(Core :: Graphics: WebRender, enhancement, P3)

Other Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: kats, Assigned: kats)

References

Details

(Whiteboard: [gfx-noted])

Attachments

(2 files)

Now that we have webrender building by default (in bug 1342450), we want to be able to turn it on and off with a single pref flip. That's almost happening already, but in all.js there are some additional advanced layers prefs that also get flipped by the MOZ_ENABLE_WEBRENDER define [1]. That means if a user builds with --enable-webrender=build, and wants to turn on webrender to behave the same as it would with --enable-webrender=true, they would have to manually flip gfx.webrender.enabled and also flip the necessary advanced layers prefs.

Note that this affects not just the user flow, but also the automation flow, because otherwise to implement bug 1342488 we will need to flip a bunch of prefs instead of just one.

I have an idea for making those advanced layers prefs conditional on webrender being enabled so that we don't have to enumerate all of these prefs separately.

[1] http://searchfox.org/mozilla-central/rev/9af9b1c7946ebef6056d2ee4c368d496395cf1c8/modules/libpref/init/all.js#5648
Try push at https://treeherder.mozilla.org/#/jobs?repo=try&revision=13984d568ca5a4d634b822de02e9b77f6249f5a8 (with my patches rebased onto the graphics branch). But I intend to land the patches on inbound/autoland after the next m-c -> graphics merge.

/cc mchang and ethlin, please take a look at the patches in the try push above (particularly the second one) as I think you guys are the ones most likely to be adding new prefs of this sort in the near future.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #1)
> But I intend to land the patches
> on inbound/autoland after the next m-c -> graphics merge.

This should say graphics -> m-c merge. Which happened earlier today. So now graphics and m-c are pretty close. I'll land these patches on m-c and merge them back to graphics the next day. As part of the merge I'll need to fix up any other prefs that landed in the meantime (Mason already landed layers.advanced.boxshadow-inset-layers which I'll have to fix up, for example).
Comment on attachment 8850641 [details]
Bug 1349949 - Add the notion of an "override pref" to gfxPrefs.h.

https://reviewboard.mozilla.org/r/123188/#review125600

::: gfx/thebes/gfxPrefs.h:85
(Diff revision 1)
> +// - A value of 0 means that it has been force-disabled, and is exposed as a
> +//   false-valued bool.
> +// - A value of 1 means that it has been force-enabled, and is exposed as a
> +//   true-valued bool.
> +// - A value of 2 (the default) means that it returns the value of a different
> +//   function (which is evaluated to a boolean).

nit: returns the base value as a bool
Attachment #8850641 - Flags: review?(mchang) → review+
Comment on attachment 8850642 [details]
Bug 1349949 - Turn some advanced layers prefs into override prefs.

https://reviewboard.mozilla.org/r/123190/#review125606
Attachment #8850642 - Flags: review?(mchang) → review+
Thanks. Updated patches to address nit. Turns out the graphics -> m-c merge has not yet propagated to autoland, so I need to wait for that before I can push this.
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/617ef3603a8e
Add the notion of an "override pref" to gfxPrefs.h. r=mchang
https://hg.mozilla.org/integration/autoland/rev/cc8baab97e60
Turn some advanced layers prefs into override prefs. r=mchang
https://hg.mozilla.org/mozilla-central/rev/617ef3603a8e
https://hg.mozilla.org/mozilla-central/rev/cc8baab97e60
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/projects/graphics/rev/db0dbb918eeb
Convert another pref to an override pref as part of merging m-c to graphics. r=me
You need to log in before you can comment on or make changes to this bug.