Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1941353003:
Allow stencils to be attached to render targets created via SkSurface::MakeFromBackendTextureAsRend… (Closed)
DescriptionAllow stencils to be attached to render targets created via SkSurface::MakeFromBackendTextureAsRenderTarget
This is a regression from "Refactor to separate backend object lifecycle
and GpuResource budget decision".
GrGLRenderer::canAttemptStencilAttachment was incorrectly returning false
for all wrapped render targets. This function should return false only if
the FBO is wrapped (unowned). If the FBO is owned by Skia, we can attach
stencils.
BUG=608238
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1924183003
Committed: https://skia.googlesource.com/skia/+/c4025189d31efcb0d54bf14b7712b38725f86c13
Patch Set 1 : #
Total comments: 1
Patch Set 2 : Fix unit tests to avoid ANGLE issues #
Messages
Total messages: 20 (10 generated)
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Patchset #1 (id:40001) has been deleted
ericrk@chromium.org changed reviewers: + bsalomon@google.com, kkinnunen@nvidia.com
Dug into the failure with the other CL and took a different approach to the fix (previous approach doesn't work, as it doesn't allow Chrome to bind texture rectangles as RTs). Pulled most of the unit tests from the previous CL, but tweaked the stencil test to directly check stencil capabilities rather than using rendering (which seemed to work around stencil in some cases).
lgtm
The CQ bit was checked by ericrk@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1941353003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1941353003/60001
Message was sent while issue was closed.
Description was changed from ========== Allow stencils to be attached to render targets created via SkSurface::MakeFromBackendTextureAsRenderTarget This is a regression from "Refactor to separate backend object lifecycle and GpuResource budget decision". GrGLRenderer::canAttemptStencilAttachment was incorrectly returning false for all wrapped render targets. This function should return false only if the FBO is wrapped (unowned). If the FBO is owned by Skia, we can attach stencils. BUG=608238 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Allow stencils to be attached to render targets created via SkSurface::MakeFromBackendTextureAsRenderTarget This is a regression from "Refactor to separate backend object lifecycle and GpuResource budget decision". GrGLRenderer::canAttemptStencilAttachment was incorrectly returning false for all wrapped render targets. This function should return false only if the FBO is wrapped (unowned). If the FBO is owned by Skia, we can attach stencils. BUG=608238 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/0736f3386820f19c0fe90b5dda2094e253780071 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:60001) as https://skia.googlesource.com/skia/+/0736f3386820f19c0fe90b5dda2094e253780071
Message was sent while issue was closed.
Thanks for fixing this, I did not understand to think about the rectangle textures. Small nitpicking inline. The resulting bug or change in behavior mentioned is much more benign than the original.. https://codereview.chromium.org/1941353003/diff/60001/src/gpu/gl/GrGLRenderTa... File src/gpu/gl/GrGLRenderTarget.cpp (right): https://codereview.chromium.org/1941353003/diff/60001/src/gpu/gl/GrGLRenderTa... src/gpu/gl/GrGLRenderTarget.cpp:163: return this->fRTFBOOwnership == GrBackendObjectOwnership::kOwned; This was not the intention, though. The "owns" does not mean "created". Previously objects created outside Skia were not modified. I supposed this is because you can not really know how they are created and what features they support.
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:60001) has been created in https://codereview.chromium.org/1947143002/ by jvanverth@google.com. The reason for reverting is: Breaking the ANGLE bots. Message is: Caught exception 3221225477 EXCEPTION_ACCESS_VIOLATION 8.59m elapsed, 1 active, 82 queued, 315MB RAM, 1442MB peak unit test SurfaceAttachStencil_Gpu step returned non-zero exit code: -1073741819.
Message was sent while issue was closed.
Description was changed from ========== Allow stencils to be attached to render targets created via SkSurface::MakeFromBackendTextureAsRenderTarget This is a regression from "Refactor to separate backend object lifecycle and GpuResource budget decision". GrGLRenderer::canAttemptStencilAttachment was incorrectly returning false for all wrapped render targets. This function should return false only if the FBO is wrapped (unowned). If the FBO is owned by Skia, we can attach stencils. BUG=608238 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/0736f3386820f19c0fe90b5dda2094e253780071 ========== to ========== Allow stencils to be attached to render targets created via SkSurface::MakeFromBackendTextureAsRenderTarget This is a regression from "Refactor to separate backend object lifecycle and GpuResource budget decision". GrGLRenderer::canAttemptStencilAttachment was incorrectly returning false for all wrapped render targets. This function should return false only if the FBO is wrapped (unowned). If the FBO is owned by Skia, we can attach stencils. BUG=608238 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/0736f3386820f19c0fe90b5dda2094e253780071 ==========
Description was changed from ========== Allow stencils to be attached to render targets created via SkSurface::MakeFromBackendTextureAsRenderTarget This is a regression from "Refactor to separate backend object lifecycle and GpuResource budget decision". GrGLRenderer::canAttemptStencilAttachment was incorrectly returning false for all wrapped render targets. This function should return false only if the FBO is wrapped (unowned). If the FBO is owned by Skia, we can attach stencils. BUG=608238 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/0736f3386820f19c0fe90b5dda2094e253780071 ========== to ========== Allow stencils to be attached to render targets created via SkSurface::MakeFromBackendTextureAsRenderTarget This is a regression from "Refactor to separate backend object lifecycle and GpuResource budget decision". GrGLRenderer::canAttemptStencilAttachment was incorrectly returning false for all wrapped render targets. This function should return false only if the FBO is wrapped (unowned). If the FBO is owned by Skia, we can attach stencils. BUG=608238 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Touched up the CL to avoid issues on AGNLE. Also added a comment to explain why the owned/borrowed/adopted issue Kimmo points out shouldn't impact this case.
lgtm. Given that we want to eliminate GrRenderTarget and have no plans to support adopting FBO IDs I'm ok with assuming that ownership means creation here.
The CQ bit was checked by ericrk@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1941353003/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1941353003/80001
Message was sent while issue was closed.
Description was changed from ========== Allow stencils to be attached to render targets created via SkSurface::MakeFromBackendTextureAsRenderTarget This is a regression from "Refactor to separate backend object lifecycle and GpuResource budget decision". GrGLRenderer::canAttemptStencilAttachment was incorrectly returning false for all wrapped render targets. This function should return false only if the FBO is wrapped (unowned). If the FBO is owned by Skia, we can attach stencils. BUG=608238 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Allow stencils to be attached to render targets created via SkSurface::MakeFromBackendTextureAsRenderTarget This is a regression from "Refactor to separate backend object lifecycle and GpuResource budget decision". GrGLRenderer::canAttemptStencilAttachment was incorrectly returning false for all wrapped render targets. This function should return false only if the FBO is wrapped (unowned). If the FBO is owned by Skia, we can attach stencils. BUG=608238 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/c4025189d31efcb0d54bf14b7712b38725f86c13 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:80001) as https://skia.googlesource.com/skia/+/c4025189d31efcb0d54bf14b7712b38725f86c13 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
