New issue
Advanced search Search tips

Issue 641176 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Should we fold WebLayerImplFixedBounds into Layer?

Project Member Reported by vmp...@chromium.org, Aug 25 2016

Issue description

WebLayerImplFixedBounds intercepts some calls like SetBounds and converts them to other calls on a Layer. It's also an implementation of a WebLayer interface in blink.

If we want to remove WebLayer at some point, would it make sense to start by removing WebLayerImplFixedBounds? I think the natural place to move all of the special logic is Layer. Wdyt?
 

Comment 1 by danakj@chromium.org, Aug 25 2016

Or to the users of WebLayerImplFixedBounds, what would that look like?

Comment 2 by vmp...@chromium.org, Aug 26 2016

I think most of the users are actually WebLayer users (such as GraphicsLayer). So, propagating this there might look awkward... I think it would be the difference bewteen

if (webLayer->isFixedBounds())
  webLayer->invalidateRect(rect);
else
  webLayer->invalidate();

anywhere we call invalidate (2 callsites)

instead of having

Layer::SetNeedsDisplayRect(gfx::Rect rect) {
  if (fixed_bounds_)
    rect = gfx::Rect(bounds()); // (since SetNeedsDisplay just calls
                                //  SetNeedsDisplayRect(gfx::Rect(bounds()))
  ...
}
    
  
I would prefer to try to move this into Layer. Do you have reasons for moving it the other way?

Comment 3 by enne@chromium.org, Aug 26 2016

I think in practice SetNeedsDisplay doesn't get called on fixed bounds layers.  The real difference is that it doesn't change bounds, which normally creates implicit invalidations elsewhere.

I do think we should remove this class at this point.  It was a convenient way to do things in the past, but I think it's going to get in the way in the future.

Comment 4 by vmp...@chromium.org, Aug 26 2016

(I was just using SetNeedsDisplay as an example, similar transformations would apply to SetTransform and SetBounds)

Comment 5 by vmp...@chromium.org, Sep 17 2016

The latest idea here (credit: enne) is to have a raster_bounds as well as regular bounds on a layer. Then from layer to layer_impl, we can just push properties as usual. However, when we get to layer_impl (specifically picture_layer_impl), we can use raster bounds when creating the tilings and then scale the resulting tiles to fit into bounds when we append quads.

enne@ please correct me if I'm wrong. 

I still plan on taking a look at this, but not in the immediate future.

Sign in to add a comment