Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(323)

Unified Diff: third_party/WebKit/Source/core/dom/IntersectionObservation.cpp

Issue 1826323002: IntersectionObserver: use edge-inclusive geometry for intersections. (Closed) Base URL: https://chromium.googlesource.com/chromium/src@intersection-observer-inclusive-geometry
Patch Set: refactor threshold index calculating code, fix test Created 4 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/core/dom/IntersectionObservation.cpp
diff --git a/third_party/WebKit/Source/core/dom/IntersectionObservation.cpp b/third_party/WebKit/Source/core/dom/IntersectionObservation.cpp
index 8a35d27e6dcdd306ff4eb6c895ac4dc6d1438b10..621234815b3a11faae5237197a39c1bb9ac7fe3c 100644
--- a/third_party/WebKit/Source/core/dom/IntersectionObservation.cpp
+++ b/third_party/WebKit/Source/core/dom/IntersectionObservation.cpp
@@ -34,19 +34,20 @@ void IntersectionObservation::applyRootMargin(LayoutRect& rect) const
m_observer->applyRootMargin(rect);
}
+void IntersectionObservation::initializeGeometry(IntersectionGeometry& geometry) const
+{
+ initializeTargetRect(geometry.targetRect);
+ geometry.intersectionRect = geometry.targetRect;
+ initializeRootRect(geometry.rootRect);
+ geometry.doesIntersect = true;
+}
+
void IntersectionObservation::initializeTargetRect(LayoutRect& rect) const
{
ASSERT(m_target);
LayoutObject* targetLayoutObject = target()->layoutObject();
ASSERT(targetLayoutObject && targetLayoutObject->isBoxModelObject());
rect = toLayoutBoxModelObject(targetLayoutObject)->visualOverflowRect();
-
- // TODO(szager): Properly support intersection observations for zero-area targets
- // by using edge-inclusive geometry.
- if (!rect.size().width())
- rect.setWidth(LayoutUnit(1));
- if (!rect.size().height())
- rect.setHeight(LayoutUnit(1));
}
void IntersectionObservation::initializeRootRect(LayoutRect& rect) const
@@ -62,7 +63,7 @@ void IntersectionObservation::initializeRootRect(LayoutRect& rect) const
applyRootMargin(rect);
}
-void IntersectionObservation::clipToRoot(LayoutRect& rect, const LayoutRect& rootRect) const
+void IntersectionObservation::clipToRoot(IntersectionGeometry& geometry) const
{
// Map and clip rect into root element coordinates.
// TODO(szager): the writing mode flipping needs a test.
@@ -70,10 +71,12 @@ void IntersectionObservation::clipToRoot(LayoutRect& rect, const LayoutRect& roo
LayoutObject* rootLayoutObject = m_observer->rootLayoutObject();
LayoutObject* targetLayoutObject = target()->layoutObject();
- targetLayoutObject->mapToVisibleRectInAncestorSpace(toLayoutBoxModelObject(rootLayoutObject), rect, nullptr);
- LayoutRect rootClipRect(rootRect);
+ geometry.doesIntersect = targetLayoutObject->mapToVisibleRectInAncestorSpace(toLayoutBoxModelObject(rootLayoutObject), geometry.intersectionRect, nullptr, EdgeInclusive);
+ if (!geometry.doesIntersect)
+ return;
+ LayoutRect rootClipRect(geometry.rootRect);
toLayoutBox(rootLayoutObject)->flipForWritingMode(rootClipRect);
- rect.intersect(rootClipRect);
+ geometry.doesIntersect &= geometry.intersectionRect.inclusiveIntersect(rootClipRect);
}
static void mapRectUpToDocument(LayoutRect& rect, const LayoutObject& layoutObject, const Document& document)
@@ -164,22 +167,22 @@ bool IntersectionObservation::computeGeometry(IntersectionGeometry& geometry) co
if (!isContainingBlockChainDescendant(targetLayoutObject, rootLayoutObject))
return false;
- initializeTargetRect(geometry.targetRect);
- geometry.intersectionRect = geometry.targetRect;
- initializeRootRect(geometry.rootRect);
+ initializeGeometry(geometry);
- clipToRoot(geometry.intersectionRect, geometry.rootRect);
+ clipToRoot(geometry);
- // TODO(szager): there are some simple optimizations that can be done here:
- // - Don't transform rootRect if it's not going to be reported
- // - Don't transform intersectionRect if it's empty
mapTargetRectToTargetFrameCoordinates(geometry.targetRect);
- mapRootRectToTargetFrameCoordinates(geometry.intersectionRect);
- mapRootRectToRootFrameCoordinates(geometry.rootRect);
- if (geometry.intersectionRect.size().isZero())
+ if (geometry.doesIntersect)
+ mapRootRectToTargetFrameCoordinates(geometry.intersectionRect);
+ else
geometry.intersectionRect = LayoutRect();
+ if (m_shouldReportRootBounds)
+ mapRootRectToRootFrameCoordinates(geometry.rootRect);
+ else
+ geometry.rootRect = LayoutRect();
chrishtr 2016/03/24 23:35:29 Might as well kill this else clause.
szager1 2016/03/25 08:02:39 Done.
+
return true;
}
@@ -189,15 +192,30 @@ void IntersectionObservation::computeIntersectionObservations(DOMHighResTimeStam
if (!computeGeometry(geometry))
return;
- float intersectionArea = geometry.intersectionRect.size().width().toFloat() * geometry.intersectionRect.size().height().toFloat();
- float targetArea = geometry.targetRect.size().width().toFloat() * geometry.targetRect.size().height().toFloat();
- if (!targetArea)
- return;
- float newVisibleRatio = intersectionArea / targetArea;
- unsigned newThresholdIndex = observer().firstThresholdGreaterThan(newVisibleRatio);
- IntRect snappedRootBounds = pixelSnappedIntRect(geometry.rootRect);
- IntRect* rootBoundsPointer = m_shouldReportRootBounds ? &snappedRootBounds : nullptr;
szager1 2016/03/24 23:33:09 Here.
chrishtr 2016/03/24 23:35:29 ah
+ // Some corner cases for threshold index:
+ // - If target rect is zero area, only two states are recognized:
chrishtr 2016/03/24 23:33:11 Good to mention that zero-area means either width
szager1 2016/03/25 08:02:39 Done.
+ // - 0 means not intersecting.
+ // - 1 means intersecting.
chrishtr 2016/03/24 23:33:11 Change to an enum.
szager1 2016/03/25 08:02:38 OK, although it seems weird to use an enum when, i
+ // No other threshold crossings are possible.
+ // - Otherwise:
+ // - If root and target do not intersect, the threshold index is 0.
+ // - If root and target intersect but the intersection has zero-area (i.e., they
+ // have a coincident edge or corner), we consider the intersection to have
+ // "crossed" a zero threshold, but not crossed any non-zero threshold.
+ unsigned newThresholdIndex;
+ if (!geometry.doesIntersect) {
+ newThresholdIndex = 0;
+ } else if (geometry.targetRect.isEmpty()) {
+ newThresholdIndex = 1;
+ } else {
+ float intersectionArea = geometry.intersectionRect.size().width().toFloat() * geometry.intersectionRect.size().height().toFloat();
+ float targetArea = geometry.targetRect.size().width().toFloat() * geometry.targetRect.size().height().toFloat();
+ float newVisibleRatio = intersectionArea / targetArea;
+ newThresholdIndex = observer().firstThresholdGreaterThan(newVisibleRatio);
+ }
if (m_lastThresholdIndex != newThresholdIndex) {
+ IntRect snappedRootBounds = pixelSnappedIntRect(geometry.rootRect);
+ IntRect* rootBoundsPointer = m_shouldReportRootBounds ? &snappedRootBounds : nullptr;
IntersectionObserverEntry* newEntry = new IntersectionObserverEntry(
timestamp,
pixelSnappedIntRect(geometry.targetRect),

Powered by Google App Engine
This is Rietveld 408576698