1425260 - Bring back webrendest functionality

Status: RESOLVED FIXED

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

Description

[Milan Sreckovic [:milan] (needinfo for best results)]

Either re-introduce the gfx.webrendest.enabled preference, or have gfx.webrender.enabled set all the necessary values for the recommended configuration for dogfooders that don't have the detailed information about keeping up.

See bug 1410824 where we removed gfx.webrendest.enabled as we didn't need it at the time.

Make it easy for a casual user/QA to switch to full WebRender functionality, but leave it possible for the engineers to tweak different values to debug or measure performance.

Comment 1

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Bug 1425260: gfx.webrender.all turns on all preferences that are needed for webrender.

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

Comment 2

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

Comment on attachment 8939535 [details]
Bug 1425260: gfx.webrender.all turns on all preferences that are needed for webrender.

https://reviewboard.mozilla.org/r/209858/#review215584

::: gfx/thebes/gfxPrefs.h:504
(Diff revision 1)
>  
>    DECL_GFX_PREF(Live, "gfx.vsync.collect-scroll-transforms",   CollectScrollTransforms, bool, false);
>    DECL_GFX_PREF(Once, "gfx.vsync.compositor.unobserve-count",  CompositorUnobserveCount, int32_t, 10);
>  
> +  DECL_GFX_PREF(Once, "gfx.webrender.all",                     WebRenderAll, bool, false);
> +  DECL_GFX_PREF(Once, "gfx.webrender.enabled",                 WebRenderEnabled, bool, false);

We intentionally left gfx.webrender.enabled out of gfxPrefs.h so that people wouldn't accidentally use it instead of gfxVars::UseWebRender. If we add it here maybe it would be better to tack on a DoNotUseDirectly into the function name.

::: image/imgFrame.cpp:104
(Diff revision 1)
>      return Factory::CreateDataSourceSurfaceWithStride(size, format,
>                                                        stride, false);
>    }
>  #endif
>  
> -  if (!aIsAnimated && gfxPrefs::ImageMemShared()) {
> +  if (!aIsAnimated && (gfxPrefs::WebRenderAll() || gfxPrefs::ImageMemShared())) {

I would prefer making ImageMemShared an override pref (https://searchfox.org/mozilla-central/rev/b24e6342d744c5a83fab5c15972e11eeb69d68e6/gfx/thebes/gfxPrefs.h#79-90) rather than having to ensure every call site, present and future, has this WebRenderAll() check.

Ditto for the WebRenderBlobImages pref.

::: modules/libpref/init/all.js:869
(Diff revision 1)
>  pref("gfx.logging.peak-texture-usage.enabled", false);
>  
>  pref("gfx.ycbcr.accurate-conversion", false);
>  
>  #ifdef MOZ_ENABLE_WEBRENDER
> +pref("gfx.webrender.all", true);

This is going to turn on ImageMemShared and acceleration for anybody who builds with --enable-webrender in their mozconfig, not sure if that's desirable or not.

Comment 3

[Milan Sreckovic [:milan] (needinfo for best results)]

I started with the override prefs, but went the manual, but simple way of just checking on all call sites.  I expect the other preferences to (eventually) disappear.  I'll take a look again.

Comment 4

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

Comment on attachment 8939535 [details]
Bug 1425260: gfx.webrender.all turns on all preferences that are needed for webrender.

https://reviewboard.mozilla.org/r/209858/#review216618

Dropping review flag until issues are sorted out

Comment 5

[Milan Sreckovic [:milan] (needinfo for best results)]

Comment on attachment 8939535 [details]
Bug 1425260: gfx.webrender.all turns on all preferences that are needed for webrender.

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

Comment 6

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #2)
> ...              WebRenderEnabled, bool, false);
> 
> We intentionally left gfx.webrender.enabled out of gfxPrefs.h so that people
> wouldn't accidentally use it instead of gfxVars::UseWebRender. If we add it
> here maybe it would be better to tack on a DoNotUseDirectly into the
> function name.

Went with the "DoNotUseDirectly" to avoid this being a live preference, even if only evaluated on start.

> 
> I would prefer making ImageMemShared an override pref
> (https://searchfox.org/mozilla-central/rev/
> 
> Ditto for the WebRenderBlobImages pref.

Done.

> >  
> >  #ifdef MOZ_ENABLE_WEBRENDER
> > +pref("gfx.webrender.all", true);
> 
> This is going to turn on ImageMemShared and acceleration for anybody who
> builds with --enable-webrender in their mozconfig, not sure if that's
> desirable or not.

Pulled that out.

Comment 7

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

Comment on attachment 8939535 [details]
Bug 1425260: gfx.webrender.all turns on all preferences that are needed for webrender.

https://reviewboard.mozilla.org/r/209858/#review217630

LGTM, thanks!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/89d7853edd64
gfx.webrender.all turns on all preferences that are needed for webrender. r=kats

Comment 9

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/89d7853edd64