Closed
Bug 789367
Opened 12 years ago
Closed 12 years ago
[Azure] Add content preference for GTK platform
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla18
People
(Reporter: ajones, Assigned: ajones)
References
Details
Attachments
(1 file, 4 obsolete files)
15.01 KB,
patch
|
Details | Diff | Splinter Review |
Add content preference for GTK platform that disables XRender surfaces when not using the cairo backend.
Assignee | ||
Comment 1•12 years ago
|
||
Assignee | ||
Comment 2•12 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=e0b8d99babe3
Assignee | ||
Updated•12 years ago
|
QA Contact: ajones
Assignee | ||
Updated•12 years ago
|
Attachment #659097 -
Flags: review?(roc)
Comment 3•12 years ago
|
||
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).
Yeah and I think we need a cross-platform pref for that.
Assignee | ||
Updated•12 years ago
|
Attachment #659097 -
Flags: review?(roc)
Assignee | ||
Comment 5•12 years ago
|
||
Attachment #659097 -
Attachment is obsolete: true
Attachment #660664 -
Flags: review?(ncameron)
Comment 6•12 years ago
|
||
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/ajones@mozilla.com-fa6e314ed62c
Comment 7•12 years ago
|
||
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
Assignee | ||
Comment 8•12 years ago
|
||
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 <ajones@mozilla.com> > >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);
Attachment #660664 -
Flags: review?(ncameron)
Comment 9•12 years ago
|
||
Did you mean to copy the entire patch as a comment? Is there something specific to look at?
Assignee | ||
Comment 10•12 years ago
|
||
Attachment #660664 -
Attachment is obsolete: true
Attachment #661022 -
Flags: review?(ncameron)
Comment 11•12 years ago
|
||
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/ajones@mozilla.com-258762a9b7a0
Assignee | ||
Comment 12•12 years ago
|
||
(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.
Assignee | ||
Comment 13•12 years ago
|
||
Attachment #661022 -
Attachment is obsolete: true
Attachment #661022 -
Flags: review?(ncameron)
Attachment #661040 -
Flags: review?(ncameron)
Comment 14•12 years ago
|
||
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/ajones@mozilla.com-38ba0fa17cc3
Comment 15•12 years ago
|
||
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
Attachment #661040 -
Flags: review?(ncameron) → review+
Comment 16•12 years ago
|
||
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/ajones@mozilla.com-fb34ca5b887b
Comment 17•12 years ago
|
||
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/ajones@mozilla.com-fb34ca5b887b
Assignee | ||
Comment 18•12 years ago
|
||
See Bug 786938 for the M5 failure in the Try result.
Keywords: checkin-needed
Comment 19•12 years ago
|
||
(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•12 years ago
|
||
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?
Assignee | ||
Comment 21•12 years ago
|
||
Attachment #661040 -
Attachment is obsolete: true
Assignee | ||
Comment 22•12 years ago
|
||
(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.
Assignee | ||
Comment 23•12 years ago
|
||
The v4 patch includes the changes that nrc requested and is the one that was tested on try.
Keywords: checkin-needed
Comment 24•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/08f87045d076
Keywords: checkin-needed
Comment 25•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/08f87045d076
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in
before you can comment on or make changes to this bug.
Description
•