0

Revert "Move Dawn Ozone Image Representation over to Shared Texture Memory"

This reverts commit d509eb15f3.

Reason for revert: Some webgpu tests are failing

https://ci.chromium.org/ui/p/chromium/builders/ci/ChromeOS%20FYI%20Release%20Skylab%20(volteer)/3096/test-results?sortby=&groupby=


Original change's description:
> Move Dawn Ozone Image Representation over to Shared Texture Memory
>
> (Attempt )
>
> This is done as far as ozone is concerned but the issue with opaque fences still remains. Basically ozone backing works with sync fences
> and the external_vk_image_backing doesnt not.
>
> We are able to make this change only because webGPU for LINUX is not
> enabled and will not be for the foreseeable future. WebGPU on linux
> will work with this change if it uses the external_vk_image_backing
> and associated representations.
>
> This is dependent on:
> https://dawn-review.googlesource.com/c/dawn/+/194440
>
> Bug: 330385376
> Change-Id: I1a6ddb8e4a738d62f6097c62f5280b6d53dcaca6
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5925789
> Reviewed-by: ccameron chromium <ccameron@chromium.org>
> Reviewed-by: Zhenyao Mo <zmo@chromium.org>
> Commit-Queue: Peter McNeeley <petermcneeley@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1370068}

Bug: 330385376
Change-Id: Ibbd083ef0c346992854bc6b9197548de33740995
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5937143
Reviewed-by: Saifuddin Hitawala <hitawala@chromium.org>
Commit-Queue: Saifuddin Hitawala <hitawala@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Saifuddin Hitawala <hitawala@google.com>
Cr-Commit-Position: refs/heads/main@{#1370642}
This commit is contained in:
Peter McNeeley
2024-10-18 16:23:32 +00:00
committed by Chromium LUCI CQ
parent 22ce0e42a4
commit c9c6166218
17 changed files with 62 additions and 204 deletions

@ -66,7 +66,6 @@ class FakeNativePixmap : public gfx::NativePixmap {
size_t GetDmaBufOffset(size_t plane) const override { return 0; }
size_t GetDmaBufPlaneSize(size_t plane) const override { return 0; }
uint64_t GetBufferFormatModifier() const override { return 0; }
uint32_t GetFourCCBufferFormat() const override { return 0; }
gfx::BufferFormat GetBufferFormat() const override { return format_; }
size_t GetNumberOfPlanes() const override { return 0; }
bool SupportsZeroCopyWebGPUImport() const override { return false; }

@ -489,7 +489,6 @@ target(link_target_type, "gles2_sources") {
"shared_image/dawn_ozone_image_representation.cc",
"shared_image/dawn_ozone_image_representation.h",
]
deps += [ "//third_party/libsync" ]
}
}

@ -5,7 +5,6 @@
#include "gpu/command_buffer/service/shared_image/dawn_ozone_image_representation.h"
#include <dawn/native/VulkanBackend.h>
#include <sync/sync.h>
#include <vulkan/vulkan.h>
#include "base/logging.h"
@ -16,16 +15,9 @@
#include "gpu/command_buffer/service/shared_image/shared_image_manager.h"
#include "gpu/command_buffer/service/shared_image/shared_image_representation.h"
#include "gpu/config/gpu_finch_features.h"
#include "ui/gfx/buffer_format_util.h"
#include "ui/gfx/geometry/size.h"
#include "ui/gfx/gpu_fence_handle.h"
#include "ui/gfx/native_pixmap.h"
#ifdef UNSAFE_BUFFERS_BUILD
// TODO(crbug.com/351564777): Remove this and convert code to safer constructs.
#pragma allow_unsafe_buffers
#endif
namespace gpu {
DawnOzoneImageRepresentation::DawnOzoneImageRepresentation(
@ -53,11 +45,7 @@ wgpu::Texture DawnOzoneImageRepresentation::BeginAccess(
wgpu::TextureUsage internal_usage) {
// It doesn't make sense to have two overlapping BeginAccess calls on the same
// representation.
// TODO(crbug.com/330385376): Switch to using the return value of
// OzoneImageBacking::BeginAccess().
if (texture_) {
LOG(ERROR)
<< "Attempting to begin access with before ending previous access.";
return nullptr;
}
@ -81,170 +69,88 @@ wgpu::Texture DawnOzoneImageRepresentation::BeginAccess(
return nullptr;
}
if (!ozone_backing()->BeginAccess(is_readonly_,
OzoneImageBacking::AccessStream::kWebGPU,
&fences, need_end_fence)) {
if (!ozone_backing()->BeginAccess(
/*readonly=*/is_readonly_, OzoneImageBacking::AccessStream::kWebGPU,
&fences, need_end_fence)) {
return nullptr;
}
DCHECK(need_end_fence || is_readonly_);
wgpu::SharedTextureMemoryBeginAccessDescriptor begin_access_desc = {};
begin_access_desc.initialized = IsCleared();
wgpu::SharedTextureMemoryVkImageLayoutBeginState begin_layout{};
// TODO(crbug.com/330385376): Track layouts correctly.
begin_layout.oldLayout = VK_IMAGE_LAYOUT_UNDEFINED;
begin_layout.newLayout = VK_IMAGE_LAYOUT_UNDEFINED;
begin_access_desc.nextInChain = &begin_layout;
// If the semaphore from BeginWrite is valid then pass it to
// SharedTextureMemory::BeginAccess() below.
std::vector<wgpu::SharedFence> shared_fences;
std::vector<uint64_t> shared_fence_signals;
begin_access_desc.fenceCount = fences.size();
if (fences.size()) {
shared_fences.resize(fences.size());
shared_fence_signals.resize(fences.size());
for (size_t i = 0; i < fences.size(); i++) {
wgpu::SharedFenceVkSemaphoreSyncFDDescriptor sync_fd_desc;
// NOTE: There is no ownership transfer here, as Dawn internally dup()s
// the passed-in handle.
sync_fd_desc.handle = fences[i].Peek();
wgpu::SharedFenceDescriptor fence_desc;
fence_desc.nextInChain = &sync_fd_desc;
shared_fences[i] = device_.ImportSharedFence(&fence_desc);
// Pass 1 as the signaled value for the binary semaphore
// (Dawn's SharedTextureMemoryVk verifies that this is the value passed).
const uint64_t kSignaledValue = 1;
shared_fence_signals[i] = kSignaledValue;
}
begin_access_desc.fences = shared_fences.data();
begin_access_desc.signaledValues = shared_fence_signals.data();
}
gfx::Size pixmap_size = pixmap_->GetBufferSize();
wgpu::DawnTextureInternalUsageDescriptor internal_desc;
internal_desc.internalUsage = internal_usage;
wgpu::TextureDescriptor texture_descriptor;
texture_descriptor.format = format_;
texture_descriptor.viewFormats = view_formats_.data();
texture_descriptor.viewFormatCount = view_formats_.size();
texture_descriptor.usage = static_cast<wgpu::TextureUsage>(usage);
texture_descriptor.dimension = wgpu::TextureDimension::e2D;
texture_descriptor.size = {static_cast<uint32_t>(size().width()),
static_cast<uint32_t>(size().height()),
/*depthOrArrayLayers=*/1};
texture_descriptor.size = {static_cast<uint32_t>(pixmap_size.width()),
static_cast<uint32_t>(pixmap_size.height()), 1};
texture_descriptor.mipLevelCount = 1;
texture_descriptor.sampleCount = 1;
texture_descriptor.viewFormatCount = view_formats_.size();
texture_descriptor.viewFormats = view_formats_.data();
texture_descriptor.nextInChain = &internal_desc;
wgpu::SharedTextureMemoryDmaBufDescriptor dma_buf_desc;
dma_buf_desc.size = {static_cast<uint32_t>(pixmap_size.width()),
static_cast<uint32_t>(pixmap_size.height())};
wgpu::DawnTextureInternalUsageDescriptor internalDesc;
internalDesc.internalUsage = internal_usage;
dma_buf_desc.drmFormat = pixmap_->GetFourCCBufferFormat();
dma_buf_desc.drmModifier = pixmap_->GetBufferFormatModifier();
texture_descriptor.nextInChain = &internalDesc;
std::vector<wgpu::SharedTextureMemoryDmaBufPlane> planes(
pixmap_->GetNumberOfPlanes());
dma_buf_desc.planeCount = pixmap_->GetNumberOfPlanes();
// We assume the pixmap is not disjoint (VK_IMAGE_CREATE_DISJOINT_BIT). All
// planes will have the the same fd but different pitch/offsets. This will not
// actually be reflected in the fds for each plane due to duping of the same
// fd elsewhere. This is why we cannot (d)check for this condition.
const int fd_for_all_planes = pixmap_->GetDmaBufFd(0);
for (uint32_t plane_idx = 0; plane_idx < dma_buf_desc.planeCount;
++plane_idx) {
// Dawn is not an ownership transfer. Dawn will internally duplicate fds as
// necessary.
planes[plane_idx].fd = fd_for_all_planes;
planes[plane_idx].stride = pixmap_->GetDmaBufPitch(plane_idx);
planes[plane_idx].offset = pixmap_->GetDmaBufOffset(plane_idx);
dawn::native::vulkan::ExternalImageDescriptorDmaBuf descriptor = {};
descriptor.cTextureDescriptor =
reinterpret_cast<WGPUTextureDescriptor*>(&texture_descriptor);
descriptor.isInitialized = IsCleared();
// Import the dma-buf into Dawn via the Vulkan backend. As per the Vulkan
// documentation, importing memory from a file descriptor transfers
// ownership of the fd from the application to the Vulkan implementation.
// Thus, we need to dup the fd so the fd corresponding to the dmabuf isn't
// closed twice (once by ScopedFD and once by the Vulkan implementation).
int fd = dup(pixmap_->GetDmaBufFd(0));
descriptor.memoryFD = fd;
for (uint32_t plane = 0u; plane < pixmap_->GetNumberOfPlanes(); ++plane) {
descriptor.planeLayouts[plane].offset = pixmap_->GetDmaBufOffset(plane);
descriptor.planeLayouts[plane].stride = pixmap_->GetDmaBufPitch(plane);
}
descriptor.drmModifier = pixmap_->GetBufferFormatModifier();
descriptor.waitFDs = {};
for (auto& fence : fences) {
descriptor.waitFDs.push_back(fence.Release().release());
}
dma_buf_desc.planes = planes.data();
wgpu::SharedTextureMemoryDescriptor desc;
desc.label = "DawnOzoneImageRepresentation";
desc.nextInChain = &dma_buf_desc;
if (!shared_texture_memory_) {
shared_texture_memory_ = device_.ImportSharedTextureMemory(&desc);
if (!shared_texture_memory_) {
LOG(ERROR) << "Failed to import shared texture memory.";
return nullptr;
}
texture_ = wgpu::Texture::Acquire(
dawn::native::vulkan::WrapVulkanImage(device_.Get(), &descriptor));
if (!texture_) {
ozone_backing()->EndAccess(is_readonly_,
OzoneImageBacking::AccessStream::kWebGPU,
gfx::GpuFenceHandle());
close(fd);
}
texture_ = shared_texture_memory_.CreateTexture(&texture_descriptor);
if (!shared_texture_memory_.BeginAccess(texture_, &begin_access_desc)) {
LOG(ERROR) << "Failed to begin access for shared image.";
// End the access on the backing and restore its fence, as Dawn did not
// consume it.
ozone_backing()->EndAccess(
is_readonly_, OzoneImageBacking::AccessStream::kWebGPU,
fences.empty() ? gfx::GpuFenceHandle() : std::move(fences[0]));
// Set `texture_` to nullptr to signal failure to BeginScopedAccess(),
// which will itself then return nullptr to signal failure to the client.
texture_ = nullptr;
}
return texture_;
return texture_.Get();
}
void DawnOzoneImageRepresentation::EndAccess() {
if (!texture_) {
return;
}
wgpu::SharedTextureMemoryEndAccessState end_access_desc = {};
wgpu::SharedTextureMemoryVkImageLayoutEndState end_layout{};
end_access_desc.nextInChain = &end_layout;
if (!shared_texture_memory_.EndAccess(texture_, &end_access_desc)) {
LOG(ERROR) << "Failed to end access for DawnOzoneImageRepresentation";
texture_.Destroy();
texture_ = nullptr;
return;
}
if (end_access_desc.initialized) {
SetCleared();
}
// Note: Dawn may export zero fences if there were no begin fences,
// AND the WGPUTexture was not used on the GPU queue within the
// access scope. Otherwise, it should either export fences from Dawn
// signaled after the WGPUTexture's last use, or it should re-export
// the begin fences if the WGPUTexture was unused.
gfx::GpuFenceHandle fence;
if (end_access_desc.fenceCount) {
wgpu::SharedFenceExportInfo export_info;
wgpu::SharedFenceVkSemaphoreSyncFDExportInfo sync_fd_export_info;
export_info.nextInChain = &sync_fd_export_info;
end_access_desc.fences[0].ExportInfo(&export_info);
// Dawn will close its FD when `end_access_desc` falls out of scope, and
// so it is necessary to dup() it to give OzoneImageBacking an FD that it
// can own.
base::ScopedFD fd_handle_merged(dup(sync_fd_export_info.handle));
for (size_t i = 1; i < end_access_desc.fenceCount; i++) {
end_access_desc.fences[i].ExportInfo(&export_info);
// The 'sync_merge' returns a new handle that is unowned. Wrap in scope
// to ensure ownership.
fd_handle_merged = base::ScopedFD(
sync_merge("", fd_handle_merged.get(), sync_fd_export_info.handle));
// Grab the signal semaphore from dawn
dawn::native::vulkan::ExternalImageExportInfoOpaqueFD export_info;
if (!dawn::native::vulkan::ExportVulkanImage(
texture_.Get(), VK_IMAGE_LAYOUT_UNDEFINED, &export_info)) {
DLOG(ERROR) << "Failed to export Dawn Vulkan image.";
} else {
if (export_info.isInitialized) {
SetCleared();
}
// Avoid fence handle 'dup' by moving the scope.
fence.Adopt(std::move(fd_handle_merged));
// TODO(hob): Handle waiting on multiple semaphores from dawn.
DCHECK(export_info.semaphoreHandles.size() == 1);
gfx::GpuFenceHandle fence;
fence.Adopt(base::ScopedFD(export_info.semaphoreHandles[0]));
ozone_backing()->EndAccess(is_readonly_,
OzoneImageBacking::AccessStream::kWebGPU,
std::move(fence));
}
ozone_backing()->EndAccess(
is_readonly_, OzoneImageBacking::AccessStream::kWebGPU, std::move(fence));
texture_.Destroy();
texture_ = nullptr;
}

@ -54,7 +54,6 @@ class DawnOzoneImageRepresentation : public DawnImageRepresentation {
scoped_refptr<gfx::NativePixmap> pixmap_;
wgpu::Texture texture_;
bool is_readonly_ = false;
wgpu::SharedTextureMemory shared_texture_memory_;
};
} // namespace gpu

@ -1437,11 +1437,6 @@ WGPUFuture WebGPUDecoderImpl::RequestDeviceImpl(
wgpu::FeatureName::SharedTextureMemoryIOSurface,
wgpu::FeatureName::SharedFenceMTLSharedEvent,
#if BUILDFLAG(IS_CHROMEOS)
wgpu::FeatureName::SharedTextureMemoryDmaBuf,
wgpu::FeatureName::SharedFenceVkSemaphoreSyncFD,
#endif
#if BUILDFLAG(IS_ANDROID)
wgpu::FeatureName::SharedTextureMemoryAHardwareBuffer,
wgpu::FeatureName::SharedFenceVkSemaphoreSyncFD,
@ -1795,14 +1790,6 @@ wgpu::Adapter WebGPUDecoderImpl::CreatePreferredAdapter(
// SwiftShader adapter. For SwiftShader, we will perform a manual
// upload/readback to/from shared images.
bool supports_external_textures = false;
#if BUILDFLAG(IS_CHROMEOS)
if (!adapter.HasFeature(wgpu::FeatureName::SharedTextureMemoryDmaBuf) ||
!adapter.HasFeature(wgpu::FeatureName::SharedFenceVkSemaphoreSyncFD)) {
return false;
}
#endif
#if BUILDFLAG(IS_APPLE)
supports_external_textures =
adapter.HasFeature(wgpu::FeatureName::SharedTextureMemoryIOSurface);

@ -659,16 +659,11 @@ source_set("memory_buffer_sources") {
"linux/client_native_pixmap_factory_dmabuf.cc",
"linux/client_native_pixmap_factory_dmabuf.h",
"linux/dmabuf_uapi.h",
"linux/native_pixmap_dmabuf.cc",
"linux/native_pixmap_dmabuf.h",
]
deps += [ "//build/config/linux/libdrm" ]
if (ozone_platform_drm || ozone_platform_wayland || ozone_platform_x11) {
deps += [ "//ui/gfx/linux:drm" ]
sources += [
"linux/native_pixmap_dmabuf.cc",
"linux/native_pixmap_dmabuf.h",
]
}
deps += [ "//build/config/linux/libdrm" ]
}
if (is_linux || is_chromeos || is_android) {
@ -990,10 +985,10 @@ test("gfx_unittests") {
}
if (is_linux || is_chromeos) {
sources += [ "linux/fontconfig_util_unittest.cc" ]
if (ozone_platform_drm || ozone_platform_wayland || ozone_platform_x11) {
sources += [ "linux/native_pixmap_dmabuf_unittest.cc" ]
}
sources += [
"linux/fontconfig_util_unittest.cc",
"linux/native_pixmap_dmabuf_unittest.cc",
]
deps += [ "//third_party/fontconfig" ]
}

@ -7,7 +7,6 @@
#include <utility>
#include "base/posix/eintr_wrapper.h"
#include "ui/gfx/linux/drm_util_linux.h"
namespace gfx {
@ -53,10 +52,6 @@ uint64_t NativePixmapDmaBuf::GetBufferFormatModifier() const {
return handle_.modifier;
}
uint32_t NativePixmapDmaBuf::GetFourCCBufferFormat() const {
return ui::GetFourCCFormatFromBufferFormat(format_);
}
gfx::BufferFormat NativePixmapDmaBuf::GetBufferFormat() const {
return format_;
}

@ -33,7 +33,6 @@ class GFX_EXPORT NativePixmapDmaBuf : public gfx::NativePixmap {
size_t GetDmaBufOffset(size_t plane) const override;
size_t GetDmaBufPlaneSize(size_t plane) const override;
uint64_t GetBufferFormatModifier() const override;
uint32_t GetFourCCBufferFormat() const override;
gfx::BufferFormat GetBufferFormat() const override;
size_t GetNumberOfPlanes() const override;
bool SupportsZeroCopyWebGPUImport() const override;

@ -5,8 +5,6 @@
#ifndef UI_GFX_NATIVE_PIXMAP_H_
#define UI_GFX_NATIVE_PIXMAP_H_
#include <cstdint>
#include "base/memory/ref_counted.h"
#include "ui/gfx/buffer_types.h"
#include "ui/gfx/geometry/size.h"
@ -38,7 +36,6 @@ class NativePixmap : public base::RefCountedThreadSafe<NativePixmap> {
// The following methods return format, modifier and size of the buffer,
// respectively.
virtual gfx::BufferFormat GetBufferFormat() const = 0;
virtual uint32_t GetFourCCBufferFormat() const = 0;
virtual uint64_t GetBufferFormatModifier() const = 0;
virtual gfx::Size GetBufferSize() const = 0;

@ -58,7 +58,6 @@ class CastPixmap : public gfx::NativePixmap {
size_t GetDmaBufOffset(size_t plane) const override { return 0; }
size_t GetDmaBufPlaneSize(size_t plane) const override { return 0; }
uint64_t GetBufferFormatModifier() const override { return 0; }
uint32_t GetFourCCBufferFormat() const override { return 0; }
gfx::BufferFormat GetBufferFormat() const override {
return gfx::BufferFormat::BGRA_8888;
}

@ -5,13 +5,11 @@
#include "ui/ozone/platform/drm/gpu/gbm_pixmap.h"
#include <gbm.h>
#include <memory>
#include <utility>
#include "base/check.h"
#include "ui/gfx/gpu_fence.h"
#include "ui/gfx/linux/drm_util_linux.h"
#include "ui/gfx/native_pixmap_handle.h"
#include "ui/ozone/platform/drm/gpu/drm_overlay_plane.h"
#include "ui/ozone/platform/drm/gpu/gbm_surface_factory.h"
@ -66,10 +64,6 @@ gfx::BufferFormat GbmPixmap::GetBufferFormat() const {
return buffer_->GetBufferFormat();
}
uint32_t GbmPixmap::GetFourCCBufferFormat() const {
return ui::GetFourCCFormatFromBufferFormat(buffer_->GetBufferFormat());
}
gfx::Size GbmPixmap::GetBufferSize() const {
return buffer_->GetSize();
}

@ -37,7 +37,6 @@ class GbmPixmap : public gfx::NativePixmap {
size_t GetNumberOfPlanes() const override;
bool SupportsZeroCopyWebGPUImport() const override;
uint64_t GetBufferFormatModifier() const override;
uint32_t GetFourCCBufferFormat() const override;
gfx::BufferFormat GetBufferFormat() const override;
gfx::Size GetBufferSize() const override;
uint32_t GetUniqueId() const override;

@ -59,10 +59,6 @@ uint64_t FlatlandSysmemNativePixmap::GetBufferFormatModifier() const {
return 0;
}
uint32_t FlatlandSysmemNativePixmap::GetFourCCBufferFormat() const {
NOTREACHED();
}
gfx::BufferFormat FlatlandSysmemNativePixmap::GetBufferFormat() const {
return collection_->format();
}

@ -30,7 +30,6 @@ class FlatlandSysmemNativePixmap : public gfx::NativePixmap {
size_t GetNumberOfPlanes() const override;
bool SupportsZeroCopyWebGPUImport() const override;
uint64_t GetBufferFormatModifier() const override;
uint32_t GetFourCCBufferFormat() const override;
gfx::BufferFormat GetBufferFormat() const override;
gfx::Size GetBufferSize() const override;
uint32_t GetUniqueId() const override;

@ -155,7 +155,6 @@ class TestPixmap : public gfx::NativePixmap {
size_t GetDmaBufPlaneSize(size_t plane) const override { return 0; }
uint64_t GetBufferFormatModifier() const override { return 0; }
gfx::BufferFormat GetBufferFormat() const override { return format_; }
uint32_t GetFourCCBufferFormat() const override { return 0; }
size_t GetNumberOfPlanes() const override {
return gfx::NumberOfPlanesForLinearBufferFormat(format_);
}

@ -157,9 +157,6 @@ gfx::BufferFormat GbmPixmapWayland::GetBufferFormat() const {
return gbm_bo_->GetBufferFormat();
}
uint32_t GbmPixmapWayland::GetFourCCBufferFormat() const {
return ui::GetFourCCFormatFromBufferFormat(gbm_bo_->GetBufferFormat());
}
gfx::Size GbmPixmapWayland::GetBufferSize() const {
return gbm_bo_->GetSize();
}

@ -61,7 +61,6 @@ class GbmPixmapWayland : public gfx::NativePixmap {
size_t GetNumberOfPlanes() const override;
bool SupportsZeroCopyWebGPUImport() const override;
uint64_t GetBufferFormatModifier() const override;
uint32_t GetFourCCBufferFormat() const override;
gfx::BufferFormat GetBufferFormat() const override;
gfx::Size GetBufferSize() const override;
uint32_t GetUniqueId() const override;