Commit a5459a92 authored by Peng Huang's avatar Peng Huang Committed by Commit Bot

Reland "ExternalVkImageBacking: fix concurrent read problem"

This is a reland of 0c9696bf

Original change's description:
> ExternalVkImageBacking: fix concurrent read problem
> 
> Concurrent read access from both GL and Vulkan is not allowed.
> Get rid of the concurrent read problem by releasing the texture
> from GL temporarily, when a Vulkan read is requested. And resume
> GL read access, when the Vulkan read access is over.
> Current chrome GL and Vulkan concurrent read pattern is - Blink holds
> and caches read access for a backing shared image for a WebGL or
> 2D canvas. SkiaOutputSurfaceImplOnGpu only requests read access just
> before playing back DDL and releases the read access after playing back
> DDL and before SwapBuffers. The GPU will not schedule other tasks
> between playing back DDL and SwapBuffers. So it should be safe to
> temporarily pause GL read access.
> 
> Bug: 1104231
> Change-Id: I729eeebd5196bc5387ac148133b5926358bb4b53
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2311330
> Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
> Commit-Queue: Peng Huang <penghuang@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#791022}

Bug: 1104231
Change-Id: Ia916f250ca30b3c2b8a07016d2530fad79385aa9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2316399Reviewed-by: default avatarVasiliy Telezhnikov <vasilyt@chromium.org>
Commit-Queue: Peng Huang <penghuang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791520}
parent 5897626f
...@@ -388,6 +388,32 @@ bool ExternalVkImageBacking::BeginAccess( ...@@ -388,6 +388,32 @@ bool ExternalVkImageBacking::BeginAccess(
UpdateContent(kInGLTexture); UpdateContent(kInGLTexture);
} }
if (gl_reads_in_progress_ && need_synchronization()) {
// To avoid concurrent read access from both GL and vulkan, if there is
// unfinished GL read access, we will release the GL texture temporarily.
// And when this vulkan access is over, we will acquire the GL texture to
// resume the GL access.
DCHECK(!is_gl);
DCHECK(readonly);
DCHECK(texture_passthrough_ || texture_);
GLuint texture_id = texture_passthrough_
? texture_passthrough_->service_id()
: texture_->service_id();
context_state()->MakeCurrent(/*gl_surface=*/nullptr, /*needs_gl=*/true);
GrVkImageInfo info;
auto result = backend_texture_.getVkImageInfo(&info);
DCHECK(result);
DCHECK_EQ(info.fCurrentQueueFamily, VK_QUEUE_FAMILY_EXTERNAL);
DCHECK_NE(info.fImageLayout, VK_IMAGE_LAYOUT_UNDEFINED);
DCHECK_NE(info.fImageLayout, VK_IMAGE_LAYOUT_PREINITIALIZED);
auto release_semaphore =
ExternalVkImageGLRepresentationShared::ReleaseTexture(
external_semaphore_pool(), texture_id, info.fImageLayout);
EndAccessInternal(readonly, std::move(release_semaphore));
}
if (!BeginAccessInternal(readonly, external_semaphores)) if (!BeginAccessInternal(readonly, external_semaphores))
return false; return false;
...@@ -452,6 +478,33 @@ void ExternalVkImageBacking::EndAccess(bool readonly, ...@@ -452,6 +478,33 @@ void ExternalVkImageBacking::EndAccess(bool readonly,
latest_content_ = kInVkImage | kInGLTexture; latest_content_ = kInVkImage | kInGLTexture;
} }
} }
if (gl_reads_in_progress_ && need_synchronization()) {
// When vulkan read access is finished, if there is unfinished GL read
// access, we need to resume GL read access.
DCHECK(!is_gl);
DCHECK(readonly);
DCHECK(texture_passthrough_ || texture_);
GLuint texture_id = texture_passthrough_
? texture_passthrough_->service_id()
: texture_->service_id();
context_state()->MakeCurrent(/*gl_surface=*/nullptr, /*needs_gl=*/true);
std::vector<ExternalSemaphore> external_semaphores;
BeginAccessInternal(true, &external_semaphores);
DCHECK_LE(external_semaphores.size(), 1u);
for (auto& semaphore : external_semaphores) {
GrVkImageInfo info;
auto result = backend_texture_.getVkImageInfo(&info);
DCHECK(result);
DCHECK_EQ(info.fCurrentQueueFamily, VK_QUEUE_FAMILY_EXTERNAL);
DCHECK_NE(info.fImageLayout, VK_IMAGE_LAYOUT_UNDEFINED);
DCHECK_NE(info.fImageLayout, VK_IMAGE_LAYOUT_PREINITIALIZED);
ExternalVkImageGLRepresentationShared::AcquireTexture(
&semaphore, texture_id, info.fImageLayout);
}
AddSemaphoresToPendingListOrRelease(std::move(external_semaphores));
}
} }
void ExternalVkImageBacking::Update(std::unique_ptr<gfx::GpuFence> in_fence) { void ExternalVkImageBacking::Update(std::unique_ptr<gfx::GpuFence> in_fence) {
...@@ -470,19 +523,28 @@ bool ExternalVkImageBacking::ProduceLegacyMailbox( ...@@ -470,19 +523,28 @@ bool ExternalVkImageBacking::ProduceLegacyMailbox(
void ExternalVkImageBacking::AddSemaphoresToPendingListOrRelease( void ExternalVkImageBacking::AddSemaphoresToPendingListOrRelease(
std::vector<ExternalSemaphore> semaphores) { std::vector<ExternalSemaphore> semaphores) {
constexpr size_t kMaxPendingSemaphores = 8; constexpr size_t kMaxPendingSemaphores = 4;
DCHECK_LE(pending_semaphores_.size(), kMaxPendingSemaphores); DCHECK_LE(pending_semaphores_.size(), kMaxPendingSemaphores);
#if DCHECK_IS_ON() #if DCHECK_IS_ON()
for (auto& semaphore : semaphores) for (auto& semaphore : semaphores)
DCHECK(semaphore); DCHECK(semaphore);
#endif #endif
while (pending_semaphores_.size() < kMaxPendingSemaphores &&
std::move(semaphores.begin(), !semaphores.empty()) {
semaphores.begin() + pending_semaphores_.push_back(std::move(semaphores.back()));
std::min(semaphores.size(), semaphores.pop_back();
kMaxPendingSemaphores - pending_semaphores_.size()), }
std::back_inserter(pending_semaphores_)); if (!semaphores.empty()) {
// |semaphores| may contain VkSemephores which are submitted to queue for
// signalling but have not been signalled. In that case, we have to release
// them via fence helper to make sure all submitted GPU works is finished
// before releasing them.
fence_helper()->EnqueueCleanupTaskForSubmittedWork(
base::BindOnce([](std::vector<ExternalSemaphore>,
VulkanDeviceQueue* device_queue, bool device_lost) {},
std::move(semaphores)));
}
} }
void ExternalVkImageBacking::ReturnPendingSemaphoresWithFenceHelper( void ExternalVkImageBacking::ReturnPendingSemaphoresWithFenceHelper(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment