diff --git a/Frameworks/CoreGraphics/CGContext.mm b/Frameworks/CoreGraphics/CGContext.mm index 33699216dc..00a86e9254 100644 --- a/Frameworks/CoreGraphics/CGContext.mm +++ b/Frameworks/CoreGraphics/CGContext.mm @@ -156,8 +156,9 @@ inline HRESULT IntersectClippingGeometry(ID2D1Geometry* incomingGeometry, CGPath D2D1_FILL_MODE d2dFillMode = (pathMode & kCGPathEOFill) == kCGPathEOFill ? D2D1_FILL_MODE_ALTERNATE : D2D1_FILL_MODE_WINDING; if (!clippingGeometry) { - // If we don't have a clipping geometry, we are free to take this one wholesale (after EO/Winding conversion.) - return _CGConvertD2DGeometryToFillMode(incomingGeometry, d2dFillMode, &clippingGeometry); + // If we don't have a clipping geometry, we are free to take this one wholesale. + clippingGeometry = incomingGeometry; + return S_OK; } ComPtr factory; @@ -1053,7 +1054,7 @@ bool CGContextPathContainsPoint(CGContextRef context, CGPoint point, CGPathDrawi } ComPtr additionalClippingGeometry; - RETURN_IF_FAILED(_CGPathGetGeometry(Path(), &additionalClippingGeometry)); + RETURN_IF_FAILED(_CGPathGetGeometryWithFillMode(Path(), pathMode, &additionalClippingGeometry)); ClearPath(); auto& state = CurrentGState(); @@ -2383,12 +2384,7 @@ void CGContextClearRect(CGContextRef context, CGRect rect) { auto& state = context->CurrentGState(); if (drawMode & kCGPathFill) { state.fillBrush->SetOpacity(state.alpha); - - ComPtr geometryToFill; - D2D1_FILL_MODE d2dFillMode = (drawMode & kCGPathEOFill) == kCGPathEOFill ? D2D1_FILL_MODE_ALTERNATE : D2D1_FILL_MODE_WINDING; - RETURN_IF_FAILED(_CGConvertD2DGeometryToFillMode(geometry, d2dFillMode, &geometryToFill)); - - deviceContext->FillGeometry(geometryToFill.Get(), state.fillBrush.Get()); + deviceContext->FillGeometry(geometry, state.fillBrush.Get()); } if (drawMode & kCGPathStroke && std::fpclassify(state.lineWidth) != FP_ZERO) { @@ -2551,7 +2547,7 @@ void CGContextDrawPath(CGContextRef context, CGPathDrawingMode mode) { if (context->HasPath()) { ComPtr pGeometry; - FAIL_FAST_IF_FAILED(_CGPathGetGeometry(context->Path(), &pGeometry)); + FAIL_FAST_IF_FAILED(_CGPathGetGeometryWithFillMode(context->Path(), mode, &pGeometry)); FAIL_FAST_IF_FAILED(context->DrawGeometry(_kCGCoordinateModeDeviceSpace, pGeometry.Get(), mode)); context->ClearPath(); } diff --git a/Frameworks/CoreGraphics/CGPath.mm b/Frameworks/CoreGraphics/CGPath.mm index 1874870011..189da54a55 100644 --- a/Frameworks/CoreGraphics/CGPath.mm +++ b/Frameworks/CoreGraphics/CGPath.mm @@ -46,7 +46,7 @@ return m_geometrySink.Get(); } - _CGPathCustomSink(_In_ ID2D1GeometrySink* sink) : m_geometrySink(sink), m_lastPoint{0, 0}, m_isFigureOpen(false) { + _CGPathCustomSink(_In_ ID2D1GeometrySink* sink) : m_geometrySink(sink), m_lastPoint{ 0, 0 }, m_isFigureOpen(false) { } STDMETHOD_(void, SetFillMode)(D2D1_FILL_MODE fillMode) { @@ -271,11 +271,26 @@ HRESULT AddGeometryToPathWithTransformation(const ID2D1Geometry* geometry, const } }; -HRESULT _CGPathGetGeometry(CGPathRef path, ID2D1Geometry** pGeometry) { - RETURN_HR_IF_NULL(E_POINTER, pGeometry); +HRESULT _CGPathGetGeometryWithFillMode(CGPathRef path, CGPathDrawingMode fillMode, ID2D1Geometry** pNewGeometry) { + RETURN_HR_IF_NULL(E_POINTER, pNewGeometry); RETURN_HR_IF_NULL(E_POINTER, path); + RETURN_IF_FAILED(path->ClosePath()); - path->pathGeometry.CopyTo(pGeometry); + if (fillMode == kCGPathEOFill || fillMode == kCGPathEOFillStroke) { + ID2D1Geometry* geometry = path->GetPathGeometry(); + ComPtr factory; + geometry->GetFactory(&factory); + + ComPtr geometryGroup; + RETURN_IF_FAILED(factory->CreateGeometryGroup(D2D1_FILL_MODE_ALTERNATE, &geometry, 1, &geometryGroup)); + + ComPtr outGeometry; + RETURN_IF_FAILED(geometryGroup.As(&outGeometry)); + + *pNewGeometry = outGeometry.Detach(); + } else { + path->pathGeometry.CopyTo(pNewGeometry); + } return S_OK; } diff --git a/Frameworks/CoreGraphics/D2DWrapper.mm b/Frameworks/CoreGraphics/D2DWrapper.mm index e7c7940e54..026dd83671 100644 --- a/Frameworks/CoreGraphics/D2DWrapper.mm +++ b/Frameworks/CoreGraphics/D2DWrapper.mm @@ -31,19 +31,4 @@ HRESULT _CGGetWICFactory(IWICImagingFactory** factory) { static HRESULT sHr = CoCreateInstance(CLSID_WICImagingFactory, nullptr, CLSCTX_INPROC_SERVER, IID_PPV_ARGS(&sWicFactory)); sWicFactory.CopyTo(factory); RETURN_HR(sHr); -} - -// TODO GH#1375: Remove this when CGPath's fill mode has been worked out. -HRESULT _CGConvertD2DGeometryToFillMode(ID2D1Geometry* geometry, D2D1_FILL_MODE fillMode, ID2D1Geometry** pNewGeometry) { - ComPtr factory; - geometry->GetFactory(&factory); - - ComPtr geometryGroup; - RETURN_IF_FAILED(factory->CreateGeometryGroup(fillMode, &geometry, 1, &geometryGroup)); - - ComPtr outGeometry; - RETURN_IF_FAILED(geometryGroup.As(&outGeometry)); - - *pNewGeometry = outGeometry.Detach(); - return S_OK; } \ No newline at end of file diff --git a/Frameworks/include/CGPathInternal.h b/Frameworks/include/CGPathInternal.h index 31410b770e..e92e70de8f 100644 --- a/Frameworks/include/CGPathInternal.h +++ b/Frameworks/include/CGPathInternal.h @@ -68,8 +68,8 @@ struct CGPathElementInternal : CGPathElement { }; typedef struct CGPathElementInternal CGPathElementInternal; -HRESULT _CGPathGetGeometry(CGPathRef path, ID2D1Geometry** pGeometry); HRESULT _CGPathApplyInternal(ID2D1PathGeometry* pathGeometry, void* info, CGPathApplierFunction function); +HRESULT _CGPathGetGeometryWithFillMode(CGPathRef path, CGPathDrawingMode fillMode, ID2D1Geometry** pNewGeometry); #if defined __clang__ #pragma clang diagnostic pop diff --git a/Frameworks/include/CoreGraphics/D2DWrapper.h b/Frameworks/include/CoreGraphics/D2DWrapper.h index f4f61ed53e..11ff98697e 100644 --- a/Frameworks/include/CoreGraphics/D2DWrapper.h +++ b/Frameworks/include/CoreGraphics/D2DWrapper.h @@ -30,8 +30,6 @@ HRESULT _CGGetD2DFactory(ID2D1Factory** factory); HRESULT _CGGetWICFactory(IWICImagingFactory** factory); -HRESULT _CGConvertD2DGeometryToFillMode(ID2D1Geometry* geometry, D2D1_FILL_MODE fillMode, ID2D1Geometry** pNewGeometry); - inline D2D_POINT_2F _CGPointToD2D_F(CGPoint point) { return { point.x, point.y }; } diff --git a/build/Tests/UnitTests/CoreGraphics.Drawing/CoreGraphics.Drawing.UnitTests.vcxproj b/build/Tests/UnitTests/CoreGraphics.Drawing/CoreGraphics.Drawing.UnitTests.vcxproj index 9277523361..a98ac6a4ed 100644 --- a/build/Tests/UnitTests/CoreGraphics.Drawing/CoreGraphics.Drawing.UnitTests.vcxproj +++ b/build/Tests/UnitTests/CoreGraphics.Drawing/CoreGraphics.Drawing.UnitTests.vcxproj @@ -231,7 +231,6 @@ - @@ -246,6 +245,7 @@ + @@ -266,4 +266,4 @@ - + \ No newline at end of file diff --git a/tests/unittests/CoreGraphics.Drawing/CGPathDrawingTests.cpp b/tests/unittests/CoreGraphics.Drawing/CGPathDrawingTests.cpp index da43b33027..d6727e1de5 100644 --- a/tests/unittests/CoreGraphics.Drawing/CGPathDrawingTests.cpp +++ b/tests/unittests/CoreGraphics.Drawing/CGPathDrawingTests.cpp @@ -497,41 +497,5 @@ DRAW_TEST_F(CGPath, FillStraightLines, UIKitMimicTest<>) { CGContextFillPath(context); CGContextStrokePath(context); - CGPathRelease(thepath); -} - -DRAW_TEST_F(CGPath, FillModeCircles, UIKitMimicTest<>) { - CGContextRef context = GetDrawingContext(); - CGRect bounds = GetDrawingBounds(); - CGFloat width = bounds.size.width; - CGFloat height = bounds.size.height; - CGFloat xstart = bounds.origin.x; - CGFloat ystart = bounds.origin.y; - - CGMutablePathRef thepath = CGPathCreateMutable(); - - CGPathMoveToPoint(thepath, NULL, xstart + .5 * width + .4 * height, ystart + .5 * height); - CGPathAddArc(thepath, NULL, xstart + .5 * width, ystart + .5 * height, .4 * height, 0, M_PI, true); - CGPathAddArc(thepath, NULL, xstart + .5 * width, ystart + .5 * height, .4 * height, M_PI, 0, true); - - CGPathMoveToPoint(thepath, NULL, xstart + .5 * width + .3 * height, ystart + .5 * height); - CGPathAddArc(thepath, NULL, xstart + .5 * width, ystart + .5 * height, .3 * height, 0, M_PI, true); - CGPathAddArc(thepath, NULL, xstart + .5 * width, ystart + .5 * height, .3 * height, M_PI, 0, true); - - CGPathMoveToPoint(thepath, NULL, xstart + .5 * width + .2 * height, ystart + .5 * height); - CGPathAddArc(thepath, NULL, xstart + .5 * width, ystart + .5 * height, .2 * height, 0, M_PI, true); - CGPathAddArc(thepath, NULL, xstart + .5 * width, ystart + .5 * height, .2 * height, M_PI, 0, true); - - CGPathMoveToPoint(thepath, NULL, xstart + .5 * width + .1 * height, ystart + .5 * height); - CGPathAddArc(thepath, NULL, xstart + .5 * width, ystart + .5 * height, .1 * height, 0, M_PI, true); - CGPathAddArc(thepath, NULL, xstart + .5 * width, ystart + .5 * height, .1 * height, M_PI, 0, true); - - CGPathCloseSubpath(thepath); - - CGContextAddPath(context, thepath); - CGContextSetRGBFillColor(context, 0, 0, 1, 1); - CGContextEOFillPath(context); - CGContextStrokePath(context); - CGPathRelease(thepath); } \ No newline at end of file diff --git a/tests/unittests/CoreGraphics.drawing/CGContextDrawing_FillModeTests.cpp b/tests/unittests/CoreGraphics.drawing/CGContextDrawing_FillModeTests.cpp new file mode 100644 index 0000000000..1b73e44228 --- /dev/null +++ b/tests/unittests/CoreGraphics.drawing/CGContextDrawing_FillModeTests.cpp @@ -0,0 +1,108 @@ +//****************************************************************************** +// +// Copyright (c) Microsoft. All rights reserved. +// +// This code is licensed under the MIT License (MIT). +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. +// +//****************************************************************************** + +#include "DrawingTest.h" + +CGPathDrawingMode fillModes[] = { kCGPathFill, kCGPathFillStroke, kCGPathEOFill, kCGPathEOFillStroke }; + +class CGContextFillMode : public WhiteBackgroundTest<>, public ::testing::WithParamInterface { + CFStringRef CreateOutputFilename() { + CGPathDrawingMode fillMode = GetParam(); + + char* fillModeName; + switch (fillMode) { + case kCGPathFill: + fillModeName = "Fill"; + break; + case kCGPathFillStroke: + fillModeName = "FillStroke"; + break; + case kCGPathEOFill: + fillModeName = "EOFill"; + break; + case kCGPathEOFillStroke: + fillModeName = "EOFillStroke"; + break; + default: + break; + } + + return CFStringCreateWithFormat(nullptr, nullptr, CFSTR("TestImage.CGContext.FillMode.%s.png"), fillModeName); + } +}; + +DRAW_TEST_P(CGContextFillMode, OverlappedEllipses) { + CGContextRef context = GetDrawingContext(); + CGRect bounds = GetDrawingBounds(); + bounds = CGRectInset(bounds, 16.f, 16.f); + CGFloat width = bounds.size.width; + CGFloat height = bounds.size.height; + CGFloat xstart = bounds.origin.x; + CGFloat ystart = bounds.origin.y; + + CGPathDrawingMode fillMode = GetParam(); + + CGMutablePathRef leftCircles = CGPathCreateMutable(); + + CGPathMoveToPoint(leftCircles, NULL, xstart + .25 * width + .4 * height, ystart + .5 * height); + CGPathAddArc(leftCircles, NULL, xstart + .25 * width, ystart + .5 * height, .4 * height, 0, M_PI, true); + CGPathAddArc(leftCircles, NULL, xstart + .25 * width, ystart + .5 * height, .4 * height, M_PI, 0, true); + + CGPathMoveToPoint(leftCircles, NULL, xstart + .25 * width + .3 * height, ystart + .5 * height); + CGPathAddArc(leftCircles, NULL, xstart + .25 * width, ystart + .5 * height, .3 * height, 0, M_PI, true); + CGPathAddArc(leftCircles, NULL, xstart + .25 * width, ystart + .5 * height, .3 * height, M_PI, 0, true); + + CGPathMoveToPoint(leftCircles, NULL, xstart + .25 * width + .2 * height, ystart + .5 * height); + CGPathAddArc(leftCircles, NULL, xstart + .25 * width, ystart + .5 * height, .2 * height, 0, M_PI, true); + CGPathAddArc(leftCircles, NULL, xstart + .25 * width, ystart + .5 * height, .2 * height, M_PI, 0, true); + + CGPathMoveToPoint(leftCircles, NULL, xstart + .25 * width + .1 * height, ystart + .5 * height); + CGPathAddArc(leftCircles, NULL, xstart + .25 * width, ystart + .5 * height, .1 * height, 0, M_PI, true); + CGPathAddArc(leftCircles, NULL, xstart + .25 * width, ystart + .5 * height, .1 * height, M_PI, 0, true); + + CGPathCloseSubpath(leftCircles); + + CGMutablePathRef rightCircles = CGPathCreateMutable(); + + CGPathMoveToPoint(rightCircles, NULL, xstart + .75 * width + .4 * height, ystart + .5 * height); + CGPathAddArc(rightCircles, NULL, xstart + .75 * width, ystart + .5 * height, .4 * height, 0, M_PI, false); + CGPathAddArc(rightCircles, NULL, xstart + .75 * width, ystart + .5 * height, .4 * height, M_PI, 0, false); + + CGPathMoveToPoint(rightCircles, NULL, xstart + .75 * width + .3 * height, ystart + .5 * height); + CGPathAddArc(rightCircles, NULL, xstart + .75 * width, ystart + .5 * height, .3 * height, 0, M_PI, true); + CGPathAddArc(rightCircles, NULL, xstart + .75 * width, ystart + .5 * height, .3 * height, M_PI, 0, true); + + CGPathMoveToPoint(rightCircles, NULL, xstart + .75 * width + .2 * height, ystart + .5 * height); + CGPathAddArc(rightCircles, NULL, xstart + .75 * width, ystart + .5 * height, .2 * height, 0, M_PI, false); + CGPathAddArc(rightCircles, NULL, xstart + .75 * width, ystart + .5 * height, .2 * height, M_PI, 0, false); + + CGPathMoveToPoint(rightCircles, NULL, xstart + .75 * width + .1 * height, ystart + .5 * height); + CGPathAddArc(rightCircles, NULL, xstart + .75 * width, ystart + .5 * height, .1 * height, 0, M_PI, true); + CGPathAddArc(rightCircles, NULL, xstart + .75 * width, ystart + .5 * height, .1 * height, M_PI, 0, true); + + CGPathCloseSubpath(rightCircles); + + CGContextAddPath(context, leftCircles); + CGContextAddPath(context, rightCircles); + CGContextSetRGBFillColor(context, 0, 0, 1, 1); + CGContextSetRGBStrokeColor(context, 1, 0, 0, 1); + CGContextDrawPath(context, fillMode); + + CGPathRelease(leftCircles); + CGPathRelease(rightCircles); +} + +INSTANTIATE_TEST_CASE_P(FillModes, CGContextFillMode, ::testing::ValuesIn(fillModes)); \ No newline at end of file diff --git a/tests/unittests/CoreGraphics.drawing/data/reference/TestImage.CGContext.FillMode.EOFill.png b/tests/unittests/CoreGraphics.drawing/data/reference/TestImage.CGContext.FillMode.EOFill.png new file mode 100644 index 0000000000..787d3d9844 --- /dev/null +++ b/tests/unittests/CoreGraphics.drawing/data/reference/TestImage.CGContext.FillMode.EOFill.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:51e7d8d7f7191bc10b105bbfee3bb1848bbb83765cf953f739f2ddf131e9cbe5 +size 9010 diff --git a/tests/unittests/CoreGraphics.drawing/data/reference/TestImage.CGContext.FillMode.EOFillStroke.png b/tests/unittests/CoreGraphics.drawing/data/reference/TestImage.CGContext.FillMode.EOFillStroke.png new file mode 100644 index 0000000000..ff1403db01 --- /dev/null +++ b/tests/unittests/CoreGraphics.drawing/data/reference/TestImage.CGContext.FillMode.EOFillStroke.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:76bad121aa3f04269f6795f0aacdaf8f1576d44c18f5b1ded084ff009bd13fa6 +size 15722 diff --git a/tests/unittests/CoreGraphics.drawing/data/reference/TestImage.CGContext.FillMode.Fill.png b/tests/unittests/CoreGraphics.drawing/data/reference/TestImage.CGContext.FillMode.Fill.png new file mode 100644 index 0000000000..e996e12e5f --- /dev/null +++ b/tests/unittests/CoreGraphics.drawing/data/reference/TestImage.CGContext.FillMode.Fill.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:0fd904926a6ba1e00589bc216130c3b273a1a90b846c444dc373a1a58028a8c8 +size 8326 diff --git a/tests/unittests/CoreGraphics.drawing/data/reference/TestImage.CGContext.FillMode.FillStroke.png b/tests/unittests/CoreGraphics.drawing/data/reference/TestImage.CGContext.FillMode.FillStroke.png new file mode 100644 index 0000000000..8351197107 --- /dev/null +++ b/tests/unittests/CoreGraphics.drawing/data/reference/TestImage.CGContext.FillMode.FillStroke.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:86d7892766a42f6355a8bda1f7ca92edc84f3b75dd135f4eb62f122e659aef47 +size 17892 diff --git a/tests/unittests/CoreGraphics.drawing/data/reference/TestImage.CGPath.FillModeCircles.png b/tests/unittests/CoreGraphics.drawing/data/reference/TestImage.CGPath.FillModeCircles.png deleted file mode 100644 index db6d7d1930..0000000000 --- a/tests/unittests/CoreGraphics.drawing/data/reference/TestImage.CGPath.FillModeCircles.png +++ /dev/null @@ -1,3 +0,0 @@ -version https://git-lfs.github.com/spec/v1 -oid sha256:3f8cba6c6fe60b8a40784cd8bdec2145a0c66855dc7ce1d53f9b8f6c414e1c72 -size 18952