Implement local resources |
||
Issue descriptionLocal resources are resources used inside the same GL context and will not being sent into another process. So no need to create fence and mailbox for these resources. The detailed plan is here: https://docs.google.com/document/d/1i_xe5dTx2oUHbQI4rSmLcehV7xlI5oOq85ZMscRZQWc/edit
,
Nov 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/43d70d5dd490d5bf146cbb35c79a53edcbdf3a25 commit 43d70d5dd490d5bf146cbb35c79a53edcbdf3a25 Author: danakj <danakj@chromium.org> Date: Thu Nov 09 01:12:34 2017 When calling UseRenderPass() to restore, ensure it runs. UseRenderPass() has two use cases, either to set up a render pass as the framebuffer for drawing at the start of the draw, or to restore us back to that state after making some intermediate change to which framebuffer is bound. UseRenderPass() has some optimizations where it can return false and not do anything, if it detects that we don't want to draw the render pass. The first use case, in DrawRenderPass() makes use of this return value to avoid drawing anything. Other callers to UseRenderPass() do not check the return value because we're already drawing the pass and they are trying to restore state. If it early outs for these it would be unexpected and produce bugs. Currently DecideRenderPassAllocationsForFrame modifies render passes to try avoid such bugs, but there are other ways to encounter the bug. This changes UseRenderPass() to never early out so the latter cases will not avoid resetting the framebuffer. The former case can avoid UseRenderPass() entirely by checking CanSkipRenderPass(). If it is true, then it does not need to be drawn. Also, we will now ensure that UseRenderPass() is called after using a ScopedUseGrContext when drawing a RenderPassDrawQuad. Currently on errors it would early out before restoring the framebuffer, which could cause bugs for the next quads in the RenderPass. This was introduced when the UseRenderPass() call was moved out of the ~ScopedUseGrContext() destructor in https://codereview.chromium.org/2203033005. This all came up via https://chromium-review.googlesource.com/c/chromium/src/+/757027#message-b6e905925e834419ff448e2093a392053d5d321a R=enne@chromium.org, wutao@chromium.org Bug: 782044 , 738190 , 782042 Change-Id: Ib4662c5fee3f326642add12bbfe3d51e51dff1c9 Reviewed-on: https://chromium-review.googlesource.com/757049 Commit-Queue: danakj <danakj@chromium.org> Reviewed-by: enne <enne@chromium.org> Reviewed-by: Tao Wu <wutao@chromium.org> Cr-Commit-Position: refs/heads/master@{#515043} [modify] https://crrev.com/43d70d5dd490d5bf146cbb35c79a53edcbdf3a25/components/viz/service/display/direct_renderer.cc [modify] https://crrev.com/43d70d5dd490d5bf146cbb35c79a53edcbdf3a25/components/viz/service/display/direct_renderer.h [modify] https://crrev.com/43d70d5dd490d5bf146cbb35c79a53edcbdf3a25/components/viz/service/display/gl_renderer.cc [modify] https://crrev.com/43d70d5dd490d5bf146cbb35c79a53edcbdf3a25/components/viz/service/display/gl_renderer.h [modify] https://crrev.com/43d70d5dd490d5bf146cbb35c79a53edcbdf3a25/components/viz/service/display/skia_renderer.cc [modify] https://crrev.com/43d70d5dd490d5bf146cbb35c79a53edcbdf3a25/components/viz/service/display/skia_renderer.h [modify] https://crrev.com/43d70d5dd490d5bf146cbb35c79a53edcbdf3a25/components/viz/service/display/software_renderer.cc [modify] https://crrev.com/43d70d5dd490d5bf146cbb35c79a53edcbdf3a25/components/viz/service/display/software_renderer.h
,
Nov 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6dcd9ac14c9e5fbb60c659143b40bcc54309486e commit 6dcd9ac14c9e5fbb60c659143b40bcc54309486e Author: Ned Nguyen <nednguyen@google.com> Date: Thu Nov 09 16:08:22 2017 Revert "When calling UseRenderPass() to restore, ensure it runs." This reverts commit 43d70d5dd490d5bf146cbb35c79a53edcbdf3a25. Reason for revert: cause system_health.memory_desktop.browse:media:youtube to crash (BUG:783198) Original change's description: > When calling UseRenderPass() to restore, ensure it runs. > > UseRenderPass() has two use cases, either to set up a render pass > as the framebuffer for drawing at the start of the draw, or to restore > us back to that state after making some intermediate change to which > framebuffer is bound. > > UseRenderPass() has some optimizations where it can return false and > not do anything, if it detects that we don't want to draw the render > pass. The first use case, in DrawRenderPass() makes use of this return > value to avoid drawing anything. > > Other callers to UseRenderPass() do not check the return value because > we're already drawing the pass and they are trying to restore state. If > it early outs for these it would be unexpected and produce bugs. > > Currently DecideRenderPassAllocationsForFrame modifies render passes to > try avoid such bugs, but there are other ways to encounter the bug. > > This changes UseRenderPass() to never early out so the latter cases will > not avoid resetting the framebuffer. The former case can avoid > UseRenderPass() entirely by checking CanSkipRenderPass(). If it is true, > then it does not need to be drawn. > > Also, we will now ensure that UseRenderPass() is called after using a > ScopedUseGrContext when drawing a RenderPassDrawQuad. Currently on errors > it would early out before restoring the framebuffer, which could cause > bugs for the next quads in the RenderPass. This was introduced when the > UseRenderPass() call was moved out of the ~ScopedUseGrContext() > destructor in https://codereview.chromium.org/2203033005. > > This all came up via https://chromium-review.googlesource.com/c/chromium/src/+/757027#message-b6e905925e834419ff448e2093a392053d5d321a > > R=enne@chromium.org, wutao@chromium.org > > Bug: 782044 , 738190 , 782042 > Change-Id: Ib4662c5fee3f326642add12bbfe3d51e51dff1c9 > Reviewed-on: https://chromium-review.googlesource.com/757049 > Commit-Queue: danakj <danakj@chromium.org> > Reviewed-by: enne <enne@chromium.org> > Reviewed-by: Tao Wu <wutao@chromium.org> > Cr-Commit-Position: refs/heads/master@{#515043} TBR=danakj@chromium.org,enne@chromium.org,wutao@chromium.org Change-Id: I2acd86c21290c053b1801732599c1ab355161469 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 782044 , 738190 , 782042 Reviewed-on: https://chromium-review.googlesource.com/760557 Reviewed-by: Ned Nguyen <nednguyen@google.com> Commit-Queue: Ned Nguyen <nednguyen@google.com> Cr-Commit-Position: refs/heads/master@{#515178} [modify] https://crrev.com/6dcd9ac14c9e5fbb60c659143b40bcc54309486e/components/viz/service/display/direct_renderer.cc [modify] https://crrev.com/6dcd9ac14c9e5fbb60c659143b40bcc54309486e/components/viz/service/display/direct_renderer.h [modify] https://crrev.com/6dcd9ac14c9e5fbb60c659143b40bcc54309486e/components/viz/service/display/gl_renderer.cc [modify] https://crrev.com/6dcd9ac14c9e5fbb60c659143b40bcc54309486e/components/viz/service/display/gl_renderer.h [modify] https://crrev.com/6dcd9ac14c9e5fbb60c659143b40bcc54309486e/components/viz/service/display/skia_renderer.cc [modify] https://crrev.com/6dcd9ac14c9e5fbb60c659143b40bcc54309486e/components/viz/service/display/skia_renderer.h [modify] https://crrev.com/6dcd9ac14c9e5fbb60c659143b40bcc54309486e/components/viz/service/display/software_renderer.cc [modify] https://crrev.com/6dcd9ac14c9e5fbb60c659143b40bcc54309486e/components/viz/service/display/software_renderer.h
,
Nov 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/986d87a5fada7910886f1fc07e592c7698da128f commit 986d87a5fada7910886f1fc07e592c7698da128f Author: danakj <danakj@chromium.org> Date: Fri Nov 10 19:30:54 2017 When calling UseRenderPass() to restore, ensure it runs. UseRenderPass() has two use cases, either to set up a render pass as the framebuffer for drawing at the start of the draw, or to restore us back to that state after making some intermediate change to which framebuffer is bound. UseRenderPass() has some optimizations where it can return false and not do anything, if it detects that we don't want to draw the render pass. The first use case, in DrawRenderPass() makes use of this return value to avoid drawing anything. Other callers to UseRenderPass() do not check the return value because we're already drawing the pass and they are trying to restore state. If it early outs for these it would be unexpected and produce bugs. Currently DecideRenderPassAllocationsForFrame modifies render passes to try avoid such bugs, but there are other ways to encounter the bug. This changes UseRenderPass() to never early out so the latter cases will not avoid resetting the framebuffer. The former case can avoid UseRenderPass() entirely by checking CanSkipRenderPass(). If it is true, then it does not need to be drawn. Also, we will now ensure that UseRenderPass() is called after using a ScopedUseGrContext when drawing a RenderPassDrawQuad. Currently on errors it would early out before restoring the framebuffer, which could cause bugs for the next quads in the RenderPass. This was introduced when the UseRenderPass() call was moved out of the ~ScopedUseGrContext() destructor in https://codereview.chromium.org/2203033005. When skipping a RenderPass, current_render_pass is no longer set, which causes a crash in code that assumes it is, incorrectly. So this updates some paths in RenderPassDrawQuad drawing to avoid crashing, by passing the colorspace directly instead of trying to pull it off current_render_pass, since the caller knows where to find the correct one, and by using the root_render_pass instead for copying RenderPassDrawQuads to textures, since they draw into the root. This all came up via https://chromium-review.googlesource.com/c/chromium/src/+/757027#message-b6e905925e834419ff448e2093a392053d5d321a R=enne@chromium.org, wutao@chromium.org Bug: 782044 , 738190 , 782042, 783087 Change-Id: I62ea68be6b3008ec8b8370466db0e26321f022f9 Reviewed-on: https://chromium-review.googlesource.com/760558 Reviewed-by: enne <enne@chromium.org> Reviewed-by: ccameron <ccameron@chromium.org> Reviewed-by: Tao Wu <wutao@chromium.org> Commit-Queue: danakj <danakj@chromium.org> Cr-Commit-Position: refs/heads/master@{#515633} [modify] https://crrev.com/986d87a5fada7910886f1fc07e592c7698da128f/components/viz/service/display/direct_renderer.cc [modify] https://crrev.com/986d87a5fada7910886f1fc07e592c7698da128f/components/viz/service/display/direct_renderer.h [modify] https://crrev.com/986d87a5fada7910886f1fc07e592c7698da128f/components/viz/service/display/gl_renderer.cc [modify] https://crrev.com/986d87a5fada7910886f1fc07e592c7698da128f/components/viz/service/display/gl_renderer.h [modify] https://crrev.com/986d87a5fada7910886f1fc07e592c7698da128f/components/viz/service/display/gl_renderer_unittest.cc [modify] https://crrev.com/986d87a5fada7910886f1fc07e592c7698da128f/components/viz/service/display/renderer_pixeltest.cc [modify] https://crrev.com/986d87a5fada7910886f1fc07e592c7698da128f/components/viz/service/display/skia_renderer.cc [modify] https://crrev.com/986d87a5fada7910886f1fc07e592c7698da128f/components/viz/service/display/skia_renderer.h [modify] https://crrev.com/986d87a5fada7910886f1fc07e592c7698da128f/components/viz/service/display/software_renderer.cc [modify] https://crrev.com/986d87a5fada7910886f1fc07e592c7698da128f/components/viz/service/display/software_renderer.h
,
Nov 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bd829cadf22696834edf4aefc9033f5c8e272183 commit bd829cadf22696834edf4aefc9033f5c8e272183 Author: danakj <danakj@chromium.org> Date: Sat Nov 11 00:19:32 2017 Remove caching avoidance when there is a non-empty scissor. See https://chromium-review.googlesource.com/c/chromium/src/+/760558#message-83efb689457770dfabb6d7371847f06f8091dfd7 and https://bugs.chromium.org/p/chromium/issues/detail?id=783087#c24 for explanations, but tl;dr this check would break caching, and shouldn't be there. Before it was used to skip render passes regardless of caching, but incorrectly, and the unittest in https://chromium-review.googlesource.com/c/chromium/src/+/760558 demonstrates that, and fails with that check. This removes the check from the caching block. R=enne@chromium.org, wutao@chromium.org Bug: 782044 , 738190 , 782042, 783087 Change-Id: Idc5baea6da5e6d663da9c1aa23ce81a55e61168d Reviewed-on: https://chromium-review.googlesource.com/763710 Commit-Queue: danakj <danakj@chromium.org> Reviewed-by: Tao Wu <wutao@chromium.org> Cr-Commit-Position: refs/heads/master@{#515757} [modify] https://crrev.com/bd829cadf22696834edf4aefc9033f5c8e272183/components/viz/service/display/direct_renderer.cc [modify] https://crrev.com/bd829cadf22696834edf4aefc9033f5c8e272183/components/viz/service/display/gl_renderer_unittest.cc
,
Nov 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/493796597d3da47f760263d6d1845bda759ed292 commit 493796597d3da47f760263d6d1845bda759ed292 Author: Xu Xing <xing.xu@intel.com> Date: Sat Nov 11 12:48:55 2017 [viz] Local resource: Move render pass textures into subclass of DirectRenderer Other refinement: Use texture id directly instead of ScopedReadLockGL in ApplyImageFilter. BUG= 782044 Change-Id: I0e357a5c5831eaf5709392e852cf4e66ba2d0253 Reviewed-on: https://chromium-review.googlesource.com/749101 Commit-Queue: Xing Xu <xing.xu@intel.com> Reviewed-by: danakj <danakj@chromium.org> Cr-Commit-Position: refs/heads/master@{#515840} [modify] https://crrev.com/493796597d3da47f760263d6d1845bda759ed292/components/viz/service/display/direct_renderer.cc [modify] https://crrev.com/493796597d3da47f760263d6d1845bda759ed292/components/viz/service/display/direct_renderer.h [modify] https://crrev.com/493796597d3da47f760263d6d1845bda759ed292/components/viz/service/display/gl_renderer.cc [modify] https://crrev.com/493796597d3da47f760263d6d1845bda759ed292/components/viz/service/display/gl_renderer.h [modify] https://crrev.com/493796597d3da47f760263d6d1845bda759ed292/components/viz/service/display/gl_renderer_unittest.cc [modify] https://crrev.com/493796597d3da47f760263d6d1845bda759ed292/components/viz/service/display/skia_renderer.cc [modify] https://crrev.com/493796597d3da47f760263d6d1845bda759ed292/components/viz/service/display/skia_renderer.h [modify] https://crrev.com/493796597d3da47f760263d6d1845bda759ed292/components/viz/service/display/software_renderer.cc [modify] https://crrev.com/493796597d3da47f760263d6d1845bda759ed292/components/viz/service/display/software_renderer.h [modify] https://crrev.com/493796597d3da47f760263d6d1845bda759ed292/components/viz/service/display/vulkan_renderer.cc [modify] https://crrev.com/493796597d3da47f760263d6d1845bda759ed292/components/viz/service/display/vulkan_renderer.h
,
Dec 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9cc56b283e51de94cc373a77214da2bb646efcbd commit 9cc56b283e51de94cc373a77214da2bb646efcbd Author: Xu Xing <xing.xu@intel.com> Date: Fri Dec 22 02:20:01 2017 [viz] Implement local resource in GLRenderer Local resource is resource used inside the same GL context and will not being sent into another process, no need to create fence and mailbox for these resources. For RenderPass in render_pass_textures_, it uses texture-backed resources, no need to support GpuMemoryBuffer. Currently there are three path may possible set is_overlay_ to true: ResourceProvider::CreateGpuMemoryBufferResource LayerTreeResourceProvider::CreateResourceFromTextureMailbox ResourceProvider::CreateGpuTextureResource (Used by GLRenderer::overlay_resource_pool_). Resource for RenderPass in render_pass_textures_ is not created by above three paths. Based on this, remove the support of overlay. BUG= 782044 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel Change-Id: Ia334842b12b8297ae4a768979adb8b5a468b906a Reviewed-on: https://chromium-review.googlesource.com/771130 Commit-Queue: Xing Xu <xing.xu@intel.com> Reviewed-by: danakj <danakj@chromium.org> Cr-Commit-Position: refs/heads/master@{#525903} [modify] https://crrev.com/9cc56b283e51de94cc373a77214da2bb646efcbd/components/viz/service/BUILD.gn [modify] https://crrev.com/9cc56b283e51de94cc373a77214da2bb646efcbd/components/viz/service/display/gl_renderer.cc [modify] https://crrev.com/9cc56b283e51de94cc373a77214da2bb646efcbd/components/viz/service/display/gl_renderer.h [add] https://crrev.com/9cc56b283e51de94cc373a77214da2bb646efcbd/components/viz/service/display/scoped_render_pass_texture.cc [add] https://crrev.com/9cc56b283e51de94cc373a77214da2bb646efcbd/components/viz/service/display/scoped_render_pass_texture.h
,
Jan 23 2018
I think this bug is done?
,
Jan 23 2018
|
||
►
Sign in to add a comment |
||
Comment 1 by danakj@chromium.org
, Nov 7 2017Could it just be a struct like this? struct RenderPassTexture { GLuint id; gfx::Size size; } I'm not sure we need to track all the other things along with the id?