Diff
Modified: trunk/Source/WebCore/ChangeLog (207680 => 207681)
--- trunk/Source/WebCore/ChangeLog 2016-10-21 17:08:19 UTC (rev 207680)
+++ trunk/Source/WebCore/ChangeLog 2016-10-21 17:10:31 UTC (rev 207681)
@@ -1,3 +1,51 @@
+2016-10-20 Brent Fulgham <bfulg...@apple.com>
+
+ [Win][Direct2D] Correct some memory leaks and other minor bugs
+ https://bugs.webkit.org/show_bug.cgi?id=163769
+
+ Reviewed by Alex Christensen.
+
+ Several D2D handles were being leaked.
+
+ Direct2D sometimes returns an infinite rect containing { -inf, -inf, FloatMax, FloatMax },
+ sometimes { -FloatMax, -FloatMax, inf, inf }, and various combinations thereof. This caused
+ most SVG drawing to decide no screen rect was contained in the "infinite rect" so nothing
+ would be drawn.
+
+ Tested by existing layout tests.
+
+ * platform/graphics/GraphicsContext.h:
+ * platform/graphics/win/FloatRectDirect2D.cpp:
+ (WebCore::isInfiniteRect): Recognize various infinite rects in Windows.
+ (WebCore::FloatRect::FloatRect): Convert a Windows infinite rect to the style
+ we use inside WebKit.
+ * platform/graphics/win/FontCascadeDirect2D.cpp:
+ (WebCore::FontCascade::drawGlyphs): Use cached brushes if possible.
+ * platform/graphics/win/GlyphPageTreeNodeDirect2D.cpp:
+ (WebCore::GlyphPage::fill): Don't terminate on this error case.
+ * platform/graphics/win/GradientDirect2D.cpp:
+ (WebCore::Gradient::generateGradient): Don't leak gradients.
+ * platform/graphics/win/GraphicsContextDirect2D.cpp:
+ (WebCore::GraphicsContextPlatformPrivate::brushWithColor): Added.
+ (WebCore::GraphicsContext::brushWithColor): Added.
+ (WebCore::GraphicsContextPlatformPrivate::concatCTM): Perform transform multiplication
+ in the right order (hint: it's not distributive).
+ (WebCore::GraphicsContext::drawWithShadow): Use convenience method.
+ (WebCore::GraphicsContext::fillRect): Ditto.
+ (WebCore::GraphicsContext::platformFillRoundedRect): Ditto.
+ (WebCore::GraphicsContext::clearRect): Ditto.
+ (WebCore::GraphicsContext::setPlatformStrokeColor): Ditto.
+ (WebCore::GraphicsContext::setPlatformFillColor): Ditto.
+ * platform/graphics/win/PathDirect2D.cpp:
+ (WebCore::Path::polygonPathFromPoints): No need to convert manually.
+ (WebCore::Path::~Path): Don't leak ID2D1Geometry entities.
+ (WebCore::Path::appendGeometry): Ditto.
+ (WebCore::Path::createGeometryWithFillMode): Ditto.
+ (WebCore::Path::Path): Ditto.
+ (WebCore::Path::operator=): Ditto.
+ (WebCore::Path::strokeBoundingRect): Provide an implementation.
+ (WebCore::Path::addRect): No need for manual casting here.
+
2016-10-21 Wenson Hsieh <wenson_hs...@apple.com>
Fix minor style issue in the signature of StaticRange::create
Modified: trunk/Source/WebCore/platform/graphics/GraphicsContext.h (207680 => 207681)
--- trunk/Source/WebCore/platform/graphics/GraphicsContext.h 2016-10-21 17:08:19 UTC (rev 207680)
+++ trunk/Source/WebCore/platform/graphics/GraphicsContext.h 2016-10-21 17:10:31 UTC (rev 207681)
@@ -42,6 +42,7 @@
interface ID2D1DCRenderTarget;
interface ID2D1RenderTarget;
interface ID2D1Factory;
+interface ID2D1SolidColorBrush;
typedef ID2D1RenderTarget PlatformGraphicsContext;
#elif USE(CAIRO)
namespace WebCore {
@@ -559,6 +560,8 @@
ID2D1Brush* solidFillBrush() const;
ID2D1Brush* patternStrokeBrush() const;
ID2D1Brush* patternFillBrush() const;
+
+ ID2D1SolidColorBrush* brushWithColor(const Color&);
#endif
#else // PLATFORM(WIN)
bool shouldIncludeChildWindows() const { return false; }
Modified: trunk/Source/WebCore/platform/graphics/win/FloatRectDirect2D.cpp (207680 => 207681)
--- trunk/Source/WebCore/platform/graphics/win/FloatRectDirect2D.cpp 2016-10-21 17:08:19 UTC (rev 207680)
+++ trunk/Source/WebCore/platform/graphics/win/FloatRectDirect2D.cpp 2016-10-21 17:10:31 UTC (rev 207681)
@@ -33,10 +33,33 @@
namespace WebCore {
+static bool isInfiniteRect(const D2D1_RECT_F& rect)
+{
+ if (!std::isinf(rect.top) && rect.top != -std::numeric_limits<float>::max())
+ return false;
+
+ if (!std::isinf(rect.left) && rect.left != -std::numeric_limits<float>::max())
+ return false;
+
+ if (!std::isinf(rect.bottom) && rect.bottom != std::numeric_limits<float>::max())
+ return false;
+
+ if (!std::isinf(rect.right) && rect.right != std::numeric_limits<float>::max())
+ return false;
+
+ return true;
+}
+
FloatRect::FloatRect(const D2D1_RECT_F& r)
- : m_location(FloatPoint(r.left, r.top))
- , m_size(FloatSize(r.right - r.left, r.bottom - r.top))
{
+ // Infinite Rect case:
+ if (isInfiniteRect(r)) {
+ *this = infiniteRect();
+ return;
+ }
+
+ m_location = FloatPoint(r.left, r.top);
+ m_size = FloatSize(r.right - r.left, r.bottom - r.top);
}
FloatRect::operator D2D1_RECT_F() const
Modified: trunk/Source/WebCore/platform/graphics/win/FontCascadeDirect2D.cpp (207680 => 207681)
--- trunk/Source/WebCore/platform/graphics/win/FontCascadeDirect2D.cpp 2016-10-21 17:08:19 UTC (rev 207680)
+++ trunk/Source/WebCore/platform/graphics/win/FontCascadeDirect2D.cpp 2016-10-21 17:10:31 UTC (rev 207681)
@@ -129,13 +129,10 @@
// If shadows are ignoring transforms, then we haven't applied the Y coordinate flip yet, so down is negative.
float shadowTextY = point.y() + translation.height() + shadowOffset.height() * (graphicsContext.shadowsIgnoreTransforms() ? -1 : 1);
- COMPtr<ID2D1SolidColorBrush> shadowBrush;
- if (!SUCCEEDED(context->CreateSolidColorBrush(graphicsContext.colorWithGlobalAlpha(shadowFillColor), &shadowBrush)))
- return;
-
- context->DrawGlyphRun(D2D1::Point2F(shadowTextX, shadowTextY), &glyphRun, shadowBrush.get());
+ auto shadowBrush = graphicsContext.brushWithColor(shadowFillColor);
+ context->DrawGlyphRun(D2D1::Point2F(shadowTextX, shadowTextY), &glyphRun, shadowBrush);
if (font.syntheticBoldOffset())
- context->DrawGlyphRun(D2D1::Point2F(shadowTextX + font.syntheticBoldOffset(), shadowTextY), &glyphRun, shadowBrush.get());
+ context->DrawGlyphRun(D2D1::Point2F(shadowTextX + font.syntheticBoldOffset(), shadowTextY), &glyphRun, shadowBrush);
}
context->DrawGlyphRun(D2D1::Point2F(point.x() + translation.width(), point.y() + translation.height()), &glyphRun, graphicsContext.solidFillBrush());
Modified: trunk/Source/WebCore/platform/graphics/win/GlyphPageTreeNodeDirect2D.cpp (207680 => 207681)
--- trunk/Source/WebCore/platform/graphics/win/GlyphPageTreeNodeDirect2D.cpp 2016-10-21 17:08:19 UTC (rev 207680)
+++ trunk/Source/WebCore/platform/graphics/win/GlyphPageTreeNodeDirect2D.cpp 2016-10-21 17:10:31 UTC (rev 207681)
@@ -67,7 +67,8 @@
hr = analyzer->GetGlyphs(buffer, bufferLength, fontPlatformData.dwFontFace(), fontPlatformData.orientation() == Vertical, false,
&helper.m_analysis, nullptr, nullptr, nullptr, nullptr, 0, GlyphPage::size, clusterMap, textProperties.data(),
localGlyphBuffer, glyphProperties.data(), &returnedCount);
- RELEASE_ASSERT(SUCCEEDED(hr));
+ if (!SUCCEEDED(hr))
+ return false;
for (unsigned i = 0; i < GlyphPage::size; i++) {
Glyph glyph = localGlyphBuffer[i];
Modified: trunk/Source/WebCore/platform/graphics/win/GradientDirect2D.cpp (207680 => 207681)
--- trunk/Source/WebCore/platform/graphics/win/GradientDirect2D.cpp 2016-10-21 17:08:19 UTC (rev 207680)
+++ trunk/Source/WebCore/platform/graphics/win/GradientDirect2D.cpp 2016-10-21 17:10:31 UTC (rev 207681)
@@ -66,6 +66,11 @@
HRESULT hr = renderTarget->CreateGradientStopCollection(gradientStops.data(), gradientStops.size(), &gradientStopCollection);
RELEASE_ASSERT(SUCCEEDED(hr));
+ if (m_gradient) {
+ m_gradient->Release();
+ m_gradient = nullptr;
+ }
+
if (m_radial) {
FloatSize offset = p1() - p0();
ID2D1RadialGradientBrush* radialGradient = nullptr;
Modified: trunk/Source/WebCore/platform/graphics/win/GraphicsContextDirect2D.cpp (207680 => 207681)
--- trunk/Source/WebCore/platform/graphics/win/GraphicsContextDirect2D.cpp 2016-10-21 17:08:19 UTC (rev 207680)
+++ trunk/Source/WebCore/platform/graphics/win/GraphicsContextDirect2D.cpp 2016-10-21 17:10:31 UTC (rev 207681)
@@ -274,7 +274,7 @@
}
auto existingBrush = m_solidColoredBrushCache.ensure(colorKey, [this, color] {
- ID2D1SolidColorBrush* colorBrush = nullptr;
+ COMPtr<ID2D1SolidColorBrush> colorBrush;
m_renderTarget->CreateSolidColorBrush(color, &colorBrush);
return colorBrush;
});
@@ -282,6 +282,11 @@
return existingBrush.iterator->value;
}
+ID2D1SolidColorBrush* GraphicsContext::brushWithColor(const Color& color)
+{
+ return m_data->brushWithColor(colorWithGlobalAlpha(color)).get();
+}
+
void GraphicsContextPlatformPrivate::clip(const FloatRect& rect)
{
if (m_renderStates.isEmpty())
@@ -320,7 +325,8 @@
D2D1_MATRIX_3X2_F currentTransform;
m_renderTarget->GetTransform(¤tTransform);
- m_renderTarget->SetTransform(affineTransform * AffineTransform(currentTransform));
+ D2D1_MATRIX_3X2_F transformToConcat = affineTransform;
+ m_renderTarget->SetTransform(transformToConcat * currentTransform);
}
void GraphicsContextPlatformPrivate::flush()
@@ -882,7 +888,8 @@
deviceContext->BeginDraw();
deviceContext->DrawImage(compositor.get(), D2D1_INTERPOLATION_MODE_LINEAR);
- deviceContext->EndDraw();
+ hr = deviceContext->EndDraw();
+ ASSERT(SUCCEEDED(hr));
}
void GraphicsContext::fillPath(const Path& path)
@@ -1051,7 +1058,7 @@
drawWithoutShadow(rect, [this, rect, color](ID2D1RenderTarget* renderTarget) {
const D2D1_RECT_F d2dRect = rect;
- renderTarget->FillRectangle(&d2dRect, m_data->brushWithColor(colorWithGlobalAlpha(color)).get());
+ renderTarget->FillRectangle(&d2dRect, brushWithColor(color));
});
}
@@ -1085,8 +1092,7 @@
bool hasCustomFill = m_state.fillGradient || m_state.fillPattern;
if (!hasCustomFill && equalWidths && equalHeights && radii.topLeft().width() * 2 == r.width() && radii.topLeft().height() * 2 == r.height()) {
auto roundedRect = D2D1::RoundedRect(r, radii.topLeft().width(), radii.topLeft().height());
- COMPtr<ID2D1SolidColorBrush> fillBrush = m_data->brushWithColor(colorWithGlobalAlpha(color));
- context->FillRoundedRectangle(roundedRect, fillBrush.get());
+ context->FillRoundedRectangle(roundedRect, brushWithColor(color));
} else {
D2DContextStateSaver stateSaver(*m_data);
setFillColor(color);
@@ -1345,7 +1351,7 @@
renderTarget->SetTags(1, __LINE__);
rectToClear.intersect(renderTargetRect);
- renderTarget->FillRectangle(rectToClear, m_data->brushWithColor(colorWithGlobalAlpha(fillColor())).get());
+ renderTarget->FillRectangle(rectToClear, solidFillBrush());
});
}
@@ -1668,7 +1674,7 @@
m_data->m_solidStrokeBrush = nullptr;
- m_data->m_solidStrokeBrush = m_data->brushWithColor(colorWithGlobalAlpha(strokeColor()));
+ m_data->m_solidStrokeBrush = brushWithColor(strokeColor());
}
void GraphicsContext::setPlatformStrokeThickness(float thickness)
@@ -1683,7 +1689,7 @@
m_data->m_solidFillBrush = nullptr;
- m_data->m_solidFillBrush = m_data->brushWithColor(colorWithGlobalAlpha(fillColor()));
+ m_data->m_solidFillBrush = brushWithColor(fillColor());
}
void GraphicsContext::setPlatformShouldAntialias(bool enable)
Modified: trunk/Source/WebCore/platform/graphics/win/PathDirect2D.cpp (207680 => 207681)
--- trunk/Source/WebCore/platform/graphics/win/PathDirect2D.cpp 2016-10-21 17:08:19 UTC (rev 207680)
+++ trunk/Source/WebCore/platform/graphics/win/PathDirect2D.cpp 2016-10-21 17:10:31 UTC (rev 207681)
@@ -52,7 +52,7 @@
Vector<D2D1_POINT_2F, 32> d2dPoints;
d2dPoints.reserveInitialCapacity(points.size() - 1);
for (auto point : points)
- d2dPoints.uncheckedAppend(D2D1::Point2F(point.x(), point.y()));
+ d2dPoints.uncheckedAppend(point);
path.moveTo(points.first());
@@ -70,6 +70,8 @@
Path::~Path()
{
+ if (m_path)
+ m_path->Release();
}
PlatformPathPtr Path::ensurePlatformPath()
@@ -89,18 +91,24 @@
unsigned geometryCount = m_path ? m_path->GetSourceGeometryCount() : 0;
Vector<ID2D1Geometry*> geometries(geometryCount, nullptr);
+ // Note: 'GetSourceGeometries' returns geometries that have a +1 ref count.
+ // so they must be released before we return.
if (geometryCount)
m_path->GetSourceGeometries(geometries.data(), geometryCount);
+ geometry->AddRef();
geometries.append(geometry);
auto fillMode = m_path ? m_path->GetFillMode() : D2D1_FILL_MODE_WINDING;
- COMPtr<ID2D1GeometryGroup> protectedPath = m_path;
+ COMPtr<ID2D1GeometryGroup> protectedPath = adoptCOM(m_path);
m_path = nullptr;
HRESULT hr = GraphicsContext::systemFactory()->CreateGeometryGroup(fillMode, geometries.data(), geometries.size(), &m_path);
RELEASE_ASSERT(SUCCEEDED(hr));
+
+ for (auto entry : geometries)
+ entry->Release();
}
void Path::createGeometryWithFillMode(WindRule webkitFillMode, COMPtr<ID2D1GeometryGroup>& path) const
@@ -118,10 +126,16 @@
Vector<ID2D1Geometry*> geometries(geometryCount, nullptr);
ASSERT(geometryCount);
+
+ // Note: 'GetSourceGeometries' returns geometries that have a +1 ref count.
+ // so they must be released before we return.
m_path->GetSourceGeometries(geometries.data(), geometryCount);
HRESULT hr = GraphicsContext::systemFactory()->CreateGeometryGroup(fillMode, geometries.data(), geometries.size(), &path);
RELEASE_ASSERT(SUCCEEDED(hr));
+
+ for (auto entry : geometries)
+ entry->Release();
}
Path::Path(const Path& other)
@@ -133,15 +147,24 @@
Vector<ID2D1Geometry*> geometries(geometryCount, nullptr);
ASSERT(geometryCount);
+
+ // Note: 'GetSourceGeometries' returns geometries that have a +1 ref count.
+ // so they must be released before we return.
otherPath->GetSourceGeometries(geometries.data(), geometryCount);
HRESULT hr = GraphicsContext::systemFactory()->CreateGeometryGroup(other.m_path->GetFillMode(), geometries.data(), geometryCount, &m_path);
RELEASE_ASSERT(SUCCEEDED(hr));
+
+ for (auto entry : geometries)
+ entry->Release();
}
}
Path& Path::operator=(const Path& other)
{
+ if (m_path)
+ m_path->Release();
+
m_path = other.m_path;
if (m_path)
m_path->AddRef();
@@ -264,7 +287,7 @@
if (!SUCCEEDED(m_path->GetBounds(nullptr, &bounds)))
return FloatRect();
- return FloatRect(bounds.left, bounds.bottom, bounds.right - bounds.left, bounds.top - bounds.bottom);
+ return bounds;
}
FloatRect Path::fastBoundingRect() const
@@ -276,7 +299,7 @@
if (!SUCCEEDED(m_path->GetBounds(nullptr, &bounds)))
return FloatRect();
- return FloatRect(bounds.left, bounds.bottom, bounds.right - bounds.left, bounds.top - bounds.bottom);
+ return bounds;
}
FloatRect Path::strokeBoundingRect(StrokeStyleApplier* applier) const
@@ -284,9 +307,14 @@
if (isNull())
return FloatRect();
+ if (!applier)
+ return boundingRect();
+
UNUSED_PARAM(applier);
notImplemented();
- return FloatRect();
+
+ // Just return regular bounding rect for now.
+ return boundingRect();
}
void Path::moveTo(const FloatPoint& point)
@@ -380,8 +408,17 @@
m_activePath->EndFigure(D2D1_FIGURE_END_OPEN);
}
-static void drawArcSection(ID2D1GeometrySink* sink, FloatPoint center, float radius, float startAngle, float endAngle, bool clockwise)
+static FloatPoint arcStart(const FloatPoint& center, float radius, float startAngle)
{
+ FloatPoint startingPoint = center;
+ float startX = radius * std::cos(startAngle);
+ float startY = radius * std::sin(startAngle);
+ startingPoint.move(startX, startY);
+ return startingPoint;
+}
+
+static void drawArcSection(ID2D1GeometrySink* sink, const FloatPoint& center, float radius, float startAngle, float endAngle, bool clockwise)
+{
// Direct2D wants us to specify the end point of the arc, not the center. It will be drawn from
// whatever the current point in the 'm_activePath' is.
FloatPoint p = center;
@@ -396,13 +433,13 @@
void Path::addArc(const FloatPoint& center, float radius, float startAngle, float endAngle, bool clockwise)
{
- if (!m_activePath) {
- float startX = radius * std::cos(startAngle);
- float startY = radius * std::sin(startAngle);
-
- FloatPoint start = center;
- start.move(startX, startY);
- moveTo(start);
+ auto arcStartPoint = arcStart(center, radius, startAngle);
+ if (!m_activePath)
+ moveTo(arcStartPoint);
+ else if (!areEssentiallyEqual(currentPoint(), arcStartPoint)) {
+ // If the arc defined by the center and radius does not intersect the current position,
+ // we need to draw a line from the current position to the starting point of the arc.
+ addLineTo(arcStartPoint);
}
// Direct2D has problems drawing large arcs. It gets confused if drawing a complete (or
@@ -423,7 +460,7 @@
void Path::addRect(const FloatRect& r)
{
COMPtr<ID2D1RectangleGeometry> rectangle;
- HRESULT hr = GraphicsContext::systemFactory()->CreateRectangleGeometry(static_cast<D2D1_RECT_F>(r), &rectangle);
+ HRESULT hr = GraphicsContext::systemFactory()->CreateRectangleGeometry(r, &rectangle);
RELEASE_ASSERT(SUCCEEDED(hr));
appendGeometry(rectangle.get());
}
