Commit cd06d5d3 authored by kylechar's avatar kylechar Committed by Commit Bot

Revert "Fix Windows overlay texture lifetime"

This reverts commit 0be396a2.

Reason for revert: Speculative revert for https://crbug.com/1138919

Original change's description:
> Fix Windows overlay texture lifetime
>
> With SkiaRenderer on Windows overlay textures weren't kept alive after
> the client destroys them if still in use. Implement something in
> SkiaOutputDeviceGL that is similar SkiaOutputDeviceBufferQueue where we
> keep a map of mailboxes to textures to keep alive. Instead of using
> SharedImageRepresentationOverlay we have to use GLImage still as not all
> Windows video paths are using shared image.
>
> Bug: 1136194
> Change-Id: Ibeb5726b914058c0868a90dcc9bca74a8499c660
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2462038
> Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
> Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
> Commit-Queue: kylechar <kylechar@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#817164}

TBR=sunnyps@chromium.org,kylechar@chromium.org,vasilyt@chromium.org

Change-Id: I811b7f36afce932a359b72f5fe4e9df9cde43712
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1136194, 1138919
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2475756Reviewed-by: default avatarkylechar <kylechar@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#817497}
parent b4e1fb71
......@@ -4,11 +4,11 @@
#include "components/viz/service/display_embedder/skia_output_device_gl.h"
#include <tuple>
#include <utility>
#include "base/bind_helpers.h"
#include "base/debug/alias.h"
#include "build/build_config.h"
#include "components/viz/common/gpu/context_lost_reason.h"
#include "components/viz/service/display/dc_layer_overlay.h"
#include "gpu/command_buffer/common/swap_buffers_complete_params.h"
......@@ -49,51 +49,6 @@ NOINLINE void CheckForLoopFailures() {
} // namespace
// Holds reference needed to keep overlay textures alive. Can either hold a
// shared image or legacy GL texture.
// TODO(kylechar): Merge with SkiaOutputDeviceBufferQueue::OverlayData when we
// dont need to support TexturePassthrough anymore.
class SkiaOutputDeviceGL::OverlayData {
public:
// TODO(crbug.com/1011555): Remove ability to hold TexturePassthrough after
// all Window video paths use shared image API.
explicit OverlayData(scoped_refptr<gpu::gles2::TexturePassthrough> texture)
: texture_(std::move(texture)) {}
OverlayData(
std::unique_ptr<gpu::SharedImageRepresentationOverlay> representation,
std::unique_ptr<gpu::SharedImageRepresentationOverlay::ScopedReadAccess>
scoped_read_access)
: representation_(std::move(representation)),
scoped_read_access_(std::move(scoped_read_access)) {}
~OverlayData() = default;
OverlayData(OverlayData&& other) = default;
OverlayData& operator=(OverlayData&& other) {
texture_ = std::move(other.texture_);
// Must happen in the same order as destruction to avoid having
// |scoped_read_access_| outlive |representation_|.
scoped_read_access_ = std::move(other.scoped_read_access_);
representation_ = std::move(other.representation_);
return *this;
}
scoped_refptr<gl::GLImage> GetImage() {
if (texture_)
return texture_->GetLevelImage(texture_->target(), 0);
DCHECK(scoped_read_access_);
return scoped_read_access_->gl_image();
}
private:
std::unique_ptr<gpu::SharedImageRepresentationOverlay> representation_;
std::unique_ptr<gpu::SharedImageRepresentationOverlay::ScopedReadAccess>
scoped_read_access_;
scoped_refptr<gpu::gles2::TexturePassthrough> texture_;
};
SkiaOutputDeviceGL::SkiaOutputDeviceGL(
gpu::MailboxManager* mailbox_manager,
gpu::SharedImageRepresentationFactory* shared_image_representation_factory,
......@@ -292,14 +247,14 @@ void SkiaOutputDeviceGL::SwapBuffers(
gfx::Size(sk_surface_->width(), sk_surface_->height());
if (supports_async_swap_) {
auto callback = base::BindOnce(
&SkiaOutputDeviceGL::DoFinishSwapBuffersAsync,
weak_ptr_factory_.GetWeakPtr(), surface_size, std::move(latency_info));
auto callback = base::BindOnce(&SkiaOutputDeviceGL::DoFinishSwapBuffers,
weak_ptr_factory_.GetWeakPtr(), surface_size,
std::move(latency_info));
gl_surface_->SwapBuffersAsync(std::move(callback), std::move(feedback));
} else {
gfx::SwapResult result = gl_surface_->SwapBuffers(std::move(feedback));
DoFinishSwapBuffers(surface_size, std::move(latency_info),
gfx::SwapCompletionResult(result));
FinishSwapBuffers(gfx::SwapCompletionResult(result), surface_size,
std::move(latency_info));
}
}
......@@ -313,17 +268,17 @@ void SkiaOutputDeviceGL::PostSubBuffer(
gfx::Size(sk_surface_->width(), sk_surface_->height());
if (supports_async_swap_) {
auto callback = base::BindOnce(
&SkiaOutputDeviceGL::DoFinishSwapBuffersAsync,
weak_ptr_factory_.GetWeakPtr(), surface_size, std::move(latency_info));
auto callback = base::BindOnce(&SkiaOutputDeviceGL::DoFinishSwapBuffers,
weak_ptr_factory_.GetWeakPtr(), surface_size,
std::move(latency_info));
gl_surface_->PostSubBufferAsync(rect.x(), rect.y(), rect.width(),
rect.height(), std::move(callback),
std::move(feedback));
} else {
gfx::SwapResult result = gl_surface_->PostSubBuffer(
rect.x(), rect.y(), rect.width(), rect.height(), std::move(feedback));
DoFinishSwapBuffers(surface_size, std::move(latency_info),
gfx::SwapCompletionResult(result));
FinishSwapBuffers(gfx::SwapCompletionResult(result), surface_size,
std::move(latency_info));
}
}
......@@ -336,43 +291,24 @@ void SkiaOutputDeviceGL::CommitOverlayPlanes(
gfx::Size(sk_surface_->width(), sk_surface_->height());
if (supports_async_swap_) {
auto callback = base::BindOnce(
&SkiaOutputDeviceGL::DoFinishSwapBuffersAsync,
weak_ptr_factory_.GetWeakPtr(), surface_size, std::move(latency_info));
auto callback = base::BindOnce(&SkiaOutputDeviceGL::DoFinishSwapBuffers,
weak_ptr_factory_.GetWeakPtr(), surface_size,
std::move(latency_info));
gl_surface_->CommitOverlayPlanesAsync(std::move(callback),
std::move(feedback));
} else {
gfx::SwapResult result =
gl_surface_->CommitOverlayPlanes(std::move(feedback));
DoFinishSwapBuffers(surface_size, std::move(latency_info),
gfx::SwapCompletionResult(result));
FinishSwapBuffers(
gfx::SwapCompletionResult(
gl_surface_->CommitOverlayPlanes(std::move(feedback))),
surface_size, std::move(latency_info));
}
}
void SkiaOutputDeviceGL::DoFinishSwapBuffersAsync(
const gfx::Size& size,
std::vector<ui::LatencyInfo> latency_info,
gfx::SwapCompletionResult result) {
DCHECK(!result.gpu_fence);
FinishSwapBuffers(std::move(result), size, latency_info);
}
void SkiaOutputDeviceGL::DoFinishSwapBuffers(
const gfx::Size& size,
std::vector<ui::LatencyInfo> latency_info,
gfx::SwapCompletionResult result) {
DCHECK(!result.gpu_fence);
// Remove entries from |overlays_| for textures that weren't scheduled as an
// overlay this frame.
if (!overlays_.empty()) {
base::EraseIf(overlays_, [this](auto& entry) {
const gpu::Mailbox& mailbox = entry.first;
return !scheduled_overlay_mailboxes_.contains(mailbox);
});
scheduled_overlay_mailboxes_.clear();
}
FinishSwapBuffers(std::move(result), size, latency_info);
}
......@@ -397,17 +333,15 @@ void SkiaOutputDeviceGL::ScheduleOverlays(
// Get GLImages for DC layer textures.
bool success = true;
for (size_t i = 0; i < DCLayerOverlay::kNumResources; ++i) {
const gpu::Mailbox& mailbox = dc_layer.mailbox[i];
if (i > 0 && mailbox.IsZero())
if (i > 0 && dc_layer.mailbox[i].IsZero())
break;
auto image = GetGLImageForMailbox(mailbox);
auto image = GetGLImageForMailbox(dc_layer.mailbox[i]);
if (!image) {
success = false;
break;
}
scheduled_overlay_mailboxes_.insert(mailbox);
image->SetColorSpace(dc_layer.color_space);
params.images[i] = std::move(image);
}
......@@ -452,39 +386,38 @@ void SkiaOutputDeviceGL::EndPaint() {}
scoped_refptr<gl::GLImage> SkiaOutputDeviceGL::GetGLImageForMailbox(
const gpu::Mailbox& mailbox) {
auto it = overlays_.find(mailbox);
if (it != overlays_.end())
return it->second.GetImage();
// TODO(crbug.com/1005306): Stop using MailboxManager for lookup once all
// clients are using SharedImageInterface to create textures.
// For example, the legacy mailbox still uses GL textures (no overlay)
// and is still used.
if (!mailbox.IsSharedImage()) {
auto* texture_base = mailbox_manager_->ConsumeTexture(mailbox);
if (!texture_base)
auto* texture_base = mailbox_manager_->ConsumeTexture(mailbox);
if (!texture_base) {
auto overlay =
shared_image_representation_factory_->ProduceOverlay(mailbox);
if (!overlay)
return nullptr;
DCHECK_EQ(texture_base->GetType(), gpu::TextureBase::Type::kPassthrough);
std::tie(it, std::ignore) = overlays_.try_emplace(
mailbox,
base::WrapRefCounted(
static_cast<gpu::gles2::TexturePassthrough*>(texture_base)));
return it->second.GetImage();
// Return GLImage since the ScopedReadAccess isn't being held by anyone.
// TODO(crbug.com/1011555): Have SkiaOutputSurfaceImplOnGpu hold on to the
// ScopedReadAccess for overlays like it does for PromiseImage based
// resources.
std::unique_ptr<gpu::SharedImageRepresentationOverlay::ScopedReadAccess>
scoped_overlay_read_access =
overlay->BeginScopedReadAccess(/*need_gl_image=*/true);
DCHECK(scoped_overlay_read_access);
return scoped_overlay_read_access->gl_image();
}
auto overlay = shared_image_representation_factory_->ProduceOverlay(mailbox);
if (!overlay)
return nullptr;
std::unique_ptr<gpu::SharedImageRepresentationOverlay::ScopedReadAccess>
scoped_overlay_read_access =
overlay->BeginScopedReadAccess(/*need_gl_image=*/true);
DCHECK(scoped_overlay_read_access);
std::tie(it, std::ignore) = overlays_.try_emplace(
mailbox, std::move(overlay), std::move(scoped_overlay_read_access));
return it->second.GetImage();
if (texture_base->GetType() == gpu::TextureBase::Type::kPassthrough) {
gpu::gles2::TexturePassthrough* texture =
static_cast<gpu::gles2::TexturePassthrough*>(texture_base);
return texture->GetLevelImage(texture->target(), 0);
} else {
DCHECK_EQ(texture_base->GetType(), gpu::TextureBase::Type::kValidated);
gpu::gles2::Texture* texture =
static_cast<gpu::gles2::Texture*>(texture_base);
return texture->GetLevelImage(texture->target(), 0);
}
}
} // namespace viz
......@@ -8,8 +8,6 @@
#include <memory>
#include <vector>
#include "base/containers/flat_map.h"
#include "base/containers/flat_set.h"
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h"
......@@ -69,18 +67,11 @@ class SkiaOutputDeviceGL final : public SkiaOutputDevice {
void EndPaint() override;
private:
class OverlayData;
// Use instead of calling FinishSwapBuffers() directly. On Windows this cleans
// up old entries in |overlays_|.
// Used as callback for SwapBuffersAsync and PostSubBufferAsync to finish
// operation
void DoFinishSwapBuffers(const gfx::Size& size,
std::vector<ui::LatencyInfo> latency_info,
gfx::SwapCompletionResult result);
// Used as callback for SwapBuffersAsync and PostSubBufferAsync to finish
// operation
void DoFinishSwapBuffersAsync(const gfx::Size& size,
std::vector<ui::LatencyInfo> latency_info,
gfx::SwapCompletionResult result);
scoped_refptr<gl::GLImage> GetGLImageForMailbox(const gpu::Mailbox& mailbox);
......@@ -95,12 +86,6 @@ class SkiaOutputDeviceGL final : public SkiaOutputDevice {
sk_sp<SkSurface> sk_surface_;
// Mailboxes of overlays scheduled in the current frame.
base::flat_set<gpu::Mailbox> scheduled_overlay_mailboxes_;
// Holds references to overlay textures so they aren't destroyed while in use.
base::flat_map<gpu::Mailbox, OverlayData> overlays_;
uint64_t backbuffer_estimated_size_ = 0;
base::WeakPtrFactory<SkiaOutputDeviceGL> weak_ptr_factory_{this};
......
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