- Revision
- 268320
- Author
- wenson_hs...@apple.com
- Date
- 2020-10-11 09:33:34 -0700 (Sun, 11 Oct 2020)
Log Message
[MotionMark] Computing the fast bounding rect of an arc should not materialize a CGPathRef
https://bugs.webkit.org/show_bug.cgi?id=217563
Reviewed by Darin Adler.
Avoid hitting the slow case when asking for the fast bounding rect of a `Path`, in the case where the path
consists of a single circular arc. This fast bounding rect is intended to be a conservative way to estimate the
bounds of the path, such that the actual bounds of the path must lie within the bounds of this fast rect. At the
cost of being less accurate, we can make this fast computation much cheaper in the case of a circular arc by
simply returning the bounding rect of the circle containing the arc.
* platform/graphics/InlinePathData.h:
(WebCore::ArcData::encode const):
(WebCore::ArcData::decode):
Additionally rename what is currently `ArcData`'s `offset` member to `start` instead, and `hasOffset` to
`hasStart` for clarity. This point optionally provides the starting point of a line segment that is connected to
the circular arc defined by the center point, radius, and start and end angles. Note that this starting point
needs to be included in the fast bounding rect computation for this reason (see above).
* platform/graphics/Path.cpp:
(WebCore::Path::addArc):
(WebCore::Path::fastBoundingRect const):
(WebCore::Path::fastBoundingRectFromInlineData const):
(WebCore::Path::boundingRectFromInlineData const):
* platform/graphics/Path.h:
* platform/graphics/cg/PathCG.cpp:
(WebCore::Path::createCGPath const):
* platform/graphics/displaylists/DisplayListItems.cpp:
(WebCore::DisplayList::StrokePath::localBounds const):
Also, use the fast bounding rect when estimating `localBounds` of the `StrokePath` item; these local bounds are
only used in an optimization that skips display list items during playback, based on the extent of the item, so
it's safe to use extents that are potentially inflated. Using the fast bounding rect also helps in the case of
the more complex paths encountered in the Canvas Paths subtest of MotionMark, where each Path contains multiple
components and cannot be expressed solely with inline path data.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (268319 => 268320)
--- trunk/Source/WebCore/ChangeLog 2020-10-11 16:12:38 UTC (rev 268319)
+++ trunk/Source/WebCore/ChangeLog 2020-10-11 16:33:34 UTC (rev 268320)
@@ -1,3 +1,42 @@
+2020-10-11 Wenson Hsieh <wenson_hs...@apple.com>
+
+ [MotionMark] Computing the fast bounding rect of an arc should not materialize a CGPathRef
+ https://bugs.webkit.org/show_bug.cgi?id=217563
+
+ Reviewed by Darin Adler.
+
+ Avoid hitting the slow case when asking for the fast bounding rect of a `Path`, in the case where the path
+ consists of a single circular arc. This fast bounding rect is intended to be a conservative way to estimate the
+ bounds of the path, such that the actual bounds of the path must lie within the bounds of this fast rect. At the
+ cost of being less accurate, we can make this fast computation much cheaper in the case of a circular arc by
+ simply returning the bounding rect of the circle containing the arc.
+
+ * platform/graphics/InlinePathData.h:
+ (WebCore::ArcData::encode const):
+ (WebCore::ArcData::decode):
+
+ Additionally rename what is currently `ArcData`'s `offset` member to `start` instead, and `hasOffset` to
+ `hasStart` for clarity. This point optionally provides the starting point of a line segment that is connected to
+ the circular arc defined by the center point, radius, and start and end angles. Note that this starting point
+ needs to be included in the fast bounding rect computation for this reason (see above).
+
+ * platform/graphics/Path.cpp:
+ (WebCore::Path::addArc):
+ (WebCore::Path::fastBoundingRect const):
+ (WebCore::Path::fastBoundingRectFromInlineData const):
+ (WebCore::Path::boundingRectFromInlineData const):
+ * platform/graphics/Path.h:
+ * platform/graphics/cg/PathCG.cpp:
+ (WebCore::Path::createCGPath const):
+ * platform/graphics/displaylists/DisplayListItems.cpp:
+ (WebCore::DisplayList::StrokePath::localBounds const):
+
+ Also, use the fast bounding rect when estimating `localBounds` of the `StrokePath` item; these local bounds are
+ only used in an optimization that skips display list items during playback, based on the extent of the item, so
+ it's safe to use extents that are potentially inflated. Using the fast bounding rect also helps in the case of
+ the more complex paths encountered in the Canvas Paths subtest of MotionMark, where each Path contains multiple
+ components and cannot be expressed solely with inline path data.
+
2020-10-09 Yusuke Suzuki <ysuz...@apple.com>
[JSC] Assert Operation and HostFunction are in JITOperationsList
Modified: trunk/Source/WebCore/platform/graphics/InlinePathData.h (268319 => 268320)
--- trunk/Source/WebCore/platform/graphics/InlinePathData.h 2020-10-11 16:12:38 UTC (rev 268319)
+++ trunk/Source/WebCore/platform/graphics/InlinePathData.h 2020-10-11 16:33:34 UTC (rev 268320)
@@ -42,13 +42,13 @@
};
struct ArcData {
- FloatPoint offset;
+ FloatPoint start;
FloatPoint center;
float radius { 0 };
float startAngle { 0 };
float endAngle { 0 };
bool clockwise { false };
- bool hasOffset { false };
+ bool hasStart { false };
template<class Encoder> void encode(Encoder&) const;
template<class Decoder> static Optional<ArcData> decode(Decoder&);
@@ -113,19 +113,19 @@
template<class Encoder> void ArcData::encode(Encoder& encoder) const
{
- encoder << offset;
+ encoder << start;
encoder << center;
encoder << radius;
encoder << startAngle;
encoder << endAngle;
encoder << clockwise;
- encoder << hasOffset;
+ encoder << hasStart;
}
template<class Decoder> Optional<ArcData> ArcData::decode(Decoder& decoder)
{
ArcData data;
- if (!decoder.decode(data.offset))
+ if (!decoder.decode(data.start))
return WTF::nullopt;
if (!decoder.decode(data.center))
@@ -143,7 +143,7 @@
if (!decoder.decode(data.clockwise))
return WTF::nullopt;
- if (!decoder.decode(data.hasOffset))
+ if (!decoder.decode(data.hasStart))
return WTF::nullopt;
return data;
Modified: trunk/Source/WebCore/platform/graphics/Path.cpp (268319 => 268320)
--- trunk/Source/WebCore/platform/graphics/Path.cpp 2020-10-11 16:12:38 UTC (rev 268319)
+++ trunk/Source/WebCore/platform/graphics/Path.cpp 2020-10-11 16:33:34 UTC (rev 268320)
@@ -289,8 +289,8 @@
if (isNull() || hasInlineData<MoveData>()) {
ArcData arc;
if (hasAnyInlineData()) {
- arc.hasOffset = true;
- arc.offset = WTF::get<MoveData>(m_inlineData).location;
+ arc.hasStart = true;
+ arc.start = WTF::get<MoveData>(m_inlineData).location;
}
arc.center = point;
arc.radius = radius;
@@ -386,7 +386,7 @@
return { };
#if ENABLE(INLINE_PATH_DATA)
- if (auto rect = boundingRectFromInlineData())
+ if (auto rect = fastBoundingRectFromInlineData())
return *rect;
#endif
@@ -395,14 +395,31 @@
#if ENABLE(INLINE_PATH_DATA)
+Optional<FloatRect> Path::fastBoundingRectFromInlineData() const
+{
+ if (hasInlineData<ArcData>()) {
+ auto& arc = inlineData<ArcData>();
+ auto diameter = 2 * arc.radius;
+ FloatRect approximateBounds { arc.center, FloatSize(diameter, diameter) };
+ approximateBounds.move(-arc.radius, -arc.radius);
+ if (arc.hasStart)
+ approximateBounds.extend(arc.start);
+ return approximateBounds;
+ }
+
+ return boundingRectFromInlineData();
+}
+
Optional<FloatRect> Path::boundingRectFromInlineData() const
{
+ // FIXME: Add logic to compute the exact bounding rect for an arc in inline data.
+
if (hasInlineData<MoveData>())
return FloatRect { };
if (hasInlineData<LineData>()) {
FloatRect result;
- auto& line = WTF::get<LineData>(m_inlineData);
+ auto& line = inlineData<LineData>();
result.fitToPoints(line.start, line.end);
return result;
}
Modified: trunk/Source/WebCore/platform/graphics/Path.h (268319 => 268320)
--- trunk/Source/WebCore/platform/graphics/Path.h 2020-10-11 16:12:38 UTC (rev 268319)
+++ trunk/Source/WebCore/platform/graphics/Path.h 2020-10-11 16:33:34 UTC (rev 268320)
@@ -241,6 +241,7 @@
private:
#if ENABLE(INLINE_PATH_DATA)
bool hasAnyInlineData() const;
+ Optional<FloatRect> fastBoundingRectFromInlineData() const;
Optional<FloatRect> boundingRectFromInlineData() const;
#endif
Modified: trunk/Source/WebCore/platform/graphics/cg/PathCG.cpp (268319 => 268320)
--- trunk/Source/WebCore/platform/graphics/cg/PathCG.cpp 2020-10-11 16:12:38 UTC (rev 268319)
+++ trunk/Source/WebCore/platform/graphics/cg/PathCG.cpp 2020-10-11 16:33:34 UTC (rev 268320)
@@ -98,8 +98,8 @@
CGPathAddLineToPoint(m_path.get(), nullptr, line.end.x(), line.end.y());
},
[&](const ArcData& arc) {
- if (arc.hasOffset)
- CGPathMoveToPoint(m_path.get(), nullptr, arc.offset.x(), arc.offset.y());
+ if (arc.hasStart)
+ CGPathMoveToPoint(m_path.get(), nullptr, arc.start.x(), arc.start.y());
CGPathAddArc(m_path.get(), nullptr, arc.center.x(), arc.center.y(), arc.radius, arc.startAngle, arc.endAngle, arc.clockwise);
},
[&](const QuadCurveData& curve) {
Modified: trunk/Source/WebCore/platform/graphics/displaylists/DisplayListItems.cpp (268319 => 268320)
--- trunk/Source/WebCore/platform/graphics/displaylists/DisplayListItems.cpp 2020-10-11 16:12:38 UTC (rev 268319)
+++ trunk/Source/WebCore/platform/graphics/displaylists/DisplayListItems.cpp 2020-10-11 16:33:34 UTC (rev 268320)
@@ -1289,7 +1289,7 @@
// FIXME: Need to take stroke thickness into account correctly, via CGPathByStrokingPath().
float strokeThickness = context.strokeThickness();
- FloatRect bounds = m_path.boundingRect();
+ FloatRect bounds = m_path.fastBoundingRect();
bounds.expand(strokeThickness, strokeThickness);
return bounds;
}
