Attempt to make it more clear what FloatIntervalSearchAdaptor::collectIfNeeded is...
authorbjonesbe@adobe.com <bjonesbe@adobe.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 23 Aug 2013 16:58:18 +0000 (16:58 +0000)
committerbjonesbe@adobe.com <bjonesbe@adobe.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 23 Aug 2013 16:58:18 +0000 (16:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=119816

Reviewed by David Hyatt.

This is a port from Blink of
https://src.chromium.org/viewvc/blink?revision=155885&view=revision
Original Patch by Eric Seidel

Original comments:

"It seemed to me that template specifications would be clearer than an
if.  They also allow for compile-time error checking were a 3rd type
of float to come into existance in CSS4. :p

For any unfamiliar with this method, this the object used for
performing a search on a RedBlackTree in WTF.

We create one of these adaptors, specifying that we want to search for
values in a specific (logical) Y interval, and this adaptor is called
back for any values in the RBTree cooresponding to that interval
range.

The job of this adaptor is to collect the various values we care
about, including the left or right-most offset of the floats in that
Y-range as well as what the last (document order) float seen in that
range.

It also collects the remaining available height for the block but I'm
less clear on how that parameter is used."

Note that in addition to the original change, I have made the
updateOffsetIfNeeded and rangesIntersect methods inline, as this was
shown to be a performance win in
https://src.chromium.org/viewvc/blink?revision=156064&view=revision
and it seemed a rather trivial change to be subject to a separate
patch when porting.

No new tests, no behavior change.

* rendering/RenderBlock.cpp:
(WebCore::::updateOffsetIfNeeded):
(WebCore::::collectIfNeeded):
* rendering/RenderBlock.h:

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

Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlock.cpp
Source/WebCore/rendering/RenderBlock.h

index e55e558..f55bb27 100644 (file)
@@ -1,3 +1,50 @@
+2013-08-23  Bem Jones-Bey  <bjonesbe@adobe.com>
+
+        Attempt to make it more clear what FloatIntervalSearchAdaptor::collectIfNeeded is doing
+        https://bugs.webkit.org/show_bug.cgi?id=119816
+
+        Reviewed by David Hyatt.
+
+        This is a port from Blink of
+        https://src.chromium.org/viewvc/blink?revision=155885&view=revision
+        Original Patch by Eric Seidel
+
+        Original comments:
+
+        "It seemed to me that template specifications would be clearer than an
+        if.  They also allow for compile-time error checking were a 3rd type
+        of float to come into existance in CSS4. :p
+
+        For any unfamiliar with this method, this the object used for
+        performing a search on a RedBlackTree in WTF.
+
+        We create one of these adaptors, specifying that we want to search for
+        values in a specific (logical) Y interval, and this adaptor is called
+        back for any values in the RBTree cooresponding to that interval
+        range.
+
+        The job of this adaptor is to collect the various values we care
+        about, including the left or right-most offset of the floats in that
+        Y-range as well as what the last (document order) float seen in that
+        range.
+
+        It also collects the remaining available height for the block but I'm
+        less clear on how that parameter is used."
+
+        Note that in addition to the original change, I have made the
+        updateOffsetIfNeeded and rangesIntersect methods inline, as this was
+        shown to be a performance win in
+        https://src.chromium.org/viewvc/blink?revision=156064&view=revision
+        and it seemed a rather trivial change to be subject to a separate
+        patch when porting.
+
+        No new tests, no behavior change.
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::::updateOffsetIfNeeded):
+        (WebCore::::collectIfNeeded):
+        * rendering/RenderBlock.h:
+
 2013-08-23  David Kilzer  <ddkilzer@apple.com>
 
         WebCore fails to link due to changes in Objective-C++ ABI in trunk clang
index eb8dac9..054dd02 100644 (file)
@@ -4422,7 +4422,7 @@ void RenderBlock::clearPercentHeightDescendantsFrom(RenderBox* parent)
     }
 }
 
-static bool rangesIntersect(int floatTop, int floatBottom, int objectTop, int objectBottom)
+inline static bool rangesIntersect(int floatTop, int floatBottom, int objectTop, int objectBottom)
 {
     if (objectTop >= floatBottom || objectBottom < floatTop)
         return false;
@@ -4442,32 +4442,43 @@ static bool rangesIntersect(int floatTop, int floatBottom, int objectTop, int ob
     return false;
 }
 
+template<>
+inline bool RenderBlock::FloatIntervalSearchAdapter<RenderBlock::FloatingObject::FloatLeft>::updateOffsetIfNeeded(const FloatingObject* floatingObject) const
+{
+    if (m_renderer->logicalRightForFloat(floatingObject) > m_offset) {
+        m_offset = m_renderer->logicalRightForFloat(floatingObject);
+        return true;
+    }
+    return false;
+}
+
+template<>
+inline bool RenderBlock::FloatIntervalSearchAdapter<RenderBlock::FloatingObject::FloatRight>::updateOffsetIfNeeded(const FloatingObject* floatingObject) const
+{
+    if (m_renderer->logicalLeftForFloat(floatingObject) < m_offset) {
+        m_offset = m_renderer->logicalLeftForFloat(floatingObject);
+        return true;
+    }
+    return false;
+}
+
 template <RenderBlock::FloatingObject::Type FloatTypeValue>
 inline void RenderBlock::FloatIntervalSearchAdapter<FloatTypeValue>::collectIfNeeded(const IntervalType& interval) const
 {
-    const FloatingObject* r = interval.data();
-    if (r->type() != FloatTypeValue || !rangesIntersect(interval.low(), interval.high(), m_lowValue, m_highValue))
+    const FloatingObject* floatingObject = interval.data();
+    if (floatingObject->type() != FloatTypeValue || !rangesIntersect(interval.low(), interval.high(), m_lowValue, m_highValue))
         return;
 
     // All the objects returned from the tree should be already placed.
-    ASSERT(r->isPlaced() && rangesIntersect(m_renderer->logicalTopForFloat(r), m_renderer->logicalBottomForFloat(r), m_lowValue, m_highValue));
+    ASSERT(floatingObject->isPlaced());
+    ASSERT(rangesIntersect(m_renderer->pixelSnappedLogicalTopForFloat(floatingObject), m_renderer->pixelSnappedLogicalBottomForFloat(floatingObject), m_lowValue, m_highValue));
 
-    if (FloatTypeValue == FloatingObject::FloatLeft 
-        && m_renderer->logicalRightForFloat(r) > m_offset) {
-        m_offset = m_renderer->logicalRightForFloat(r);
-        if (m_heightRemaining)
-            *m_heightRemaining = m_renderer->logicalBottomForFloat(r) - m_lowValue;
-    }
-
-    if (FloatTypeValue == FloatingObject::FloatRight
-        && m_renderer->logicalLeftForFloat(r) < m_offset) {
-        m_offset = m_renderer->logicalLeftForFloat(r);
-        if (m_heightRemaining)
-            *m_heightRemaining = m_renderer->logicalBottomForFloat(r) - m_lowValue;
-    }
+    bool floatIsNewExtreme = updateOffsetIfNeeded(floatingObject);
+    if (floatIsNewExtreme && m_heightRemaining)
+        *m_heightRemaining = m_renderer->logicalBottomForFloat(floatingObject) - m_lowValue;
 
 #if ENABLE(CSS_SHAPES)
-    m_last = r;
+    m_last = floatingObject;
 #endif
 }
 
index cd441a1..242cce4 100644 (file)
@@ -1235,6 +1235,8 @@ protected:
 #endif
 
     private:
+        bool updateOffsetIfNeeded(const FloatingObject*) const;
+
         const RenderBlock* m_renderer;
         int m_lowValue;
         int m_highValue;