Skip to content

Commit

Permalink
Fix corrupted glDepthMask in SkImage destructor
Browse files Browse the repository at this point in the history
Quoting vasilyt:

So what happens:

* We made current virtual context of GLES2 Decoder.

* Someone deleted shared image SharedImageStub::OnDestroySharedImage.

* For performance reasons, SharedImageStub::MakeContextCurrent doesn't
switch virtual contexts, only real. It's done in assumptions, that we
don't really change GL state during it, just deleting resources (e.g
textures) [1].

* This assumption is wrong because of workaround (and maybe something
else) here [2]

This is similar to https://crbug.com/1095679, but in dtor. We need
context_state_->MakeCurrent(nullptr); in the beginning of
~WrappedSkImage [3]

What is interesting -- there is DCHECK for context_state_->IsCurrent(),
but if I read code correctly, that only checks for real context to be
current and I'm not sure if that's right.

[1] https://source.chromium.org/chromium/chromium/src/+/master:gpu/ipc/service/shared_image_stub.cc;drc=8cd33c0a2ffc73d284354e431687011534e29fb8;l=458

[2] https://source.chromium.org/chromium/chromium/src/+/master:third_party/skia/src/gpu/gl/GrGLGpu.cpp;drc=ae67c52933d2da6ea7b9cdff251738487bb84c74;l=2941

[3] https://source.chromium.org/chromium/chromium/src/+/master:gpu/command_buffer/service/wrapped_sk_image.cc;drc=67a4fbcab976e3c625bd241e10c3c3bed305137e;l=54

(cherry picked from commit 08a94c8)

Bug: 1167450,1184085
Change-Id: I6577b0d59575e499b92bda9f31dcfec4eca2f5d1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2729901
Commit-Queue: Klaus Weidner <klausw@chromium.org>
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Reviewed-by: Brandon Jones <bajones@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#859662}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2741479
Reviewed-by: Klaus Weidner <klausw@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/4430@{#206}
Cr-Branched-From: e5ce7dc-refs/heads/master@{#857950}
  • Loading branch information
klausw authored and Chromium LUCI CQ committed Mar 7, 2021
1 parent cf68e8e commit 7268813
Showing 1 changed file with 1 addition and 2 deletions.
3 changes: 1 addition & 2 deletions gpu/command_buffer/service/wrapped_sk_image.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,13 @@ SkImageInfo MakeSkImageInfo(const gfx::Size& size, viz::ResourceFormat format) {
class WrappedSkImage : public ClearTrackingSharedImageBacking {
public:
~WrappedSkImage() override {
context_state_->MakeCurrent(nullptr);
promise_texture_.reset();
context_state_->EraseCachedSkSurface(this);

if (backend_texture_.isValid())
DeleteGrBackendTexture(context_state_.get(), &backend_texture_);

DCHECK(context_state_->context_lost() ||
context_state_->IsCurrent(nullptr));
if (!context_state_->context_lost())
context_state_->set_need_context_state_reset(true);
}
Expand Down

0 comments on commit 7268813

Please sign in to comment.