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

Jump to bottom

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 1 commit
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
42 changes: 42 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,45 @@ 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 _CGContextSafeInnerBeginDraw(CGContextRef ctx) {
Copy link
Contributor

@rajsesh rajsesh Jan 13, 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.

Where are these two used, and what is their purpose? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not used in this commit, but are needed for CGD2D. it seemed vaguely more appropriate to add them now than to add them when merging to CGD2D, but i don't feel strongly about this.

As an example, CGD2D has __CGContext::DrawToCommandList - it switches render targets in the middle and switches back at the end. Retargeting is illegal if in the middle of a Begin/EndDraw pair, so we need an alternate mechanism to force an EndDraw so we can retarget, then BeginDraw again. (I made a mistake by putting the second command in the body of this function though, oops) According to @DHowett-MSFT, shadows and transparency layers also require a retarget.

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

ID2D1RenderTarget* imgRenderTarget = ctx->Backing()->DestImage()->Backing()->GetRenderTarget();
THROW_HR_IF_NULL(E_UNEXPECTED, imgRenderTarget);

if ((ctx->_beginEndDrawDepth) > 0) {
imgRenderTarget->EndDraw();
}

imgRenderTarget->BeginDraw();
}

void _CGContextSafeInnerEndDraw(CGContextRef ctx) {
ID2D1RenderTarget* imgRenderTarget = ctx->Backing()->DestImage()->Backing()->GetRenderTarget();
THROW_HR_IF_NULL(E_UNEXPECTED, imgRenderTarget);

THROW_IF_FAILED(imgRenderTarget->EndDraw());

if ((ctx->_beginEndDrawDepth) > 0) {
imgRenderTarget->BeginDraw();
}
}

#pragma endregion
6 changes: 3 additions & 3 deletions Frameworks/CoreGraphics/CGContextCairo.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1863,7 +1863,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 +1898,7 @@
D2D1::ColorF brushColor =
D2D1::ColorF(curState->curFillColor.r, curState->curFillColor.g, curState->curFillColor.b, curState->curFillColor.a);

imgRenderTarget->BeginDraw();
_CGContextPushBeginDraw(_rootContext);
// Draw the glyph using ID2D1RenderTarget
ComPtr<ID2D1SolidColorBrush> brush;
THROW_IF_FAILED(imgRenderTarget->CreateSolidColorBrush(brushColor, &brush));
Expand Down Expand Up @@ -1932,7 +1932,7 @@
}
}

THROW_IF_FAILED(imgRenderTarget->EndDraw());
_CGContextPopEndDraw(_rootContext);
}

// 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
4 changes: 4 additions & 0 deletions Frameworks/UIKit/NSLayoutManager.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <UIKit/UIGraphics.h>

#include "CoreTextInternal.h"
#include "CGContextInternal.h"

#include <vector>
#include <functional>
Expand Down Expand Up @@ -257,6 +258,9 @@ - (void)drawGlyphsForGlyphRange:(NSRange)range atPoint:(CGPoint)position {
CGContextSaveGState(curCtx);
CGContextSetTextMatrix(curCtx, CGAffineTransformMakeScale(1.0f, -1.0f));

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

int count = [_ctLines count];
for (int curLine = 0; curLine < count; curLine++) {
CTLineRef line = (CTLineRef)_ctLines[curLine];
Expand Down
3 changes: 3 additions & 0 deletions Frameworks/UIKit/NSString+UIKitAdditions.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#import <Foundation/NSMutableDictionary.h>
#import "CoreGraphics/CGContext.h"
#import "CoreTextInternal.h"
#import "CGContextInternal.h"
#import "NSParagraphStyleInternal.h"
#import <assert.h>
#import "LoggingNative.h"
Expand Down Expand Up @@ -145,6 +146,8 @@ - (CGSize)drawInRect:(CGRect)rect withFont:(UIFont*)font lineBreakMode:(UILineBr
}

CGContextRef context = UIGraphicsGetCurrentContext();
_CGContextPushBeginDraw(context);
auto popEnd = wil::ScopeExit([context]() { _CGContextPopEndDraw(context); });

// Invert text matrix so glyphs are drawn with correct orientation
CGContextSetTextMatrix(context, CGAffineTransformMakeScale(1.0f, -1.0f));
Expand Down
15 changes: 9 additions & 6 deletions Frameworks/UIKit/UIView.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#import "UIGestureRecognizerInternal.h"
#import "CALayerInternal.h"
#import "CAAnimationInternal.h"
#import "CGContextInternal.h"
#import <AutoLayout.h>
#import <NSLayoutConstraint+AutoLayout.h>
#import "NSLayoutAnchorInternal.h"
Expand Down Expand Up @@ -243,7 +244,7 @@ - (bool)_processGesturesForTouch:(UITouch*)touch event:(UIEvent*)event touchEven
// scanning DManip Gestures, if one gesture is ongoing, cancel all DManip Gestures
// otherwise, send Touch to DManip gestures
for (int i = 0; i < dManipGestureCount; i++) {
UIGestureRecognizer* dManipGesture = dManipRecognizers[i];
UIGestureRecognizer* dManipGesture = dManipRecognizers[i];
if (gestureOnGoing) {
[dManipGesture _cancelIfActive];
if (DEBUG_GESTURES) {
Expand Down Expand Up @@ -527,9 +528,7 @@ - (UITouchPhase)_processPointerEvent:(WUXIPointerRoutedEventArgs*)pointerEventAr
if (!touchPoint.touch->_view) {
// Ignore if the pointer isn't captured
if (DEBUG_TOUCHES_LIGHT) {
TraceVerbose(TAG,
L"View for touch is nil!, ignoring touch for touchPhase %d.",
touchPhase);
TraceVerbose(TAG, L"View for touch is nil!, ignoring touch for touchPhase %d.", touchPhase);
}
} else if (touchPhase != UITouchPhaseBegan && ![touchPoint.touch->_view->priv->currentTouches containsObject:touchPoint.touch]) {
// Ignore if the pointer isn't captured
Expand Down Expand Up @@ -2252,12 +2251,16 @@ - (CGPoint)convertPoint:(CGPoint)pos fromView:(UIView*)fromView {
*/
- (void)drawLayer:(CALayer*)layer inContext:(CGContextRef)context {
UIGraphicsPushContext(context);
_CGContextPushBeginDraw(context);
Copy link
Contributor

@rajsesh rajsesh Jan 13, 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.

This should really go into CALayer display, which displays a layer for one frame. #Resolved

Copy link
Contributor

@rajsesh rajsesh Jan 13, 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.

Also into UIGraphicsPushContext() and PopContext(). #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had the same idea, but it ended up regressing XAMLCatalog (Custom text view would not update at all).

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Contexts that go onto the UIGraphics context stack cannot actually be safely assumed to only need their results after PopContext().

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

Copy link
Contributor

@rajsesh rajsesh Jan 13, 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.

Fair enough. We should still do this in CALayer display. #Resolved

Copy link
Contributor Author

@ms-jihua ms-jihua Jan 13, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Er, that is, putting it into [CALayer display] broke things. I could go investigate why if you'd like, but I'm not sure how fruitful that'd be - we'd probably have to keep this out of CALayer display anyway.

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


auto popEnd = wil::ScopeExit([context]() {
_CGContextPopEndDraw(context);
UIGraphicsPopContext();
});

CGRect bounds;
bounds = CGContextGetClipBoundingBox(context);
[self drawRect:bounds];

UIGraphicsPopContext();
}

/**
Expand Down
8 changes: 8 additions & 0 deletions Frameworks/include/CGContextInternal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ COREGRAPHICS_EXPORT bool CGContextIsPointInPath(CGContextRef c, bool eoFill, flo

COREGRAPHICS_EXPORT void CGContextDrawGlyphRun(CGContextRef ctx, const DWRITE_GLYPH_RUN* glyphRun);

COREGRAPHICS_EXPORT void _CGContextPushBeginDraw(CGContextRef ctx);
COREGRAPHICS_EXPORT void _CGContextPopEndDraw(CGContextRef ctx);
COREGRAPHICS_EXPORT void _CGContextSafeInnerBeginDraw(CGContextRef ctx);
COREGRAPHICS_EXPORT void _CGContextSafeInnerEndDraw(CGContextRef ctx);

// TODO 1077:: Remove once D2D render target is implemented
COREGRAPHICS_EXPORT void _CGContextSetScaleFactor(CGContextRef ctx, float scale);

Expand All @@ -57,6 +62,9 @@ class __CGContext : private objc_object {
float scale;
CGContextImpl* _backing;

// Reduces the number of BeginDraw() and EndDraw() calls needed, by counting in a stack-like manner
uint32_t _beginEndDrawDepth = 0;
Copy link
Contributor

@aballway aballway Jan 13, 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.

Should we have a mutex for updating this? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'll std::atomic it

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


__CGContext(CGImageRef pDest);
~__CGContext();

Expand Down
4 changes: 4 additions & 0 deletions build/CoreGraphics/dll/CoreGraphics.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,10 @@ LIBRARY CoreGraphics
CGContextIsPointInPath
EbrCenterTextInRectVertically
_CGContextSetScaleFactor
_CGContextPushBeginDraw
_CGContextPopEndDraw
_CGContextSafeInnerBeginDraw
_CGContextSafeInnerEndDraw

; CGDataConsumer.mm
CGDataConsumerCreate
Expand Down
Binary file modified docs/CoreText/WinObjC.CoreText.docx
View file
Open in desktop
Binary file not shown.
2 changes: 1 addition & 1 deletion samples/XAMLCatalog/XAMLCatalog/CustomTextControlViewController.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -874,7 +874,7 @@ - (void)viewDidLoad {
textEdit.autoresizingMask = infoLabel.autoresizingMask = focusButton.autoresizingMask =
UIViewAutoresizingFlexibleLeftMargin | UIViewAutoresizingFlexibleRightMargin | UIViewAutoresizingFlexibleBottomMargin;

infoLabel.text = @"Click the control to gain focus. Use the keyboard to type and caret nagivate. Hold shift to update the "
infoLabel.text = @"Click the control to gain focus. Use the keyboard to type and caret navigate. Hold shift to update the "
Copy link
Contributor

@aballway aballway Jan 13, 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.

+1 #Closed

@"selection. Right click or Ctrl+C or V to copy and paste.";
infoLabel.numberOfLines = 0;
infoLabel.textAlignment = NSTextAlignmentCenter;
Expand Down
2 changes: 1 addition & 1 deletion samples/XAMLCatalog/XAMLCatalog/XAMLCatalog.storyboard
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@
<constraint firstAttribute="height" constant="100" id="jzf-wl-pPs"/>
<constraint firstAttribute="width" constant="300" id="oPb-uK-RMZ"/>
</constraints>
<string key="text">Click the control to gain focus. Use the keyboard to type and caret nagivate. Hold shift to update the selection. Right click or Ctrl+C or V to copy and paste.</string>
<string key="text">Click the control to gain focus. Use the keyboard to type and caret navigate. Hold shift to update the selection. Right click or Ctrl+C or V to copy and paste.</string>
<fontDescription key="fontDescription" type="system" pointSize="17"/>
<color key="textColor" red="0.0" green="0.0" blue="0.0" alpha="1" colorSpace="calibratedRGB"/>
<nil key="highlightedColor"/>
Expand Down