Skip to content

Commit

Permalink
Make OverlayImageRepresentation::GetGLImage windows specific
Browse files Browse the repository at this point in the history
GetGLImage is deprecated way of passing image to the overlay, all
platforms were converted to pass native buffers directly, so mark this
as windows specific.

Bug: 1342316
Change-Id: I5e55b15a74d137c39da715b5f47f01c59311c0ca
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3988149
Reviewed-by: Saifuddin Hitawala <hitawala@chromium.org>
Commit-Queue: Vasiliy Telezhnikov <vasilyt@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1065641}
  • Loading branch information
vasilyt authored and Chromium LUCI CQ committed Oct 31, 2022
1 parent a58022a commit f32b16c
Show file tree
Hide file tree
Showing 23 changed files with 40 additions and 88 deletions.
2 changes: 1 addition & 1 deletion android_webview/browser/gfx/overlay_processor_webview.cc
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class OverlayProcessorWebView::Manager
return;
}

read_access_ = representation_->BeginScopedReadAccess(false);
read_access_ = representation_->BeginScopedReadAccess();
if (!read_access_) {
LOG(ERROR) << "Couldn't access shared image for read.";
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,7 @@ void PresenterImageFuchsia::BeginPresent() {
DCHECK(read_begin_fences_.empty());

scoped_overlay_read_access_ =
overlay_representation_->BeginScopedReadAccess(
/*needs_gl_image=*/false);
overlay_representation_->BeginScopedReadAccess();
DCHECK(scoped_overlay_read_access_);

// Take ownership of acquire fence.
Expand Down
12 changes: 2 additions & 10 deletions components/viz/service/display_embedder/output_presenter_gl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,8 @@ void PresenterImageGL::BeginPresent() {
DCHECK(!sk_surface());
DCHECK(!scoped_overlay_read_access_);

#if defined(USE_OZONE) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_ANDROID)
const bool needs_gl_image = false;
#else
const bool needs_gl_image = true;
#endif // defined(USE_OZONE)

scoped_overlay_read_access_ =
overlay_representation_->BeginScopedReadAccess(needs_gl_image);
overlay_representation_->BeginScopedReadAccess();
DCHECK(scoped_overlay_read_access_);
}

Expand Down Expand Up @@ -120,7 +114,7 @@ gl::OverlayImage PresenterImageGL::GetOverlayImage(
#elif BUILDFLAG(IS_ANDROID)
return scoped_overlay_read_access_->GetAHardwareBufferFenceSync();
#else
return scoped_overlay_read_access_->gl_image();
LOG(FATAL) << "GetOverlayImage() is not implemented on this platform".
#endif
}

Expand Down Expand Up @@ -383,8 +377,6 @@ void OutputPresenterGL::ScheduleOverlayPlane(
#elif BUILDFLAG(IS_ANDROID)
gl::OverlayImage overlay_image =
access ? access->GetAHardwareBufferFenceSync() : nullptr;
#else
auto* overlay_image = access ? access->gl_image() : nullptr;
#endif
// TODO(msisov): Once shared image factory allows creating a non backed
// images and ScheduleOverlayPlane does not rely on GLImage, remove the if
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
#include "ui/gfx/gpu_fence_handle.h"
#include "ui/gfx/swap_result.h"
#include "ui/gl/gl_fence.h"
#include "ui/gl/gl_image.h"
#include "ui/gl/gl_surface.h"

#if BUILDFLAG(IS_OZONE)
Expand Down Expand Up @@ -334,24 +333,12 @@ SkiaOutputDeviceBufferQueue::GetOrCreateOverlayData(const gpu::Mailbox& mailbox,
return nullptr;
}

#if defined(USE_OZONE) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_ANDROID)
const bool needs_gl_image = false;
#else
const bool needs_gl_image = true;
#endif // defined(USE_OZONE)

// TODO(penghuang): do not depend on GLImage.
auto shared_image_access =
shared_image->BeginScopedReadAccess(needs_gl_image);
auto shared_image_access = shared_image->BeginScopedReadAccess();
if (!shared_image_access) {
LOG(ERROR) << "Could not access SharedImage for read.";
return nullptr;
}

// TODO(penghuang): do not depend on GLImage.
DLOG_IF(FATAL, needs_gl_image && !shared_image_access->gl_image())
<< "Cannot get GLImage.";

bool result;
std::tie(it, result) = overlays_.emplace(std::move(shared_image),
std::move(shared_image_access));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class SkiaOutputDeviceDComp::OverlayData {

gpu::OverlayImageRepresentation::ScopedReadAccess* BeginOverlayAccess() {
DCHECK(representation_);
access_ = representation_->BeginScopedReadAccess(/*needs_gl_image=*/true);
access_ = representation_->BeginScopedReadAccess();
DCHECK(access_);
return access_.get();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,11 +246,6 @@ class OverlayAHBImageRepresentation : public OverlayImageRepresentation {
return gl_image_->GetAHardwareBuffer();
}

gl::GLImage* GetGLImage() override {
NOTREACHED();
return nullptr;
}

raw_ptr<OverlayImage> gl_image_ = nullptr;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -565,8 +565,7 @@ TEST_F(AHardwareBufferImageBackingFactoryTest, Overlay) {
gl_legacy_shared_image.mailbox());
EXPECT_TRUE(overlay_representation);

auto scoped_read_access =
overlay_representation->BeginScopedReadAccess(/*needs_gl_image=*/false);
auto scoped_read_access = overlay_representation->BeginScopedReadAccess();
EXPECT_TRUE(scoped_read_access);
auto buffer = scoped_read_access->GetAHardwareBufferFenceSync();
DCHECK(buffer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,9 @@ class WrappedOverlayCompoundImageRepresentation
void EndReadAccess(gfx::GpuFenceHandle release_fence) final {
return wrapped_->EndReadAccess(std::move(release_fence));
}
#if BUILDFLAG(IS_WIN)
gl::GLImage* GetGLImage() final { return wrapped_->GetGLImage(); }
#endif

private:
const SharedImageAccessStream access_stream_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ TEST_F(CompoundImageBackingTest, UploadOnAccess) {
EXPECT_FALSE(GetGpuHasLatestContent(compound_backing));

// First access should trigger upload from memory to GPU.
overlay_rep->BeginScopedReadAccess(false);
overlay_rep->BeginScopedReadAccess();
EXPECT_TRUE(gpu_backing->GetUploadFromMemoryCalledAndReset());

// After GPU read access both should have latest content.
Expand All @@ -212,13 +212,13 @@ TEST_F(CompoundImageBackingTest, UploadOnAccess) {

// Second access shouldn't trigger upload since no shared memory updates
// happened.
overlay_rep->BeginScopedReadAccess(false);
overlay_rep->BeginScopedReadAccess();
EXPECT_FALSE(gpu_backing->GetUploadFromMemoryCalledAndReset());

// Notify compound backing of shared memory update. Next access should
// trigger a new upload.
compound_backing->Update(nullptr);
overlay_rep->BeginScopedReadAccess(false);
overlay_rep->BeginScopedReadAccess();
EXPECT_TRUE(gpu_backing->GetUploadFromMemoryCalledAndReset());

// Test that GLTexturePassthrough access causes upload.
Expand Down Expand Up @@ -292,6 +292,7 @@ TEST_F(CompoundImageBackingTest, ReadbackToMemory) {
EXPECT_TRUE(GetGpuHasLatestContent(compound_backing));
}

#if BUILDFLAG(IS_WIN)
TEST_F(CompoundImageBackingTest, NoUploadOnOverlayMemoryAccess) {
auto backing = CreateCompoundBacking(/*allow_shm_overlays=*/true);
auto* compound_backing = static_cast<CompoundImageBacking*>(backing.get());
Expand All @@ -300,14 +301,15 @@ TEST_F(CompoundImageBackingTest, NoUploadOnOverlayMemoryAccess) {

auto overlay_rep =
manager_.ProduceOverlay(compound_backing->mailbox(), &tracker_);
auto access = overlay_rep->BeginScopedReadAccess(/*needs_gl_image=*/true);
auto access = overlay_rep->BeginScopedReadAccess();

// This should produce a GLImageMemory but there will still be no GPU backing.
auto* gl_image = access->gl_image();
ASSERT_TRUE(gl_image);
EXPECT_EQ(gl_image->GetType(), gl::GLImage::Type::MEMORY);
EXPECT_FALSE(HasGpuBacking(compound_backing));
}
#endif

TEST_F(CompoundImageBackingTest, LazyAllocationFailsCreate) {
auto backing = CreateCompoundBacking();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1865,8 +1865,7 @@ void D3DImageBackingFactoryTest::RunOverlayTest(bool use_shared_handle,
shared_image_representation_factory_->ProduceOverlay(
shared_image_refs[0]->mailbox());

auto scoped_read_access =
overlay_representation->BeginScopedReadAccess(/*needs_gl_image=*/true);
auto scoped_read_access = overlay_representation->BeginScopedReadAccess();
ASSERT_TRUE(scoped_read_access);

auto* gl_image_d3d =
Expand Down Expand Up @@ -2081,8 +2080,7 @@ TEST_F(D3DImageBackingFactoryTest, CreateFromSharedMemory) {
shared_image_representation_factory_->ProduceOverlay(
shared_image_refs[0]->mailbox());

auto scoped_read_access =
overlay_representation->BeginScopedReadAccess(/*needs_gl_image=*/true);
auto scoped_read_access = overlay_representation->BeginScopedReadAccess();
ASSERT_TRUE(scoped_read_access);

auto* gl_image_memory =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,12 @@ void ExternalVkImageOverlayImageRepresentation::EndReadAccess(
read_begin_semaphores_.clear();
}

#if BUILDFLAG(IS_WIN)
gl::GLImage* ExternalVkImageOverlayImageRepresentation::GetGLImage() {
NOTREACHED();
return nullptr;
}
#endif

void ExternalVkImageOverlayImageRepresentation::GetAcquireFence(
gfx::GpuFenceHandle& fence) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ class ExternalVkImageOverlayImageRepresentation
// OverlayImageRepresentation implementation
bool BeginReadAccess(gfx::GpuFenceHandle& acquire_fence) override;
void EndReadAccess(gfx::GpuFenceHandle release_fence) override;

#if BUILDFLAG(IS_WIN)
gl::GLImage* GetGLImage() override;
#endif

private:
void GetAcquireFence(gfx::GpuFenceHandle& fence);
Expand Down
2 changes: 2 additions & 0 deletions gpu/command_buffer/service/shared_image/gl_image_backing.cc
Original file line number Diff line number Diff line change
Expand Up @@ -291,9 +291,11 @@ void OverlayGLImageRepresentation::EndReadAccess(
gl_backing->SetReleaseFence(std::move(release_fence));
}

#if BUILDFLAG(IS_WIN)
gl::GLImage* OverlayGLImageRepresentation::GetGLImage() {
return gl_image_.get();
}
#endif

///////////////////////////////////////////////////////////////////////////////
// GLImageBacking
Expand Down
3 changes: 3 additions & 0 deletions gpu/command_buffer/service/shared_image/gl_image_backing.h
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,10 @@ class OverlayGLImageRepresentation : public OverlayImageRepresentation {
private:
bool BeginReadAccess(gfx::GpuFenceHandle& acquire_fence) override;
void EndReadAccess(gfx::GpuFenceHandle release_fence) override;

#if BUILDFLAG(IS_WIN)
gl::GLImage* GetGLImage() override;
#endif

scoped_refptr<gl::GLImage> gl_image_;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ class OverlayIOSurfaceRepresentation : public OverlayImageRepresentation {
void EndReadAccess(gfx::GpuFenceHandle release_fence) override;
gfx::ScopedIOSurface GetIOSurface() const override;
bool IsInUseByWindowServer() const override;
gl::GLImage* GetGLImage() override;

scoped_refptr<gl::GLImage> gl_image_;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,11 +266,6 @@
static_cast<gl::GLImageIOSurface*>(gl_image_.get())->io_surface());
}

gl::GLImage* OverlayIOSurfaceRepresentation::GetGLImage() {
NOTREACHED();
return nullptr;
}

///////////////////////////////////////////////////////////////////////////////
// IOSurfaceImageBacking

Expand Down
17 changes: 0 additions & 17 deletions gpu/command_buffer/service/shared_image/ozone_image_backing.cc
Original file line number Diff line number Diff line change
Expand Up @@ -106,23 +106,6 @@ class OzoneImageBacking::OverlayOzoneImageRepresentation
ozone_backing->EndAccess(/*readonly=*/true, AccessStream::kOverlay,
std::move(release_fence));
}

gl::GLImage* GetGLImage() override {
if (!gl_image_) {
gfx::BufferFormat buffer_format = viz::BufferFormat(format());
auto pixmap =
static_cast<OzoneImageBacking*>(backing())->GetNativePixmap();
gl_image_ = base::MakeRefCounted<gl::GLImageNativePixmap>(
pixmap->GetBufferSize(), buffer_format);
if (backing()->color_space().IsValid())
gl_image_->SetColorSpace(backing()->color_space());
gl_image_->InitializeForOverlay(pixmap);
}

return gl_image_.get();
}

scoped_refptr<gl::GLImageNativePixmap> gl_image_;
};

OzoneImageBacking::~OzoneImageBacking() = default;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -338,18 +338,16 @@ bool OverlayImageRepresentation::IsInUseByWindowServer() const {
OverlayImageRepresentation::ScopedReadAccess::ScopedReadAccess(
base::PassKey<OverlayImageRepresentation> pass_key,
OverlayImageRepresentation* representation,
gl::GLImage* gl_image,
gfx::GpuFenceHandle acquire_fence)
: ScopedAccessBase(representation),
gl_image_(gl_image),
acquire_fence_(std::move(acquire_fence)) {}

OverlayImageRepresentation::ScopedReadAccess::~ScopedReadAccess() {
representation()->EndReadAccess(std::move(release_fence_));
}

std::unique_ptr<OverlayImageRepresentation::ScopedReadAccess>
OverlayImageRepresentation::BeginScopedReadAccess(bool needs_gl_image) {
OverlayImageRepresentation::BeginScopedReadAccess() {
if (!IsCleared()) {
LOG(ERROR) << "Attempt to read from an uninitialized SharedImage";
return nullptr;
Expand All @@ -363,7 +361,7 @@ OverlayImageRepresentation::BeginScopedReadAccess(bool needs_gl_image) {

return std::make_unique<ScopedReadAccess>(
base::PassKey<OverlayImageRepresentation>(), this,
needs_gl_image ? GetGLImage() : nullptr, std::move(acquire_fence));
std::move(acquire_fence));
}

DawnImageRepresentation::ScopedAccess::ScopedAccess(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -488,12 +488,9 @@ class GPU_GLES2_EXPORT OverlayImageRepresentation
public:
ScopedReadAccess(base::PassKey<OverlayImageRepresentation> pass_key,
OverlayImageRepresentation* representation,
gl::GLImage* gl_image,
gfx::GpuFenceHandle acquire_fence);
~ScopedReadAccess();

gl::GLImage* gl_image() const { return gl_image_; }

#if BUILDFLAG(IS_ANDROID)
AHardwareBuffer* GetAHardwareBuffer() {
return representation()->GetAHardwareBuffer();
Expand All @@ -510,6 +507,8 @@ class GPU_GLES2_EXPORT OverlayImageRepresentation
return representation()->GetNativePixmap();
}
#elif BUILDFLAG(IS_WIN)
gl::GLImage* gl_image() { return representation()->GetGLImage(); }

scoped_refptr<gl::DCOMPSurfaceProxy> GetDCOMPSurfaceProxy() {
return representation()->GetDCOMPSurfaceProxy();
}
Expand All @@ -532,12 +531,11 @@ class GPU_GLES2_EXPORT OverlayImageRepresentation
}

private:
const raw_ptr<gl::GLImage, DanglingUntriaged> gl_image_;
gfx::GpuFenceHandle acquire_fence_;
gfx::GpuFenceHandle release_fence_;
};

std::unique_ptr<ScopedReadAccess> BeginScopedReadAccess(bool needs_gl_image);
std::unique_ptr<ScopedReadAccess> BeginScopedReadAccess();

protected:
friend class WrappedOverlayCompoundImageRepresentation;
Expand All @@ -563,16 +561,13 @@ class GPU_GLES2_EXPORT OverlayImageRepresentation
scoped_refptr<gfx::NativePixmap> GetNativePixmap();
#elif BUILDFLAG(IS_WIN)
virtual scoped_refptr<gl::DCOMPSurfaceProxy> GetDCOMPSurfaceProxy();
virtual gl::GLImage* GetGLImage() = 0;
#elif BUILDFLAG(IS_MAC)
virtual gfx::ScopedIOSurface GetIOSurface() const;
// Return true if the macOS WindowServer is currently using the underlying
// storage for the image.
virtual bool IsInUseByWindowServer() const;
#endif

// TODO(penghuang): Refactor it to not depend on GL.
// Get the backing as GLImage for GLSurface::ScheduleOverlayPlane.
virtual gl::GLImage* GetGLImage() = 0;
};

#if BUILDFLAG(IS_ANDROID)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,7 @@ TEST_F(SharedImageRepresentationTest, OverlayClearing) {

// We should not be able to begin read ccess.
{
auto scoped_access =
representation->BeginScopedReadAccess(false /* needs_gl_image */);
auto scoped_access = representation->BeginScopedReadAccess();
EXPECT_FALSE(scoped_access);
}
EXPECT_FALSE(representation->IsCleared());
Expand All @@ -240,8 +239,7 @@ TEST_F(SharedImageRepresentationTest, OverlayClearing) {

// We can now begin read access.
{
auto scoped_access =
representation->BeginScopedReadAccess(false /* needs_gl_image */);
auto scoped_access = representation->BeginScopedReadAccess();
EXPECT_TRUE(scoped_access);
}
EXPECT_TRUE(representation->IsCleared());
Expand Down
Loading

0 comments on commit f32b16c

Please sign in to comment.