Refactor ShapeOutsideInfo so it isn't mutated for each line
authorbjonesbe@adobe.com <bjonesbe@adobe.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Aug 2014 19:05:21 +0000 (19:05 +0000)
committerbjonesbe@adobe.com <bjonesbe@adobe.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Aug 2014 19:05:21 +0000 (19:05 +0000)
https://bugs.webkit.org/show_bug.cgi?id=135781

Reviewed by Zoltan Horvath.

Encapsulate the per line state into a ShapeOutsideDeltas object.
updateDeltasForContainingBlockLine has been renamed to
computeDeltasForContainingBlockLine, and it returns an instance of the
ShapeOutsideDeltas object for that line. This object is cached, but
none of the functionality of ShapeOutsideInfo is dependant on any line
specific data anymore.

No new tests, no behavior change.

* rendering/FloatingObjects.cpp:
(WebCore::ComputeFloatOffsetForFloatLayoutAdapter<FloatingObject::FloatLeft>::updateOffsetIfNeeded):
    Update to take a FloatingObject reference because the FloatingObject cannot be null.
(WebCore::ComputeFloatOffsetForFloatLayoutAdapter<FloatingObject::FloatRight>::updateOffsetIfNeeded):
    Ditto.
(WebCore::ComputeFloatOffsetAdapter<FloatTypeValue>::collectIfNeeded): Pass FloatingObject to
    updateOffsetIfNeeded as a reference, since it cannot be null.
(WebCore::ComputeFloatOffsetForLineLayoutAdapter<FloatingObject::FloatLeft>::updateOffsetIfNeeded):
    Use ShapeOutsideDeltas object to calculate the offset and take the FloatingObject as a
    reference.
(WebCore::ComputeFloatOffsetForLineLayoutAdapter<FloatingObject::FloatRight>::updateOffsetIfNeeded):
    Ditto.
(WebCore::shapeInfoForFloat): Deleted.
* rendering/line/LineWidth.cpp:
(WebCore::LineWidth::shrinkAvailableWidthForNewFloatIfNeeded): Update to use ShapeOutsideDeltas object.
* rendering/shapes/ShapeOutsideInfo.cpp:
(WebCore::ShapeOutsideInfo::computeDeltasForContainingBlockLine): Return a ShaoeOutsideDeltas object
    instead of storing line specific data in instance variables.
(WebCore::ShapeOutsideInfo::updateDeltasForContainingBlockLine): Deleted.
* rendering/shapes/ShapeOutsideInfo.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@172529 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/rendering/FloatingObjects.cpp
Source/WebCore/rendering/line/LineWidth.cpp
Source/WebCore/rendering/shapes/ShapeOutsideInfo.cpp
Source/WebCore/rendering/shapes/ShapeOutsideInfo.h

index d2d0ca7..3c3e62f 100644 (file)
@@ -1,3 +1,40 @@
+2014-08-13  Bem Jones-Bey  <bjonesbe@adobe.com>
+
+        Refactor ShapeOutsideInfo so it isn't mutated for each line
+        https://bugs.webkit.org/show_bug.cgi?id=135781
+
+        Reviewed by Zoltan Horvath.
+
+        Encapsulate the per line state into a ShapeOutsideDeltas object.
+        updateDeltasForContainingBlockLine has been renamed to
+        computeDeltasForContainingBlockLine, and it returns an instance of the
+        ShapeOutsideDeltas object for that line. This object is cached, but
+        none of the functionality of ShapeOutsideInfo is dependant on any line
+        specific data anymore.
+
+        No new tests, no behavior change.
+
+        * rendering/FloatingObjects.cpp:
+        (WebCore::ComputeFloatOffsetForFloatLayoutAdapter<FloatingObject::FloatLeft>::updateOffsetIfNeeded):
+            Update to take a FloatingObject reference because the FloatingObject cannot be null.
+        (WebCore::ComputeFloatOffsetForFloatLayoutAdapter<FloatingObject::FloatRight>::updateOffsetIfNeeded):
+            Ditto.
+        (WebCore::ComputeFloatOffsetAdapter<FloatTypeValue>::collectIfNeeded): Pass FloatingObject to
+            updateOffsetIfNeeded as a reference, since it cannot be null.
+        (WebCore::ComputeFloatOffsetForLineLayoutAdapter<FloatingObject::FloatLeft>::updateOffsetIfNeeded):
+            Use ShapeOutsideDeltas object to calculate the offset and take the FloatingObject as a
+            reference.
+        (WebCore::ComputeFloatOffsetForLineLayoutAdapter<FloatingObject::FloatRight>::updateOffsetIfNeeded):
+            Ditto.
+        (WebCore::shapeInfoForFloat): Deleted.
+        * rendering/line/LineWidth.cpp:
+        (WebCore::LineWidth::shrinkAvailableWidthForNewFloatIfNeeded): Update to use ShapeOutsideDeltas object.
+        * rendering/shapes/ShapeOutsideInfo.cpp:
+        (WebCore::ShapeOutsideInfo::computeDeltasForContainingBlockLine): Return a ShaoeOutsideDeltas object
+            instead of storing line specific data in instance variables.
+        (WebCore::ShapeOutsideInfo::updateDeltasForContainingBlockLine): Deleted.
+        * rendering/shapes/ShapeOutsideInfo.h:
+
 2014-08-13  Zoltan Horvath  <zoltan@webkit.org>
 
         [CSS3-Text] Add rendering support for the none value of text-justify property
index 5b5bf06..b407a7d 100644 (file)
@@ -141,7 +141,7 @@ public:
     LayoutUnit offset() const { return m_offset; }
 
 protected:
-    virtual bool updateOffsetIfNeeded(const FloatingObject*) = 0;
+    virtual bool updateOffsetIfNeeded(const FloatingObject&) = 0;
 
     const RenderBlockFlow& m_renderer;
     LayoutUnit m_lineTop;
@@ -151,7 +151,7 @@ protected:
 };
 
 template <FloatingObject::Type FloatTypeValue>
-class ComputeFloatOffsetForFloatLayoutAdapter : public ComputeFloatOffsetAdapter<FloatTypeValue> { 
+class ComputeFloatOffsetForFloatLayoutAdapter : public ComputeFloatOffsetAdapter<FloatTypeValue> {
 public:
     ComputeFloatOffsetForFloatLayoutAdapter(const RenderBlockFlow& renderer, LayoutUnit lineTop, LayoutUnit lineBottom, LayoutUnit offset)
         : ComputeFloatOffsetAdapter<FloatTypeValue>(renderer, lineTop, lineBottom, offset)
@@ -163,7 +163,7 @@ public:
     LayoutUnit heightRemaining() const;
 
 protected:
-    virtual bool updateOffsetIfNeeded(const FloatingObject*) override final;
+    virtual bool updateOffsetIfNeeded(const FloatingObject&) override final;
 };
 
 template <FloatingObject::Type FloatTypeValue>
@@ -177,7 +177,7 @@ public:
     virtual ~ComputeFloatOffsetForLineLayoutAdapter() { }
 
 protected:
-    virtual bool updateOffsetIfNeeded(const FloatingObject*) override final;
+    virtual bool updateOffsetIfNeeded(const FloatingObject&) override final;
 };
 
 class FindNextFloatLogicalBottomAdapter {
@@ -425,9 +425,9 @@ LayoutUnit FloatingObjects::logicalRightOffset(LayoutUnit fixedOffset, LayoutUni
 }
 
 template<>
-inline bool ComputeFloatOffsetForFloatLayoutAdapter<FloatingObject::FloatLeft>::updateOffsetIfNeeded(const FloatingObject* floatingObject)
+inline bool ComputeFloatOffsetForFloatLayoutAdapter<FloatingObject::FloatLeft>::updateOffsetIfNeeded(const FloatingObject& floatingObject)
 {
-    LayoutUnit logicalRight = m_renderer.logicalRightForFloat(floatingObject);
+    LayoutUnit logicalRight = m_renderer.logicalRightForFloat(&floatingObject);
     if (logicalRight > m_offset) {
         m_offset = logicalRight;
         return true;
@@ -436,9 +436,9 @@ inline bool ComputeFloatOffsetForFloatLayoutAdapter<FloatingObject::FloatLeft>::
 }
 
 template<>
-inline bool ComputeFloatOffsetForFloatLayoutAdapter<FloatingObject::FloatRight>::updateOffsetIfNeeded(const FloatingObject* floatingObject)
+inline bool ComputeFloatOffsetForFloatLayoutAdapter<FloatingObject::FloatRight>::updateOffsetIfNeeded(const FloatingObject& floatingObject)
 {
-    LayoutUnit logicalLeft = m_renderer.logicalLeftForFloat(floatingObject);
+    LayoutUnit logicalLeft = m_renderer.logicalLeftForFloat(&floatingObject);
     if (logicalLeft < m_offset) {
         m_offset = logicalLeft;
         return true;
@@ -463,35 +463,22 @@ inline void ComputeFloatOffsetAdapter<FloatTypeValue>::collectIfNeeded(const Int
     ASSERT(floatingObject->isPlaced());
     ASSERT(rangesIntersect(m_renderer.logicalTopForFloat(floatingObject), m_renderer.logicalBottomForFloat(floatingObject), m_lineTop, m_lineBottom));
 
-    bool floatIsNewExtreme = updateOffsetIfNeeded(floatingObject);
+    bool floatIsNewExtreme = updateOffsetIfNeeded(*floatingObject);
     if (floatIsNewExtreme)
         m_outermostFloat = floatingObject;
 }
 
-#if ENABLE(CSS_SHAPES)
-static inline ShapeOutsideInfo* shapeInfoForFloat(const FloatingObject* floatingObject, const RenderBlockFlow& containingBlock, LayoutUnit lineTop, LayoutUnit lineBottom)
-{
-    if (floatingObject) {
-        if (ShapeOutsideInfo* shapeOutside = floatingObject->renderer().shapeOutsideInfo()) {
-            shapeOutside->updateDeltasForContainingBlockLine(containingBlock, *floatingObject, lineTop, lineBottom - lineTop);
-            return shapeOutside;
-        }
-    }
-
-    return nullptr;
-}
-#endif
-
 template<>
-inline bool ComputeFloatOffsetForLineLayoutAdapter<FloatingObject::FloatLeft>::updateOffsetIfNeeded(const FloatingObject* floatingObject)
+inline bool ComputeFloatOffsetForLineLayoutAdapter<FloatingObject::FloatLeft>::updateOffsetIfNeeded(const FloatingObject& floatingObject)
 {
-    LayoutUnit logicalRight = m_renderer.logicalRightForFloat(floatingObject);
+    LayoutUnit logicalRight = m_renderer.logicalRightForFloat(&floatingObject);
 #if ENABLE(CSS_SHAPES)
-    if (ShapeOutsideInfo* shapeOutside = shapeInfoForFloat(floatingObject, m_renderer, m_lineTop, m_lineBottom)) {
-        if (!shapeOutside->lineOverlapsShape())
+    if (ShapeOutsideInfo* shapeOutside = floatingObject.renderer().shapeOutsideInfo()) {
+        ShapeOutsideDeltas shapeDeltas = shapeOutside->computeDeltasForContainingBlockLine(m_renderer, floatingObject, m_lineTop, m_lineBottom - m_lineTop);
+        if (!shapeDeltas.lineOverlapsShape())
             return false;
 
-        logicalRight += shapeOutside->rightMarginBoxDelta();
+        logicalRight += shapeDeltas.rightMarginBoxDelta();
     }
 #endif
     if (logicalRight > m_offset) {
@@ -503,15 +490,16 @@ inline bool ComputeFloatOffsetForLineLayoutAdapter<FloatingObject::FloatLeft>::u
 }
 
 template<>
-inline bool ComputeFloatOffsetForLineLayoutAdapter<FloatingObject::FloatRight>::updateOffsetIfNeeded(const FloatingObject* floatingObject)
+inline bool ComputeFloatOffsetForLineLayoutAdapter<FloatingObject::FloatRight>::updateOffsetIfNeeded(const FloatingObject& floatingObject)
 {
-    LayoutUnit logicalLeft = m_renderer.logicalLeftForFloat(floatingObject);
+    LayoutUnit logicalLeft = m_renderer.logicalLeftForFloat(&floatingObject);
 #if ENABLE(CSS_SHAPES)
-    if (ShapeOutsideInfo* shapeOutside = shapeInfoForFloat(floatingObject, m_renderer, m_lineTop, m_lineBottom)) {
-        if (!shapeOutside->lineOverlapsShape())
+    if (ShapeOutsideInfo* shapeOutside = floatingObject.renderer().shapeOutsideInfo()) {
+        ShapeOutsideDeltas shapeDeltas = shapeOutside->computeDeltasForContainingBlockLine(m_renderer, floatingObject, m_lineTop, m_lineBottom - m_lineTop);
+        if (!shapeDeltas.lineOverlapsShape())
             return false;
 
-        logicalLeft += shapeOutside->leftMarginBoxDelta();
+        logicalLeft += shapeDeltas.leftMarginBoxDelta();
     }
 #endif
     if (logicalLeft < m_offset) {
index 8ccc7fc..e62ae99 100644 (file)
@@ -83,10 +83,10 @@ void LineWidth::shrinkAvailableWidthForNewFloatIfNeeded(FloatingObject* newFloat
         return;
 
 #if ENABLE(CSS_SHAPES)
-    ShapeOutsideInfo* shapeOutsideInfo = newFloat->renderer().shapeOutsideInfo();
-    if (shapeOutsideInfo) {
+    ShapeOutsideDeltas shapeDeltas;
+    if (ShapeOutsideInfo* shapeOutsideInfo = newFloat->renderer().shapeOutsideInfo()) {
         LayoutUnit lineHeight = m_block.lineHeight(m_isFirstLine, m_block.isHorizontalWritingMode() ? HorizontalLine : VerticalLine, PositionOfInteriorLineBoxes);
-        shapeOutsideInfo->updateDeltasForContainingBlockLine(m_block, *newFloat, m_block.logicalHeight(), lineHeight);
+        shapeDeltas = shapeOutsideInfo->computeDeltasForContainingBlockLine(m_block, *newFloat, m_block.logicalHeight(), lineHeight);
     }
 #endif
 
@@ -95,9 +95,9 @@ void LineWidth::shrinkAvailableWidthForNewFloatIfNeeded(FloatingObject* newFloat
         if (shouldIndentText() && m_block.style().isLeftToRightDirection())
             newLeft += floorToInt(m_block.textIndentOffset());
 #if ENABLE(CSS_SHAPES)
-        if (shapeOutsideInfo) {
-            if (shapeOutsideInfo->lineOverlapsShape())
-                newLeft += shapeOutsideInfo->rightMarginBoxDelta();
+        if (shapeDeltas.isValid()) {
+            if (shapeDeltas.lineOverlapsShape())
+                newLeft += shapeDeltas.rightMarginBoxDelta();
             else // If the line doesn't overlap the shape, then we need to act as if this float didn't exist.
                 newLeft = m_left;
         }
@@ -108,9 +108,9 @@ void LineWidth::shrinkAvailableWidthForNewFloatIfNeeded(FloatingObject* newFloat
         if (shouldIndentText() && !m_block.style().isLeftToRightDirection())
             newRight -= floorToInt(m_block.textIndentOffset());
 #if ENABLE(CSS_SHAPES)
-        if (shapeOutsideInfo) {
-            if (shapeOutsideInfo->lineOverlapsShape())
-                newRight += shapeOutsideInfo->leftMarginBoxDelta();
+        if (shapeDeltas.isValid()) {
+            if (shapeDeltas.lineOverlapsShape())
+                newRight += shapeDeltas.leftMarginBoxDelta();
             else // If the line doesn't overlap the shape, then we need to act as if this float didn't exist.
                 newRight = m_right;
         }
index 945f7ec..34f1ba1 100644 (file)
@@ -303,42 +303,39 @@ bool ShapeOutsideInfo::isEnabledFor(const RenderBox& box)
     return false;
 }
 
-void ShapeOutsideInfo::updateDeltasForContainingBlockLine(const RenderBlockFlow& containingBlock, const FloatingObject& floatingObject, LayoutUnit lineTop, LayoutUnit lineHeight)
+ShapeOutsideDeltas ShapeOutsideInfo::computeDeltasForContainingBlockLine(const RenderBlockFlow& containingBlock, const FloatingObject& floatingObject, LayoutUnit lineTop, LayoutUnit lineHeight)
 {
     ASSERT(lineHeight >= 0);
-
     LayoutUnit borderBoxTop = containingBlock.logicalTopForFloat(&floatingObject) + containingBlock.marginBeforeForChild(m_renderer);
     LayoutUnit borderBoxLineTop = lineTop - borderBoxTop;
 
-    if (isShapeDirty() || m_borderBoxLineTop != borderBoxLineTop || m_lineHeight != lineHeight) {
-        m_borderBoxLineTop = borderBoxLineTop;
-        m_referenceBoxLineTop = borderBoxLineTop - logicalTopOffset();
-        m_lineHeight = lineHeight;
-
+    if (isShapeDirty() || !m_shapeOutsideDeltas.isForLine(borderBoxLineTop, lineHeight)) {
+        LayoutUnit referenceBoxLineTop = borderBoxLineTop - logicalTopOffset();
         LayoutUnit floatMarginBoxWidth = containingBlock.logicalWidthForFloat(&floatingObject);
 
-        if (computedShape().lineOverlapsShapeMarginBounds(m_referenceBoxLineTop, m_lineHeight)) {
+        if (computedShape().lineOverlapsShapeMarginBounds(referenceBoxLineTop, lineHeight)) {
             LineSegment segment = computedShape().getExcludedInterval((borderBoxLineTop - logicalTopOffset()), std::min(lineHeight, shapeLogicalBottom() - borderBoxLineTop));
             if (segment.isValid) {
                 LayoutUnit logicalLeftMargin = containingBlock.style().isLeftToRightDirection() ? containingBlock.marginStartForChild(m_renderer) : containingBlock.marginEndForChild(m_renderer);
                 LayoutUnit rawLeftMarginBoxDelta = segment.logicalLeft + logicalLeftOffset() + logicalLeftMargin;
-                m_leftMarginBoxDelta = clampTo<LayoutUnit>(rawLeftMarginBoxDelta, LayoutUnit(), floatMarginBoxWidth);
+                LayoutUnit leftMarginBoxDelta = clampTo<LayoutUnit>(rawLeftMarginBoxDelta, LayoutUnit(), floatMarginBoxWidth);
 
                 LayoutUnit logicalRightMargin = containingBlock.style().isLeftToRightDirection() ? containingBlock.marginEndForChild(m_renderer) : containingBlock.marginStartForChild(m_renderer);
                 LayoutUnit rawRightMarginBoxDelta = segment.logicalRight + logicalLeftOffset() - containingBlock.logicalWidthForChild(m_renderer) - logicalRightMargin;
-                m_rightMarginBoxDelta = clampTo<LayoutUnit>(rawRightMarginBoxDelta, -floatMarginBoxWidth, LayoutUnit());
-                m_lineOverlapsShape = true;
-                return;
+                LayoutUnit rightMarginBoxDelta = clampTo<LayoutUnit>(rawRightMarginBoxDelta, -floatMarginBoxWidth, LayoutUnit());
+
+                m_shapeOutsideDeltas = ShapeOutsideDeltas(leftMarginBoxDelta, rightMarginBoxDelta, true, borderBoxLineTop, lineHeight);
+                return m_shapeOutsideDeltas;
             }
         }
 
         // Lines that do not overlap the shape should act as if the float
         // wasn't there for layout purposes. So we set the deltas to remove the
         // entire width of the float
-        m_leftMarginBoxDelta = floatMarginBoxWidth;
-        m_rightMarginBoxDelta = -floatMarginBoxWidth;
-        m_lineOverlapsShape = false;
+        m_shapeOutsideDeltas = ShapeOutsideDeltas(floatMarginBoxWidth, -floatMarginBoxWidth, false, borderBoxLineTop, lineHeight);
     }
+
+    return m_shapeOutsideDeltas;
 }
 
 }
index 71beb06..a48d9c9 100644 (file)
@@ -43,22 +43,58 @@ class RenderBox;
 class StyleImage;
 class FloatingObject;
 
+class ShapeOutsideDeltas final {
+public:
+    ShapeOutsideDeltas()
+        : m_leftMarginBoxDelta(0)
+        , m_rightMarginBoxDelta(0)
+        , m_borderBoxLineTop(0)
+        , m_lineHeight(0)
+        , m_lineOverlapsShape(false)
+        , m_isValid(false)
+    {
+    }
+
+    ShapeOutsideDeltas(LayoutUnit leftMarginBoxDelta, LayoutUnit rightMarginBoxDelta, bool lineOverlapsShape, LayoutUnit borderBoxLineTop, LayoutUnit lineHeight)
+        : m_leftMarginBoxDelta(leftMarginBoxDelta)
+        , m_rightMarginBoxDelta(rightMarginBoxDelta)
+        , m_borderBoxLineTop(borderBoxLineTop)
+        , m_lineHeight(lineHeight)
+        , m_lineOverlapsShape(lineOverlapsShape)
+        , m_isValid(true)
+    {
+    }
+
+    bool isForLine(LayoutUnit borderBoxLineTop, LayoutUnit lineHeight)
+    {
+        return m_isValid && m_borderBoxLineTop == borderBoxLineTop && m_lineHeight == lineHeight;
+    }
+
+    bool isValid() { return m_isValid; }
+    LayoutUnit leftMarginBoxDelta() { ASSERT(m_isValid); return m_leftMarginBoxDelta; }
+    LayoutUnit rightMarginBoxDelta() { ASSERT(m_isValid); return m_rightMarginBoxDelta; }
+    bool lineOverlapsShape() { ASSERT(m_isValid); return m_lineOverlapsShape; }
+
+private:
+    LayoutUnit m_leftMarginBoxDelta;
+    LayoutUnit m_rightMarginBoxDelta;
+    LayoutUnit m_borderBoxLineTop;
+    LayoutUnit m_lineHeight;
+    bool m_lineOverlapsShape;
+    bool m_isValid;
+};
+
 class ShapeOutsideInfo final {
     WTF_MAKE_FAST_ALLOCATED;
 public:
     ShapeOutsideInfo(const RenderBox& renderer)
         : m_renderer(renderer)
-        , m_lineOverlapsShape(false)
     {
     }
 
     static bool isEnabledFor(const RenderBox&);
 
-    LayoutUnit leftMarginBoxDelta() const { return m_leftMarginBoxDelta; }
-    LayoutUnit rightMarginBoxDelta() const { return m_rightMarginBoxDelta; }
-    bool lineOverlapsShape() const { return m_lineOverlapsShape; }
-
-    void updateDeltasForContainingBlockLine(const RenderBlockFlow&, const FloatingObject&, LayoutUnit lineTop, LayoutUnit lineHeight);
+    ShapeOutsideDeltas computeDeltasForContainingBlockLine(const RenderBlockFlow&, const FloatingObject&, LayoutUnit lineTop, LayoutUnit lineHeight);
 
     void setReferenceBoxLogicalSize(LayoutSize);
 
@@ -69,10 +105,6 @@ public:
     LayoutUnit shapeLogicalWidth() const { return computedShape().shapeMarginLogicalBoundingBox().width(); }
     LayoutUnit shapeLogicalHeight() const { return computedShape().shapeMarginLogicalBoundingBox().height(); }
 
-    LayoutUnit logicalLineTop() const { return m_referenceBoxLineTop + logicalTopOffset(); }
-    LayoutUnit logicalLineBottom() const { return m_referenceBoxLineTop + m_lineHeight + logicalTopOffset(); }
-    LayoutUnit logicalLineBottom(LayoutUnit lineHeight) const { return m_referenceBoxLineTop + lineHeight + logicalTopOffset(); }
-
     void markShapeAsDirty() { m_shape = nullptr; }
     bool isShapeDirty() { return !m_shape; }
 
@@ -110,13 +142,8 @@ private:
 
     mutable std::unique_ptr<Shape> m_shape;
     LayoutSize m_referenceBoxLogicalSize;
-    LayoutUnit m_referenceBoxLineTop;
-    LayoutUnit m_lineHeight;
 
-    LayoutUnit m_leftMarginBoxDelta;
-    LayoutUnit m_rightMarginBoxDelta;
-    LayoutUnit m_borderBoxLineTop;
-    bool m_lineOverlapsShape;
+    ShapeOutsideDeltas m_shapeOutsideDeltas;
 };
 
 }