cc: Refactor content scale/bounds into draw properties

cc/layer.h

Index: cc/layer.h
diff --git a/cc/layer.h b/cc/layer.h
index 91a7cdbcd94c76b89476ea9473909f1bd46f4f60..5d20e1f747e26f78112f6cd379258031ddb1b3db 100644
--- a/cc/layer.h
+++ b/cc/layer.h
@@ -84,8 +84,8 @@ public:
     // 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; }
@@ -227,9 +227,10 @@ public:
     // 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
@@ -316,6 +317,8 @@ protected:
     virtual scoped_ptr<LayerImpl> createLayerImpl(LayerTreeHostImpl* hostImpl);
     int m_layerId;
 
+    DrawProperties<Layer, RenderSurface> m_drawProperties;
+
 private:
     friend class base::RefCounted<Layer>;
 
@@ -389,8 +392,6 @@ private:
 
     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*);