cc: Remove tile rasterize on demand for GPU rasterization

cc: Remove tile rasterize on demand for GPU rasterization

This code was recently added and isn't needed because it signals to the tile
that it should raster on demand (checkerboard), but this will happen anyway with
the delgated renderer.

I've also removed a test that was breaking due to the change.

The test wasn't really testing what we thought, because we wouldn't end up
rasterizing anything with GPU, even though force was on.

Committed: https://crrev.com/01a4b8027c2e9e678cd6afd0a97bf83807d55f5a
Cr-Commit-Position: refs/heads/master@{#311109}

[hendrikw, 2015-01-09 23:51:24]

hendrikw@chromium.org changed reviewers:
+ vmpstr@chromium.org

[hendrikw, 2015-01-09 23:53:59]

PTAL, thanks!

[danakj, 2015-01-10 00:02:45]

On Fri, Jan 9, 2015 at 3:53 PM, <hendrikw@chromium.org> wrote:

> Reviewers: vmpstr,
>
> Message:
> PTAL, thanks!
>
>
> Description:
> cc: Remove tile rasterize on demand for GPU rasterization
>
> This code was recently added and isn't needed because it signals to the
> tile
> that it should raster on demand (checkerboard), but this will happen
> anyway with
>
> the delgated renderer.
>
> I've also removed a test that was breaking due to the change.
>
> The test wasn't really testing what we thought, because the we wouldn't
> end up
>

"the we"


> rasterizing anything with GPU, even though force was on.
>
> Please review this at https://codereview.chromium.org/842483003/
>
> Base URL: https://chromium.googlesource.com/chromium/src.git@master
>
> Affected files (+0, -35 lines):
>   M cc/resources/tile_manager.cc
>   M cc/trees/layer_tree_host_pixeltest_on_demand_raster.cc
>
>
> Index: cc/resources/tile_manager.cc
> diff --git a/cc/resources/tile_manager.cc b/cc/resources/tile_manager.cc
> index dff3a52f871ac6a80cd05b77e92fc25ab6b14fed..
> a6693529fc1dda2fedf6e4e4fe38bd0f09af97bd 100644
> --- a/cc/resources/tile_manager.cc
> +++ b/cc/resources/tile_manager.cc
> @@ -418,28 +418,6 @@ void TileManager::SynchronouslyRasterizeTiles(
>        tiles_that_need_to_be_rasterized, resource_pool_,
>        base::Bind(&TileManager::UpdateTileDrawInfo,
> base::Unretained(this)));
>
> -  // Use on-demand raster for any required-for-activation tiles that have
> not
> -  // been been assigned memory after reaching a steady memory state. This
> -  // ensures that we activate even when OOM. Note that we have to rebuilt
> the
> -  // queue in case the last AssignGpuMemoryToTiles evicted some tiles
> that would
> -  // otherwise not be picked up by the old raster queue.
> -  client_->BuildRasterQueue(&raster_priority_queue_,
> -                            global_state_.tree_priority);
> -
> -  // Use on-demand raster for any tiles that have not been been assigned
> -  // memory. This ensures that we draw even when OOM.
> -  while (!raster_priority_queue_.IsEmpty()) {
> -    Tile* tile = raster_priority_queue_.Top();
> -    TileDrawInfo& draw_info = tile->draw_info();
> -
> -    if (tile->required_for_draw() && !draw_info.IsReadyToDraw()) {
> -      draw_info.set_rasterize_on_demand();
> -      client_->NotifyTileStateChanged(tile);
> -    }
> -    raster_priority_queue_.Pop();
> -  }
> -  raster_priority_queue_.Reset();
> -
>    TRACE_EVENT_INSTANT1("cc", "DidRasterize", TRACE_EVENT_SCOPE_THREAD,
> "state",
>                         BasicStateAsValue());
>
> Index: cc/trees/layer_tree_host_pixeltest_on_demand_raster.cc
> diff --git a/cc/trees/layer_tree_host_pixeltest_on_demand_raster.cc
> b/cc/trees/layer_tree_host_pixeltest_on_demand_raster.cc
> index a5ccb8b65c3bfdad1b0595caf859cfbf8d3a847c..
> 4d60baf355e89ef5437a964c28f032162cad8da1 100644
> --- a/cc/trees/layer_tree_host_pixeltest_on_demand_raster.cc
> +++ b/cc/trees/layer_tree_host_pixeltest_on_demand_raster.cc
> @@ -111,19 +111,6 @@ TEST_F(LayerTreeHostOnDemandRasterPixelTest,
> RasterPictureLayer) {
>    RunOnDemandRasterPixelTest();
>  }
>
> -class LayerTreeHostOnDemandRasterPixelTestWithGpuRasterizationForced
> -    : public LayerTreeHostOnDemandRasterPixelTest {
> -  void InitializeSettings(LayerTreeSettings* settings) override {
> -    LayerTreeHostOnDemandRasterPixelTest::InitializeSettings(settings);
> -    settings->gpu_rasterization_forced = true;
> -  }
> -};
> -
> -TEST_F(LayerTreeHostOnDemandRasterPixelTestWithGpuRasterizationForced,
> -       RasterPictureLayer) {
> -  RunOnDemandRasterPixelTest();
> -}
> -
>  }  // namespace
>  }  // namespace cc
>
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[hendrikw, 2015-01-10 00:06:18]

On 2015/01/10 00:02:45, danakj wrote:
> On Fri, Jan 9, 2015 at 3:53 PM, <mailto:hendrikw@chromium.org> wrote:
> 
> > Reviewers: vmpstr,
> >
> > Message:
> > PTAL, thanks!
> >
> >
> > Description:
> > cc: Remove tile rasterize on demand for GPU rasterization
> >
> > This code was recently added and isn't needed because it signals to the
> > tile
> > that it should raster on demand (checkerboard), but this will happen
> > anyway with
> >
> > the delgated renderer.
> >
> > I've also removed a test that was breaking due to the change.
> >
> > The test wasn't really testing what we thought, because the we wouldn't
> > end up
> >
> 
> "the we"
> 
> 
> > rasterizing anything with GPU, even though force was on.
> >
> > Please review this at https://codereview.chromium.org/842483003/
> >
> > Base URL: https://chromium.googlesource.com/chromium/src.git@master
> >
> > Affected files (+0, -35 lines):
> >   M cc/resources/tile_manager.cc
> >   M cc/trees/layer_tree_host_pixeltest_on_demand_raster.cc
> >
> >
> > Index: cc/resources/tile_manager.cc
> > diff --git a/cc/resources/tile_manager.cc b/cc/resources/tile_manager.cc
> > index dff3a52f871ac6a80cd05b77e92fc25ab6b14fed..
> > a6693529fc1dda2fedf6e4e4fe38bd0f09af97bd 100644
> > --- a/cc/resources/tile_manager.cc
> > +++ b/cc/resources/tile_manager.cc
> > @@ -418,28 +418,6 @@ void TileManager::SynchronouslyRasterizeTiles(
> >        tiles_that_need_to_be_rasterized, resource_pool_,
> >        base::Bind(&TileManager::UpdateTileDrawInfo,
> > base::Unretained(this)));
> >
> > -  // Use on-demand raster for any required-for-activation tiles that have
> > not
> > -  // been been assigned memory after reaching a steady memory state. This
> > -  // ensures that we activate even when OOM. Note that we have to rebuilt
> > the
> > -  // queue in case the last AssignGpuMemoryToTiles evicted some tiles
> > that would
> > -  // otherwise not be picked up by the old raster queue.
> > -  client_->BuildRasterQueue(&raster_priority_queue_,
> > -                            global_state_.tree_priority);
> > -
> > -  // Use on-demand raster for any tiles that have not been been assigned
> > -  // memory. This ensures that we draw even when OOM.
> > -  while (!raster_priority_queue_.IsEmpty()) {
> > -    Tile* tile = raster_priority_queue_.Top();
> > -    TileDrawInfo& draw_info = tile->draw_info();
> > -
> > -    if (tile->required_for_draw() && !draw_info.IsReadyToDraw()) {
> > -      draw_info.set_rasterize_on_demand();
> > -      client_->NotifyTileStateChanged(tile);
> > -    }
> > -    raster_priority_queue_.Pop();
> > -  }
> > -  raster_priority_queue_.Reset();
> > -
> >    TRACE_EVENT_INSTANT1("cc", "DidRasterize", TRACE_EVENT_SCOPE_THREAD,
> > "state",
> >                         BasicStateAsValue());
> >
> > Index: cc/trees/layer_tree_host_pixeltest_on_demand_raster.cc
> > diff --git a/cc/trees/layer_tree_host_pixeltest_on_demand_raster.cc
> > b/cc/trees/layer_tree_host_pixeltest_on_demand_raster.cc
> > index a5ccb8b65c3bfdad1b0595caf859cfbf8d3a847c..
> > 4d60baf355e89ef5437a964c28f032162cad8da1 100644
> > --- a/cc/trees/layer_tree_host_pixeltest_on_demand_raster.cc
> > +++ b/cc/trees/layer_tree_host_pixeltest_on_demand_raster.cc
> > @@ -111,19 +111,6 @@ TEST_F(LayerTreeHostOnDemandRasterPixelTest,
> > RasterPictureLayer) {
> >    RunOnDemandRasterPixelTest();
> >  }
> >
> > -class LayerTreeHostOnDemandRasterPixelTestWithGpuRasterizationForced
> > -    : public LayerTreeHostOnDemandRasterPixelTest {
> > -  void InitializeSettings(LayerTreeSettings* settings) override {
> > -    LayerTreeHostOnDemandRasterPixelTest::InitializeSettings(settings);
> > -    settings->gpu_rasterization_forced = true;
> > -  }
> > -};
> > -
> > -TEST_F(LayerTreeHostOnDemandRasterPixelTestWithGpuRasterizationForced,
> > -       RasterPictureLayer) {
> > -  RunOnDemandRasterPixelTest();
> > -}
> > -
> >  }  // namespace
> >  }  // namespace cc
> >
> >
> >
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

the oops

[vmpstr, 2015-01-12 18:54:20]

lgtm

[hendrikw, 2015-01-12 18:54:52]

The CQ bit was checked by hendrikw@chromium.org

[commit-bot: I haz the power, 2015-01-12 18:56:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/842483003/20001

[commit-bot: I haz the power, 2015-01-12 21:09:55]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-01-12 21:11:05]

Patchset 2 (id:??) landed as
https://crrev.com/01a4b8027c2e9e678cd6afd0a97bf83807d55f5a
Cr-Commit-Position: refs/heads/master@{#311109}

[hendrikw, 2015-01-15 23:22:11]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/832353004/ by hendrikw@chromium.org.

The reason for reverting is: Speculative revert for
https://code.google.com/p/chromium/issues/detail?id=449288.