789367 - [Azure] Add content preference for GTK platform

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Add content preference for GTK platform that disables XRender surfaces when not using the cairo backend.

Comment 1

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Attachment: Add content pref for GTK platform

Comment 2

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

https://tbpl.mozilla.org/?tree=Try&rev=e0b8d99babe3

Comment 3

[Nick Cameron [:nrc]]

I don't think you should use the canvas preferred backend for the content backend, you should add a pref for content backends, like canvas has (you won't need a fallback though).

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yeah and I think we need a cross-platform pref for that.

Comment 5

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Attachment: Add content pref in a more general sense similar to canvas

Comment 6

[Mozilla RelEng Bot]

Try run for fa6e314ed62c is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=fa6e314ed62c
Results (out of 50 total builds):
    exception: 11
    success: 23
    warnings: 4
    failure: 12
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-fa6e314ed62c

Comment 7

[Nick Cameron [:nrc]]

Comment on attachment 660664 [details] [diff] [review]
Add content pref in a more general sense similar to canvas

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

::: gfx/thebes/gfxPlatform.cpp
@@ +1184,5 @@
>      }
>  }
>  
>  void
> +gfxPlatform::InitCanvasBackend(uint32_t aCanvasBitmask, uint32_t aContentBitmask)

This is initialising more than the canvas backend, so the name should change

::: gfx/thebes/gfxPlatform.h
@@ +483,5 @@
>       * returns the first backend named in the pref gfx.canvas.azure.backends
>       * which is a component of aBackendBitmask, a bitmask of backend types
>       */
>      static mozilla::gfx::BackendType GetCanvasBackendPref(uint32_t aBackendBitmask);
> +    static mozilla::gfx::BackendType GetBackendPref(const char* aEnabledPref,

add a comment, maybe change aEnabledPref to aEnabledPrefName or something, and likewise for aBackendPref.

add a GetContentBackendPref method, or remove GetCanvasBackendPref

::: gfx/thebes/gfxPlatformGtk.cpp
@@ +147,5 @@
>      // in more context, including the display and/or target surface type that
>      // we should try to match
>      GdkScreen *gdkScreen = gdk_screen_get_default();
>      if (gdkScreen) {
> +        if (UseXRender() && (GetContentBackend() == BACKEND_NONE ||

nit: && => new line

::: modules/libpref/src/init/all.js
@@ +249,5 @@
>  pref("gfx.content.azure.enabled", true);
>  #else
>  #ifdef XP_MACOSX
>  pref("gfx.canvas.azure.backends", "cg");
> +pref("gfx.content.azure.backend", "cg");

if we are not supporting azure content on mac, then we should not have this pref

Comment 8

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Comment on attachment 660664 [details] [diff] [review]
Add content pref in a more general sense similar to canvas

>Bug 789367 - Add content preference for GTK platform
>
>From: Anthony Jones <[e-mail]>
>
>diff --git a/gfx/thebes/gfxPlatform.cpp b/gfx/thebes/gfxPlatform.cpp
>--- a/gfx/thebes/gfxPlatform.cpp
>+++ b/gfx/thebes/gfxPlatform.cpp
>@@ -223,18 +223,19 @@ gfxPlatform::gfxPlatform()
>     mDownloadableFontsSanitize = UNINITIALIZED_VALUE;
>     mFallbackUsesCmaps = UNINITIALIZED_VALUE;
> 
> #ifdef MOZ_GRAPHITE
>     mGraphiteShapingEnabled = UNINITIALIZED_VALUE;
> #endif
>     mBidiNumeralOption = UNINITIALIZED_VALUE;
> 
>-    uint32_t backendMask = (1 << BACKEND_CAIRO) | (1 << BACKEND_SKIA);
>-    InitCanvasBackend(backendMask);
>+    uint32_t canvasMask = (1 << BACKEND_CAIRO) | (1 << BACKEND_SKIA);
>+    uint32_t contentMask = 0;
>+    InitCanvasBackend(canvasMask, contentMask);
> }
> 
> gfxPlatform*
> gfxPlatform::GetPlatform()
> {
>     if (!gPlatform) {
>         Init();
>     }
>@@ -1179,35 +1180,40 @@ gfxPlatform::AppendPrefLang(eFontPrefLan
>     
>     if (i == aLen) {
>         aPrefLangs[aLen] = aAddLang;
>         aLen++;
>     }
> }
> 
> void
>-gfxPlatform::InitCanvasBackend(uint32_t aBackendBitmask)
>+gfxPlatform::InitCanvasBackend(uint32_t aCanvasBitmask, uint32_t aContentBitmask)
> {
>-    if (!Preferences::GetBool("gfx.canvas.azure.enabled", false)) {
>-        mPreferredCanvasBackend = BACKEND_NONE;
>-        mFallbackCanvasBackend = BACKEND_NONE;
>-        return;
>-    }
>-
>-    mPreferredCanvasBackend = GetCanvasBackendPref(aBackendBitmask);
>-    mFallbackCanvasBackend = GetCanvasBackendPref(aBackendBitmask & ~(1 << mPreferredCanvasBackend));
>+    mPreferredCanvasBackend = GetCanvasBackendPref(aCanvasBitmask);
>+    mFallbackCanvasBackend = GetCanvasBackendPref(aCanvasBitmask & ~(1 << mPreferredCanvasBackend));
>+    mContentBackend = GetBackendPref("gfx.content.azure.enabled", "gfx.content.azure.backend", aContentBitmask);
> }
> 
> /* static */ BackendType
> gfxPlatform::GetCanvasBackendPref(uint32_t aBackendBitmask)
> {
>+    return GetBackendPref("gfx.canvas.azure.enabled", "gfx.canvas.azure.backends", aBackendBitmask);
>+}
>+
>+/* static */ BackendType
>+gfxPlatform::GetBackendPref(const char* aEnabledPref, const char* aBackendPref, uint32_t aBackendBitmask)
>+{
>+    if (!Preferences::GetBool(aEnabledPref, false)) {
>+        return BACKEND_NONE;
>+    }
>+
>     if (!gBackendList) {
>         gBackendList = new nsTArray<nsCString>();
>         nsCString prefString;
>-        if (NS_SUCCEEDED(Preferences::GetCString("gfx.canvas.azure.backends", &prefString))) {
>+        if (NS_SUCCEEDED(Preferences::GetCString(aBackendPref, &prefString))) {
>             ParseString(prefString, ',', *gBackendList);
>         }
>     }
> 
>     for (uint32_t i = 0; i < gBackendList->Length(); ++i) {
>         BackendType result = BackendTypeForName((*gBackendList)[i]);
>         if ((1 << result) & aBackendBitmask) {
>             return result;
>diff --git a/gfx/thebes/gfxPlatform.h b/gfx/thebes/gfxPlatform.h
>--- a/gfx/thebes/gfxPlatform.h
>+++ b/gfx/thebes/gfxPlatform.h
>@@ -197,17 +197,17 @@ public:
>       CreateDrawTargetForData(unsigned char* aData, const mozilla::gfx::IntSize& aSize, 
>                               int32_t aStride, mozilla::gfx::SurfaceFormat aFormat);
> 
>     bool SupportsAzureCanvas();
> 
>     void GetAzureBackendInfo(mozilla::widget::InfoObject &aObj) {
>       aObj.DefineProperty("AzureCanvasBackend", GetBackendName(mPreferredCanvasBackend));
>       aObj.DefineProperty("AzureFallbackCanvasBackend", GetBackendName(mFallbackCanvasBackend));
>-      aObj.DefineProperty("AzureContentBackend", GetBackendName(GetContentBackend()));
>+      aObj.DefineProperty("AzureContentBackend", GetBackendName(mContentBackend));
>     }
> 
>     mozilla::gfx::BackendType GetPreferredCanvasBackend() {
>       return mPreferredCanvasBackend;
>     }
> 
>     /*
>      * Font bits
>@@ -473,27 +473,29 @@ protected:
>                                  mozilla::gfx::SurfaceFormat aFormat);
> 
>     /**
>      * Initialise the preferred and fallback canvas backends
>      * aBackendBitmask specifies the backends which are acceptable to the caller.
>      * The backend used is determined by aBackendBitmask and the order specified
>      * by the gfx.canvas.azure.backends pref.
>      */
>-    void InitCanvasBackend(uint32_t aBackendBitmask);
>+    void InitCanvasBackend(uint32_t aCanvasBitmask, uint32_t aContentBitmask);
>     /**
>      * returns the first backend named in the pref gfx.canvas.azure.backends
>      * which is a component of aBackendBitmask, a bitmask of backend types
>      */
>     static mozilla::gfx::BackendType GetCanvasBackendPref(uint32_t aBackendBitmask);
>+    static mozilla::gfx::BackendType GetBackendPref(const char* aEnabledPref,
>+                                                    const char* aBackendPref,
>+                                                    uint32_t aBackendBitmask);
>     static mozilla::gfx::BackendType BackendTypeForName(const nsCString& aName);
> 
>-    virtual mozilla::gfx::BackendType GetContentBackend()
>-    {
>-      return mozilla::gfx::BACKEND_NONE;
>+    mozilla::gfx::BackendType GetContentBackend() {
>+      return mContentBackend;
>     }
> 
>     int8_t  mAllowDownloadableFonts;
>     int8_t  mDownloadableFontsSanitize;
> #ifdef MOZ_GRAPHITE
>     int8_t  mGraphiteShapingEnabled;
> #endif
> 
>@@ -518,14 +520,16 @@ private:
>     nsTArray<uint32_t> mCJKPrefLangs;
>     nsCOMPtr<nsIObserver> mSRGBOverrideObserver;
>     nsCOMPtr<nsIObserver> mFontPrefsObserver;
> 
>     // The preferred draw target backend to use for canvas
>     mozilla::gfx::BackendType mPreferredCanvasBackend;
>     // The fallback draw target backend to use for canvas, if the preferred backend fails
>     mozilla::gfx::BackendType mFallbackCanvasBackend;
>+    // The backend to use for content
>+    mozilla::gfx::BackendType mContentBackend;
> 
>     mozilla::widget::GfxInfoCollector<gfxPlatform> mAzureCanvasBackendCollector;
>     bool mWorkAroundDriverBugs;
> };
> 
> #endif /* GFX_PLATFORM_H */
>diff --git a/gfx/thebes/gfxPlatformGtk.cpp b/gfx/thebes/gfxPlatformGtk.cpp
>--- a/gfx/thebes/gfxPlatformGtk.cpp
>+++ b/gfx/thebes/gfxPlatformGtk.cpp
>@@ -88,16 +88,19 @@ gfxPlatformGtk::gfxPlatformGtk()
>     gPlatformFonts->Init(100);
>     gPlatformFontAliases = new FontTable();
>     gPlatformFontAliases->Init(100);
>     gPrefFonts = new PrefFontTable();
>     gPrefFonts->Init(100);
>     gCodepointsWithNoFonts = new gfxSparseBitSet();
>     UpdateFontList();
> #endif
>+    uint32_t canvasMask = (1 << BACKEND_CAIRO) | (1 << BACKEND_SKIA);
>+    uint32_t contentMask = (1 << BACKEND_CAIRO);
>+    InitCanvasBackend(canvasMask, contentMask);
> }
> 
> gfxPlatformGtk::~gfxPlatformGtk()
> {
>     gfxFontconfigUtils::Shutdown();
>     sFontconfigUtils = nullptr;
> 
> #ifdef MOZ_PANGO
>@@ -140,32 +143,33 @@ gfxPlatformGtk::CreateOffscreenSurface(c
>     bool needsClear = true;
>     gfxASurface::gfxImageFormat imageFormat = OptimalFormatForContent(contentType);
> #ifdef MOZ_X11
>     // XXX we really need a different interface here, something that passes
>     // in more context, including the display and/or target surface type that
>     // we should try to match
>     GdkScreen *gdkScreen = gdk_screen_get_default();
>     if (gdkScreen) {
>-        if (!UseXRender()) {
>-            // We're not going to use XRender, so we don't need to
>-            // search for a render format
>-            newSurface = new gfxImageSurface(size, imageFormat);
>-            // The gfxImageSurface ctor zeroes this for us, no need to
>-            // waste time clearing again
>-            needsClear = false;
>-        } else {
>+        if (UseXRender() && (GetContentBackend() == BACKEND_NONE ||
>+                             GetContentBackend() == BACKEND_CAIRO)) {
>             Screen *screen = gdk_x11_screen_get_xscreen(gdkScreen);
>             XRenderPictFormat* xrenderFormat =
>                 gfxXlibSurface::FindRenderFormat(DisplayOfScreen(screen),
>                                                  imageFormat);
> 
>             if (xrenderFormat) {
>                 newSurface = gfxXlibSurface::Create(screen, xrenderFormat, size);
>             }
>+        } else {
>+            // We're not going to use XRender, so we don't need to
>+            // search for a render format
>+            newSurface = new gfxImageSurface(size, imageFormat);
>+            // The gfxImageSurface ctor zeroes this for us, no need to
>+            // waste time clearing again
>+            needsClear = false;
>         }
>     }
> #endif
> 
>     if (!newSurface) {
>         // We couldn't create a native surface for whatever reason;
>         // e.g., no display, no RENDER, bad size, etc.
>         // Fall back to image surface for the data.
>diff --git a/gfx/thebes/gfxWindowsPlatform.cpp b/gfx/thebes/gfxWindowsPlatform.cpp
>--- a/gfx/thebes/gfxWindowsPlatform.cpp
>+++ b/gfx/thebes/gfxWindowsPlatform.cpp
>@@ -511,23 +511,26 @@ gfxWindowsPlatform::UpdateRenderMode()
>             SetupClearTypeParams();
> 
>             if (hr == S_OK)
>               reporter.SetSuccessful();
>         }
>     }
> #endif
> 
>-    uint32_t backendMask = 1 << BACKEND_CAIRO;
>+    uint32_t canvasMask = 1 << BACKEND_CAIRO;
>+    uint32_t contentMask;
>     if (mRenderMode == RENDER_DIRECT2D) {
>-      backendMask |= 1 << BACKEND_DIRECT2D;
>+      canvasMask |= 1 << BACKEND_DIRECT2D;
>+      contentMask = BACKEND_DIRECT2D;
>     } else {
>-      backendMask |= 1 << BACKEND_SKIA;
>+      canvasMask |= 1 << BACKEND_SKIA;
>+      contentMask = 0;
>     }
>-    InitCanvasBackend(backendMask);
>+    InitCanvasBackend(canvasMask, contentMask);
> }
> 
> void
> gfxWindowsPlatform::VerifyD2DDevice(bool aAttemptForce)
> {
> #ifdef CAIRO_HAS_D2D_SURFACE
>     if (mD2DDevice) {
>         ID3D10Device1 *device = cairo_d2d_device_get_device(mD2DDevice);
>diff --git a/gfx/thebes/gfxWindowsPlatform.h b/gfx/thebes/gfxWindowsPlatform.h
>--- a/gfx/thebes/gfxWindowsPlatform.h
>+++ b/gfx/thebes/gfxWindowsPlatform.h
>@@ -238,23 +238,16 @@ public:
>     cairo_device_t *GetD2DDevice() { return mD2DDevice; }
>     ID3D10Device1 *GetD3D10Device() { return mD2DDevice ? cairo_d2d_device_get_device(mD2DDevice) : nullptr; }
> #endif
> 
>     static bool IsOptimus();
>     static bool IsRunningInWindows8Metro();
> 
> protected:
>-    virtual mozilla::gfx::BackendType GetContentBackend()
>-    {
>-      return UseAzureContentDrawing() && mRenderMode == RENDER_DIRECT2D ?
>-               mozilla::gfx::BACKEND_DIRECT2D :
>-               mozilla::gfx::BACKEND_NONE;
>-    }
>-
>     RenderMode mRenderMode;
> 
>     int8_t mUseClearTypeForDownloadableFonts;
>     int8_t mUseClearTypeAlways;
>     HDC mScreenDC;
> 
> private:
>     void Init();
>diff --git a/modules/libpref/src/init/all.js b/modules/libpref/src/init/all.js
>--- a/modules/libpref/src/init/all.js
>+++ b/modules/libpref/src/init/all.js
>@@ -240,27 +240,27 @@ pref("gfx.font_rendering.directwrite.use
> 
> pref("gfx.font_rendering.opentype_svg.enabled", false);
> 
> pref("gfx.canvas.azure.enabled", true);
> #ifdef XP_WIN
> // comma separated list of backends to use in order of preference
> // e.g., pref("gfx.canvas.azure.backends", "direct2d,skia,cairo");
> pref("gfx.canvas.azure.backends", "direct2d,cairo");
>+pref("gfx.content.azure.backend", "direct2d");
> pref("gfx.content.azure.enabled", true);
> #else
> #ifdef XP_MACOSX
> pref("gfx.canvas.azure.backends", "cg");
>-#else
>-#ifdef ANDROID
>-pref("gfx.canvas.azure.backends", "cairo");
>+pref("gfx.content.azure.backend", "cg");
> #else
> pref("gfx.canvas.azure.backends", "cairo");
>+pref("gfx.content.azure.backend", "cairo");
> #endif
>-#endif
>+pref("gfx.content.azure.enabled", false);
> #endif
> 
> #ifdef ANDROID
> pref("gfx.textures.poweroftwo.force-enabled", false);
> #endif
> 
> pref("gfx.work-around-driver-bugs", true);
> pref("gfx.prefer-mesa-llvmpipe", false);

Comment 9

[Nick Cameron [:nrc]]

Did you mean to copy the entire patch as a comment? Is there something specific to look at?

Comment 10

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Attachment: Content pref to match the canvas pref v2

Comment 11

[Mozilla RelEng Bot]

Try run for 258762a9b7a0 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=258762a9b7a0
Results (out of 6 total builds):
    exception: 5
    failure: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-258762a9b7a0

Comment 12

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

(In reply to Nick Cameron [:nrc] from comment #9)
> Did you mean to copy the entire patch as a comment?

I don't know how that happened.

Comment 13

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Attachment: Content pref to match the canvas pref v3

Comment 14

[Mozilla RelEng Bot]

Try run for 38ba0fa17cc3 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=38ba0fa17cc3
Results (out of 54 total builds):
    exception: 16
    success: 26
    warnings: 8
    failure: 4
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-38ba0fa17cc3

Comment 15

[Nick Cameron [:nrc]]

Comment on attachment 661040 [details] [diff] [review]
Content pref to match the canvas pref v3

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

r+ with the nit below addressed

::: modules/libpref/src/init/all.js
@@ +254,2 @@
>  pref("gfx.canvas.azure.backends", "cairo");
> +pref("gfx.content.azure.backend", "cairo");

Rename this gfx.content.azure.backends, since it can take multiple backend names like canvas can

Comment 16

[Mozilla RelEng Bot]

Try run for fb34ca5b887b is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=fb34ca5b887b
Results (out of 93 total builds):
    exception: 2
    success: 76
    warnings: 15
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-fb34ca5b887b

Comment 17

[Mozilla RelEng Bot]

Try run for fb34ca5b887b is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=fb34ca5b887b
Results (out of 95 total builds):
    exception: 2
    success: 76
    warnings: 17
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-fb34ca5b887b

Comment 18

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

See Bug 786938 for the M5 failure in the Try result.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

(In reply to Mozilla RelEng Bot from comment #17)
>     https://tbpl.mozilla.org/?tree=Try&rev=fb34ca5b887b

Green on Try.

https://hg.mozilla.org/integration/mozilla-inbound/rev/9805ecdb8f66

Comment 20

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/3435b7e0ecc5 since the build was busted on every platform - did the world change out from under you?

Comment 21

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Attachment: Content pref to match the canvas pref v4

Comment 22

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

(In reply to Phil Ringnalda (:philor) from comment #20)
> Backed out in
> https://hg.mozilla.org/integration/mozilla-inbound/rev/3435b7e0ecc5 since
> the build was busted on every platform - did the world change out from under
> you?

It turns out that I suck at uploading patches.

Comment 23

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

The v4 patch includes the changes that nrc requested and is the one that was tested on try.

Comment 24

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/08f87045d076

Comment 25

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/08f87045d076