Adding support for ClearRect with clipping geometry #2053
Conversation
|
|
||
| HRESULT __CGContext::ClearRect(CGRect rect) { | ||
| PushBeginDraw(); | ||
| auto endDraw = wil::ScopeExit([this]() { PopEndDraw(); }); |
There was a problem hiding this comment.
This will ignore the antialias modes set in @MSFTFox's change #Resolved
Frameworks/CoreGraphics/CGContext.mm
Outdated
| auto endDraw = wil::ScopeExit([this]() { PopEndDraw(); }); | ||
|
|
||
| ComPtr<ID2D1RectangleGeometry> rectangle; | ||
| RETURN_IF_FAILED(Factory()->CreateRectangleGeometry(__CGRectToD2D_F(rect), &rectangle)); |
There was a problem hiding this comment.
Consider caching the ID2D1Factory. #Resolved
Frameworks/CoreGraphics/CGContext.mm
Outdated
| RETURN_IF_FAILED(deviceContext->CreateSolidColorBrush(D2D1::ColorF(D2D1::ColorF::Black, 0.0f), &brush)); | ||
|
|
||
| deviceContext->SetPrimitiveBlend(D2D1_PRIMITIVE_BLEND_COPY); | ||
| deviceContext->FillGeometry(CurrentGState().clippingGeometry.Get(), brush.Get()); |
There was a problem hiding this comment.
I wonder how this works in combination with transparency layers. Can you do a test on our platform and the reference platform where you draw red, begin a transparency layer, draw blue, clear a sub-region, and end the layer? I'd expect the blue to have a "hole" in it where the red is visible. #Resolved
| } | ||
| }; | ||
|
|
||
| TEST_P(CGClearRect, Transformed) { |
There was a problem hiding this comment.
These must be DRAW_TEST_P for TAEF. #Resolved
|
|
||
| return CFStringCreateWithFormat(nullptr, | ||
| nullptr, | ||
| CFSTR("TestImage.CGContext.ClearRect.rect.(%0.0f.%0.0f)%0.0fx%0.0f.transformationAffine.[%0.1f,%0." |
There was a problem hiding this comment.
nit: filename doesn't need the word rect or transformationAffine in it. #Resolved
There was a problem hiding this comment.
@msft-Jeyaram we are running up against Windows' relatively short MAX_PATH. Please reduce these filenames. #Resolved
There was a problem hiding this comment.
|
|
||
| CGContextAddRect(context, CGRectMake(0, 0, 250, 250)); | ||
| CGContextConcatCTM(context, transformation); | ||
| CGContextClip(context); |
There was a problem hiding this comment.
Is this a test for two different clipping regions? #ByDesign
There was a problem hiding this comment.
yes, i would of made both of them parameterized but then we would end up with a spew of files.
In reply to: 102640945 [](ancestors = 102640945)
| } | ||
|
|
||
| static CGRect rects[] = { CGRectMake(0, 0, 100, 100), CGRectMake(0, 0, 50, 250), CGRectMake(100, 100, 125, 250) }; | ||
| static CGAffineTransform transformation[] = { CGAffineTransformMakeRotation(0.4), |
There was a problem hiding this comment.
include an identity test! #Resolved
| CGClearRect, | ||
| ::testing::Combine(::testing::ValuesIn(rects), ::testing::ValuesIn(transformation))); | ||
|
|
||
| DRAW_TEST_F(CGContextRectClear, ClearRect, WhiteBackgroundTest<>) { |
There was a problem hiding this comment.
you can remove RectClear from this test name; it's implied and it's part of CGContext. #Resolved
| CGContextClearRect(context, borderRect); | ||
| } | ||
|
|
||
| DISABLED_DRAW_TEST_F(CGContextRectClear, ClearRectRam, WhiteBackgroundTest<>) { |
There was a problem hiding this comment.
Disabled?
This one is named Ram; perhaps Arc is better!
Also, why is it disabled? #ByDesign
| CGRect bounds = GetDrawingBounds(); | ||
|
|
||
| CGRect cirleRect = CGRectMake(0, 0, 100, 100); | ||
| CGContextAddArc(context, 50, 50, 50, 0.0, 2 * M_PI, 0); |
There was a problem hiding this comment.
when we attempt to fill the geometry with an arc.
The fill is a success but the rendering doesn't show any signs of being filled.
I tried with a custom circle geometry of the same size and it works.
investigating, it looks like some issues with the arc geometry. #ByDesign
There was a problem hiding this comment.
that's not good. please file an issue and tag this code with that issue. #Resolved
Frameworks/CoreGraphics/CGContext.mm
Outdated
| PushBeginDraw(); | ||
| auto endDraw = wil::ScopeExit([this]() { PopEndDraw(); }); | ||
|
|
||
| auto factory = Factory(); |
There was a problem hiding this comment.
nit: shouldn't use auto for this since type isn't specified on right/unrepresentable #Resolved
Frameworks/CoreGraphics/CGContext.mm
Outdated
| RETURN_IF_FAILED(CurrentGState().IntersectClippingGeometry(transformedRectangle.Get(), kCGPathEOFill)); | ||
|
|
||
| ComPtr<ID2D1SolidColorBrush> brush; | ||
| RETURN_IF_FAILED(deviceContext->CreateSolidColorBrush(D2D1::ColorF(D2D1::ColorF::Black, 0.0f), &brush)); |
There was a problem hiding this comment.
Nit: this should be in the gstate as a clear brush, we are likely creating too many brushes. #Resolved
msft-Jeyaram
force-pushed
the
ClearRect
branch
from
764f8ac to
4d91173
Compare
msft-Jeyaram
force-pushed
the
ClearRect
branch
from
4d91173 to
b05c965
Compare
|
@DHowett-MSFT @rajsesh-msft Ping |
#1744