cc: Refactor content scale/bounds into draw properties

cc/layer.h

 // Copyright 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CC_LAYER_H_
 #define CC_LAYER_H_
 
 #include <public/WebFilterOperations.h>
 #include <string>
 #include <vector>
@@ skipping 66 matching lines @@
 
     void setAnchorPointZ(float);
     float anchorPointZ() const { return m_anchorPointZ; }
 
     virtual void setBackgroundColor(SkColor);
     SkColor backgroundColor() const { return m_backgroundColor; }
 
     // A layer's bounds are in logical, non-page-scaled pixels (however, the
     // root layer's bounds are in physical pixels).
     void setBounds(const gfx::Size&);
+    virtual void didUpdateBounds();

[shawnsingh, 2012/12/11 21:54:05]

I think we can do without this.  Instead, I think it might be better to have a
one-way flow of logic from "given properties" --> "computed drawProperties".

So, when layer's given properties change, we could just note to ourselves that
"calcDrawProperties will need to be run" and accept the change.  Then
calcDrawProperties can call setIdealContentsScale as appropriate, where the
layer updates its draw properties.

[enne (OOO), 2012/12/11 23:23:44]

Sadly, no.  Unfortunately, we do so much in content space that we pretty much
need to immediately update content bounds to reflect changes in bounds.  I don't
think this patch is possible without didUpdateBounds().  This is the equivalent
of the previous virtual contentBounds() function.

There may be some later refactoring possible to make this cleaner, but I don't
see any way to do this easily right now.

     const gfx::Size& bounds() const { return m_bounds; }
-    virtual gfx::Size contentBounds() const;
 
     void setMasksToBounds(bool);
     bool masksToBounds() const { return m_masksToBounds; }
 
     void setMaskLayer(Layer*);
     Layer* maskLayer() { return m_maskLayer.get(); }
     const Layer* maskLayer() const { return m_maskLayer.get(); }
 
     virtual void setNeedsDisplayRect(const gfx::RectF& dirtyRect);
     void setNeedsDisplay() { setNeedsDisplayRect(gfx::RectF(gfx::PointF(), bounds())); }
@@ skipping 121 matching lines @@
     void setDebugName(const std::string&);
 
     virtual void pushPropertiesTo(LayerImpl*);
 
     void clearRenderSurface() { m_drawProperties.render_surface.reset(); }
     void createRenderSurface();
 
     // The contentsScale converts from logical, non-page-scaled pixels to target pixels.
     // The contentsScale is 1 for the root layer as it is already in physical pixels.
     // By default contentsScale is forced to be 1 except for subclasses of ContentsScalingLayer.
-    virtual float contentsScaleX() const;
-    virtual float contentsScaleY() const;
-    virtual void setContentsScale(float contentsScale) { }
+    float contentsScaleX() const { return m_drawProperties.contents_scale_x; }
+    float contentsScaleY() const { return m_drawProperties.contents_scale_y; }

[shawnsingh, 2012/12/11 21:54:05]

maybe to avoid confusion with the previous state of the code, we should remove
these accessors, and require people to use drawProperties().contents_scale_x
instead.

That way we can be more explicitly aware of any accidental cycles in our data
flow?  For example, I was briefly confused why image_layer used contentsScaleX()
to compute contentBounds, until I realized that image_layer implemented their
own contentsScaleX() accessor.  removing it I think would be much clearer.

Eventually, I'm thinking we should remove all accessors that return
m_drawProperties values, for the same reason.

[enne (OOO), 2012/12/11 23:23:44]

I think that's just an issue with ImageLayer shadowing these functions.  I
should rename them there.  There's no cycle, there's just an accidental
shadowing.

Callers should really not care about whether these values are lazily computed or
computed and stored.  If you want to change things to call drawProperties() more
explicitly, then that can be a separate patch.

+    gfx::Size contentBounds() const { return m_drawProperties.content_bounds; }
+    virtual void setIdealContentsScale(float ideal_contents_scale);

[shawnsingh, 2012/12/11 21:54:05]

instead of "set" which makes it feel like it's part of the user's public API,
how about we say "calculateActualContentsScale(float ideal_contents_scale)"?   I
personally think that helps separate this function from the settors that are
intended to be a sort of public layer API

[enne (OOO), 2012/12/11 23:23:44]

Sure.  I can rename this for clarity.

 
     // The scale at which contents should be rastered, to match the scale at
     // which they will drawn to the screen. This scale is a component of the
     // contentsScale() but does not include page/device scale factors.
     float rasterScale() const { return m_rasterScale; }
     void setRasterScale(float scale);
 
     // When true, the rasterScale() will be set by the compositor. If false, it
     // will use whatever value is given to it by the embedder.
     bool automaticallyComputeRasterScale() { return m_automaticallyComputeRasterScale; }
@@ skipping 66 matching lines @@
     // is not available and the update rect will remain empty.
     // Note this rect is in layer space (not content space).
     gfx::RectF m_updateRect;
 
     scoped_refptr<Layer> m_maskLayer;
 
     // Constructs a LayerImpl of the correct runtime type for this Layer type.
     virtual scoped_ptr<LayerImpl> createLayerImpl(LayerTreeHostImpl* hostImpl);
     int m_layerId;
 
+    DrawProperties<Layer, RenderSurface> m_drawProperties;
+
 private:
     friend class base::RefCounted<Layer>;
 
     void setParent(Layer*);
     bool hasAncestor(Layer*) const;
     bool descendantIsFixedToContainerLayer() const;
 
     size_t numChildren() const { return m_children.size(); }
 
     // Returns the index of the child or -1 if not found.
@@ skipping 53 matching lines @@
 
     // Transient properties.
     float m_rasterScale;
     bool m_automaticallyComputeRasterScale;
     bool m_boundsContainPageScale;
 
     gfx::Transform m_implTransform;
 
     WebKit::WebAnimationDelegate* m_layerAnimationDelegate;
     WebKit::WebLayerScrollClient* m_layerScrollClient;
-
-    DrawProperties<Layer, RenderSurface> m_drawProperties;
 };
 
 void sortLayers(std::vector<scoped_refptr<Layer> >::iterator, std::vector<scoped_refptr<Layer> >::iterator, void*);
 
 }  // namespace cc
 
 #endif  // CC_LAYER_H_