Commit 3a7d5ecf authored by Jonathan Backer's avatar Jonathan Backer Committed by Commit Bot

Make context current on teardown

Normally, SkiaOutputSurfaceImplOnGpu is responsible for making a GL
context current before any calls into DirectContextProvider (e.g.
ImplOnGpu::CopyResult, ImplOnGpu::PerformDelayedWork, ~ImplOnGpu).

One exception to this is if there is an asynchronous GLRendererCopier
readback in flight as ~ImplOnGpu is called (e.g. for tab picker on
Android). Specifically, if either ReadPixelsWorkflow::Finish or
~ReadPixelsWorkflow has not been called yet.

This CL does 2 things:

1) It does a glFinish() to guaranteee that ReadPixelsWorkflow::Finish
   will be called before ~ImplOnGpu finishes. This may cause some jank,
   but it is the simplest method to avoid a leak of GPU resources for
   in flight readbacks.

2) It does a MakeCurrent in DirectContextProvider::Destroy to make sure
   that a context is current when ~GLPixelBufferRGBAResult is called
   (see [1]).

[1] https://cs.chromium.org/chromium/src/components/viz/service/display/gl_renderer_copier.cc?rcl=ec3242c55275c8f1c2ea6ac2a0211c391eba7ddc&l=543

Bug: 1020558
Change-Id: I5092594b879efaedd65f2ed9ee3fce297930d294
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1898126
Commit-Queue: Jonathan Backer <backer@chromium.org>
Reviewed-by: default avatarkylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712274}
parent 8b759729
......@@ -96,6 +96,7 @@ DirectContextProvider::DirectContextProvider(
command_buffer_ = std::move(command_buffer);
decoder_ = std::move(decoder);
gl_context_ = std::move(gl_context);
gl_surface_ = std::move(gl_surface);
gles2_implementation_ = std::make_unique<gpu::gles2::GLES2Implementation>(
gles2_cmd_helper_.get(), nullptr, transfer_buffer_.get(),
......@@ -123,7 +124,11 @@ DirectContextProvider::~DirectContextProvider() {
void DirectContextProvider::Destroy() {
DCHECK(decoder_);
bool have_context = !decoder_->WasContextLost();
bool have_context = !decoder_->WasContextLost() &&
(gl_context_->IsCurrent(nullptr) ||
gl_context_->MakeCurrent(gl_surface_.get()));
if (have_context && framebuffer_id_ != 0) {
gles2_implementation_->DeleteFramebuffers(1, &framebuffer_id_);
framebuffer_id_ = 0;
......@@ -360,4 +365,9 @@ void DirectContextProvider::MarkContextLost() {
}
}
void DirectContextProvider::FinishQueries() {
if (decoder_->HasPendingQueries())
gles2_implementation_->Finish();
}
} // namespace viz
......@@ -102,6 +102,8 @@ class VIZ_SERVICE_EXPORT DirectContextProvider
GLuint GenClientTextureId();
void DeleteClientTextureId(GLuint client_id);
void MarkContextLost();
// Call a glFinish() to complete any pending queries.
void FinishQueries();
// ContextProvider implementation.
void AddRef() const override;
......@@ -172,6 +174,7 @@ class VIZ_SERVICE_EXPORT DirectContextProvider
std::unique_ptr<gpu::gles2::GLES2Decoder> decoder_;
std::unique_ptr<gpu::TransferBuffer> transfer_buffer_;
scoped_refptr<gl::GLContext> gl_context_;
scoped_refptr<gl::GLSurface> gl_surface_;
std::unique_ptr<gpu::gles2::GLES2Implementation> gles2_implementation_;
GLuint framebuffer_id_ = 0;
......
......@@ -651,6 +651,8 @@ SkiaOutputSurfaceImplOnGpu::~SkiaOutputSurfaceImplOnGpu() {
}
if (copier_) {
context_provider_->FinishQueries();
copier_ = nullptr;
texture_deleter_ = nullptr;
context_provider_ = nullptr;
......
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