When IsGraphiteContextThreadSafe() is true, only one graphite::Context
is created and shared by all SharedContextStates. This CL ensures only
one GraphiteCacheController is responsible for cleaning up Graphite
resource cache.
Bug: 407874799
Change-Id: I7caab4af368c250c5cac9faa037366795858058f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6503536
Reviewed-by: Kyle Charbonneau <kylechar@chromium.org>
Commit-Queue: Maggie Chen <magchen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1454673}
The new set uses the latest collected Pipeline labels. Together they
generate 141 Pipelines of which 113 were in the corpus (yielding an over-generation of 28 Pipelines).
The UMA stats also behave as expected with this set. Compared to a baseline of no web page, loading one of the corpus' web pages yields an increased usage of the Precompiled Pipelines.
Low-Coverage-Reason: EXPERIMENTAL_CODE - not yet enabled in Chrome
Bug: 358074434
Change-Id: I809e3c854e99f28a98c424145c1aafd9b8d1f8dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6500977
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Cr-Commit-Position: refs/heads/main@{#1454491}
EGLSurfaces fail to bind to target (eglBindTexImage) if they are not on
the original context or thread. Now add
base::SingleThreadTaskRunner*along along with EGLDisplay to the key for
egl_state_map_ so IOSurfaceBackingEGLState is only reused on the same
thread.
Bug: 405946930
Change-Id: I87a635d2ed326804d4091c3c6a23780e8d0a95f0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6499888
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Commit-Queue: Maggie Chen <magchen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1453893}
if ~IOSurfaceImageBacking() is called on a different thread, send its
|egl_state_for_skia_gl_context_| to the created thread so its egl_state
can be destroyed in the GLContext where egl_state is created.
Bug: 405946930
Change-Id: I8200732b9607e44528b9202b6aeaa238ec23f9dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6497445
Commit-Queue: Maggie Chen <magchen@chromium.org>
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1453479}
This moves GpuMemoryBufferHandle a bit closer to the world where the
various fields are encapsulated within GpuMemoryBufferHandle. Similar to
previous CLs:
- the only way to extract a mutable gfx::NativePixmapHandle from a
gfx::GpuMemoryBufferHandle is to consume the original GMBH.
- non-mutable access is still possible via a getter
- gfx::GpuMemoryBufferHandle has a new constructor overload to construct
a NATIVE_PIXMAP directly from a gfx::NativePixmapHandle.
This does reveal a few issues with code that assumes it always has a
gfx::NATIVE_PIXMAP; this is not actually the case in many tests, which
use TestSharedImageInterface, which only supports shmem GMB handles. To
reflect this reality, non-test code that is affected by this now has an
explicit check that the GMB handle is of the appropriate type, with an
else branch to indicate that this situation only occurs in tests.
This CL was tested using the CQ :) Due to the potential of unknown
unknowns, all newly-introduced type-safety checks are not fatal until
M139.
Bug: 40584691
Change-Id: I05ad7ff6c9904d64edfc570586be36df10eff52d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6374406
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Reviewed-by: Yury Khmel <khmel@chromium.org>
Reviewed-by: Shik Chen <shik@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: vikas soni <vikassoni@chromium.org>
Reviewed-by: Kramer Ge <fangzhoug@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1453431}
We now use the compound image backing path for shared memory GMBs on ios
blink so it's important that ProduceOverlay work as expected. This CL
also fixes the other virtual accessors so a similar issue doesn't arise
if we ever use this backing on other platforms.
Change-Id: I127723b9b4a7c2d121084a1f5a3fefb43ec57b9e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6490729
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Auto-Submit: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1451944}
- 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}