Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CoreText Performance: Call ID2D1RenderTarget::BeginDraw()/EndDraw() f… #1705

Merged
merged 2 commits into from
Jan 20, 2017
Merged

CoreText Performance: Call ID2D1RenderTarget::BeginDraw()/EndDraw() f… #1705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fcd7cf9
CoreText Performance: Call ID2D1RenderTarget::BeginDraw()/EndDraw() f…
ms-jihua Jan 4, 2017
cc44c46
- Added Begin/EndDraw pairs to a few more places
ms-jihua Jan 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions Frameworks/CoreGraphics/CGContext.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1238,3 +1238,39 @@ void CGContextDrawGlyphRun(CGContextRef ctx, const DWRITE_GLYPH_RUN* glyphRun) {
void _CGContextSetScaleFactor(CGContextRef ctx, float scale) {
ctx->Backing()->_CGContextSetScaleFactor(scale);
}

#pragma region CGContextBeginDrawEndDraw

void _CGContextPushBeginDraw(CGContextRef ctx) {
if ((ctx->_beginEndDrawDepth)++ == 0) {
ID2D1RenderTarget* imgRenderTarget = ctx->Backing()->DestImage()->Backing()->GetRenderTarget();
THROW_HR_IF_NULL(E_UNEXPECTED, imgRenderTarget);
imgRenderTarget->BeginDraw();
}
}

void _CGContextPopEndDraw(CGContextRef ctx) {
if (--(ctx->_beginEndDrawDepth) == 0) {
ID2D1RenderTarget* imgRenderTarget = ctx->Backing()->DestImage()->Backing()->GetRenderTarget();
THROW_HR_IF_NULL(E_UNEXPECTED, imgRenderTarget);
THROW_IF_FAILED(imgRenderTarget->EndDraw());
}
}

void _CGContextEscapeBeginEndDrawStack(CGContextRef ctx) {
if ((ctx->_beginEndDrawDepth > 0) && ((ctx->_escapeBeginEndDrawDepth)++ == 0)) {
ID2D1RenderTarget* imgRenderTarget = ctx->Backing()->DestImage()->Backing()->GetRenderTarget();
THROW_HR_IF_NULL(E_UNEXPECTED, imgRenderTarget);
THROW_IF_FAILED(imgRenderTarget->EndDraw());
}
}

void _CGContextUnescapeBeginEndDrawStack(CGContextRef ctx) {
if ((ctx->_beginEndDrawDepth > 0) && (--(ctx->_escapeBeginEndDrawDepth) == 0)) {
ID2D1RenderTarget* imgRenderTarget = ctx->Backing()->DestImage()->Backing()->GetRenderTarget();
THROW_HR_IF_NULL(E_UNEXPECTED, imgRenderTarget);
imgRenderTarget->BeginDraw();
}
}

#pragma endregion
113 changes: 109 additions & 4 deletions Frameworks/CoreGraphics/CGContextCairo.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,11 @@
// An OSS request has been submitted. The link for the request is:
// https://osstool.microsoft.com/palamida/RequestDetails.htm?rid=40072&projectId=1
void CGContextCairo::_cairoImageSurfaceBlur(cairo_surface_t* surface) {
// TODO #1635: It's unsafe to edit the bitmap not through D2D1RenderTarget, during a BeginDraw()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are a fairly significant hit to performance for the non-text CGContextCairo drawing functions, eating up >50% of time in CGContextFillRect, for instance. This may be unavoidable until the CGD2D merge, and is a tradeoff - text-drawing is still a much bigger bottleneck (CGContextFillRect, while slower, still does not take up much time). Please comment if you have any objections.

// Escape any Begin/EndDraw() pairs for the lifetime of this function (to be removed during the CGD2D merge)
_CGContextEscapeBeginEndDrawStack(_rootContext);
Copy link
Contributor

@aballway aballway Jan 19, 2017 •
edited by ms-jihua
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: as much as I hate them, this would make a nice macro #WontFix

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tried it, but it obscures too much for my taste.

In reply to: 96971228 [](ancestors = 96971228)

auto popEnd = wil::ScopeExit([this]() { _CGContextUnescapeBeginEndDrawStack(this->_rootContext); });

double blur = curState->shadowBlur;

if (surface == NULL || blur <= 0) {
Expand Down Expand Up @@ -171,6 +176,11 @@
}

void CGContextCairo::_cairoContextStrokePathShadow() {
// TODO #1635: It's unsafe to edit the bitmap not through D2D1RenderTarget, during a BeginDraw()
// Escape any Begin/EndDraw() pairs for the lifetime of this function (to be removed during the CGD2D merge)
_CGContextEscapeBeginEndDrawStack(_rootContext);
auto popEnd = wil::ScopeExit([this]() { _CGContextUnescapeBeginEndDrawStack(this->_rootContext); });

int i;
double width, height, deltaWidth, deltaHeight;

Expand Down Expand Up @@ -283,6 +293,11 @@
}

void CGContextCairo::Clear(float r, float g, float b, float a) {
// TODO #1635: It's unsafe to edit the bitmap not through D2D1RenderTarget, during a BeginDraw()
// Escape any Begin/EndDraw() pairs for the lifetime of this function (to be removed during the CGD2D merge)
_CGContextEscapeBeginEndDrawStack(_rootContext);
auto popEnd = wil::ScopeExit([this]() { _CGContextUnescapeBeginEndDrawStack(this->_rootContext); });

ObtainLock();

CGRect rct;
Expand All @@ -303,6 +318,11 @@
}

void CGContextCairo::DrawImage(CGImageRef img, CGRect src, CGRect dest, bool tiled) {
// TODO #1635: It's unsafe to edit the bitmap not through D2D1RenderTarget, during a BeginDraw()
// Escape any Begin/EndDraw() pairs for the lifetime of this function (to be removed during the CGD2D merge)
_CGContextEscapeBeginEndDrawStack(_rootContext);
auto popEnd = wil::ScopeExit([this]() { _CGContextUnescapeBeginEndDrawStack(this->_rootContext); });

ObtainLock();

if (dest.size.width == 0.0f || dest.size.height == 0.0f)
Expand Down Expand Up @@ -854,6 +874,11 @@
}

void CGContextCairo::CGContextClearRect(CGRect rct) {
// TODO #1635: It's unsafe to edit the bitmap not through D2D1RenderTarget, during a BeginDraw()
// Escape any Begin/EndDraw() pairs for the lifetime of this function (to be removed during the CGD2D merge)
_CGContextEscapeBeginEndDrawStack(_rootContext);
auto popEnd = wil::ScopeExit([this]() { _CGContextUnescapeBeginEndDrawStack(this->_rootContext); });

ObtainLock();

_isDirty = true;
Expand Down Expand Up @@ -886,6 +911,11 @@
}

void CGContextCairo::CGContextFillRect(CGRect rct) {
// TODO #1635: It's unsafe to edit the bitmap not through D2D1RenderTarget, during a BeginDraw()
// Escape any Begin/EndDraw() pairs for the lifetime of this function (to be removed during the CGD2D merge)
_CGContextEscapeBeginEndDrawStack(_rootContext);
auto popEnd = wil::ScopeExit([this]() { _CGContextUnescapeBeginEndDrawStack(this->_rootContext); });

ObtainLock();

_isDirty = true;
Expand Down Expand Up @@ -1102,6 +1132,11 @@
}

void CGContextCairo::CGContextStrokeEllipseInRect(CGRect rct) {
// TODO #1635: It's unsafe to edit the bitmap not through D2D1RenderTarget, during a BeginDraw()
// Escape any Begin/EndDraw() pairs for the lifetime of this function (to be removed during the CGD2D merge)
_CGContextEscapeBeginEndDrawStack(_rootContext);
auto popEnd = wil::ScopeExit([this]() { _CGContextUnescapeBeginEndDrawStack(this->_rootContext); });

ObtainLock();

_isDirty = true;
Expand Down Expand Up @@ -1135,6 +1170,11 @@
}

void CGContextCairo::CGContextFillEllipseInRect(CGRect rct) {
// TODO #1635: It's unsafe to edit the bitmap not through D2D1RenderTarget, during a BeginDraw()
// Escape any Begin/EndDraw() pairs for the lifetime of this function (to be removed during the CGD2D merge)
_CGContextEscapeBeginEndDrawStack(_rootContext);
auto popEnd = wil::ScopeExit([this]() { _CGContextUnescapeBeginEndDrawStack(this->_rootContext); });

ObtainLock();

_isDirty = true;
Expand Down Expand Up @@ -1174,6 +1214,11 @@
}

void CGContextCairo::CGContextStrokePath() {
// TODO #1635: It's unsafe to edit the bitmap not through D2D1RenderTarget, during a BeginDraw()
// Escape any Begin/EndDraw() pairs for the lifetime of this function (to be removed during the CGD2D merge)
_CGContextEscapeBeginEndDrawStack(_rootContext);
auto popEnd = wil::ScopeExit([this]() { _CGContextUnescapeBeginEndDrawStack(this->_rootContext); });

ObtainLock();

_isDirty = true;
Expand All @@ -1197,6 +1242,11 @@
}

void CGContextCairo::CGContextStrokeRect(CGRect rct) {
// TODO #1635: It's unsafe to edit the bitmap not through D2D1RenderTarget, during a BeginDraw()
// Escape any Begin/EndDraw() pairs for the lifetime of this function (to be removed during the CGD2D merge)
_CGContextEscapeBeginEndDrawStack(_rootContext);
auto popEnd = wil::ScopeExit([this]() { _CGContextUnescapeBeginEndDrawStack(this->_rootContext); });

ObtainLock();

_isDirty = true;
Expand Down Expand Up @@ -1230,6 +1280,11 @@
}

void CGContextCairo::CGContextStrokeRectWithWidth(CGRect rct, float width) {
// TODO #1635: It's unsafe to edit the bitmap not through D2D1RenderTarget, during a BeginDraw()
// Escape any Begin/EndDraw() pairs for the lifetime of this function (to be removed during the CGD2D merge)
_CGContextEscapeBeginEndDrawStack(_rootContext);
auto popEnd = wil::ScopeExit([this]() { _CGContextUnescapeBeginEndDrawStack(this->_rootContext); });

ObtainLock();

_isDirty = true;
Expand Down Expand Up @@ -1260,6 +1315,11 @@
}

void CGContextCairo::CGContextFillPath() {
// TODO #1635: It's unsafe to edit the bitmap not through D2D1RenderTarget, during a BeginDraw()
// Escape any Begin/EndDraw() pairs for the lifetime of this function (to be removed during the CGD2D merge)
_CGContextEscapeBeginEndDrawStack(_rootContext);
auto popEnd = wil::ScopeExit([this]() { _CGContextUnescapeBeginEndDrawStack(this->_rootContext); });

ObtainLock();

_isDirty = true;
Expand Down Expand Up @@ -1288,6 +1348,11 @@
}

void CGContextCairo::CGContextEOFillPath() {
// TODO #1635: It's unsafe to edit the bitmap not through D2D1RenderTarget, during a BeginDraw()
// Escape any Begin/EndDraw() pairs for the lifetime of this function (to be removed during the CGD2D merge)
_CGContextEscapeBeginEndDrawStack(_rootContext);
auto popEnd = wil::ScopeExit([this]() { _CGContextUnescapeBeginEndDrawStack(this->_rootContext); });

ObtainLock();

_isDirty = true;
Expand Down Expand Up @@ -1316,6 +1381,11 @@
}

void CGContextCairo::CGContextEOClip() {
// TODO #1635: It's unsafe to edit the bitmap not through D2D1RenderTarget, during a BeginDraw()
// Escape any Begin/EndDraw() pairs for the lifetime of this function (to be removed during the CGD2D merge)
_CGContextEscapeBeginEndDrawStack(_rootContext);
auto popEnd = wil::ScopeExit([this]() { _CGContextUnescapeBeginEndDrawStack(this->_rootContext); });

ObtainLock();

TraceWarning(TAG, L"CGContextEOClip not supported");
Expand All @@ -1326,6 +1396,11 @@
}

void CGContextCairo::CGContextDrawPath(CGPathDrawingMode mode) {
// TODO #1635: It's unsafe to edit the bitmap not through D2D1RenderTarget, during a BeginDraw()
// Escape any Begin/EndDraw() pairs for the lifetime of this function (to be removed during the CGD2D merge)
_CGContextEscapeBeginEndDrawStack(_rootContext);
auto popEnd = wil::ScopeExit([this]() { _CGContextUnescapeBeginEndDrawStack(this->_rootContext); });

ObtainLock();

_isDirty = true;
Expand Down Expand Up @@ -1426,6 +1501,11 @@
}

void CGContextCairo::CGContextDrawLinearGradient(CGGradientRef gradient, CGPoint startPoint, CGPoint endPoint, DWORD options) {
// TODO #1635: It's unsafe to edit the bitmap not through D2D1RenderTarget, during a BeginDraw()
// Escape any Begin/EndDraw() pairs for the lifetime of this function (to be removed during the CGD2D merge)
_CGContextEscapeBeginEndDrawStack(_rootContext);
auto popEnd = wil::ScopeExit([this]() { _CGContextUnescapeBeginEndDrawStack(this->_rootContext); });

ObtainLock();

_isDirty = true;
Expand Down Expand Up @@ -1467,6 +1547,11 @@

void CGContextCairo::CGContextDrawRadialGradient(
CGGradientRef gradient, CGPoint startCenter, float startRadius, CGPoint endCenter, float endRadius, DWORD options) {
// TODO #1635: It's unsafe to edit the bitmap not through D2D1RenderTarget, during a BeginDraw()
// Escape any Begin/EndDraw() pairs for the lifetime of this function (to be removed during the CGD2D merge)
_CGContextEscapeBeginEndDrawStack(_rootContext);
auto popEnd = wil::ScopeExit([this]() { _CGContextUnescapeBeginEndDrawStack(this->_rootContext); });

ObtainLock();

_isDirty = true;
Expand Down Expand Up @@ -1673,6 +1758,11 @@
}

void CGContextCairo::CGContextClip() {
// TODO #1635: It's unsafe to edit the bitmap not through D2D1RenderTarget, during a BeginDraw()
// Escape any Begin/EndDraw() pairs for the lifetime of this function (to be removed during the CGD2D merge)
_CGContextEscapeBeginEndDrawStack(_rootContext);
auto popEnd = wil::ScopeExit([this]() { _CGContextUnescapeBeginEndDrawStack(this->_rootContext); });

ObtainLock();

LOCK_CAIRO();
Expand Down Expand Up @@ -1710,6 +1800,11 @@
}

void CGContextCairo::CGContextClipToRect(CGRect rect) {
// TODO #1635: It's unsafe to edit the bitmap not through D2D1RenderTarget, during a BeginDraw()
// Escape any Begin/EndDraw() pairs for the lifetime of this function (to be removed during the CGD2D merge)
_CGContextEscapeBeginEndDrawStack(_rootContext);
auto popEnd = wil::ScopeExit([this]() { _CGContextUnescapeBeginEndDrawStack(this->_rootContext); });

ObtainLock();
LOCK_CAIRO();
cairo_path_t* oldPath = cairo_copy_path(_drawContext);
Expand Down Expand Up @@ -1738,6 +1833,11 @@
}

void CGContextCairo::CGContextBeginTransparencyLayerWithRect(CGRect rect, id auxInfo) {
// TODO #1635: It's unsafe to edit the bitmap not through D2D1RenderTarget, during a BeginDraw()
// Escape any Begin/EndDraw() pairs for the lifetime of this function (to be removed during the CGD2D merge)
_CGContextEscapeBeginEndDrawStack(_rootContext);
auto popEnd = wil::ScopeExit([this]() { _CGContextUnescapeBeginEndDrawStack(this->_rootContext); });

ObtainLock();
LOCK_CAIRO();
cairo_save(_drawContext);
Expand All @@ -1756,6 +1856,11 @@
}

void CGContextCairo::CGContextEndTransparencyLayer() {
// TODO #1635: It's unsafe to edit the bitmap not through D2D1RenderTarget, during a BeginDraw()
// Escape any Begin/EndDraw() pairs for the lifetime of this function (to be removed during the CGD2D merge)
_CGContextEscapeBeginEndDrawStack(_rootContext);
auto popEnd = wil::ScopeExit([this]() { _CGContextUnescapeBeginEndDrawStack(this->_rootContext); });

ObtainLock();
LOCK_CAIRO();
// Retrieve the group as a pattern
Expand Down Expand Up @@ -1863,7 +1968,7 @@
CGContextStrokePath();

ID2D1RenderTarget* imgRenderTarget = _imgDest->Backing()->GetRenderTarget();
THROW_NS_IF_NULL(E_UNEXPECTED, imgRenderTarget);
THROW_HR_IF_NULL(E_UNEXPECTED, imgRenderTarget);

// Apply the required transformations as set in the context.
// We need some special handling in transform as CoreText in iOS renders from bottom left but DWrite on Windows does top left.
Expand Down Expand Up @@ -1898,7 +2003,9 @@
D2D1::ColorF brushColor =
D2D1::ColorF(curState->curFillColor.r, curState->curFillColor.g, curState->curFillColor.b, curState->curFillColor.a);

imgRenderTarget->BeginDraw();
_CGContextPushBeginDraw(_rootContext);
auto popEnd = wil::ScopeExit([this]() { _CGContextPopEndDraw(this->_rootContext); });

// Draw the glyph using ID2D1RenderTarget
ComPtr<ID2D1SolidColorBrush> brush;
THROW_IF_FAILED(imgRenderTarget->CreateSolidColorBrush(brushColor, &brush));
Expand Down Expand Up @@ -1931,8 +2038,6 @@
userTransform = CGAffineTransformTranslate(userTransform, glyphRun->glyphAdvances[i], 0);
}
}

THROW_IF_FAILED(imgRenderTarget->EndDraw());
}

// TODO 1077:: Remove once D2D render target is implemented
Expand Down
2 changes: 1 addition & 1 deletion Frameworks/CoreGraphics/CGContextImpl.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -743,4 +743,4 @@

// TODO 1077:: Remove once D2D render target is implemented
void CGContextImpl::_CGContextSetScaleFactor(float scale) {
}
}
4 changes: 4 additions & 0 deletions Frameworks/CoreText/CTFrame.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#import "DWriteWrapper_CoreText.h"
#import "CoreTextInternal.h"
#import "CGContextInternal.h"
#import "CGPathInternal.h"

const CFStringRef kCTFrameProgressionAttributeName = static_cast<CFStringRef>(@"kCTFrameProgressionAttributeName");
Expand Down Expand Up @@ -123,6 +124,9 @@ void CTFrameDraw(CTFrameRef frameRef, CGContextRef ctx) {
ctx, CGAffineTransformMake(textMatrix.a, -textMatrix.b, textMatrix.c, -textMatrix.d, textMatrix.tx, textMatrix.ty));
CGContextScaleCTM(ctx, 1.0f, -1.0f);

_CGContextPushBeginDraw(ctx);
auto popEnd = wil::ScopeExit([ctx]() { _CGContextPopEndDraw(ctx); });

for (size_t i = 0; i < frame->_lineOrigins.size() && (frame->_lineOrigins[i].y < frame->_frameRect.size.height); ++i) {
_CTLine* line = static_cast<_CTLine*>([frame->_lines objectAtIndex:i]);
CGContextSetTextPosition(ctx, frame->_lineOrigins[i].x, frame->_lineOrigins[i].y);
Expand Down
3 changes: 3 additions & 0 deletions Frameworks/CoreText/CTLine.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,9 @@ void CTLineDraw(CTLineRef lineRef, CGContextRef ctx) {

_CTLine* line = static_cast<_CTLine*>(lineRef);

_CGContextPushBeginDraw(ctx);
auto popEnd = wil::ScopeExit([ctx]() { _CGContextPopEndDraw(ctx); });

for (size_t i = 0; i < [line->_runs count]; ++i) {
_CTRun* curRun = [line->_runs objectAtIndex:i];
if (i > 0) {
Expand Down
Loading