- Move AutoLock into RetainGLTexture().
- Add Autolock in ~GLTextureIRepresentation() and
~SkiaGaneshRepresentation() for deleting IOSurfaceBackingEGLState.
IOSurfaceImageBacking::IOSurfaceBackingEGLStateBeingDestroyed() has
AssertLockAcquired() and |lock_| needs to be hold from the caller.
Bug: 405946930
Change-Id: I59360c3f44f7673adfa84dfd1566ddfb4e17eb96
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6485629
Commit-Queue: Maggie Chen <magchen@chromium.org>
Reviewed-by: vikas soni <vikassoni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1451543}
DumpBackgroundGraphiteMemoryStatistics() is called from
SharedContextState(). In the new thread safe architecture where two
threads (or two SharedContextStates) share the same graphite context,
CHECK is triggered in CreateAllocatorDump() when a duplicate name is
used for Create. The solution here is to skip the second identical
graphite context dump when an existing MemoryAllocatorDump is detected.
Bug: 407874799
Change-Id: I102d2374eab27396992af734a047e7b629fd3126
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6485454
Commit-Queue: Maggie Chen <magchen@chromium.org>
Reviewed-by: vikas soni <vikassoni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1451414}
The toggle was enabled for only Samsung devices due to required
texture memory size being larger than AHardwareBuffer allocation size.[1]
This led to SharedTextureMemory creation failure which led to
failures when beginning to read access.
We know that one of the reasons for fallback textures being created
more often with Graphite than Ganesh is FailedBeginReadAccess.[2]
Also from recent crashes in crbug.com/411835797 we see a variety of
devices crashing on same codepath with failure to BeginAccess.
This change enabled the toggle for all devices on Android so the
VkImage size check in Dawn is skipped.
[1] crbug.com/377489264#comment2
[2] https://uma.googleplex.com/p/chrome/timeline_v2?sid=f5e0dcaab32e98d5a1d55c6c8e0f5471
Bug: 383528569, 411835797
Change-Id: I86daf9d3b00e724cf225d231191837b6f5cbfd00
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6483711
Commit-Queue: Saifuddin Hitawala <hitawala@chromium.org>
Reviewed-by: Bo Liu <boliu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1451283}
This reverts commit 5170aa7c3e.
This reverts part of commit f5a5655498.
Reason for revert: Lack of waitUntilScheduled causes WebGL test
failures.
Changes over a plain revert:
1) Update some comments to say why the original optimization isn't
possible.
2) Revert the glFlush in CreateMetalSharedEvent that was needed to fix
issues with the original CL and is now unnecessary.
Bug: 40273077, 412679364
Original change's description:
> [gpu] Do not waitUntilScheduled on same Metal device
>
> Skip waitUntilScheduled calls when waiting on the same MTLDevice as the
> pending flush. In particular, this means that on single GPU systems, we
> will never call waitUntilScheduled except when handing off the IOSurface
> to CoreAnimation.
>
> Bug: 40273077
> Change-Id: I7c3c8ce30f061caf217d8107f12b0bc243b2cb49
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6464979
> Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
> Commit-Queue: Maggie Chen <magchen@chromium.org>
> Auto-Submit: Sunny Sachanandani <sunnyps@chromium.org>
> Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
> Reviewed-by: Maggie Chen <magchen@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1448456}
Bug: 40273077
Change-Id: Iea0008e00d16e87dc0525b73c824e50128e8f4da
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6485450
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Kenneth Russell <kbr@chromium.org>
Auto-Submit: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1450889}
For SwANGLE, we need to ReleaseTexImage after the last GL access is done
to copy from Swiftshader's shadow texture memory to the IOSurface. Today
this looks at `num_ongoing_read_accesses_` in the backing, but this is
wrong since it considers all kinds of accesses. It's not necessary that
the last access being dropped is a GL access so it's possible that the
SwANGLE ReleaseTexImage never runs because the last GL access is not
the last read access overall.
This CL fixes that by tracking a separate `num_ongoing_accesses` in the
IOSurfaceBackingEGLState and using that for calling ReleaseTexImage.
Additionally, it makes `num_ongoing_read_accesses` a signed int instead
of unsigned int so that it's possible to assert that is >= 0 always - we
generally want to avoid unsigned types unless it's for serialization or
interop with other systems per the style guide.
Also, clean up some of the surrounding code a bit and remove a legacy
DCHECK from the GLImageBacking days.
Bug: 412679361, 40273077, 412679364
Change-Id: Idd06af0a8eef35338934fbddf1a4c8f8cf1af989
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6480226
Auto-Submit: Sunny Sachanandani <sunnyps@chromium.org>
Commit-Queue: Saifuddin Hitawala <hitawala@chromium.org>
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Saifuddin Hitawala <hitawala@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1450750}
Found while investigating another issue: we only use the texture base
`usage` as the key in DawnSharedTextureCache - we should also take into
account `internal_usage` and `view_formats` since those can vary between
BeginAccess calls and we don't want to return the same wgpu::Texture if
those change.
This CL makes RemoveWGPUTextureFromCache and DestroyWGPUTextureIfCached
slightly slower since they now scan the entire map for the texture, but
it's not going to be an issue in practice since we only have a handful
of cached textures at most.
Bug: 412679361, 40273077
Change-Id: Ie7824fd52939391bea0a98ddaf0d1c3a4625140d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6483991
Auto-Submit: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Saifuddin Hitawala <hitawala@chromium.org>
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1450685}
After https://crrev.com/c/6464979, we don't call waitUntilScheduled when
the waiting MTLDevice is the same as the WGPUDevice/EGLDisplay that's
pending flush. This seems to have caused some test failures with Ganesh
like Pixel_WebGPUCopyExternalImageWebGPUCanvas which indicates broken
synchronization between ANGLE and Dawn.
One possible issue is that since we don't always flush the GL context
in GL EndAccess, the shared event signal that was just enqueued wouldn't
ever be executed causing the next Dawn BeginAccess to encode a shared
event wait that will never be executed.
This CL attempts to fix that by making GLDisplayEGL always flush the GL
context after it enqueues a shared event signal so that we don't ever
encounter a shared event with an enqueued but not flushed signal value.
This allows us to get rid of the ReleaseTexImage call for Graphite in GL
EndAccess since the only reason for that call was to flush the context.
Bug: 412679361, 40273077
Change-Id: I8f4e838d99185d264e3f40ceea377eaf137ceaac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6480877
Auto-Submit: Sunny Sachanandani <sunnyps@chromium.org>
Commit-Queue: Saifuddin Hitawala <hitawala@chromium.org>
Reviewed-by: Saifuddin Hitawala <hitawala@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1450625}
Instead of retry with OpenGLES Compatibility mode, just skip the
test if the adapter of Core mode is not available. The Dawn
OpenGLES backend is currently not thread-safe.
Tested with the try bot gpu-fyi-try-android-m-nexus-5x-64
Bug: 407874799, 411656649
Change-Id: I228b19b00fbb56c20305189280041ddfd6ecf187
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6480813
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Commit-Queue: Maggie Chen <magchen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1450172}
This is a reland of commit a698becfd2
gpu_unittests fails on Android device Nexus 5X with the previous CL. It
could not find a dawn adaptor with the given Vulkan backend type. This
CL will retry with a different backend OpenGLES for old Android
phones if no adapter is found.
Tested with the try bot gpu-fyi-try-android-m-nexus-5x-64
Original change's description:
> Add GraphiteSharedContext thread safe unit tests
>
> GraphiteSharedContext is a wrapper class with AutoLock on top of
> graphite::context. The unit tests test the thread safe functions on
> graphite::context
>
> Bug: 407874799
> Change-Id: I66796392b6a2e3411697b3e4c9fcd30092dfd55d
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6465024
> Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
> Commit-Queue: Maggie Chen <magchen@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1448621}
Bug: 407874799, 411656649
Change-Id: I56709d18a71907739be1dbdfb30846df7aaed5d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6476632
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Commit-Queue: Maggie Chen <magchen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1450028}
Ganesh shader cache purges cache memory on memory pressure that is
currently not implemented for graphite shader cache. This change
adds similar support that should make the functionality equivalent
between the two skia backends. Also, move the common code to helper
method in service utils.
NOTE: This is currently only supported for GraphiteDawn. We can add
support for WebGPU dawn as well in a follow-up.
Bug: 408234440
Change-Id: I3d622f4a9a0ddcf3e665341b6e11e0d99d326363
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6457634
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Commit-Queue: Saifuddin Hitawala <hitawala@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1449949}
When feature kGraphiteContextIsThreadSafe is enabled,
GraphiteSharedContext is shared by all threads and is owned by
DawnSharedContext. Therefore, move the creation into DawnSharedContext
to prevent from being created more than once.
Bug: 407874799
Change-Id: I74737d03cb449710e750c1d35d25c74595f3eca7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6471450
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Commit-Queue: Maggie Chen <magchen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1449929}
If the Redirection bitmap is removed from the main window, ANGLE can't
render content when not using DComp. The reason being that ANGLE
EGLSurface swap chains use a bitBlit approach. The solution to this
problem is to supply a child hwnd with a Redirection bitmap.
Bug: 400454999
Change-Id: Ib113d37696cef0355ba45c200d94e5c26e72874d
Skip-Clang-Tidy-Checks: modernize-use-nullptr
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6426631
Reviewed-by: Rafael Cintron <rafael.cintron@microsoft.com>
Commit-Queue: Sahir Vellani <sahir.vellani@microsoft.com>
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1449042}
GraphiteSharedContext is a wrapper class with AutoLock on top of
graphite::context. The unit tests test the thread safe functions on
graphite::context
Bug: 407874799
Change-Id: I66796392b6a2e3411697b3e4c9fcd30092dfd55d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6465024
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Commit-Queue: Maggie Chen <magchen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1448621}
- Start to always create and use the wrapper, GraphiteSharedContext (regardless of the feature flag GraphiteContextIsThreadSafe).
- Now only GraphiteSharedContext can communicate with skgpu::graphite::Context directly. All clients must use GraphiteSharedContext for all skgpu::graphite::Context functions.
- When features::kGraphiteContextIsThreadSafe is enabled, there is only one skgpu::graphite::Context and one wrapper GraphiteSharedContext. They are shared by all threads in the Gpu process. DawnSharedContext owns GraphiteSharedContext.
- When features::kGraphiteContextIsThreadSafe is disabled, each thread has its own skgpu::graphite::Context and GraphiteSharedContext. DawnContextProvider owns GraphiteSharedContext.
- Remove GetGraphiteContext() from SharedContextState, DawnContentProvider and MetalContentProvider.
Bug: 407874799
Change-Id: Ieab98a560d3da8c0f7c05a5d720883bba2de5fc6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6442809
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Commit-Queue: Maggie Chen <magchen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1448463}
Skip waitUntilScheduled calls when waiting on the same MTLDevice as the
pending flush. In particular, this means that on single GPU systems, we
will never call waitUntilScheduled except when handing off the IOSurface
to CoreAnimation.
Bug: 40273077
Change-Id: I7c3c8ce30f061caf217d8107f12b0bc243b2cb49
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6464979
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Commit-Queue: Maggie Chen <magchen@chromium.org>
Auto-Submit: Sunny Sachanandani <sunnyps@chromium.org>
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Maggie Chen <magchen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1448456}
The Dawn/Vulkan allocator uses a heap block size of 8mb by default.
We're seeing significant GPU memory fragmentation with
Graphite/Dawn/Vulkan when compared to Ganesh/Vulkan which uses 64kb
heap blocks. Previous experiments showed that 256kb results in minimal
additional fragmentation so start with that.
WebGPU dawn devices are unchanged and still using default the heap block
size.
Bug: 407730048
Change-Id: Ib0c380dc9f013812bcff49f9bbbed36157c11d49
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6438217
Commit-Queue: Kyle Charbonneau <kylechar@chromium.org>
Reviewed-by: Saifuddin Hitawala <hitawala@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1448113}
When D3DImageBacking is not shared across devices, we can give Dawn a
hint that fence signaling is not needed in EndAccess. That can reduce
overheads in some cases.
The same for DXGISwapChainImageBacking & DCompSurfaceImageBacking.
Note that this option is currently a no-op in Dawn. But future
optimizations would be possible. For example, Dawn doesn't need to
signal any fence when this flag is false. Its Queue.Submit() could defer
the ID3D11DeviceContext::Flush() call to later or asynchronuously.
Bug: 335003893
Bug: 377716220
Change-Id: I37fb87651b219ea8783649b22820fa5ccce28d5b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6448769
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Auto-Submit: Quyen Le <lehoangquyen@chromium.org>
Commit-Queue: Quyen Le <lehoangquyen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1447583}
This feature is not completed and is disabled by default.When this
feature is enabled, the AutoLocks in GraphiteSharedContext ensure thread
safety for graphite::context.
This CL adds the following things.
1) Add a feature flag.
2) Create GraphiteSharedContext, the graphite::context wrapper, in
DownContextProvider and MetalContextProvider.
3) Move GraphiteSharedContext into DawnSharedContext. In the new
architecture, DawnSharedContext owns GraphiteSharedContext.
Bug: 407874799
Change-Id: I1d1ca8a3b79eee5f25fb9a6eed6add52dd27c1ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6442544
Reviewed-by: vikas soni <vikassoni@chromium.org>
Commit-Queue: Maggie Chen <magchen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1447261}
This is a reland of commit 16a0b272da
The reland fixes a compile error in the ipc fuzzer that was not caught
by the CQ.
Original change's description:
> Improve encapsulation of shmem and dxgi GpuMemoryBufferHandles
>
> This CL attempts to move shared memory and DXGI GpuMemoryBufferHandles
> towards a model that are easier to move into a std::variant. The
> trickiest part of this is updating use of the shared memory region:
> previous refactorings here ignored this by simply dynamically
> dispatching depending on the type of the std::variant. While it is
> possible to continue implementing this behavior, being explicit here
> probably has long-term benefits for code readability...
>
> Summary of changes:
> - Extracting a shared memory regions or DXGI handle from a
> GpuMemoryBufferHandle is now a consuming operation, i.e. the
> GpuMemoryBufferHandle becomes an EMPTY_BUFFER.
> - Mutable getters and setters for these two subtypes no longer exist,
> to make it impossible to have a SHARED_MEMORY_BUFFER with an invalid
> region.
> - Add overloads for constructing a GpuMemoryBufferHandle from a shared
> memory region or DXGI handle.
>
> Bug: 40584691
> Change-Id: I580a85be460035e063d1bf65e3c031104cb5410c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6381717
> Reviewed-by: Frank Liberato <liberato@chromium.org>
> Reviewed-by: Piotr Bialecki <bialpio@chromium.org>
> Reviewed-by: Ilya Nikolaevskiy <ilnik@chromium.org>
> Reviewed-by: Colin Blundell <blundell@chromium.org>
> Reviewed-by: vikas soni <vikassoni@chromium.org>
> Commit-Queue: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Min Chen <minch@chromium.org>
> Reviewed-by: Peter McNeeley <petermcneeley@google.com>
> Cr-Commit-Position: refs/heads/main@{#1445133}
Bug: 40584691
Change-Id: I031b616ad43262c95e6e20eb8f0ad7df886ee78f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6448471
Reviewed-by: Frank Liberato <liberato@chromium.org>
Reviewed-by: Eugene Zemtsov <eugene@chromium.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@chromium.org>
Reviewed-by: Brian Sheedy <bsheedy@chromium.org>
Reviewed-by: Peter McNeeley <petermcneeley@google.com>
Reviewed-by: Min Chen <minch@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: vikas soni <vikassoni@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1446154}
Add max_texture_size crash key that will help investigate the maximum
size supported for creating NV12 textures when creating D3D textures
fails for NV12 formats due to large sizes.
Bug: 374461672
Change-Id: I0b1377eb6d6f72c5d0d6cfe0a9fc61e9ea3e359a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6447094
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Commit-Queue: Saifuddin Hitawala <hitawala@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1445370}
This reverts commit 16a0b272da.
Reason for revert: Tree is closed due to "error: no member named 'set_region' in 'gfx::DXGIHandle'" error in https://ci.chromium.org/ui/p/chromium/builders/ci/win32-archive-rel/49415/overview
Original change's description:
> Improve encapsulation of shmem and dxgi GpuMemoryBufferHandles
>
> This CL attempts to move shared memory and DXGI GpuMemoryBufferHandles
> towards a model that are easier to move into a std::variant. The
> trickiest part of this is updating use of the shared memory region:
> previous refactorings here ignored this by simply dynamically
> dispatching depending on the type of the std::variant. While it is
> possible to continue implementing this behavior, being explicit here
> probably has long-term benefits for code readability...
>
> Summary of changes:
> - Extracting a shared memory regions or DXGI handle from a
> GpuMemoryBufferHandle is now a consuming operation, i.e. the
> GpuMemoryBufferHandle becomes an EMPTY_BUFFER.
> - Mutable getters and setters for these two subtypes no longer exist,
> to make it impossible to have a SHARED_MEMORY_BUFFER with an invalid
> region.
> - Add overloads for constructing a GpuMemoryBufferHandle from a shared
> memory region or DXGI handle.
>
> Bug: 40584691
> Change-Id: I580a85be460035e063d1bf65e3c031104cb5410c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6381717
> Reviewed-by: Frank Liberato <liberato@chromium.org>
> Reviewed-by: Piotr Bialecki <bialpio@chromium.org>
> Reviewed-by: Ilya Nikolaevskiy <ilnik@chromium.org>
> Reviewed-by: Colin Blundell <blundell@chromium.org>
> Reviewed-by: vikas soni <vikassoni@chromium.org>
> Commit-Queue: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Min Chen <minch@chromium.org>
> Reviewed-by: Peter McNeeley <petermcneeley@google.com>
> Cr-Commit-Position: refs/heads/main@{#1445133}
Bug: 40584691
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Change-Id: I2c0b29869003b6ebd6f96a24c1d207b0b3546d01
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6438850
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Nicola Tommasi <tommasin@chromium.org>
Auto-Submit: Nicola Tommasi <tommasin@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1445159}
This CL attempts to move shared memory and DXGI GpuMemoryBufferHandles
towards a model that are easier to move into a std::variant. The
trickiest part of this is updating use of the shared memory region:
previous refactorings here ignored this by simply dynamically
dispatching depending on the type of the std::variant. While it is
possible to continue implementing this behavior, being explicit here
probably has long-term benefits for code readability...
Summary of changes:
- Extracting a shared memory regions or DXGI handle from a
GpuMemoryBufferHandle is now a consuming operation, i.e. the
GpuMemoryBufferHandle becomes an EMPTY_BUFFER.
- Mutable getters and setters for these two subtypes no longer exist,
to make it impossible to have a SHARED_MEMORY_BUFFER with an invalid
region.
- Add overloads for constructing a GpuMemoryBufferHandle from a shared
memory region or DXGI handle.
Bug: 40584691
Change-Id: I580a85be460035e063d1bf65e3c031104cb5410c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6381717
Reviewed-by: Frank Liberato <liberato@chromium.org>
Reviewed-by: Piotr Bialecki <bialpio@chromium.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: vikas soni <vikassoni@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Min Chen <minch@chromium.org>
Reviewed-by: Peter McNeeley <petermcneeley@google.com>
Cr-Commit-Position: refs/heads/main@{#1445133}
If the viewFormats passed to GPUCanvasContext.configure() are invalid,
the spec requires that a validation error occur. Subsequent calls to
GPUCanvasContext.getCurrentTexture() must also generate a validation
error.
The SwiftShader path in Chrome uses
SharedImageRepresentationAndAccessSkiaFallback for WebGPU canvas
contents. When GPUCanvasContext.getCurrentTexture() causes
AssociateMailbox() to be called on an invalid context, this causes the
GPUTexture allocation in the SIRAASF constructor to fail. When the
canvas is later garbage collected, DissociateMailbox() calls
SharedImageRepresentationAndAccessSkiaFallback::UploadContentsToSkia(),
which fails on that invalid texture in
GPUCommandEncoder.copyTextureToBuffer(). This can cause validation
errors in unrelated code (leading to the flaky failed tests in the bug).
The fix is to move the texture creation to SIRAASF::Create() and detect
the texture creation failure with a a push/popErrorScope(). If it fails,
return nullptr, causing
WebGPUDecoderImpl::HandleAssociateMailboxImmediate() to create a
ErrorSharedImageRepresentationAndAccess instead, as the other backends
do. This will avoid doing anything with an invalid texture.
However, it turns out that the validation error from CreateTexture is
actually load-bearing, and is the only reason that getCurrentTexture()
generates a validation error at all! Removing it causes the viewFormats
CTS tests to fail, since they don't get a validation error they're
expecting. The fix for that is to explicitly validate the texture
descriptor each time GPUCanvasContext.getCurrentTexture() is called.
Bug: 40853211
Change-Id: I445b0aff0f05efac01e0d95966ade9945876f9fb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6435571
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Stephen White <senorblanco@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1444466}
These functions mirror the public functions in skgpu::graphite:Context.
When |is_thread_safe| is true, the lock in each wrapper function ensures
calling to the same function in skgpu::graphite:Context is thread safe.
When |is_thread_safe| is false, the lock has no effect.
Bug: 407874799
Change-Id: I3590d4a2ce5f236bceb0732ca2b111b343b6e7d8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6440343
Commit-Queue: Maggie Chen <magchen@chromium.org>
Reviewed-by: vikas soni <vikassoni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1444279}
This uses `kern_return_t` for the return type of
`IOSurface{Lock,Unlock}`. In the current code base, `IOReturn` is used
for the return type of `IOSurface{Lock, Unlock}` but the return type is
not available for tvOS. According to the function signature,
`IOSurface{Lock, Unlock}` returns `kern_return_t`. So, this change uses
`kern_return_t` and compares the return value with KERN_SUCCESS.
Bug: 391914246
Change-Id: I296c3a8ab77adc9283d3b02911eff5713916b269
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6429008
Reviewed-by: John Rummell <jrummell@chromium.org>
Reviewed-by: Matthew Denton <mpdenton@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Julie Jeongeun Kim <jkim@igalia.com>
Reviewed-by: Kyle Charbonneau <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1443779}
If skia failed to initialize or during wierd tear-down ordering, the
transfer cache can be null at raster decoder destruction. Defensively
null check it.
Bug: 408729616
Change-Id: Iae29700f3e7fa294ab338d6e6510ce275a02e065
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6438235
Commit-Queue: Geoff Lang <geofflang@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1443623}
This will surface the Skia UMA stat:
Skia.Graphite.PipelineCache.PipelineUsesInEpoch
which reports the number of Pipelines used since the last call to reportPipelineStats(kPipelineCache). We will use this to set Skia's constant:
kGlobalGraphicsPipelineCacheSizeLimit
which is currently set (somewhat arbitrarily) to 256.
Low-Coverage-Reason: EXPERIMENTAL_CODE Precompilation is not yet enabled
Bug: 358074434
Change-Id: Ic2a37bb0062bfa54553ae232492604f995e2e5d5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6426285
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1443500}
To support multithreading for graphite::Context, we introduce a new
wrapper class GraphiteSharedContext that uses a lock to ensure
graphite::Context is thread safe.
GraphiteSharedContext in this CL is pretty much empty. More content will
be added later.
Bug: 407874799
Change-Id: I58bd2f3f4f834bbc280ec44b95b015e43676bfca
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6425059
Reviewed-by: vikas soni <vikassoni@chromium.org>
Commit-Queue: Maggie Chen <magchen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1442992}
We currently try to use the Graphite Dawn Metal backend which fails to
initialize causing fallback to Ganesh GL Metal. Use the Graphite native
Metal backend instead which performs far better than Ganesh GL. We
eventually want to use the Graphite Dawn backend so this is intended to
be temporary.
This CL updates the skia_use_metal GN flags to the new behavior and also
removes the requirement to explicitly pass a command line flag to enable
the Graphite Metal backend on iOS.
Also, update SkiaBackendType to include the GraphiteMetal case.
Cq-Include-Trybots: luci.chromium.try:ios-blink-dbg-fyi
Change-Id: I685d3a5ae497af9550cd572221fa61b2fb36dcba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6409054
Auto-Submit: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1442882}
GpuInfoCollector::GetDawnTogglesForSkiaGraphite is currently out of
sync with the toggles enabled/disabled in DawnContextProvider.
Ideally the toggles in GpuInfoCollector should not be a separate list
and also come from DawnContextProvider. But for now, update
GpuInfoCollector to have most of the toggles similar to that in
DawnContextProvider along with TODOs for those that require extra
flags and are platform specific.
Bug: 407497928
Change-Id: Ib687a6f806ba9a846eeb5887bc16b836a373a932
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6416722
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Commit-Queue: Saifuddin Hitawala <hitawala@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1442700}
Create `texture_holder_` in WrappedGraphiteBacking::InitializeWithData
so that on backing construction failure, the texture will be deleted
as well.
Bug: 383528569
Change-Id: Ic0f621e69766d42a542787f679cdf0ab6c3e1cff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6430128
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Commit-Queue: Saifuddin Hitawala <hitawala@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1442381}
This removes features and code associated with these two related features:
- WriteMetalShaderCacheToDisk
- UseBuiltInMetalShaderCache
These are two halves of the same functionality, and the combined system has been confirmed obsolete.
Bug: 356625448, 356625491
Change-Id: Ic4ba3439c457c94ad051e5c8be370a722c9ca90a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6416542
Commit-Queue: Christopher Grant <cjgrant@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1441913}
- AutoLock is null op when is_thread_safe is false.
- Use class ClearTrackingSharedImageBacking to handle locks in
IOSurfaceImageBacking clear.
- Add AutoLock for memory Begin/End Read/Write access.
- The base class SharedImageRepresentation has already had a lock in
place. No duplicate locks are added to ProduceDawn/Overlay/...etc.
- ProduceSkiaGraphite() calls ProduceDawn(). No duplicate locks are
added.
- No AutoLock is added to IOSurfaceBackingEGLState since it's an
intermediate layer in memory access calls.
- Tested with Feature "EnableDrDc" enabled and disabled for deadlock.
- Tested with --enable-skia-graphite and --disable-skia-graphite for
deadlock.
Bug: 405946930
Change-Id: I9a7ace979cdf629b918ef6d3830b9256ffd8886e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6389800
Commit-Queue: Maggie Chen <magchen@chromium.org>
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1441887}
