From 0afcd4879bd4ce369b53458b40ad70d1ef137214 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Wed, 8 Sep 2021 12:41:51 +0000 Subject: [PATCH] Bug 1729441 - Add a pref to set the blob tile size. r=gfx-reviewers,bradwerth The prefs name is gfx.webrender.blob-tile-size and can be changed at runtime. It changes the behavior of a wrench reftest that was ensuring that we don't crash with unreasonable tile sizes. The new behavior (sanitizing the tile size) means we can render the image while we would previously skip it. Differential Revision: https://phabricator.services.mozilla.com/D124789 --- gfx/config/gfxVars.h | 1 + gfx/layers/ipc/CompositorBridgeParent.cpp | 13 +++++++------ gfx/layers/ipc/CompositorBridgeParent.h | 4 ++-- gfx/layers/wr/WebRenderBridgeParent.cpp | 8 ++++++-- gfx/layers/wr/WebRenderBridgeParent.h | 3 ++- gfx/thebes/gfxPlatform.cpp | 16 ++++++++++++++++ gfx/webrender_bindings/WebRenderAPI.cpp | 5 +++-- gfx/webrender_bindings/WebRenderAPI.h | 2 +- gfx/webrender_bindings/src/bindings.rs | 3 ++- gfx/wr/webrender/src/resource_cache.rs | 5 +++++ .../reftests/image/very-big-tile-size-ref.yaml | 2 +- modules/libpref/init/StaticPrefList.yaml | 5 +++++ 12 files changed, 51 insertions(+), 16 deletions(-) diff --git a/gfx/config/gfxVars.h b/gfx/config/gfxVars.h index 79dd31d577e8..64b463826ea3 100644 --- a/gfx/config/gfxVars.h +++ b/gfx/config/gfxVars.h @@ -56,6 +56,7 @@ class gfxVarReceiver; _(WebRenderMaxPartialPresentRects, int32_t, 0) \ _(WebRenderDebugFlags, int32_t, 0) \ _(WebRenderBatchingLookback, int32_t, 10) \ + _(WebRenderBlobTileSize, int32_t, 256) \ _(UseSoftwareWebRender, bool, false) \ _(AllowSoftwareWebRenderD3D11, bool, false) \ _(ScreenDepth, int32_t, 0) \ diff --git a/gfx/layers/ipc/CompositorBridgeParent.cpp b/gfx/layers/ipc/CompositorBridgeParent.cpp index 465635c997c3..0677c74f5646 100644 --- a/gfx/layers/ipc/CompositorBridgeParent.cpp +++ b/gfx/layers/ipc/CompositorBridgeParent.cpp @@ -1334,8 +1334,9 @@ void CompositorBridgeParent::InitializeStatics() { gfxVars::SetWebRenderDebugFlagsListener(&UpdateDebugFlags); gfxVars::SetUseWebRenderMultithreadingListener( &UpdateWebRenderMultithreading); - gfxVars::SetWebRenderBatchingLookbackListener( - &UpdateWebRenderBatchingParameters); + gfxVars::SetWebRenderBatchingLookbackListener(&UpdateWebRenderParameters); + gfxVars::SetWebRenderBlobTileSizeListener(&UpdateWebRenderParameters); + gfxVars::SetWebRenderProfilerUIListener(&UpdateWebRenderProfilerUI); } @@ -1398,12 +1399,12 @@ void CompositorBridgeParent::UpdateWebRenderMultithreading() { } /*static*/ -void CompositorBridgeParent::UpdateWebRenderBatchingParameters() { +void CompositorBridgeParent::UpdateWebRenderParameters() { if (!CompositorThreadHolder::IsInCompositorThread()) { if (CompositorThread()) { CompositorThread()->Dispatch(NewRunnableFunction( - "CompositorBridgeParent::UpdateWebRenderBatchingParameters", - &CompositorBridgeParent::UpdateWebRenderBatchingParameters)); + "CompositorBridgeParent::UpdateWebRenderParameters", + &CompositorBridgeParent::UpdateWebRenderParameters)); } return; @@ -1411,7 +1412,7 @@ void CompositorBridgeParent::UpdateWebRenderBatchingParameters() { MonitorAutoLock lock(*sIndirectLayerTreesLock); ForEachWebRenderBridgeParent([&](WebRenderBridgeParent* wrBridge) -> void { - wrBridge->UpdateBatchingParameters(); + wrBridge->UpdateParameters(); }); } diff --git a/gfx/layers/ipc/CompositorBridgeParent.h b/gfx/layers/ipc/CompositorBridgeParent.h index 14b34232acbd..dde13eca97ee 100644 --- a/gfx/layers/ipc/CompositorBridgeParent.h +++ b/gfx/layers/ipc/CompositorBridgeParent.h @@ -619,9 +619,9 @@ class CompositorBridgeParent final : public CompositorBridgeParentBase, static void UpdateWebRenderMultithreading(); /** - * Notify the compositor webrender batching parameters have been updated. + * Notify the compositor some webrender parameters have been updated. */ - static void UpdateWebRenderBatchingParameters(); + static void UpdateWebRenderParameters(); /** * Notify the compositor webrender profiler UI string has been updated. diff --git a/gfx/layers/wr/WebRenderBridgeParent.cpp b/gfx/layers/wr/WebRenderBridgeParent.cpp index 18b27197f291..01638bd82ad2 100644 --- a/gfx/layers/wr/WebRenderBridgeParent.cpp +++ b/gfx/layers/wr/WebRenderBridgeParent.cpp @@ -336,6 +336,7 @@ WebRenderBridgeParent::WebRenderBridgeParent( #if defined(MOZ_WIDGET_ANDROID) mScreenPixelsTarget(nullptr), #endif + mBlobTileSize(256), mDestroyed(false), mReceivedDisplayList(false), mIsFirstPaint(true), @@ -555,7 +556,8 @@ bool WebRenderBridgeParent::UpdateResources( gfxCriticalNote << "TOpAddBlobImage failed"; return false; } - aUpdates.AddBlobImage(op.key(), op.descriptor(), bytes, + + aUpdates.AddBlobImage(op.key(), op.descriptor(), mBlobTileSize, bytes, wr::ToDeviceIntRect(op.visibleRect())); break; } @@ -1653,9 +1655,11 @@ void WebRenderBridgeParent::UpdateMultithreading() { mApi->EnableMultithreading(gfxVars::UseWebRenderMultithreading()); } -void WebRenderBridgeParent::UpdateBatchingParameters() { +void WebRenderBridgeParent::UpdateParameters() { uint32_t count = gfxVars::WebRenderBatchingLookback(); mApi->SetBatchingLookback(count); + + mBlobTileSize = gfxVars::WebRenderBlobTileSize(); } #if defined(MOZ_WIDGET_ANDROID) diff --git a/gfx/layers/wr/WebRenderBridgeParent.h b/gfx/layers/wr/WebRenderBridgeParent.h index 789797b91efb..67cc6b2d14d0 100644 --- a/gfx/layers/wr/WebRenderBridgeParent.h +++ b/gfx/layers/wr/WebRenderBridgeParent.h @@ -93,7 +93,7 @@ class WebRenderBridgeParent final : public PWebRenderBridgeParent, void UpdateQualitySettings(); void UpdateDebugFlags(); void UpdateMultithreading(); - void UpdateBatchingParameters(); + void UpdateParameters(); void UpdateProfilerUI(); mozilla::ipc::IPCResult RecvEnsureConnected( @@ -502,6 +502,7 @@ class WebRenderBridgeParent final : public PWebRenderBridgeParent, #if defined(MOZ_WIDGET_ANDROID) UiCompositorControllerParent* mScreenPixelsTarget; #endif + uint16_t mBlobTileSize; bool mDestroyed; bool mReceivedDisplayList; bool mIsFirstPaint; diff --git a/gfx/thebes/gfxPlatform.cpp b/gfx/thebes/gfxPlatform.cpp index d86a73af96fc..31fffdb654e2 100644 --- a/gfx/thebes/gfxPlatform.cpp +++ b/gfx/thebes/gfxPlatform.cpp @@ -556,6 +556,13 @@ static void WebRenderBatchingPrefChangeCallback(const char* aPrefName, void*) { gfx::gfxVars::SetWebRenderBatchingLookback(count); } +static void WebRenderBlobTileSizePrefChangeCallback(const char* aPrefName, + void*) { + uint32_t tileSize = Preferences::GetUint( + StaticPrefs::GetPrefName_gfx_webrender_blob_tile_size(), 256); + gfx::gfxVars::SetWebRenderBlobTileSize(tileSize); +} + static uint32_t GetSkiaGlyphCacheSize() { // Only increase font cache size on non-android to save memory. #if !defined(MOZ_WIDGET_ANDROID) @@ -1272,6 +1279,10 @@ void gfxPlatform::ShutdownLayersIPC() { WR_DEBUG_PREF); Preferences::UnregisterCallback(WebRendeProfilerUIPrefChangeCallback, "gfx.webrender.debug.profiler-ui"); + Preferences::UnregisterCallback( + WebRenderBlobTileSizePrefChangeCallback, + nsDependentCString( + StaticPrefs::GetPrefName_gfx_webrender_blob_tile_size())); } } else { @@ -2611,6 +2622,11 @@ void gfxPlatform::InitWebRenderConfig() { nsDependentCString( StaticPrefs::GetPrefName_gfx_webrender_batching_lookback())); + Preferences::RegisterCallbackAndCall( + WebRenderBlobTileSizePrefChangeCallback, + nsDependentCString( + StaticPrefs::GetPrefName_gfx_webrender_blob_tile_size())); + if (WebRenderResourcePathOverride()) { CrashReporter::AnnotateCrashReport( CrashReporter::Annotation::IsWebRenderResourcePathOverridden, true); diff --git a/gfx/webrender_bindings/WebRenderAPI.cpp b/gfx/webrender_bindings/WebRenderAPI.cpp index 63bd304e4174..f0956662383d 100644 --- a/gfx/webrender_bindings/WebRenderAPI.cpp +++ b/gfx/webrender_bindings/WebRenderAPI.cpp @@ -817,10 +817,11 @@ void TransactionBuilder::AddImage(ImageKey key, void TransactionBuilder::AddBlobImage(BlobImageKey key, const ImageDescriptor& aDescriptor, + uint16_t aTileSize, wr::Vec& aBytes, const wr::DeviceIntRect& aVisibleRect) { - wr_resource_updates_add_blob_image(mTxn, key, &aDescriptor, &aBytes.inner, - aVisibleRect); + wr_resource_updates_add_blob_image(mTxn, key, &aDescriptor, aTileSize, + &aBytes.inner, aVisibleRect); } void TransactionBuilder::AddExternalImage(ImageKey key, diff --git a/gfx/webrender_bindings/WebRenderAPI.h b/gfx/webrender_bindings/WebRenderAPI.h index 4818b5228de3..c1f83b03d881 100644 --- a/gfx/webrender_bindings/WebRenderAPI.h +++ b/gfx/webrender_bindings/WebRenderAPI.h @@ -140,7 +140,7 @@ class TransactionBuilder final { wr::Vec& aBytes); void AddBlobImage(wr::BlobImageKey aKey, const ImageDescriptor& aDescriptor, - wr::Vec& aBytes, + uint16_t aTileSize, wr::Vec& aBytes, const wr::DeviceIntRect& aVisibleRect); void AddExternalImageBuffer(ImageKey key, const ImageDescriptor& aDescriptor, diff --git a/gfx/webrender_bindings/src/bindings.rs b/gfx/webrender_bindings/src/bindings.rs index a05631e0e9a1..37aced0393d6 100644 --- a/gfx/webrender_bindings/src/bindings.rs +++ b/gfx/webrender_bindings/src/bindings.rs @@ -2003,6 +2003,7 @@ pub extern "C" fn wr_resource_updates_add_blob_image( txn: &mut Transaction, image_key: BlobImageKey, descriptor: &WrImageDescriptor, + tile_size: u16, bytes: &mut WrVecU8, visible_rect: DeviceIntRect, ) { @@ -2012,7 +2013,7 @@ pub extern "C" fn wr_resource_updates_add_blob_image( Arc::new(bytes.flush_into_vec()), visible_rect, if descriptor.format == ImageFormat::BGRA8 { - Some(256) + Some(tile_size) } else { None }, diff --git a/gfx/wr/webrender/src/resource_cache.rs b/gfx/wr/webrender/src/resource_cache.rs index d5a86f2d8828..de1c8d6584cb 100644 --- a/gfx/wr/webrender/src/resource_cache.rs +++ b/gfx/wr/webrender/src/resource_cache.rs @@ -780,6 +780,11 @@ impl ResourceCache { visible_rect: &DeviceIntRect, mut tiling: Option, ) { + if let Some(ref mut tile_size) = tiling { + // Sanitize the value since it can be set by a pref. + *tile_size = (*tile_size).max(16).min(2048); + } + if tiling.is_none() && Self::should_tile(self.tiling_threshold(), &descriptor, &data) { // We aren't going to be able to upload a texture this big, so tile it, even // if tiling was not requested. diff --git a/gfx/wr/wrench/reftests/image/very-big-tile-size-ref.yaml b/gfx/wr/wrench/reftests/image/very-big-tile-size-ref.yaml index 5001c2812c48..6d7ea1b4b1cf 100644 --- a/gfx/wr/wrench/reftests/image/very-big-tile-size-ref.yaml +++ b/gfx/wr/wrench/reftests/image/very-big-tile-size-ref.yaml @@ -2,4 +2,4 @@ root: items: - type: rect bounds: 0 0 500 500 - color: green + color: red diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index 11040d235187..9a792daca6e0 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/modules/libpref/init/StaticPrefList.yaml @@ -5188,6 +5188,11 @@ value: 10 mirror: always +- name: gfx.webrender.blob-tile-size + type: uint32_t + value: 256 + mirror: always + - name: gfx.webrender.compositor type: bool #if defined(XP_WIN) || defined(XP_MACOSX)