Skip to content

Commit

Permalink
[m110] Revert "gpu: Pass plane format to compound image gpu backing f…
Browse files Browse the repository at this point in the history 
…actory"

This reverts commit dda2dce.
https://chromiumdash.appspot.com/commit/542b8ddee86659267fbf6c761d4ef168545572d7

Reason for revert: Incorrect GMB plane offset causes crbug.com/1415122

This CL is only being reverted on M110 branch since multiple subsequent
CLs have landed on top of it which also fixes the bug in a different way
so the bug only occurs on M110 branch.

Bug: 1415122
Change-Id: Iba783e0dd7c5f946b7ac6e99c2c1408dcec73f2f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4252540
Reviewed-by: Kyle Charbonneau <kylechar@chromium.org>
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/branch-heads/5481@{#1147}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}
  • Loading branch information
@sunnyps
sunnyps authored and Chromium LUCI CQ committed Feb 15, 2023
1 parent 37ba462 commit 7e34d30
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 77 deletions.
66 changes: 26 additions & 40 deletions gpu/command_buffer/service/shared_image/compound_image_backing.cc
View file
Expand Up @@ -7,7 +7,6 @@
#include "base/feature_list.h"
#include "base/functional/bind.h"
#include "base/logging.h"
#include "base/memory/ptr_util.h"
#include "base/memory/scoped_refptr.h"
#include "base/trace_event/memory_allocator_dump_guid.h"
#include "base/trace_event/process_memory_dump.h"
Expand Down Expand Up @@ -41,6 +40,26 @@ BASE_FEATURE(kSkipReadbackToSharedMemory,

constexpr AccessStreamSet kMemoryStreamSet = {SharedImageAccessStream::kMemory};

bool IsValidSharedMemoryBufferFormat(const gfx::Size& size,
gfx::BufferFormat buffer_format,
gfx::BufferPlane plane) {
if (!gpu::IsImageSizeValidForGpuMemoryBufferFormat(size, buffer_format)) {
DLOG(ERROR) << "Invalid image size for format.";
return false;
}
switch (plane) {
case gfx::BufferPlane::DEFAULT:
case gfx::BufferPlane::Y:
case gfx::BufferPlane::UV:
break;
default:
DLOG(ERROR) << "Invalid plane " << gfx::BufferPlaneToString(plane);
return false;
}

return true;
}

// Unique GUIDs for child backings.
base::trace_event::MemoryAllocatorDumpGuid GetSubBackingGUIDForTracing(
const Mailbox& mailbox,
Expand All @@ -50,23 +69,6 @@ base::trace_event::MemoryAllocatorDumpGuid GetSubBackingGUIDForTracing(
mailbox.ToDebugString().c_str(), backing_index));
}

bool IsBufferPlaneAndFormatSupported(gfx::BufferPlane plane,
gfx::BufferFormat format) {
switch (format) {
case gfx::BufferFormat::YVU_420:
return plane == gfx::BufferPlane::Y || plane == gfx::BufferPlane::U ||
plane == gfx::BufferPlane::V;
case gfx::BufferFormat::YUV_420_BIPLANAR:
case gfx::BufferFormat::P010:
return plane == gfx::BufferPlane::Y || plane == gfx::BufferPlane::UV;
case gfx::BufferFormat::YUVA_420_TRIPLANAR:
return plane == gfx::BufferPlane::Y || plane == gfx::BufferPlane::UV ||
plane == gfx::BufferPlane::A;
default:
return plane == gfx::BufferPlane::DEFAULT;
}
}

} // namespace

// Wrapped representation types are not in the anonymous namespace because they
Expand Down Expand Up @@ -285,24 +287,6 @@ class WrappedOverlayCompoundImageRepresentation
std::unique_ptr<OverlayImageRepresentation> wrapped_;
};

// static
bool CompoundImageBacking::IsValidSharedMemoryBufferFormat(
const gfx::Size& size,
gfx::BufferFormat format,
gfx::BufferPlane plane) {
if (!gpu::IsImageSizeValidForGpuMemoryBufferFormat(size, format)) {
DVLOG(1) << "Invalid image size: " << size.ToString()
<< " for format: " << gfx::BufferFormatToString(format);
return false;
}
if (!IsBufferPlaneAndFormatSupported(plane, format)) {
DVLOG(1) << "Unsupported buffer plane: " << gfx::BufferPlaneToString(plane)
<< " for format: " << gfx::BufferFormatToString(format);
return false;
}
return true;
}

// static
std::unique_ptr<SharedImageBacking> CompoundImageBacking::CreateSharedMemory(
SharedImageBackingFactory* gpu_backing_factory,
Expand All @@ -316,13 +300,15 @@ std::unique_ptr<SharedImageBacking> CompoundImageBacking::CreateSharedMemory(
GrSurfaceOrigin surface_origin,
SkAlphaType alpha_type,
uint32_t usage) {
DCHECK(IsValidSharedMemoryBufferFormat(size, buffer_format, plane));
if (!IsValidSharedMemoryBufferFormat(size, buffer_format, plane)) {
return nullptr;
}

const gfx::Size plane_size = GetPlaneSize(plane, size);
const viz::ResourceFormat plane_format =
viz::GetResourceFormat(GetPlaneBufferFormat(plane, buffer_format));

const size_t plane_index = GetPlaneIndex(plane, buffer_format);
const size_t plane_index = plane == gfx::BufferPlane::UV ? 1 : 0;
handle.offset +=
gfx::BufferOffsetForBufferFormat(size, buffer_format, plane_index);

Expand All @@ -339,10 +325,10 @@ std::unique_ptr<SharedImageBacking> CompoundImageBacking::CreateSharedMemory(
SHARED_IMAGE_USAGE_CPU_WRITE, std::move(shm_wrapper));
shm_backing->SetNotRefCounted();

return base::WrapUnique(new CompoundImageBacking(
return std::make_unique<CompoundImageBacking>(
mailbox, si_format, plane_size, color_space, surface_origin, alpha_type,
usage, allow_shm_overlays, std::move(shm_backing),
gpu_backing_factory->GetWeakPtr()));
gpu_backing_factory->GetWeakPtr());
}

CompoundImageBacking::CompoundImageBacking(
Expand Down
28 changes: 12 additions & 16 deletions gpu/command_buffer/service/shared_image/compound_image_backing.h
View file
Expand Up @@ -47,10 +47,6 @@ class GPU_GLES2_EXPORT CompoundImageBacking : public SharedImageBacking {
using CreateBackingCallback =
base::OnceCallback<void(std::unique_ptr<SharedImageBacking>&)>;

static bool IsValidSharedMemoryBufferFormat(const gfx::Size& size,
gfx::BufferFormat buffer_format,
gfx::BufferPlane plane);

// Creates a backing that contains a shared memory backing and GPU backing
// provided by `gpu_backing_factory`.
static std::unique_ptr<SharedImageBacking> CreateSharedMemory(
Expand All @@ -66,6 +62,18 @@ class GPU_GLES2_EXPORT CompoundImageBacking : public SharedImageBacking {
SkAlphaType alpha_type,
uint32_t usage);

CompoundImageBacking(
const Mailbox& mailbox,
viz::SharedImageFormat format,
const gfx::Size& size,
const gfx::ColorSpace& color_space,
GrSurfaceOrigin surface_origin,
SkAlphaType alpha_type,
uint32_t usage,
bool allow_shm_overlays,
std::unique_ptr<SharedMemoryImageBacking> shm_backing,
base::WeakPtr<SharedImageBackingFactory> gpu_backing_factory);

~CompoundImageBacking() override;

// Called by wrapped representations before access. This will update
Expand Down Expand Up @@ -126,18 +134,6 @@ class GPU_GLES2_EXPORT CompoundImageBacking : public SharedImageBacking {
std::unique_ptr<SharedImageBacking> backing;
};

CompoundImageBacking(
const Mailbox& mailbox,
viz::SharedImageFormat format,
const gfx::Size& size,
const gfx::ColorSpace& color_space,
GrSurfaceOrigin surface_origin,
SkAlphaType alpha_type,
uint32_t usage,
bool allow_shm_overlays,
std::unique_ptr<SharedMemoryImageBacking> shm_backing,
base::WeakPtr<SharedImageBackingFactory> gpu_backing_factory);

void OnMemoryDump(const std::string& dump_name,
base::trace_event::MemoryAllocatorDumpGuid client_guid,
base::trace_event::ProcessMemoryDump* pmd,
Expand Down
20 changes: 12 additions & 8 deletions gpu/command_buffer/service/shared_image/d3d_image_backing_factory.cc
View file
Expand Up @@ -67,10 +67,6 @@ absl::optional<DXGI_FORMAT> GetSupportedRGBAFormat(
return DXGI_FORMAT_R8_UNORM;
case viz::RG_88:
return DXGI_FORMAT_R8G8_UNORM;
case viz::R16_EXT:
return DXGI_FORMAT_R16_UNORM;
case viz::RG16_EXT:
return DXGI_FORMAT_R16G16_UNORM;
default:
NOTREACHED();
return {};
Expand All @@ -93,7 +89,7 @@ DXGI_FORMAT GetDXGIFormat(gfx::BufferFormat buffer_format) {
}
}

// Typeless formats supported by CreateSharedImage(GMB) for XR.
// Formats supported by CreateSharedImage(GMB).
DXGI_FORMAT GetDXGITypelessFormat(gfx::BufferFormat buffer_format) {
switch (buffer_format) {
case gfx::BufferFormat::RGBA_8888:
Expand Down Expand Up @@ -516,9 +512,17 @@ bool D3DImageBackingFactory::IsSupported(uint32_t usage,
}

if (gmb_type == gfx::EMPTY_BUFFER) {
// We only support rendering or uploading to RGBA formats.
if (!GetSupportedRGBAFormat(format))
return false;
if (usage & SHARED_IMAGE_USAGE_CPU_UPLOAD) {
// Only allow single NV12 shared memory GMBs for now. This excludes
// dual shared memory GMBs used by software video decoder.
if (format.resource_format() != viz::YUV_420_BIPLANAR) {
return false;
}
} else {
if (!GetSupportedRGBAFormat(format)) {
return false;
}
}
} else if (gmb_type == gfx::DXGI_SHARED_HANDLE) {
if (GetDXGIFormat(ToBufferFormat(format)) == DXGI_FORMAT_UNKNOWN)
return false;
Expand Down
17 changes: 4 additions & 13 deletions gpu/command_buffer/service/shared_image/shared_image_factory.cc
View file
Expand Up @@ -427,19 +427,10 @@ bool SharedImageFactory::CreateSharedImage(const Mailbox& mailbox,
!IsSharedBetweenThreads(usage)) {
// Check if CompoundImageBacking can hold shared memory buffer plus
// another GPU backing type to satisfy requirements.
if (CompoundImageBacking::IsValidSharedMemoryBufferFormat(size, format,
plane)) {
// For shared memory backed compound backings, we need to check if the
// corresponding GPU backing can support the format and size for the given
// plane rather than the original GMB format and size.
const auto plane_format = viz::SharedImageFormat::SinglePlane(
viz::GetResourceFormat(GetPlaneBufferFormat(plane, format)));
const gfx::Size plane_size = GetPlaneSize(plane, size);
factory =
GetFactoryByUsage(usage | SHARED_IMAGE_USAGE_CPU_UPLOAD, plane_format,
plane_size, /*pixel_data=*/{}, gfx::EMPTY_BUFFER);
use_compound = factory != nullptr;
}
use_compound = true;
factory = GetFactoryByUsage(usage | SHARED_IMAGE_USAGE_CPU_UPLOAD,
si_format, size,
/*pixel_data=*/{}, gfx::EMPTY_BUFFER);
}

if (!factory) {
Expand Down

0 comments on commit 7e34d30

Please sign in to comment.