Display/SoftwareRenderer has dependency on cc::RasterSource |
||||||
Issue descriptionThis breaks our layering paradigm, with components/viz/service/display (and components/viz/common/quads) depending on cc/raster. This relationship exists for resourceless software draws in android webview, so it only exists for software renderer. Looking at how the RasterSource is used, it goes from SoftwareRenderer through raster_source->PlaybackToCanvas(), but does so in a way that avoids all the interesting code in there, and just ends up basically as a call to DisplayItemList::Raster(). We can resolve this by: - Having viz quads and viz display depend on cc/paint (note: separate lower level component than cc, similar to skia in the layer cake) - Have PictureDrawQuad have a cc::DisplayItemList instead of a RasterSource. - Have SoftwareRenderer raster the DisplayItemList directly. If this is problematic in any way, then we can add a Rasterizer interface for SoftwareRenderer, which SynchronousLayerTreeFrameSink (for webview) makes and gives to the Display, and it could implement anything problematic and depend on cc/raster etc.
,
Jul 26 2017
,
Jul 27 2017
I haven't gotten that far on this yet. Marking it as available.
,
Aug 9 2017
,
Sep 5 2017
,
Sep 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bc77ada8ee01afa7ae89894572f9dcf812e50af3 commit bc77ada8ee01afa7ae89894572f9dcf812e50af3 Author: danakj <danakj@chromium.org> Date: Wed Sep 06 01:07:05 2017 Have PictureDrawQuad hold a DisplayItemList Currently it has a RasterSource which is part of the layer compositor in cc/raster, but that makes a dependency from viz's display compositor onto the layer compositor. Instead, all it needs is the DisplayItemList from cc/paint/ so we replace it with that. A few lines of canvas setup need to be copied from RasterSource out to the SoftwareRenderer (and SkiaRenderer), but most of it is skipped already with the settings passed to the RasterSource, so can be omitted. R=vmpstr@chromium.org TBR=dcheng Bug: 749235 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel Change-Id: If860d0525381fe514c2bf96faeaac4c63489543d Reviewed-on: https://chromium-review.googlesource.com/651077 Commit-Queue: danakj <danakj@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Vladimir Levin <vmpstr@chromium.org> Reviewed-by: danakj <danakj@chromium.org> Cr-Commit-Position: refs/heads/master@{#499827} [modify] https://crrev.com/bc77ada8ee01afa7ae89894572f9dcf812e50af3/cc/ipc/cc_param_traits_unittest.cc [modify] https://crrev.com/bc77ada8ee01afa7ae89894572f9dcf812e50af3/cc/layers/picture_layer_impl.cc [modify] https://crrev.com/bc77ada8ee01afa7ae89894572f9dcf812e50af3/cc/output/renderer_pixeltest.cc [modify] https://crrev.com/bc77ada8ee01afa7ae89894572f9dcf812e50af3/cc/output/software_renderer.cc [modify] https://crrev.com/bc77ada8ee01afa7ae89894572f9dcf812e50af3/cc/quads/draw_quad_unittest.cc [modify] https://crrev.com/bc77ada8ee01afa7ae89894572f9dcf812e50af3/cc/quads/picture_draw_quad.cc [modify] https://crrev.com/bc77ada8ee01afa7ae89894572f9dcf812e50af3/cc/quads/picture_draw_quad.h [modify] https://crrev.com/bc77ada8ee01afa7ae89894572f9dcf812e50af3/cc/raster/gpu_raster_buffer_provider.cc [modify] https://crrev.com/bc77ada8ee01afa7ae89894572f9dcf812e50af3/cc/raster/raster_source.cc [modify] https://crrev.com/bc77ada8ee01afa7ae89894572f9dcf812e50af3/cc/raster/raster_source.h [modify] https://crrev.com/bc77ada8ee01afa7ae89894572f9dcf812e50af3/cc/raster/raster_source_unittest.cc [modify] https://crrev.com/bc77ada8ee01afa7ae89894572f9dcf812e50af3/components/viz/service/BUILD.gn [modify] https://crrev.com/bc77ada8ee01afa7ae89894572f9dcf812e50af3/components/viz/service/display/skia_renderer.cc
,
Sep 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7c47854c6f100793a4678b6cbfcc7fb308d211bc commit 7c47854c6f100793a4678b6cbfcc7fb308d211bc Author: Takeshi Yoshino <tyoshino@chromium.org> Date: Wed Sep 06 02:04:13 2017 Revert "Have PictureDrawQuad hold a DisplayItemList" This reverts commit bc77ada8ee01afa7ae89894572f9dcf812e50af3. Reason for revert: Broke RasterSourceTest.RasterTransformWithoutRecordingScale on Mac10.9 Tests (dbg) Original change's description: > Have PictureDrawQuad hold a DisplayItemList > > Currently it has a RasterSource which is part of the layer compositor > in cc/raster, but that makes a dependency from viz's display compositor > onto the layer compositor. > > Instead, all it needs is the DisplayItemList from cc/paint/ so we > replace it with that. A few lines of canvas setup need to be copied > from RasterSource out to the SoftwareRenderer (and SkiaRenderer), > but most of it is skipped already with the settings passed to the > RasterSource, so can be omitted. > > R=vmpstr@chromium.org > TBR=dcheng > > Bug: 749235 > Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel > Change-Id: If860d0525381fe514c2bf96faeaac4c63489543d > Reviewed-on: https://chromium-review.googlesource.com/651077 > Commit-Queue: danakj <danakj@chromium.org> > Reviewed-by: Daniel Cheng <dcheng@chromium.org> > Reviewed-by: Vladimir Levin <vmpstr@chromium.org> > Reviewed-by: danakj <danakj@chromium.org> > Cr-Commit-Position: refs/heads/master@{#499827} TBR=danakj@chromium.org,dcheng@chromium.org,vmpstr@chromium.org Change-Id: I3fd6a31ed1769ff41032a537ae6c57b991b1a874 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 749235 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel Reviewed-on: https://chromium-review.googlesource.com/651986 Reviewed-by: Takeshi Yoshino <tyoshino@chromium.org> Commit-Queue: Takeshi Yoshino <tyoshino@chromium.org> Cr-Commit-Position: refs/heads/master@{#499846} [modify] https://crrev.com/7c47854c6f100793a4678b6cbfcc7fb308d211bc/cc/ipc/cc_param_traits_unittest.cc [modify] https://crrev.com/7c47854c6f100793a4678b6cbfcc7fb308d211bc/cc/layers/picture_layer_impl.cc [modify] https://crrev.com/7c47854c6f100793a4678b6cbfcc7fb308d211bc/cc/output/renderer_pixeltest.cc [modify] https://crrev.com/7c47854c6f100793a4678b6cbfcc7fb308d211bc/cc/output/software_renderer.cc [modify] https://crrev.com/7c47854c6f100793a4678b6cbfcc7fb308d211bc/cc/quads/draw_quad_unittest.cc [modify] https://crrev.com/7c47854c6f100793a4678b6cbfcc7fb308d211bc/cc/quads/picture_draw_quad.cc [modify] https://crrev.com/7c47854c6f100793a4678b6cbfcc7fb308d211bc/cc/quads/picture_draw_quad.h [modify] https://crrev.com/7c47854c6f100793a4678b6cbfcc7fb308d211bc/cc/raster/gpu_raster_buffer_provider.cc [modify] https://crrev.com/7c47854c6f100793a4678b6cbfcc7fb308d211bc/cc/raster/raster_source.cc [modify] https://crrev.com/7c47854c6f100793a4678b6cbfcc7fb308d211bc/cc/raster/raster_source.h [modify] https://crrev.com/7c47854c6f100793a4678b6cbfcc7fb308d211bc/cc/raster/raster_source_unittest.cc [modify] https://crrev.com/7c47854c6f100793a4678b6cbfcc7fb308d211bc/components/viz/service/BUILD.gn [modify] https://crrev.com/7c47854c6f100793a4678b6cbfcc7fb308d211bc/components/viz/service/display/skia_renderer.cc
,
Sep 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/135f8e843bd7ba16feb82eedfd86e893ff90cf75 commit 135f8e843bd7ba16feb82eedfd86e893ff90cf75 Author: danakj <danakj@chromium.org> Date: Wed Sep 06 20:53:08 2017 Reland: Have PictureDrawQuad hold a DisplayItemList Currently it has a RasterSource which is part of the layer compositor in cc/raster, but that makes a dependency from viz's display compositor onto the layer compositor. Instead, all it needs is the DisplayItemList from cc/paint/ so we replace it with that. A few lines of canvas setup need to be copied from RasterSource out to the SoftwareRenderer (and SkiaRenderer), but most of it is skipped already with the settings passed to the RasterSource, so can be omitted. R=vmpstr@chromium.org TBR=dcheng Bug: 749235 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel Change-Id: I59fe7be25d61a39e85de53b671b30ee5c290be24 Reviewed-on: https://chromium-review.googlesource.com/652979 Commit-Queue: danakj <danakj@chromium.org> Reviewed-by: Vladimir Levin <vmpstr@chromium.org> Reviewed-by: danakj <danakj@chromium.org> Cr-Commit-Position: refs/heads/master@{#500081} [modify] https://crrev.com/135f8e843bd7ba16feb82eedfd86e893ff90cf75/cc/ipc/cc_param_traits_unittest.cc [modify] https://crrev.com/135f8e843bd7ba16feb82eedfd86e893ff90cf75/cc/layers/picture_layer_impl.cc [modify] https://crrev.com/135f8e843bd7ba16feb82eedfd86e893ff90cf75/cc/output/renderer_pixeltest.cc [modify] https://crrev.com/135f8e843bd7ba16feb82eedfd86e893ff90cf75/cc/output/software_renderer.cc [modify] https://crrev.com/135f8e843bd7ba16feb82eedfd86e893ff90cf75/cc/quads/draw_quad_unittest.cc [modify] https://crrev.com/135f8e843bd7ba16feb82eedfd86e893ff90cf75/cc/quads/picture_draw_quad.cc [modify] https://crrev.com/135f8e843bd7ba16feb82eedfd86e893ff90cf75/cc/quads/picture_draw_quad.h [modify] https://crrev.com/135f8e843bd7ba16feb82eedfd86e893ff90cf75/cc/raster/gpu_raster_buffer_provider.cc [modify] https://crrev.com/135f8e843bd7ba16feb82eedfd86e893ff90cf75/cc/raster/raster_source.cc [modify] https://crrev.com/135f8e843bd7ba16feb82eedfd86e893ff90cf75/cc/raster/raster_source.h [modify] https://crrev.com/135f8e843bd7ba16feb82eedfd86e893ff90cf75/cc/raster/raster_source_unittest.cc [modify] https://crrev.com/135f8e843bd7ba16feb82eedfd86e893ff90cf75/components/viz/service/BUILD.gn [modify] https://crrev.com/135f8e843bd7ba16feb82eedfd86e893ff90cf75/components/viz/service/display/skia_renderer.cc
,
Sep 6 2017
,
Sep 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fd751b0f7d36086c8c2d974a98e8466e89344b5a commit fd751b0f7d36086c8c2d974a98e8466e89344b5a Author: Jayson Adams <shrike@chromium.org> Date: Wed Sep 06 23:47:32 2017 Revert "Reland: Have PictureDrawQuad hold a DisplayItemList" This reverts commit 135f8e843bd7ba16feb82eedfd86e893ff90cf75. Reason for revert: RasterSourceTest.RasterTransformWithoutRecordingScale test failure on Win7 Tests, cl flagged by Sheriff-o-Matic https://uberchromegw.corp.google.com/i/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/62999 Original change's description: > Reland: Have PictureDrawQuad hold a DisplayItemList > > Currently it has a RasterSource which is part of the layer compositor > in cc/raster, but that makes a dependency from viz's display compositor > onto the layer compositor. > > Instead, all it needs is the DisplayItemList from cc/paint/ so we > replace it with that. A few lines of canvas setup need to be copied > from RasterSource out to the SoftwareRenderer (and SkiaRenderer), > but most of it is skipped already with the settings passed to the > RasterSource, so can be omitted. > > R=vmpstr@chromium.org > TBR=dcheng > > Bug: 749235 > Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel > Change-Id: I59fe7be25d61a39e85de53b671b30ee5c290be24 > Reviewed-on: https://chromium-review.googlesource.com/652979 > Commit-Queue: danakj <danakj@chromium.org> > Reviewed-by: Vladimir Levin <vmpstr@chromium.org> > Reviewed-by: danakj <danakj@chromium.org> > Cr-Commit-Position: refs/heads/master@{#500081} TBR=danakj@chromium.org,dcheng@chromium.org,vmpstr@chromium.org Change-Id: I6afe019ff735db041c2494a467dbbdfda78eab36 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 749235 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel Reviewed-on: https://chromium-review.googlesource.com/653189 Reviewed-by: Jayson Adams <shrike@chromium.org> Commit-Queue: Jayson Adams <shrike@chromium.org> Cr-Commit-Position: refs/heads/master@{#500130} [modify] https://crrev.com/fd751b0f7d36086c8c2d974a98e8466e89344b5a/cc/ipc/cc_param_traits_unittest.cc [modify] https://crrev.com/fd751b0f7d36086c8c2d974a98e8466e89344b5a/cc/layers/picture_layer_impl.cc [modify] https://crrev.com/fd751b0f7d36086c8c2d974a98e8466e89344b5a/cc/output/renderer_pixeltest.cc [modify] https://crrev.com/fd751b0f7d36086c8c2d974a98e8466e89344b5a/cc/output/software_renderer.cc [modify] https://crrev.com/fd751b0f7d36086c8c2d974a98e8466e89344b5a/cc/quads/draw_quad_unittest.cc [modify] https://crrev.com/fd751b0f7d36086c8c2d974a98e8466e89344b5a/cc/quads/picture_draw_quad.cc [modify] https://crrev.com/fd751b0f7d36086c8c2d974a98e8466e89344b5a/cc/quads/picture_draw_quad.h [modify] https://crrev.com/fd751b0f7d36086c8c2d974a98e8466e89344b5a/cc/raster/gpu_raster_buffer_provider.cc [modify] https://crrev.com/fd751b0f7d36086c8c2d974a98e8466e89344b5a/cc/raster/raster_source.cc [modify] https://crrev.com/fd751b0f7d36086c8c2d974a98e8466e89344b5a/cc/raster/raster_source.h [modify] https://crrev.com/fd751b0f7d36086c8c2d974a98e8466e89344b5a/cc/raster/raster_source_unittest.cc [modify] https://crrev.com/fd751b0f7d36086c8c2d974a98e8466e89344b5a/components/viz/service/BUILD.gn [modify] https://crrev.com/fd751b0f7d36086c8c2d974a98e8466e89344b5a/components/viz/service/display/skia_renderer.cc
,
Sep 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b075024f163f3b60e4f485e27717cb430044fc06 commit b075024f163f3b60e4f485e27717cb430044fc06 Author: danakj <danakj@chromium.org> Date: Fri Sep 08 17:45:41 2017 Reland (2): Have PictureDrawQuad hold a DisplayItemList Currently it has a RasterSource which is part of the layer compositor in cc/raster, but that makes a dependency from viz's display compositor onto the layer compositor. Instead, all it needs is the DisplayItemList from cc/paint/ so we replace it with that. A few lines of canvas setup need to be copied from RasterSource out to the SoftwareRenderer (and SkiaRenderer), but most of it is skipped already with the settings passed to the RasterSource, so can be omitted. In this reland, we ensure the failing test only runs on non-debug builds. TBR=vmpstr@chromium.org TBR=dcheng Bug: 749235 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel Change-Id: I33104ce1610743327ce456efdef768307b544383 Reviewed-on: https://chromium-review.googlesource.com/655477 Commit-Queue: danakj <danakj@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: danakj <danakj@chromium.org> Cr-Commit-Position: refs/heads/master@{#500618} [modify] https://crrev.com/b075024f163f3b60e4f485e27717cb430044fc06/cc/ipc/cc_param_traits_unittest.cc [modify] https://crrev.com/b075024f163f3b60e4f485e27717cb430044fc06/cc/layers/picture_layer_impl.cc [modify] https://crrev.com/b075024f163f3b60e4f485e27717cb430044fc06/cc/output/renderer_pixeltest.cc [modify] https://crrev.com/b075024f163f3b60e4f485e27717cb430044fc06/cc/output/software_renderer.cc [modify] https://crrev.com/b075024f163f3b60e4f485e27717cb430044fc06/cc/quads/draw_quad_unittest.cc [modify] https://crrev.com/b075024f163f3b60e4f485e27717cb430044fc06/cc/quads/picture_draw_quad.cc [modify] https://crrev.com/b075024f163f3b60e4f485e27717cb430044fc06/cc/quads/picture_draw_quad.h [modify] https://crrev.com/b075024f163f3b60e4f485e27717cb430044fc06/cc/raster/gpu_raster_buffer_provider.cc [modify] https://crrev.com/b075024f163f3b60e4f485e27717cb430044fc06/cc/raster/raster_source.cc [modify] https://crrev.com/b075024f163f3b60e4f485e27717cb430044fc06/cc/raster/raster_source.h [modify] https://crrev.com/b075024f163f3b60e4f485e27717cb430044fc06/cc/raster/raster_source_unittest.cc [modify] https://crrev.com/b075024f163f3b60e4f485e27717cb430044fc06/components/viz/service/BUILD.gn [modify] https://crrev.com/b075024f163f3b60e4f485e27717cb430044fc06/components/viz/service/display/skia_renderer.cc |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by danakj@chromium.org
, Jul 26 2017