Commit 2814d325 authored by Jonathan Backer's avatar Jonathan Backer Committed by Commit Bot

SkiaRenderer: Handle context lost errors

With this CL, SkiaRenderer will receive a signal from
GpuServiceImpl::LoseAllContexts and mark the shared context_state_ as
lost. This will cause us to abandon all future draws and no-op the
releases.

Unfortunately, we cannot delete fulfilled promise images after
GrContext::abandonContext is called. So an explicit leak in
ImageContextImpl::OnContextLost was added.

TBR=boliu@chromium.org

Bug: 1011420
Change-Id: I5f5e178b6a51d770534c2260274f0b9c75c21831
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1864413
Commit-Queue: Jonathan Backer <backer@chromium.org>
Reviewed-by: default avatarkylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706922}
parent 4e491965
...@@ -108,4 +108,14 @@ SkiaOutputSurfaceDependencyWebView::CreateGLSurface( ...@@ -108,4 +108,14 @@ SkiaOutputSurfaceDependencyWebView::CreateGLSurface(
return gl_surface_; return gl_surface_;
} }
void SkiaOutputSurfaceDependencyWebView::RegisterDisplayContext(
gpu::DisplayContext* display_context) {
// No GpuChannelManagerDelegate here, so leave it no-op for now.
}
void SkiaOutputSurfaceDependencyWebView::UnregisterDisplayContext(
gpu::DisplayContext* display_context) {
// No GpuChannelManagerDelegate here, so leave it no-op for now.
}
} // namespace android_webview } // namespace android_webview
...@@ -43,6 +43,8 @@ class SkiaOutputSurfaceDependencyWebView ...@@ -43,6 +43,8 @@ class SkiaOutputSurfaceDependencyWebView
gpu::SurfaceHandle GetSurfaceHandle() override; gpu::SurfaceHandle GetSurfaceHandle() override;
scoped_refptr<gl::GLSurface> CreateGLSurface( scoped_refptr<gl::GLSurface> CreateGLSurface(
base::WeakPtr<gpu::ImageTransportSurfaceDelegate> stub) override; base::WeakPtr<gpu::ImageTransportSurfaceDelegate> stub) override;
void RegisterDisplayContext(gpu::DisplayContext* display_context) override;
void UnregisterDisplayContext(gpu::DisplayContext* display_context) override;
private: private:
gl::GLSurface* const gl_surface_; gl::GLSurface* const gl_surface_;
......
...@@ -20,6 +20,10 @@ ExternalUseClient::ImageContext::ImageContext( ...@@ -20,6 +20,10 @@ ExternalUseClient::ImageContext::ImageContext(
ExternalUseClient::ImageContext::~ImageContext() = default; ExternalUseClient::ImageContext::~ImageContext() = default;
void ExternalUseClient::ImageContext::OnContextLost() {
NOTREACHED();
}
void ExternalUseClient::ImageContext::SetImage(sk_sp<SkImage> image, void ExternalUseClient::ImageContext::SetImage(sk_sp<SkImage> image,
GrBackendFormat backend_format) { GrBackendFormat backend_format) {
DCHECK(!image_); DCHECK(!image_);
......
...@@ -38,6 +38,7 @@ class VIZ_SERVICE_EXPORT ExternalUseClient { ...@@ -38,6 +38,7 @@ class VIZ_SERVICE_EXPORT ExternalUseClient {
const base::Optional<gpu::VulkanYCbCrInfo>& ycbcr_info, const base::Optional<gpu::VulkanYCbCrInfo>& ycbcr_info,
sk_sp<SkColorSpace> color_space); sk_sp<SkColorSpace> color_space);
virtual ~ImageContext(); virtual ~ImageContext();
virtual void OnContextLost();
// //
// Thread safety is guaranteed by these invariants: (a) only the compositor // Thread safety is guaranteed by these invariants: (a) only the compositor
......
...@@ -42,6 +42,13 @@ ImageContextImpl::ImageContextImpl(RenderPassId render_pass_id, ...@@ -42,6 +42,13 @@ ImageContextImpl::ImageContextImpl(RenderPassId render_pass_id,
render_pass_id_(render_pass_id), render_pass_id_(render_pass_id),
mipmap_(mipmap ? GrMipMapped::kYes : GrMipMapped::kNo) {} mipmap_(mipmap ? GrMipMapped::kYes : GrMipMapped::kNo) {}
void ImageContextImpl::OnContextLost() {
if (representation_) {
representation_->OnContextLost();
representation_ = nullptr;
}
}
ImageContextImpl::~ImageContextImpl() { ImageContextImpl::~ImageContextImpl() {
DCHECK(!representation_scoped_read_access_); DCHECK(!representation_scoped_read_access_);
......
...@@ -58,6 +58,8 @@ class ImageContextImpl final : public ExternalUseClient::ImageContext { ...@@ -58,6 +58,8 @@ class ImageContextImpl final : public ExternalUseClient::ImageContext {
sk_sp<SkColorSpace> color_space); sk_sp<SkColorSpace> color_space);
~ImageContextImpl() final; ~ImageContextImpl() final;
void OnContextLost() final;
RenderPassId render_pass_id() const { return render_pass_id_; } RenderPassId render_pass_id() const { return render_pass_id_; }
GrMipMapped mipmap() const { return mipmap_; } GrMipMapped mipmap() const { return mipmap_; }
......
...@@ -21,6 +21,7 @@ class GLSurface; ...@@ -21,6 +21,7 @@ class GLSurface;
namespace gpu { namespace gpu {
class DisplayContext;
class GpuDriverBugWorkarounds; class GpuDriverBugWorkarounds;
class ImageFactory; class ImageFactory;
class ImageTransportSurfaceDelegate; class ImageTransportSurfaceDelegate;
...@@ -86,6 +87,10 @@ class VIZ_SERVICE_EXPORT SkiaOutputSurfaceDependency { ...@@ -86,6 +87,10 @@ class VIZ_SERVICE_EXPORT SkiaOutputSurfaceDependency {
gpu::SurfaceHandle parent_window, gpu::SurfaceHandle parent_window,
gpu::SurfaceHandle child_window) = 0; gpu::SurfaceHandle child_window) = 0;
#endif #endif
virtual void RegisterDisplayContext(gpu::DisplayContext* display_context) = 0;
virtual void UnregisterDisplayContext(
gpu::DisplayContext* display_context) = 0;
}; };
} // namespace viz } // namespace viz
......
...@@ -115,4 +115,14 @@ void SkiaOutputSurfaceDependencyImpl::DidCreateAcceleratedSurfaceChildWindow( ...@@ -115,4 +115,14 @@ void SkiaOutputSurfaceDependencyImpl::DidCreateAcceleratedSurfaceChildWindow(
} }
#endif #endif
void SkiaOutputSurfaceDependencyImpl::RegisterDisplayContext(
gpu::DisplayContext* display_context) {
gpu_service_impl_->RegisterDisplayContext(display_context);
}
void SkiaOutputSurfaceDependencyImpl::UnregisterDisplayContext(
gpu::DisplayContext* display_context) {
gpu_service_impl_->UnregisterDisplayContext(display_context);
}
} // namespace viz } // namespace viz
...@@ -49,6 +49,9 @@ class VIZ_SERVICE_EXPORT SkiaOutputSurfaceDependencyImpl ...@@ -49,6 +49,9 @@ class VIZ_SERVICE_EXPORT SkiaOutputSurfaceDependencyImpl
gpu::SurfaceHandle child_window) override; gpu::SurfaceHandle child_window) override;
#endif #endif
void RegisterDisplayContext(gpu::DisplayContext* display_context) override;
void UnregisterDisplayContext(gpu::DisplayContext* display_context) override;
private: private:
GpuServiceImpl* const gpu_service_impl_; GpuServiceImpl* const gpu_service_impl_;
const gpu::SurfaceHandle surface_handle_; const gpu::SurfaceHandle surface_handle_;
......
...@@ -52,18 +52,37 @@ sk_sp<SkPromiseImageTexture> Fulfill(void* texture_context) { ...@@ -52,18 +52,37 @@ sk_sp<SkPromiseImageTexture> Fulfill(void* texture_context) {
void DoNothing(void* texture_context) {} void DoNothing(void* texture_context) {}
template <typename... Args> template <typename... Args>
void PostAsyncTask(SkiaOutputSurfaceDependency* dependency, void PostAsyncTaskRepeatedly(
const base::RepeatingCallback<void(Args...)>& callback, SkiaOutputSurfaceDependency* dependency,
Args... args) { const base::RepeatingCallback<void(Args...)>& callback,
Args... args) {
dependency->PostTaskToClientThread(base::BindOnce(callback, args...)); dependency->PostTaskToClientThread(base::BindOnce(callback, args...));
} }
template <typename... Args> template <typename... Args>
base::RepeatingCallback<void(Args...)> CreateSafeCallback( base::RepeatingCallback<void(Args...)> CreateSafeRepeatingCallback(
SkiaOutputSurfaceDependency* dependency, SkiaOutputSurfaceDependency* dependency,
const base::RepeatingCallback<void(Args...)>& callback) { const base::RepeatingCallback<void(Args...)>& callback) {
DCHECK(dependency); DCHECK(dependency);
return base::BindRepeating(&PostAsyncTask<Args...>, dependency, callback); return base::BindRepeating(&PostAsyncTaskRepeatedly<Args...>, dependency,
callback);
}
template <typename... Args>
void PostAsyncTaskOnce(SkiaOutputSurfaceDependency* dependency,
base::OnceCallback<void(Args...)> callback,
Args... args) {
dependency->PostTaskToClientThread(
base::BindOnce(std::move(callback), args...));
}
template <typename... Args>
base::OnceCallback<void(Args...)> CreateSafeOnceCallback(
SkiaOutputSurfaceDependency* dependency,
base::OnceCallback<void(Args...)> callback) {
DCHECK(dependency);
return base::BindOnce(&PostAsyncTaskOnce<Args...>, dependency,
std::move(callback));
} }
} // namespace } // namespace
...@@ -656,17 +675,17 @@ void SkiaOutputSurfaceImpl::InitializeOnGpuThread(base::WaitableEvent* event, ...@@ -656,17 +675,17 @@ void SkiaOutputSurfaceImpl::InitializeOnGpuThread(base::WaitableEvent* event,
base::BindOnce(&base::WaitableEvent::Signal, base::Unretained(event))); base::BindOnce(&base::WaitableEvent::Signal, base::Unretained(event)));
} }
auto did_swap_buffer_complete_callback = CreateSafeCallback( auto did_swap_buffer_complete_callback = CreateSafeRepeatingCallback(
dependency_.get(), dependency_.get(),
base::BindRepeating(&SkiaOutputSurfaceImpl::DidSwapBuffersComplete, base::BindRepeating(&SkiaOutputSurfaceImpl::DidSwapBuffersComplete,
weak_ptr_)); weak_ptr_));
auto buffer_presented_callback = CreateSafeCallback( auto buffer_presented_callback = CreateSafeRepeatingCallback(
dependency_.get(), dependency_.get(),
base::BindRepeating(&SkiaOutputSurfaceImpl::BufferPresented, weak_ptr_)); base::BindRepeating(&SkiaOutputSurfaceImpl::BufferPresented, weak_ptr_));
auto context_lost_callback = CreateSafeCallback( auto context_lost_callback = CreateSafeOnceCallback(
dependency_.get(), dependency_.get(),
base::BindRepeating(&SkiaOutputSurfaceImpl::ContextLost, weak_ptr_)); base::BindOnce(&SkiaOutputSurfaceImpl::ContextLost, weak_ptr_));
auto gpu_vsync_callback = CreateSafeCallback( auto gpu_vsync_callback = CreateSafeRepeatingCallback(
dependency_.get(), dependency_.get(),
base::BindRepeating(&SkiaOutputSurfaceImpl::OnGpuVSync, weak_ptr_)); base::BindRepeating(&SkiaOutputSurfaceImpl::OnGpuVSync, weak_ptr_));
......
...@@ -626,11 +626,14 @@ SkiaOutputSurfaceImplOnGpu::SkiaOutputSurfaceImplOnGpu( ...@@ -626,11 +626,14 @@ SkiaOutputSurfaceImplOnGpu::SkiaOutputSurfaceImplOnGpu(
gpu_vsync_callback_(std::move(gpu_vsync_callback)), gpu_vsync_callback_(std::move(gpu_vsync_callback)),
gpu_preferences_(dependency_->GetGpuPreferences()) { gpu_preferences_(dependency_->GetGpuPreferences()) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
dependency_->RegisterDisplayContext(this);
} }
SkiaOutputSurfaceImplOnGpu::~SkiaOutputSurfaceImplOnGpu() { SkiaOutputSurfaceImplOnGpu::~SkiaOutputSurfaceImplOnGpu() {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
dependency_->UnregisterDisplayContext(this);
// |context_provider_| and clients want either the context to be lost or made // |context_provider_| and clients want either the context to be lost or made
// current on destruction. // current on destruction.
if (context_state_ && MakeCurrent(false /* need_fbo0 */)) { if (context_state_ && MakeCurrent(false /* need_fbo0 */)) {
...@@ -1214,7 +1217,10 @@ void SkiaOutputSurfaceImplOnGpu::ReleaseImageContexts( ...@@ -1214,7 +1217,10 @@ void SkiaOutputSurfaceImplOnGpu::ReleaseImageContexts(
DCHECK(!image_contexts.empty()); DCHECK(!image_contexts.empty());
// The window could be destroyed already, and the MakeCurrent will fail with // The window could be destroyed already, and the MakeCurrent will fail with
// an destroyed window, so MakeCurrent without requiring the fbo0. // an destroyed window, so MakeCurrent without requiring the fbo0.
MakeCurrent(false /* need_fbo0 */); if (!MakeCurrent(false /* need_fbo0 */)) {
for (const auto& context : image_contexts)
context->OnContextLost();
}
// |image_contexts| goes out of scope here. // |image_contexts| goes out of scope here.
} }
...@@ -1354,13 +1360,14 @@ bool SkiaOutputSurfaceImplOnGpu::InitializeForVulkan() { ...@@ -1354,13 +1360,14 @@ bool SkiaOutputSurfaceImplOnGpu::InitializeForVulkan() {
bool SkiaOutputSurfaceImplOnGpu::MakeCurrent(bool need_fbo0) { bool SkiaOutputSurfaceImplOnGpu::MakeCurrent(bool need_fbo0) {
if (!is_using_vulkan()) { if (!is_using_vulkan()) {
if (context_state_->context_lost())
return false;
// Only make current with |gl_surface_|, if following operations will use // Only make current with |gl_surface_|, if following operations will use
// fbo0. // fbo0.
if (!context_state_->MakeCurrent(need_fbo0 ? gl_surface_.get() : nullptr)) { if (!context_state_->MakeCurrent(need_fbo0 ? gl_surface_.get() : nullptr)) {
LOG(ERROR) << "Failed to make current."; LOG(ERROR) << "Failed to make current.";
context_lost_callback_.Run(); MarkContextLost();
if (context_provider_)
context_provider_->MarkContextLost();
return false; return false;
} }
context_state_->set_need_context_state_reset(true); context_state_->set_need_context_state_reset(true);
...@@ -1474,4 +1481,13 @@ void SkiaOutputSurfaceImplOnGpu::RenderToOverlay( ...@@ -1474,4 +1481,13 @@ void SkiaOutputSurfaceImplOnGpu::RenderToOverlay(
#endif #endif
} }
void SkiaOutputSurfaceImplOnGpu::MarkContextLost() {
context_state_->MarkContextLost();
if (context_lost_callback_) {
std::move(context_lost_callback_).Run();
if (context_provider_)
context_provider_->MarkContextLost();
}
}
} // namespace viz } // namespace viz
...@@ -26,6 +26,7 @@ ...@@ -26,6 +26,7 @@
#include "gpu/command_buffer/service/shared_context_state.h" #include "gpu/command_buffer/service/shared_context_state.h"
#include "gpu/command_buffer/service/sync_point_manager.h" #include "gpu/command_buffer/service/sync_point_manager.h"
#include "gpu/ipc/in_process_command_buffer.h" #include "gpu/ipc/in_process_command_buffer.h"
#include "gpu/ipc/service/display_context.h"
#include "gpu/ipc/service/image_transport_surface_delegate.h" #include "gpu/ipc/service/image_transport_surface_delegate.h"
#include "third_party/skia/include/core/SkPromiseImageTexture.h" #include "third_party/skia/include/core/SkPromiseImageTexture.h"
#include "third_party/skia/include/core/SkSurface.h" #include "third_party/skia/include/core/SkSurface.h"
...@@ -71,14 +72,15 @@ struct RenderPassGeometry; ...@@ -71,14 +72,15 @@ struct RenderPassGeometry;
// The SkiaOutputSurface implementation running on the GPU thread. This class // The SkiaOutputSurface implementation running on the GPU thread. This class
// should be created, used and destroyed on the GPU thread. // should be created, used and destroyed on the GPU thread.
class SkiaOutputSurfaceImplOnGpu : public gpu::ImageTransportSurfaceDelegate { class SkiaOutputSurfaceImplOnGpu : public gpu::ImageTransportSurfaceDelegate,
public gpu::DisplayContext {
public: public:
using DidSwapBufferCompleteCallback = using DidSwapBufferCompleteCallback =
base::RepeatingCallback<void(gpu::SwapBuffersCompleteParams, base::RepeatingCallback<void(gpu::SwapBuffersCompleteParams,
const gfx::Size& pixel_size)>; const gfx::Size& pixel_size)>;
using BufferPresentedCallback = using BufferPresentedCallback =
base::RepeatingCallback<void(const gfx::PresentationFeedback& feedback)>; base::RepeatingCallback<void(const gfx::PresentationFeedback& feedback)>;
using ContextLostCallback = base::RepeatingCallback<void()>; using ContextLostCallback = base::OnceClosure;
static std::unique_ptr<SkiaOutputSurfaceImplOnGpu> Create( static std::unique_ptr<SkiaOutputSurfaceImplOnGpu> Create(
SkiaOutputSurfaceDependency* deps, SkiaOutputSurfaceDependency* deps,
...@@ -189,6 +191,9 @@ class SkiaOutputSurfaceImplOnGpu : public gpu::ImageTransportSurfaceDelegate { ...@@ -189,6 +191,9 @@ class SkiaOutputSurfaceImplOnGpu : public gpu::ImageTransportSurfaceDelegate {
void RenderToOverlay(gpu::Mailbox overlay_candidate_mailbox, void RenderToOverlay(gpu::Mailbox overlay_candidate_mailbox,
const gfx::Rect& bounds); const gfx::Rect& bounds);
// gpu::DisplayContext implementation:
void MarkContextLost() override;
private: private:
class ScopedPromiseImageAccess; class ScopedPromiseImageAccess;
...@@ -229,7 +234,7 @@ class SkiaOutputSurfaceImplOnGpu : public gpu::ImageTransportSurfaceDelegate { ...@@ -229,7 +234,7 @@ class SkiaOutputSurfaceImplOnGpu : public gpu::ImageTransportSurfaceDelegate {
const gpu::SequenceId sequence_id_; const gpu::SequenceId sequence_id_;
const DidSwapBufferCompleteCallback did_swap_buffer_complete_callback_; const DidSwapBufferCompleteCallback did_swap_buffer_complete_callback_;
const BufferPresentedCallback buffer_presented_callback_; const BufferPresentedCallback buffer_presented_callback_;
const ContextLostCallback context_lost_callback_; ContextLostCallback context_lost_callback_;
const GpuVSyncCallback gpu_vsync_callback_; const GpuVSyncCallback gpu_vsync_callback_;
#if defined(USE_OZONE) #if defined(USE_OZONE)
......
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