Title: [154494] trunk/Source/WebCore
- Revision
- 154494
- Author
- [email protected]
- Date
- 2013-08-23 09:58:18 -0700 (Fri, 23 Aug 2013)
Log Message
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:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (154493 => 154494)
--- trunk/Source/WebCore/ChangeLog 2013-08-23 16:56:31 UTC (rev 154493)
+++ trunk/Source/WebCore/ChangeLog 2013-08-23 16:58:18 UTC (rev 154494)
@@ -1,3 +1,50 @@
+2013-08-23 Bem Jones-Bey <[email protected]>
+
+ 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 <[email protected]>
WebCore fails to link due to changes in Objective-C++ ABI in trunk clang
Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (154493 => 154494)
--- trunk/Source/WebCore/rendering/RenderBlock.cpp 2013-08-23 16:56:31 UTC (rev 154493)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp 2013-08-23 16:58:18 UTC (rev 154494)
@@ -4422,7 +4422,7 @@
}
}
-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 @@
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;
- }
+ bool floatIsNewExtreme = updateOffsetIfNeeded(floatingObject);
+ if (floatIsNewExtreme && m_heightRemaining)
+ *m_heightRemaining = m_renderer->logicalBottomForFloat(floatingObject) - 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;
- }
-
#if ENABLE(CSS_SHAPES)
- m_last = r;
+ m_last = floatingObject;
#endif
}
Modified: trunk/Source/WebCore/rendering/RenderBlock.h (154493 => 154494)
--- trunk/Source/WebCore/rendering/RenderBlock.h 2013-08-23 16:56:31 UTC (rev 154493)
+++ trunk/Source/WebCore/rendering/RenderBlock.h 2013-08-23 16:58:18 UTC (rev 154494)
@@ -1235,6 +1235,8 @@
#endif
private:
+ bool updateOffsetIfNeeded(const FloatingObject*) const;
+
const RenderBlock* m_renderer;
int m_lowValue;
int m_highValue;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes