Commit b8db3fa6 authored by Natasha Lee's avatar Natasha Lee Committed by Commit Bot

Correctly SetCleared in EndAccess()

Also have SharedImageBackingD3D derive from
ClearTrackingSharedImageBacking so that SetClearedRect and ClearedRect
is correctly implemented and tracked.

Bug: chromium:1036080
Change-Id: I8099dbaa0475baa392bc804c726e967b2d7e4a96
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2095410Reviewed-by: default avatarAustin Eng <enga@chromium.org>
Reviewed-by: default avatarKhushal <khushalsagar@chromium.org>
Commit-Queue: Natasha Lee <natlee@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#749307}
parent ed91af03
......@@ -87,14 +87,6 @@ WGPUTexture ExternalVkImageDawnRepresentation::BeginAccess(
// Keep a reference to the texture so that it stays valid (its content
// might be destroyed).
dawn_procs_.textureReference(texture_);
// Assume that the user of this representation will write to the texture
// so set the cleared flag so that other representations don't overwrite
// the result.
// TODO(cwallez@chromium.org): This is incorrect and allows reading
// uninitialized data. When !IsCleared we should tell dawn_native to
// consider the texture lazy-cleared. crbug.com/1036080
SetCleared();
}
return texture_;
......@@ -105,13 +97,14 @@ void ExternalVkImageDawnRepresentation::EndAccess() {
return;
}
// TODO(cwallez@chromium.org): query dawn_native to know if the texture was
// cleared and set IsCleared appropriately.
// Grab the signal semaphore from dawn
int signal_semaphore_fd =
dawn_native::vulkan::ExportSignalSemaphoreOpaqueFD(device_, texture_);
if (dawn_native::IsTextureSubresourceInitialized(texture_, 0, 1, 0, 1)) {
SetCleared();
}
// Wrap file descriptor in a handle
SemaphoreHandle signal_semaphore(
VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_OPAQUE_FD_BIT_KHR,
......
......@@ -48,13 +48,13 @@ SharedImageBackingD3D::SharedImageBackingD3D(
Microsoft::WRL::ComPtr<ID3D11Texture2D> d3d11_texture,
base::win::ScopedHandle shared_handle,
Microsoft::WRL::ComPtr<IDXGIKeyedMutex> dxgi_keyed_mutex)
: SharedImageBacking(mailbox,
format,
size,
color_space,
usage,
texture->estimated_size(),
false /* is_thread_safe */),
: ClearTrackingSharedImageBacking(mailbox,
format,
size,
color_space,
usage,
texture->estimated_size(),
false /* is_thread_safe */),
swap_chain_(std::move(swap_chain)),
texture_(std::move(texture)),
image_(std::move(image)),
......@@ -78,11 +78,6 @@ SharedImageBackingD3D::~SharedImageBackingD3D() {
shared_handle_.Close();
}
// Texture is cleared on initialization.
gfx::Rect SharedImageBackingD3D::ClearedRect() const {
return gfx::Rect(size());
}
void SharedImageBackingD3D::Update(std::unique_ptr<gfx::GpuFence> in_fence) {
DLOG(ERROR) << "SharedImageBackingD3D::Update : Trying to update "
"Shared Images associated with swap chain.";
......
......@@ -32,7 +32,7 @@ struct Mailbox;
// Implementation of SharedImageBacking that holds buffer (front buffer/back
// buffer of swap chain) texture (as gles2::Texture/gles2::TexturePassthrough)
// and a reference to created swap chain.
class SharedImageBackingD3D : public SharedImageBacking {
class SharedImageBackingD3D : public ClearTrackingSharedImageBacking {
public:
SharedImageBackingD3D(
const Mailbox& mailbox,
......@@ -50,10 +50,6 @@ class SharedImageBackingD3D : public SharedImageBacking {
~SharedImageBackingD3D() override;
// Texture is cleared on initialization.
gfx::Rect ClearedRect() const override;
void SetClearedRect(const gfx::Rect& cleared_rect) override {}
void Update(std::unique_ptr<gfx::GpuFence> in_fence) override;
bool ProduceLegacyMailbox(MailboxManager* mailbox_manager) override;
......
......@@ -500,6 +500,8 @@ TEST_F(SharedImageBackingFactoryD3DTest, GL_SkiaGL) {
// Set the clear color to green.
api->glClearColorFn(0.0f, 1.0f, 0.0f, 1.0f);
api->glClearFn(GL_COLOR_BUFFER_BIT);
gl_representation->SetCleared();
scoped_access.reset();
gl_representation.reset();
......@@ -555,7 +557,7 @@ TEST_F(SharedImageBackingFactoryD3DTest, Dawn_SkiaGL) {
auto scoped_access = dawn_representation->BeginScopedAccess(
WGPUTextureUsage_OutputAttachment,
SharedImageRepresentation::AllowUnclearedAccess::kNo);
SharedImageRepresentation::AllowUnclearedAccess::kYes);
ASSERT_TRUE(scoped_access);
wgpu::Texture texture = wgpu::Texture::Acquire(scoped_access->texture());
......
......@@ -303,14 +303,6 @@ class SharedImageRepresentationDawnIOSurface
// Keep a reference to the texture so that it stays valid (its content
// might be destroyed).
dawn_procs_.textureReference(texture_);
// Assume that the user of this representation will write to the texture
// so set the cleared flag so that other representations don't overwrite
// the result.
// TODO(cwallez@chromium.org): This is incorrect and allows reading
// uninitialized data. When !IsCleared we should tell dawn_native to
// consider the texture lazy-cleared. crbug.com/1036080
SetCleared();
}
return texture_;
......@@ -320,8 +312,10 @@ class SharedImageRepresentationDawnIOSurface
if (!texture_) {
return;
}
// TODO(cwallez@chromium.org): query dawn_native to know if the texture was
// cleared and set IsCleared appropriately.
if (dawn_native::IsTextureSubresourceInitialized(texture_, 0, 1, 0, 1)) {
SetCleared();
}
// All further operations on the textures are errors (they would be racy
// with other backings).
......
......@@ -99,14 +99,6 @@ WGPUTexture SharedImageRepresentationDawnD3D::BeginAccess(
// Keep a reference to the texture so that it stays valid (its content
// might be destroyed).
dawn_procs_.textureReference(texture_);
// Assume that the user of this representation will write to the texture
// so set the cleared flag so that other representations don't overwrite
// the result.
// TODO(cwallez@chromium.org): This is incorrect and allows reading
// uninitialized data. When !IsCleared we should tell dawn_native to
// consider the texture lazy-cleared. crbug.com/1036080
SetCleared();
} else {
d3d_image_backing->EndAccessD3D12();
}
......@@ -122,8 +114,9 @@ void SharedImageRepresentationDawnD3D::EndAccess() {
SharedImageBackingD3D* d3d_image_backing =
static_cast<SharedImageBackingD3D*>(backing());
// TODO(cwallez@chromium.org): query dawn_native to know if the texture was
// cleared and set IsCleared appropriately.
if (dawn_native::IsTextureSubresourceInitialized(texture_, 0, 1, 0, 1)) {
SetCleared();
}
// All further operations on the textures are errors (they would be racy
// with other backings).
......
......@@ -84,14 +84,6 @@ WGPUTexture SharedImageRepresentationDawnOzone::BeginAccess(
// Keep a reference to the texture so that it stays valid (its content
// might be destroyed).
dawn_procs_->data.textureReference(texture_);
// Assume that the user of this representation will write to the texture
// so set the cleared flag so that other representations don't overwrite
// the result.
// TODO(cwallez@chromium.org): This is incorrect and allows reading
// uninitialized data. When !IsCleared we should tell dawn_native to
// consider the texture lazy-cleared. crbug.com/1036080
SetCleared();
} else {
close(fd);
}
......@@ -104,6 +96,10 @@ void SharedImageRepresentationDawnOzone::EndAccess() {
return;
}
if (dawn_native::IsTextureSubresourceInitialized(texture_, 0, 1, 0, 1)) {
SetCleared();
}
// TODO(hob): Synchronize access to the dma-buf by exporting the VkSemaphore
// from the WebGPU texture.
dawn_procs_->data.textureDestroy(texture_);
......
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