Decouple RasterDecoder from virtual contexts |
|||||||||
Issue descriptionRasterDecoder does not really use virtual contexts. It has a virtual context to change how virtual context state is restored. Specifically, when RasterDecoder is made current after a different virtual context was current, it resets it's state. When a virtual context is made current after a RasterDecoder was current, that virtual context does a full state reset. We need to do something similar for SkiaRenderer using SkDDL (SkiaOutputSurfaceImplOnGpu). As per comments #7 and #8 below, we could modify GLContext to permit this sort of thing without directly relying on GLContextVirtual.
,
Nov 7
To be more concrete, I think that we would push the GLVirtualContext up to the RasterDecoderContextState for sharing. One thing I'm unsure is dealing with different GetCompatibilityKey on Linux. Maybe we would have to have a different RasterDecoderContextState per compatibility key?
,
Nov 7
Do we even need GLVirtualContext for RasterDecoder and SkiaRenderer? What's important is that we use the same driver-level context, but do we actually need a GLVirtualContext, meaning, one that automatically resets all state on MakeCurrent? Both SkiaRenderer and RasterDecoder can reset the exact state they need (or invalidate Skia state).
,
Nov 7
Re: multiple RasterDecoderContextState, I don't think it's desirable, because that would mean multiple GrContexts, meaning sharing of SkImages etc. would be problematic, duplicate skia caches etc. One option to look into is to have 1 context per "compatibility key" in the RasterDecoderContextState, but I don't know if Skia would behave sanely with multiple contexts under one GrContext (e.g. for unshared resources such as framebuffer objects, vertex array objects, queries, etc.). Another option is to see if we could just get rid of this. It's only for linux, and at the end of the day, for an optimization (avoiding 1 full-screen blit on presentation when not using a compositing manager). I don't think linux optimizations punishing other platforms would be the right tradeoff at this point.
,
Nov 7
Filed https://bugs.chromium.org/p/chromium/issues/detail?id=902943 dumping thoughts there, I think we could remove the compatibility key concept, maybe even without a perf hit.
,
Nov 8
Re #3: You are correct that the main advantage is that we are using the same driver-level context. I like your idea of not using a virtual context at all, because we could avoid unnecessary virtual context switching. We would have to audit the RasterDecoder. Here are the issues I am aware of that are unaddressed by SharedImage. We are using CopyTexImageResourceManager and CopyTextureCHROMIUMResourceManager for CopySubTextureCHROMIUM in RasterDecoder. This is used for partial raster on the one copy raster path. Both of these managers call back into the Restore* hooks typically just used for virtual context state restoration. RasterDecoder::CopySubTexture calls TextureManager::ClearTextureLevel, which calls back into RasterDecoder::ClearLevel, which does a ScopedPixelUnpackState.
,
Nov 8
Peng and I were just chatting about this and I'd like to share our thoughts so that you all can give feedback. Here are the main use cases that we are trying to address when virtual contexts are active: (1) SkiaRenderer,SkDDL and RasterDecoder,OOP-R interleaved: we don't want to reset GrContext and we want to ignore virtual context state; we do need to make the screen surface current with the context when SkDDL runs; we don't need to make offscreen surface current when OOP-R runs (except when screen surface is destroyed) (2) SkiaRenderer,SkDDL interleaved with WebGL: when we transition from WebGL to SkDDL, we need to reset the GrContext but we don't need to reset to ground context state; when we transition from SkDDL to WebGL, we need to do a hard reset of the WebGL virtual context state piman@ is correct that SkDLL and RasterDecoder don't have to have a virtual context to do this. RasterDecoder is currently using it's virtual context (one per RasterDecoder) to hook into the transitions between other clients (like WebGL) by looking for calls on RasterDecoderImpl::GetContextState (called when transitioning to WebGL) and RasterDecoderImpl::RestoreState (called when transitioning from WebGL). The current implementation has a disadvantage that we do a ContextState::RestoreState in RasterDecoder when we transition from WebGL to OOP-R. This RestoreState is still necessary for CopySubTexture, but that is only called on the one copy raster path. As per comment #6, we can separately audit RasterDecoder and remove the resetting to ground state. We were thinking of two paths forward: (1) Give RasterDecoderContextState a GLContextVirtual and have RDCS be called during virtual context switches (e.g. give it GetContextState and RestoreState). This way RDCS can reset GrContext when WebGL becomes current. Initially, there would be one ground ContextState shared by all RasterDecoders and SkDDL. We would restore it when transitioning from WebGL (at least until we audit RasterDecoder). (2) Remove GLContextVirtual from RasterDecoder and add hooks into GLContext so that (a) SkDDL,RasterDecoder can detect when WebGL has made a virtual context current and GrContext needs a reset and (b) SkDDL,RasterDecoder can force WebGL to do a hard reset (something like GLContext::OnReleaseVirtuallyCurrent).
,
Nov 8
Sounds pretty good. I like option 2. One thing that I was thinking about is to replace the GLContextVirtual + GLStateRestorer logic by a simple "state key" that various clients could tag on a shared GLContext when making current, as well as querying the previous key. This would be to detect the conditions in (2.a) and (2.b). GLES2DecoderImpl would keep its ContextState, but do the state restore explicitly rather than implicitly in GLES2DecoderImpl::MakeCurrent, tagging the GLContext with a key corresponding to the ContextState. Skia use would have its own key (for SkDDL and RasterDecoder::*RasterCHROMIUM), and so would RasterDecoder for the GL use cases (CopySubTexture + whatever else we might want to add as we port other parts of the pipeline). We'd want some encoding of the key so that switching between 2 GLES2DecoderImpl can reset only the subset of the ContextState that's needed (as is today). So the key would be something like enum (GLES2/Skia/GL-for-RasterDecoder) + pointer to object (ContextState/...). Not sure if Option 1 is more expedient. It certainly has a higher cost.
,
Nov 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bea0d6b3c467e05183fb88d06a829a6d7e7d136a commit bea0d6b3c467e05183fb88d06a829a6d7e7d136a Author: Peng Huang <penghuang@chromium.org> Date: Fri Nov 09 10:03:03 2018 Move context state related methods out of DecoderContext This CL adds a new GLContextVirtualDelegate interface to contain all context state related methods from DecoderContext. After this one, I am going to implement GLContextVirtualDelegate in RasterDecoderContextState and only create one GLContextVirtual for all raster decoder and display compositor. Bug: 900941 , 902904 Change-Id: I10d80bea8531885c449a2e676ce0552e9363b34c Reviewed-on: https://chromium-review.googlesource.com/c/1312338 Reviewed-by: Antoine Labour <piman@chromium.org> Commit-Queue: Peng Huang <penghuang@chromium.org> Cr-Commit-Position: refs/heads/master@{#606777} [modify] https://crrev.com/bea0d6b3c467e05183fb88d06a829a6d7e7d136a/gpu/command_buffer/service/BUILD.gn [modify] https://crrev.com/bea0d6b3c467e05183fb88d06a829a6d7e7d136a/gpu/command_buffer/service/decoder_context.h [modify] https://crrev.com/bea0d6b3c467e05183fb88d06a829a6d7e7d136a/gpu/command_buffer/service/gl_context_virtual.cc [modify] https://crrev.com/bea0d6b3c467e05183fb88d06a829a6d7e7d136a/gpu/command_buffer/service/gl_context_virtual.h [add] https://crrev.com/bea0d6b3c467e05183fb88d06a829a6d7e7d136a/gpu/command_buffer/service/gl_context_virtual_delegate.h [modify] https://crrev.com/bea0d6b3c467e05183fb88d06a829a6d7e7d136a/gpu/command_buffer/service/gl_state_restorer_impl.cc [modify] https://crrev.com/bea0d6b3c467e05183fb88d06a829a6d7e7d136a/gpu/command_buffer/service/gl_state_restorer_impl.h [modify] https://crrev.com/bea0d6b3c467e05183fb88d06a829a6d7e7d136a/gpu/command_buffer/service/raster_decoder.cc [modify] https://crrev.com/bea0d6b3c467e05183fb88d06a829a6d7e7d136a/gpu/command_buffer/service/raster_decoder_context_state.cc [modify] https://crrev.com/bea0d6b3c467e05183fb88d06a829a6d7e7d136a/gpu/command_buffer/service/raster_decoder_context_state.h [modify] https://crrev.com/bea0d6b3c467e05183fb88d06a829a6d7e7d136a/gpu/command_buffer/service/wrapped_sk_image.cc
,
Nov 14
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/83884c4057b6da2959d519a7e1f7b91b7e54fa99 commit 83884c4057b6da2959d519a7e1f7b91b7e54fa99 Author: Peng Huang <penghuang@chromium.org> Date: Wed Nov 14 00:16:06 2018 Remove vertex attrib related code in RasterDecoder. RatserDecoder uses Skia to do rendering, and skia can restore vertex attribs. So we don't need maintain and restore it in RasterDecoder. Bug: 902904 Change-Id: I3e85d3960e0b782e41ae6d45c07ddf3494e63ca9 Reviewed-on: https://chromium-review.googlesource.com/c/1334201 Commit-Queue: Peng Huang <penghuang@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Cr-Commit-Position: refs/heads/master@{#607826} [modify] https://crrev.com/83884c4057b6da2959d519a7e1f7b91b7e54fa99/gpu/command_buffer/service/context_state.cc [modify] https://crrev.com/83884c4057b6da2959d519a7e1f7b91b7e54fa99/gpu/command_buffer/service/raster_decoder.cc [modify] https://crrev.com/83884c4057b6da2959d519a7e1f7b91b7e54fa99/gpu/command_buffer/service/raster_decoder_unittest_base.cc
,
Nov 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c52da365c963bca29b850ceeb36a4a0614bf3ee4 commit c52da365c963bca29b850ceeb36a4a0614bf3ee4 Author: Peng Huang <penghuang@chromium.org> Date: Thu Nov 15 12:57:49 2018 Remove indexed uniform buffer related code in RasterDecoder RatserDecoder uses Skia to do rendering, and skia can restore indexed uniform buffer, So we don't need maintain and restore it in RasterDecoder. Bug: 902904 Change-Id: I234611646625a412d65bd632011ddd8e6647ca51 Reviewed-on: https://chromium-review.googlesource.com/c/1335733 Reviewed-by: Antoine Labour <piman@chromium.org> Commit-Queue: Peng Huang <penghuang@chromium.org> Cr-Commit-Position: refs/heads/master@{#608338} [modify] https://crrev.com/c52da365c963bca29b850ceeb36a4a0614bf3ee4/gpu/command_buffer/service/context_state.cc [modify] https://crrev.com/c52da365c963bca29b850ceeb36a4a0614bf3ee4/gpu/command_buffer/service/raster_decoder.cc
,
Nov 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/61eccfe452e2fc818d90c1fa253c315f4de8e23b commit 61eccfe452e2fc818d90c1fa253c315f4de8e23b Author: Peng Huang <penghuang@chromium.org> Date: Thu Nov 15 22:10:13 2018 Decouple gles2::ContextState and gles2::Logger from DecoderClient We would like to share one ContextState for all raster decoders and display compositor. To do that, we need make ContextState work without a decoder client. Bug: 902904 Change-Id: I0ebabbf084e864f18b5e5e1bc5d069a3d436718e Reviewed-on: https://chromium-review.googlesource.com/c/1338383 Commit-Queue: Peng Huang <penghuang@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Cr-Commit-Position: refs/heads/master@{#608539} [modify] https://crrev.com/61eccfe452e2fc818d90c1fa253c315f4de8e23b/gpu/command_buffer/service/gles2_cmd_decoder.cc [modify] https://crrev.com/61eccfe452e2fc818d90c1fa253c315f4de8e23b/gpu/command_buffer/service/gles2_cmd_decoder_passthrough.cc [modify] https://crrev.com/61eccfe452e2fc818d90c1fa253c315f4de8e23b/gpu/command_buffer/service/logger.cc [modify] https://crrev.com/61eccfe452e2fc818d90c1fa253c315f4de8e23b/gpu/command_buffer/service/logger.h [modify] https://crrev.com/61eccfe452e2fc818d90c1fa253c315f4de8e23b/gpu/command_buffer/service/raster_decoder.cc
,
Nov 16
,
Nov 16
,
Nov 16
,
Nov 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c6a76075a1712c9c28f2a427948946884406ed8f commit c6a76075a1712c9c28f2a427948946884406ed8f Author: Peng Huang <penghuang@chromium.org> Date: Tue Nov 27 23:17:31 2018 Remove texture and sampler related code in RasterDecoder. RasterDecoder uses skia to do rendering, and skia can restore texure and sampler bindings by itself. So we don't need use ContextState to do it. Bug: 902904 Change-Id: I5ce2510b4616aab9087cd61bfbfb88368c6e34ab Reviewed-on: https://chromium-review.googlesource.com/c/1338159 Commit-Queue: Peng Huang <penghuang@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Cr-Commit-Position: refs/heads/master@{#611396} [modify] https://crrev.com/c6a76075a1712c9c28f2a427948946884406ed8f/gpu/command_buffer/service/context_state.cc [modify] https://crrev.com/c6a76075a1712c9c28f2a427948946884406ed8f/gpu/command_buffer/service/context_state.h [modify] https://crrev.com/c6a76075a1712c9c28f2a427948946884406ed8f/gpu/command_buffer/service/raster_decoder.cc [modify] https://crrev.com/c6a76075a1712c9c28f2a427948946884406ed8f/gpu/command_buffer/service/raster_decoder_unittest_base.cc
,
Nov 28
,
Nov 30
,
Dec 3
Changing summary as per comment #7 and #8 to remove confusion.
,
Dec 3
,
Dec 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e89c2a05ed132e26c1936f2d3ba411e2fcfdcff7 commit e89c2a05ed132e26c1936f2d3ba411e2fcfdcff7 Author: Peng Huang <penghuang@chromium.org> Date: Tue Dec 04 11:35:11 2018 RasterDecoder: only reset necessary gl state for DoCopySubTexture Because RasterDecoder::DoCopySubTexture only depends on several pixel unpack related state. Bug: 902904 Change-Id: I08fe3996c85818c745605ca6e80c5af8fbf713a7 Reviewed-on: https://chromium-review.googlesource.com/c/1354135 Commit-Queue: Peng Huang <penghuang@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Cr-Commit-Position: refs/heads/master@{#613511} [modify] https://crrev.com/e89c2a05ed132e26c1936f2d3ba411e2fcfdcff7/gpu/command_buffer/service/feature_info.cc [modify] https://crrev.com/e89c2a05ed132e26c1936f2d3ba411e2fcfdcff7/gpu/command_buffer/service/feature_info.h [modify] https://crrev.com/e89c2a05ed132e26c1936f2d3ba411e2fcfdcff7/gpu/command_buffer/service/raster_decoder.cc [modify] https://crrev.com/e89c2a05ed132e26c1936f2d3ba411e2fcfdcff7/gpu/command_buffer/service/raster_decoder_unittest.cc [modify] https://crrev.com/e89c2a05ed132e26c1936f2d3ba411e2fcfdcff7/gpu/command_buffer/service/raster_decoder_unittest_base.cc [modify] https://crrev.com/e89c2a05ed132e26c1936f2d3ba411e2fcfdcff7/gpu/command_buffer/service/raster_decoder_unittest_base.h
,
Dec 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b4ed185bebb4d8167ab4b03e29d8068e19fa9813 commit b4ed185bebb4d8167ab4b03e29d8068e19fa9813 Author: Peng Huang <penghuang@chromium.org> Date: Wed Dec 05 03:35:29 2018 Decouple ErrorState and ContextState. Bug: 902904 Change-Id: Idaf970c574597d2436befccc4f0979392c125d89 Reviewed-on: https://chromium-review.googlesource.com/c/1362220 Reviewed-by: Antoine Labour <piman@chromium.org> Commit-Queue: Peng Huang <penghuang@chromium.org> Cr-Commit-Position: refs/heads/master@{#613851} [modify] https://crrev.com/b4ed185bebb4d8167ab4b03e29d8068e19fa9813/gpu/command_buffer/service/buffer_manager.cc [modify] https://crrev.com/b4ed185bebb4d8167ab4b03e29d8068e19fa9813/gpu/command_buffer/service/buffer_manager.h [modify] https://crrev.com/b4ed185bebb4d8167ab4b03e29d8068e19fa9813/gpu/command_buffer/service/context_state.cc [modify] https://crrev.com/b4ed185bebb4d8167ab4b03e29d8068e19fa9813/gpu/command_buffer/service/context_state.h [modify] https://crrev.com/b4ed185bebb4d8167ab4b03e29d8068e19fa9813/gpu/command_buffer/service/gles2_cmd_decoder.cc [modify] https://crrev.com/b4ed185bebb4d8167ab4b03e29d8068e19fa9813/gpu/command_buffer/service/gles2_cmd_decoder_unittest_context_state.cc [modify] https://crrev.com/b4ed185bebb4d8167ab4b03e29d8068e19fa9813/gpu/command_buffer/service/raster_decoder.cc [modify] https://crrev.com/b4ed185bebb4d8167ab4b03e29d8068e19fa9813/gpu/command_buffer/service/texture_manager.cc [modify] https://crrev.com/b4ed185bebb4d8167ab4b03e29d8068e19fa9813/gpu/command_buffer/service/texture_manager.h
,
Dec 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1808a743048d147521eaffb386dc9f29b4ecae0e commit 1808a743048d147521eaffb386dc9f29b4ecae0e Author: Peng Huang <penghuang@chromium.org> Date: Fri Dec 07 14:40:58 2018 Share one GLContext for all RasterDecoder when virtual context is used. With this CL, we will create a GLContextVirtual in RasterDecoderContextState, and share it with all raster decoders, display compositoer, etc. It is a temporary solution. In follow up CLs, we will share the one GLContext with GLES2 as well. In that case, we will use GLContext directly instead of GLContextVirtual. Change-Id: If9b8d1f32e58b32a9d9f8cd07aa10c30432e3183 Bug: 902904 Reviewed-on: https://chromium-review.googlesource.com/c/1357463 Commit-Queue: Peng Huang <penghuang@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Cr-Commit-Position: refs/heads/master@{#614694} [modify] https://crrev.com/1808a743048d147521eaffb386dc9f29b4ecae0e/components/viz/service/display_embedder/skia_output_surface_impl_on_gpu.h [modify] https://crrev.com/1808a743048d147521eaffb386dc9f29b4ecae0e/components/viz/service/gl/gpu_service_impl.cc [modify] https://crrev.com/1808a743048d147521eaffb386dc9f29b4ecae0e/gpu/command_buffer/service/decoder_context.h [modify] https://crrev.com/1808a743048d147521eaffb386dc9f29b4ecae0e/gpu/command_buffer/service/gl_context_virtual_delegate.h [modify] https://crrev.com/1808a743048d147521eaffb386dc9f29b4ecae0e/gpu/command_buffer/service/gl_state_restorer_impl.cc [modify] https://crrev.com/1808a743048d147521eaffb386dc9f29b4ecae0e/gpu/command_buffer/service/gr_cache_controller.cc [modify] https://crrev.com/1808a743048d147521eaffb386dc9f29b4ecae0e/gpu/command_buffer/service/raster_decoder.cc [modify] https://crrev.com/1808a743048d147521eaffb386dc9f29b4ecae0e/gpu/command_buffer/service/raster_decoder_context_state.cc [modify] https://crrev.com/1808a743048d147521eaffb386dc9f29b4ecae0e/gpu/command_buffer/service/raster_decoder_context_state.h [modify] https://crrev.com/1808a743048d147521eaffb386dc9f29b4ecae0e/gpu/command_buffer/service/raster_decoder_unittest.cc [modify] https://crrev.com/1808a743048d147521eaffb386dc9f29b4ecae0e/gpu/command_buffer/service/raster_decoder_unittest_base.cc [modify] https://crrev.com/1808a743048d147521eaffb386dc9f29b4ecae0e/gpu/command_buffer/service/raster_decoder_unittest_base.h [modify] https://crrev.com/1808a743048d147521eaffb386dc9f29b4ecae0e/gpu/command_buffer/service/raster_decoder_unittest_context_lost.cc [modify] https://crrev.com/1808a743048d147521eaffb386dc9f29b4ecae0e/gpu/command_buffer/service/wrapped_sk_image.cc [modify] https://crrev.com/1808a743048d147521eaffb386dc9f29b4ecae0e/gpu/ipc/in_process_command_buffer.cc [modify] https://crrev.com/1808a743048d147521eaffb386dc9f29b4ecae0e/gpu/ipc/service/gpu_channel_manager.cc [modify] https://crrev.com/1808a743048d147521eaffb386dc9f29b4ecae0e/gpu/ipc/service/raster_command_buffer_stub.cc [modify] https://crrev.com/1808a743048d147521eaffb386dc9f29b4ecae0e/gpu/ipc/service/shared_image_stub.cc
,
Dec 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/be615a8821d04fb4bf6d371be95bd323d2d4155c commit be615a8821d04fb4bf6d371be95bd323d2d4155c Author: Peng Huang <penghuang@chromium.org> Date: Fri Dec 07 18:00:38 2018 Revert "Share one GLContext for all RasterDecoder when virtual context is used." This reverts commit 1808a743048d147521eaffb386dc9f29b4ecae0e. Reason for revert: Null-dereference READ in gpu::gles2::ContextState::api https://crbug.com/912981 Original change's description: > Share one GLContext for all RasterDecoder when virtual context is used. > > With this CL, we will create a GLContextVirtual in RasterDecoderContextState, > and share it with all raster decoders, display compositoer, etc. It is a > temporary solution. In follow up CLs, we will share the one GLContext with > GLES2 as well. In that case, we will use GLContext directly instead of > GLContextVirtual. > > Change-Id: If9b8d1f32e58b32a9d9f8cd07aa10c30432e3183 > Bug: 902904 > Reviewed-on: https://chromium-review.googlesource.com/c/1357463 > Commit-Queue: Peng Huang <penghuang@chromium.org> > Reviewed-by: Antoine Labour <piman@chromium.org> > Cr-Commit-Position: refs/heads/master@{#614694} TBR=penghuang@chromium.org,piman@chromium.org Change-Id: I2eb66f211d408988f30bfb870425da80e0eb7a4b No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 902904 Reviewed-on: https://chromium-review.googlesource.com/c/1368284 Reviewed-by: Peng Huang <penghuang@chromium.org> Commit-Queue: Peng Huang <penghuang@chromium.org> Cr-Commit-Position: refs/heads/master@{#614738} [modify] https://crrev.com/be615a8821d04fb4bf6d371be95bd323d2d4155c/components/viz/service/display_embedder/skia_output_surface_impl_on_gpu.h [modify] https://crrev.com/be615a8821d04fb4bf6d371be95bd323d2d4155c/components/viz/service/gl/gpu_service_impl.cc [modify] https://crrev.com/be615a8821d04fb4bf6d371be95bd323d2d4155c/gpu/command_buffer/service/decoder_context.h [modify] https://crrev.com/be615a8821d04fb4bf6d371be95bd323d2d4155c/gpu/command_buffer/service/gl_context_virtual_delegate.h [modify] https://crrev.com/be615a8821d04fb4bf6d371be95bd323d2d4155c/gpu/command_buffer/service/gl_state_restorer_impl.cc [modify] https://crrev.com/be615a8821d04fb4bf6d371be95bd323d2d4155c/gpu/command_buffer/service/gr_cache_controller.cc [modify] https://crrev.com/be615a8821d04fb4bf6d371be95bd323d2d4155c/gpu/command_buffer/service/raster_decoder.cc [modify] https://crrev.com/be615a8821d04fb4bf6d371be95bd323d2d4155c/gpu/command_buffer/service/raster_decoder_context_state.cc [modify] https://crrev.com/be615a8821d04fb4bf6d371be95bd323d2d4155c/gpu/command_buffer/service/raster_decoder_context_state.h [modify] https://crrev.com/be615a8821d04fb4bf6d371be95bd323d2d4155c/gpu/command_buffer/service/raster_decoder_unittest.cc [modify] https://crrev.com/be615a8821d04fb4bf6d371be95bd323d2d4155c/gpu/command_buffer/service/raster_decoder_unittest_base.cc [modify] https://crrev.com/be615a8821d04fb4bf6d371be95bd323d2d4155c/gpu/command_buffer/service/raster_decoder_unittest_base.h [modify] https://crrev.com/be615a8821d04fb4bf6d371be95bd323d2d4155c/gpu/command_buffer/service/raster_decoder_unittest_context_lost.cc [modify] https://crrev.com/be615a8821d04fb4bf6d371be95bd323d2d4155c/gpu/command_buffer/service/wrapped_sk_image.cc [modify] https://crrev.com/be615a8821d04fb4bf6d371be95bd323d2d4155c/gpu/ipc/in_process_command_buffer.cc [modify] https://crrev.com/be615a8821d04fb4bf6d371be95bd323d2d4155c/gpu/ipc/service/gpu_channel_manager.cc [modify] https://crrev.com/be615a8821d04fb4bf6d371be95bd323d2d4155c/gpu/ipc/service/raster_command_buffer_stub.cc [modify] https://crrev.com/be615a8821d04fb4bf6d371be95bd323d2d4155c/gpu/ipc/service/shared_image_stub.cc
,
Dec 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/66a7a37669a359059140065d35dc324191f85ab8 commit 66a7a37669a359059140065d35dc324191f85ab8 Author: Peng Huang <penghuang@chromium.org> Date: Fri Dec 07 20:05:38 2018 Reland "Share one GLContext for all RasterDecoder when virtual context is used." Original change's description: > Revert "Share one GLContext for all RasterDecoder when virtual context is used." > > This reverts commit 1808a743048d147521eaffb386dc9f29b4ecae0e. > > Reason for revert: Null-dereference READ in gpu::gles2::ContextState::api > https://crbug.com/912981 > > Original change's description: > > Share one GLContext for all RasterDecoder when virtual context is used. > > > > With this CL, we will create a GLContextVirtual in RasterDecoderContextState, > > and share it with all raster decoders, display compositoer, etc. It is a > > temporary solution. In follow up CLs, we will share the one GLContext with > > GLES2 as well. In that case, we will use GLContext directly instead of > > GLContextVirtual. > > > > Change-Id: If9b8d1f32e58b32a9d9f8cd07aa10c30432e3183 > > Bug: 902904 > > Reviewed-on: https://chromium-review.googlesource.com/c/1357463 > > Commit-Queue: Peng Huang <penghuang@chromium.org> > > Reviewed-by: Antoine Labour <piman@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#614694} > > TBR=penghuang@chromium.org,piman@chromium.org > > Change-Id: I2eb66f211d408988f30bfb870425da80e0eb7a4b > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: 902904 > Reviewed-on: https://chromium-review.googlesource.com/c/1368284 > Reviewed-by: Peng Huang <penghuang@chromium.org> > Commit-Queue: Peng Huang <penghuang@chromium.org> > Cr-Commit-Position: refs/heads/master@{#614738} TBR=penghuang@chromium.org,piman@chromium.org Change-Id: Ie409d5d631509efa978a8741381cedbbbcdb3904 Bug: 902904 Reviewed-on: https://chromium-review.googlesource.com/c/1368031 Reviewed-by: Peng Huang <penghuang@chromium.org> Commit-Queue: Peng Huang <penghuang@chromium.org> Cr-Commit-Position: refs/heads/master@{#614796} [modify] https://crrev.com/66a7a37669a359059140065d35dc324191f85ab8/components/viz/service/display_embedder/skia_output_surface_impl_on_gpu.h [modify] https://crrev.com/66a7a37669a359059140065d35dc324191f85ab8/components/viz/service/gl/gpu_service_impl.cc [modify] https://crrev.com/66a7a37669a359059140065d35dc324191f85ab8/gpu/command_buffer/service/decoder_context.h [modify] https://crrev.com/66a7a37669a359059140065d35dc324191f85ab8/gpu/command_buffer/service/gl_context_virtual_delegate.h [modify] https://crrev.com/66a7a37669a359059140065d35dc324191f85ab8/gpu/command_buffer/service/gl_state_restorer_impl.cc [modify] https://crrev.com/66a7a37669a359059140065d35dc324191f85ab8/gpu/command_buffer/service/gr_cache_controller.cc [modify] https://crrev.com/66a7a37669a359059140065d35dc324191f85ab8/gpu/command_buffer/service/gr_cache_controller_unittest.cc [modify] https://crrev.com/66a7a37669a359059140065d35dc324191f85ab8/gpu/command_buffer/service/raster_decoder.cc [modify] https://crrev.com/66a7a37669a359059140065d35dc324191f85ab8/gpu/command_buffer/service/raster_decoder_context_state.cc [modify] https://crrev.com/66a7a37669a359059140065d35dc324191f85ab8/gpu/command_buffer/service/raster_decoder_context_state.h [modify] https://crrev.com/66a7a37669a359059140065d35dc324191f85ab8/gpu/command_buffer/service/raster_decoder_unittest.cc [modify] https://crrev.com/66a7a37669a359059140065d35dc324191f85ab8/gpu/command_buffer/service/raster_decoder_unittest_base.cc [modify] https://crrev.com/66a7a37669a359059140065d35dc324191f85ab8/gpu/command_buffer/service/raster_decoder_unittest_base.h [modify] https://crrev.com/66a7a37669a359059140065d35dc324191f85ab8/gpu/command_buffer/service/raster_decoder_unittest_context_lost.cc [modify] https://crrev.com/66a7a37669a359059140065d35dc324191f85ab8/gpu/command_buffer/service/shared_image_backing_factory_ahardwarebuffer_unittest.cc [modify] https://crrev.com/66a7a37669a359059140065d35dc324191f85ab8/gpu/command_buffer/service/shared_image_backing_factory_gl_texture_unittest.cc [modify] https://crrev.com/66a7a37669a359059140065d35dc324191f85ab8/gpu/command_buffer/service/wrapped_sk_image.cc [modify] https://crrev.com/66a7a37669a359059140065d35dc324191f85ab8/gpu/command_buffer/tests/fuzzer_main.cc [modify] https://crrev.com/66a7a37669a359059140065d35dc324191f85ab8/gpu/ipc/in_process_command_buffer.cc [modify] https://crrev.com/66a7a37669a359059140065d35dc324191f85ab8/gpu/ipc/service/gpu_channel_manager.cc [modify] https://crrev.com/66a7a37669a359059140065d35dc324191f85ab8/gpu/ipc/service/raster_command_buffer_stub.cc [modify] https://crrev.com/66a7a37669a359059140065d35dc324191f85ab8/gpu/ipc/service/shared_image_stub.cc
,
Dec 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e4accf842d18254cad95947e47660f291b6c5ef6 commit e4accf842d18254cad95947e47660f291b6c5ef6 Author: Peng Huang <penghuang@chromium.org> Date: Wed Dec 12 22:22:47 2018 Enable virualized context for SkiaRenderer code path Bug: 900941 , 902904 Change-Id: I1b374fe1efd9c0612fc6f42a84eefd8bc3b1e320 Reviewed-on: https://chromium-review.googlesource.com/c/1370189 Commit-Queue: Peng Huang <penghuang@chromium.org> Reviewed-by: Jonathan Backer <backer@chromium.org> Cr-Commit-Position: refs/heads/master@{#616083} [modify] https://crrev.com/e4accf842d18254cad95947e47660f291b6c5ef6/components/viz/service/display_embedder/skia_output_surface_impl_on_gpu.cc [modify] https://crrev.com/e4accf842d18254cad95947e47660f291b6c5ef6/components/viz/service/display_embedder/skia_output_surface_impl_on_gpu.h [modify] https://crrev.com/e4accf842d18254cad95947e47660f291b6c5ef6/components/viz/service/gl/gpu_service_impl.cc [modify] https://crrev.com/e4accf842d18254cad95947e47660f291b6c5ef6/gpu/ipc/in_process_command_buffer.cc [modify] https://crrev.com/e4accf842d18254cad95947e47660f291b6c5ef6/gpu/ipc/service/gles2_command_buffer_stub.cc [modify] https://crrev.com/e4accf842d18254cad95947e47660f291b6c5ef6/gpu/ipc/service/gpu_channel_manager.cc
,
Jan 16
(6 days ago)
I think this one has been fixed. Next, we will remove GLContextVirtual and let GLES2Decoder, RasterDecoder, display compositor and etc to use GLContext directly. I will file a new issue for it. |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by backer@chromium.org
, Nov 7