From 05117f3c63b35771160a29f652144047678ce7e1 Mon Sep 17 00:00:00 2001 From: AJ Ballway Date: Tue, 1 Nov 2016 10:09:37 -0700 Subject: [PATCH 1/6] Fix rotation and nslayout line drawing (#1274) * Fix rotation and nslayout line drawing * Remove descent and add comment * Add CTCatalog page for testing CTM transformation * Add new test page to xcodeproj * Address CR feedback * Remove vestigial colorspace Fixes incorrect rotation introduced by my recent math changes and adds descent to nslayoutmanager line drawing to make exclusion zones more accurate Fixes #1275 --- Frameworks/CoreGraphics/CGContextCairo.mm | 5 +- Frameworks/UIKit/NSLayoutManager.mm | 1 + .../CTCatalog-Headers.vcxitems | 1 + .../CTCatalog-Headers.vcxitems.filters | 3 + .../CTCatalog-WinStore10/CTCatalog.vcxproj | 1 + .../CTCatalog.vcxproj.filters | 3 + .../CTCatalog.xcodeproj/project.pbxproj | 6 + .../CTCatalog/CTCRootViewController.m | 4 +- ...TCAffineTransformationTestViewController.h | 22 ++ ...CAffineTransformationTestViewController.mm | 201 ++++++++++++++++++ .../Tests/CTCAlignmentTestViewController.m | 2 - .../Tests/CTCFontTestViewController.mm | 2 - .../Tests/CTCFrameTestViewController.mm | 2 - .../Tests/CTCFramesetterTestViewController.mm | 2 - .../Tests/CTCLineTestViewController.mm | 2 - .../CTCParagraphStyleTestViewController.mm | 2 - .../Tests/CTCRunTestViewController.mm | 3 - 17 files changed, 244 insertions(+), 18 deletions(-) create mode 100644 tests/testapps/CTCatalog/CTCatalog/Tests/CTCAffineTransformationTestViewController.h create mode 100644 tests/testapps/CTCatalog/CTCatalog/Tests/CTCAffineTransformationTestViewController.mm diff --git a/Frameworks/CoreGraphics/CGContextCairo.mm b/Frameworks/CoreGraphics/CGContextCairo.mm index a66b96cc94..335b61006f 100644 --- a/Frameworks/CoreGraphics/CGContextCairo.mm +++ b/Frameworks/CoreGraphics/CGContextCairo.mm @@ -1868,6 +1868,7 @@ // Undo transform to text space transform = CGAffineTransformConcat(CGAffineTransformMake(1, 0, 0, -1, 0, lineAscent / 2.0f), transform); + // Translate to the drawing point transform = CGAffineTransformTranslate(transform, curState->curTextPosition.x, curState->curTextPosition.y); // Find transform that user created by multiplying given transform by necessary transforms to draw with CoreText @@ -1875,11 +1876,11 @@ CGAffineTransform userTransform = CGAffineTransformConcat(curState->curTransform, CGAffineTransformMake(1.0f / _scale, 0, 0, 1.0f / _scale, 0, -height / _scale)); - // Apply the context CTM + // Apply the context CTM to get total matrix transform = CGAffineTransformConcat(transform, userTransform); // Perform anti-clockwise rotation required to match the reference platform. - imgRenderTarget->SetTransform(D2D1::Matrix3x2F(transform.a, -transform.b, transform.c, transform.d, transform.tx, -transform.ty)); + imgRenderTarget->SetTransform(D2D1::Matrix3x2F(transform.a, -transform.b, -transform.c, transform.d, transform.tx, -transform.ty)); // Draw the glyph using ID2D1RenderTarget ComPtr brush; diff --git a/Frameworks/UIKit/NSLayoutManager.mm b/Frameworks/UIKit/NSLayoutManager.mm index 757c06a21f..821b73e3fb 100644 --- a/Frameworks/UIKit/NSLayoutManager.mm +++ b/Frameworks/UIKit/NSLayoutManager.mm @@ -247,6 +247,7 @@ - (void)drawGlyphsForGlyphRange:(NSRange)range atPoint:(CGPoint)position { CTLineRef line = (CTLineRef)_ctLines[curLine]; CGContextSaveGState(curCtx); + // Ignore descent to keep from drawing descending characters from drawing into exclusion zones CGFloat ascent, leading; CTLineGetTypographicBounds(line, &ascent, nullptr, &leading); CGContextSetTextPosition(curCtx, _lineOrigins[curLine].x, -(_lineOrigins[curLine].y + ascent + leading)); diff --git a/tests/testapps/CTCatalog/CTCatalog.vsimporter/CTCatalog-Headers-WinStore10/CTCatalog-Headers.vcxitems b/tests/testapps/CTCatalog/CTCatalog.vsimporter/CTCatalog-Headers-WinStore10/CTCatalog-Headers.vcxitems index 445a3b19f9..934157aac1 100644 --- a/tests/testapps/CTCatalog/CTCatalog.vsimporter/CTCatalog-Headers-WinStore10/CTCatalog-Headers.vcxitems +++ b/tests/testapps/CTCatalog/CTCatalog.vsimporter/CTCatalog-Headers-WinStore10/CTCatalog-Headers.vcxitems @@ -10,6 +10,7 @@ + diff --git a/tests/testapps/CTCatalog/CTCatalog.vsimporter/CTCatalog-Headers-WinStore10/CTCatalog-Headers.vcxitems.filters b/tests/testapps/CTCatalog/CTCatalog.vsimporter/CTCatalog-Headers-WinStore10/CTCatalog-Headers.vcxitems.filters index 5d3f701e3f..fef0b0a787 100644 --- a/tests/testapps/CTCatalog/CTCatalog.vsimporter/CTCatalog-Headers-WinStore10/CTCatalog-Headers.vcxitems.filters +++ b/tests/testapps/CTCatalog/CTCatalog.vsimporter/CTCatalog-Headers-WinStore10/CTCatalog-Headers.vcxitems.filters @@ -39,5 +39,8 @@ CTCatalog\Test + + CTCatalog\Test + \ No newline at end of file diff --git a/tests/testapps/CTCatalog/CTCatalog.vsimporter/CTCatalog-WinStore10/CTCatalog.vcxproj b/tests/testapps/CTCatalog/CTCatalog.vsimporter/CTCatalog-WinStore10/CTCatalog.vcxproj index dee550b266..c0a577b0cd 100644 --- a/tests/testapps/CTCatalog/CTCatalog.vsimporter/CTCatalog-WinStore10/CTCatalog.vcxproj +++ b/tests/testapps/CTCatalog/CTCatalog.vsimporter/CTCatalog-WinStore10/CTCatalog.vcxproj @@ -177,6 +177,7 @@ Designer + diff --git a/tests/testapps/CTCatalog/CTCatalog.vsimporter/CTCatalog-WinStore10/CTCatalog.vcxproj.filters b/tests/testapps/CTCatalog/CTCatalog.vsimporter/CTCatalog-WinStore10/CTCatalog.vcxproj.filters index ca3c84923b..164f0187a0 100644 --- a/tests/testapps/CTCatalog/CTCatalog.vsimporter/CTCatalog-WinStore10/CTCatalog.vcxproj.filters +++ b/tests/testapps/CTCatalog/CTCatalog.vsimporter/CTCatalog-WinStore10/CTCatalog.vcxproj.filters @@ -110,5 +110,8 @@ CTCatalog + + CTCatalog\Test + \ No newline at end of file diff --git a/tests/testapps/CTCatalog/CTCatalog.xcodeproj/project.pbxproj b/tests/testapps/CTCatalog/CTCatalog.xcodeproj/project.pbxproj index bf820a201f..abc6424bdf 100644 --- a/tests/testapps/CTCatalog/CTCatalog.xcodeproj/project.pbxproj +++ b/tests/testapps/CTCatalog/CTCatalog.xcodeproj/project.pbxproj @@ -7,6 +7,7 @@ objects = { /* Begin PBXBuildFile section */ + 0005A0501DC7D76C00921DDB /* CTCAffineTransformationTestViewController.mm in Sources */ = {isa = PBXBuildFile; fileRef = 0005A04F1DC7D76C00921DDB /* CTCAffineTransformationTestViewController.mm */; }; 002AB41B1D6CA1F1002082A5 /* CTCFontTestViewController.mm in Sources */ = {isa = PBXBuildFile; fileRef = 002AB4101D6CA1F1002082A5 /* CTCFontTestViewController.mm */; }; 002AB41C1D6CA1F1002082A5 /* CTCFramesetterTestViewController.mm in Sources */ = {isa = PBXBuildFile; fileRef = 002AB4121D6CA1F1002082A5 /* CTCFramesetterTestViewController.mm */; }; 002AB41D1D6CA1F1002082A5 /* CTCFrameTestViewController.mm in Sources */ = {isa = PBXBuildFile; fileRef = 002AB4141D6CA1F1002082A5 /* CTCFrameTestViewController.mm */; }; @@ -24,6 +25,8 @@ /* End PBXBuildFile section */ /* Begin PBXFileReference section */ + 0005A04E1DC7D76C00921DDB /* CTCAffineTransformationTestViewController.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = CTCAffineTransformationTestViewController.h; path = Tests/CTCAffineTransformationTestViewController.h; sourceTree = ""; }; + 0005A04F1DC7D76C00921DDB /* CTCAffineTransformationTestViewController.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = CTCAffineTransformationTestViewController.mm; path = Tests/CTCAffineTransformationTestViewController.mm; sourceTree = ""; }; 002AB40F1D6CA1F1002082A5 /* CTCFontTestViewController.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = CTCFontTestViewController.h; path = Tests/CTCFontTestViewController.h; sourceTree = ""; }; 002AB4101D6CA1F1002082A5 /* CTCFontTestViewController.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = CTCFontTestViewController.mm; path = Tests/CTCFontTestViewController.mm; sourceTree = ""; }; 002AB4111D6CA1F1002082A5 /* CTCFramesetterTestViewController.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = CTCFramesetterTestViewController.h; path = Tests/CTCFramesetterTestViewController.h; sourceTree = ""; }; @@ -109,6 +112,8 @@ 002E8DAD1D59314E00D5DC64 /* Test */ = { isa = PBXGroup; children = ( + 0005A04E1DC7D76C00921DDB /* CTCAffineTransformationTestViewController.h */, + 0005A04F1DC7D76C00921DDB /* CTCAffineTransformationTestViewController.mm */, 002AB40F1D6CA1F1002082A5 /* CTCFontTestViewController.h */, 002AB4101D6CA1F1002082A5 /* CTCFontTestViewController.mm */, 002AB4111D6CA1F1002082A5 /* CTCFramesetterTestViewController.h */, @@ -198,6 +203,7 @@ buildActionMask = 2147483647; files = ( 002AB41F1D6CA1F1002082A5 /* CTCParagraphStyleTestViewController.mm in Sources */, + 0005A0501DC7D76C00921DDB /* CTCAffineTransformationTestViewController.mm in Sources */, 002AB41C1D6CA1F1002082A5 /* CTCFramesetterTestViewController.mm in Sources */, 002AB41D1D6CA1F1002082A5 /* CTCFrameTestViewController.mm in Sources */, 002E8D9A1D592A3500D5DC64 /* CTCRootViewController.m in Sources */, diff --git a/tests/testapps/CTCatalog/CTCatalog/CTCRootViewController.m b/tests/testapps/CTCatalog/CTCatalog/CTCRootViewController.m index 3c0332e34c..7efa8bd7ab 100644 --- a/tests/testapps/CTCatalog/CTCatalog/CTCRootViewController.m +++ b/tests/testapps/CTCatalog/CTCatalog/CTCRootViewController.m @@ -22,6 +22,7 @@ #import "CTCFrameTestViewController.h" #import "CTCParagraphStyleTestViewController.h" #import "CTCFramesetterTestViewController.h" +#import "CTCAffineTransformationTestViewController.h" @interface TestRow : NSObject @@ -59,7 +60,8 @@ - (NSArray*)tests { [TestRow row:@"Frame Functions" testClass:[CTCFrameTestViewController class]], [TestRow row:@"ParagraphStyle Tests" testClass:[CTCParagraphStyleTestViewController class]], [TestRow row:@"Line Functions" testClass:[CTCLineTestViewController class]], - [TestRow row:@"Framesetter Functions" testClass:[CTCFramesetterTestViewController class]] + [TestRow row:@"Framesetter Functions" testClass:[CTCFramesetterTestViewController class]], + [TestRow row:@"Affine Transformation Tests" testClass:[CTCAffineTransformationTestViewController class]] ]; } return _tests; diff --git a/tests/testapps/CTCatalog/CTCatalog/Tests/CTCAffineTransformationTestViewController.h b/tests/testapps/CTCatalog/CTCatalog/Tests/CTCAffineTransformationTestViewController.h new file mode 100644 index 0000000000..cde1501647 --- /dev/null +++ b/tests/testapps/CTCatalog/CTCatalog/Tests/CTCAffineTransformationTestViewController.h @@ -0,0 +1,22 @@ +//****************************************************************************** +// +// 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. +// +//***************************************************************************** + +#pragma once + +#import "CTCBaseViewController.h" + +@interface CTCAffineTransformationTestViewController : CTCBaseViewController +@end \ No newline at end of file diff --git a/tests/testapps/CTCatalog/CTCatalog/Tests/CTCAffineTransformationTestViewController.mm b/tests/testapps/CTCatalog/CTCatalog/Tests/CTCAffineTransformationTestViewController.mm new file mode 100644 index 0000000000..0b537e866b --- /dev/null +++ b/tests/testapps/CTCatalog/CTCatalog/Tests/CTCAffineTransformationTestViewController.mm @@ -0,0 +1,201 @@ +//****************************************************************************** +// +// 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. +// +//***************************************************************************** + +#import "CTCAffineTransformationTestViewController.h" + +static const NSString* text = @"This frame is manipulated by the above sliders. The leftmost slider changes rotation, which should be " + @"about the origin in the bottom-left of this frame with left being clockwise rotation and right being " + @"counterclockwise. The middle sliders change the X and Y scale respectively. The rightmost sliders " + @"change the position of the X, Y origins of the drawn frame respectively."; + +@interface CTAffineTransformationTestView : UIView { +} + +@property (nonatomic) CGFloat rotationCTM; +@property (nonatomic) CGPoint translationCTM; +@property (nonatomic) CGPoint scaleCTM; + +@end + +@implementation CTAffineTransformationTestView { +} + +- (void)drawRect:(CGRect)rect { + UIColor* color = [UIColor blackColor]; + + CGContextRef context = UIGraphicsGetCurrentContext(); + + // Aligns origin for our frame + CGContextTranslateCTM(context, 0.0f, self.bounds.size.height); + + // Flips y-axis for our frame + CGContextScaleCTM(context, 1.0f, -1.0f); + + // Apply user given transformations + CGContextRotateCTM(context, _rotationCTM); + CGContextScaleCTM(context, _scaleCTM.x, _scaleCTM.y); + CGContextTranslateCTM(context, _translationCTM.x, _translationCTM.y); + + // Creates path with current rectangle + CGMutablePathRef path = CGPathCreateMutable(); + CGPathAddRect(path, NULL, self.bounds); + + // Create style setting to match given alignment + CTParagraphStyleSetting setting; + + setting.spec = kCTParagraphStyleSpecifierAlignment; + setting.valueSize = sizeof(CTTextAlignment); + CTTextAlignment alignment = kCTLeftTextAlignment; + setting.value = &alignment; + CTParagraphStyleRef paragraphStyle = CTParagraphStyleCreate(&setting, 1); + CFAutorelease(paragraphStyle); + + UIFont* font = [UIFont systemFontOfSize:20]; + CTFontRef myCFFont = CTFontCreateWithName((__bridge CFStringRef)[font fontName], [font pointSize], NULL); + CFAutorelease(myCFFont); + // Make dictionary for attributed string with font, color, and alignment + NSDictionary* attributesDict = [NSDictionary dictionaryWithObjectsAndKeys:(__bridge id)myCFFont, + (id)kCTFontAttributeName, + color.CGColor, + (id)kCTForegroundColorAttributeName, + (__bridge id)paragraphStyle, + (id)kCTParagraphStyleAttributeName, + nil]; + + CFAttributedStringRef attrString = + CFAttributedStringCreate(kCFAllocatorDefault, (__bridge CFStringRef)text, (__bridge CFDictionaryRef)attributesDict); + CFAutorelease(attrString); + + CTFramesetterRef framesetter = CTFramesetterCreateWithAttributedString(attrString); + CFAutorelease(framesetter); + + // Creates frame for framesetter with current attributed string + CTFrameRef frame = CTFramesetterCreateFrame(framesetter, CFRangeMake(0, 0), path, NULL); + CFAutorelease(frame); + + // Draws the text in the frame + CTFrameDraw(frame, context); + + // Creates outline + CGContextSetLineWidth(context, 2.0); + CGContextSetStrokeColorWithColor(context, color.CGColor); + CGContextMoveToPoint(context, 0, 0); + CGContextAddRect(context, rect); + CGContextStrokePath(context); + + CGPathRelease(path); +} + +@end + +@implementation CTCAffineTransformationTestViewController { + CTAffineTransformationTestView* _textView; + UISlider* _rotationSlider; + UISlider* _scaleXSlider; + UISlider* _scaleYSlider; + UISlider* _translateXSlider; + UISlider* _translateYSlider; +} + +- (void)viewDidLoad { + [super viewDidLoad]; + self.view.backgroundColor = [UIColor whiteColor]; + CGFloat width = CGRectGetWidth(self.view.bounds); + + _rotationSlider = [[UISlider alloc] initWithFrame:CGRectMake(0, 10, width / 4, 50)]; + _rotationSlider.minimumValue = -2.0f * M_PI; + _rotationSlider.maximumValue = 2.0f * M_PI; + _rotationSlider.value = 0.0f; + _rotationSlider.continuous = YES; + [_rotationSlider addTarget:self action:@selector(drawTests) forControlEvents:UIControlEventValueChanged]; + [self.view addSubview:_rotationSlider]; + + _scaleXSlider = [[UISlider alloc] initWithFrame:CGRectMake(width / 3, 10, width / 4, 50)]; + _scaleXSlider.minimumValue = -2.0f; + _scaleXSlider.maximumValue = 2.0f; + _scaleXSlider.value = 1.0f; + _scaleXSlider.continuous = YES; + [_scaleXSlider addTarget:self action:@selector(drawTests) forControlEvents:UIControlEventValueChanged]; + [self.view addSubview:_scaleXSlider]; + + _scaleYSlider = [[UISlider alloc] initWithFrame:CGRectMake(width / 3, 70, width / 4, 50)]; + _scaleYSlider.minimumValue = -2.0f; + _scaleYSlider.maximumValue = 2.0f; + _scaleYSlider.value = 1.0f; + _scaleYSlider.continuous = YES; + [_scaleYSlider addTarget:self action:@selector(drawTests) forControlEvents:UIControlEventValueChanged]; + [self.view addSubview:_scaleYSlider]; + + _translateXSlider = [[UISlider alloc] initWithFrame:CGRectMake(2.0f * width / 3, 10, width / 4, 50)]; + _translateXSlider.minimumValue = -200; + _translateXSlider.maximumValue = 200; + _translateXSlider.value = 0.0f; + _translateXSlider.continuous = YES; + [_translateXSlider addTarget:self action:@selector(drawTests) forControlEvents:UIControlEventValueChanged]; + [self.view addSubview:_translateXSlider]; + + _translateYSlider = [[UISlider alloc] initWithFrame:CGRectMake(2.0f * width / 3, 70, width / 4, 50)]; + _translateYSlider.minimumValue = -200; + _translateYSlider.maximumValue = 200; + _translateYSlider.value = 0.0f; + _translateYSlider.continuous = YES; + [_translateYSlider addTarget:self action:@selector(drawTests) forControlEvents:UIControlEventValueChanged]; + [self.view addSubview:_translateYSlider]; + + // Create frame of text + _textView = [[CTAffineTransformationTestView alloc] initWithFrame:CGRectMake(width / 4, 150, width / 2, 200)]; + _textView.backgroundColor = [UIColor whiteColor]; + _textView.autoresizingMask = UIViewAutoresizingFlexibleWidth; + // Sets view to call updateTableViews when done drawing + _textView.rotationCTM = 0.0f; + _textView.scaleCTM = { 1.0f, 1.0f }; + _textView.translationCTM = { 0.0f, 0.0f }; + [self.view addSubview:_textView]; + + // Draws the frameview + [self drawTests]; +} + +- (void)viewDidLayoutSubviews { + CGFloat width = CGRectGetWidth(self.view.bounds); + _textView.frame = CGRectMake(width / 4, 150, width / 2, 200); + [_textView setNeedsDisplay]; + + _rotationSlider.frame = CGRectMake(0, 10, width / 4, 50); + [_rotationSlider setNeedsDisplay]; + + _scaleXSlider.frame = CGRectMake(width / 3, 10, width / 4, 50); + [_scaleXSlider setNeedsDisplay]; + + _scaleYSlider.frame = CGRectMake(width / 3, 70, width / 4, 50); + [_scaleYSlider setNeedsDisplay]; + + _translateXSlider.frame = CGRectMake(2.0f * width / 3, 10, width / 4, 50); + [_translateXSlider setNeedsDisplay]; + + _translateYSlider.frame = CGRectMake(2.0f * width / 3, 70, width / 4, 50); + [_translateYSlider setNeedsDisplay]; +} + +- (void)drawTests { + // Update the text in the frame + // Allows input of \n and \t to insert newlines and tabs respectively + _textView.rotationCTM = _rotationSlider.value; + _textView.scaleCTM = { _scaleXSlider.value, _scaleYSlider.value }; + _textView.translationCTM = { _translateXSlider.value, _translateYSlider.value }; + [_textView setNeedsDisplay]; +} +@end \ No newline at end of file diff --git a/tests/testapps/CTCatalog/CTCatalog/Tests/CTCAlignmentTestViewController.m b/tests/testapps/CTCatalog/CTCatalog/Tests/CTCAlignmentTestViewController.m index f5b5473f35..c65c1a5313 100644 --- a/tests/testapps/CTCatalog/CTCatalog/Tests/CTCAlignmentTestViewController.m +++ b/tests/testapps/CTCatalog/CTCatalog/Tests/CTCAlignmentTestViewController.m @@ -80,14 +80,12 @@ - (void)drawRect:(CGRect)rect { // Creates outline CGContextSetLineWidth(context, 2.0); - CGColorSpaceRef colorspace = CGColorSpaceCreateDeviceRGB(); CGContextSetStrokeColorWithColor(context, color.CGColor); CGContextMoveToPoint(context, 0, 0); CGContextAddRect(context, rect); CGContextStrokePath(context); CGPathRelease(path); - CGColorSpaceRelease(colorspace); } @end diff --git a/tests/testapps/CTCatalog/CTCatalog/Tests/CTCFontTestViewController.mm b/tests/testapps/CTCatalog/CTCatalog/Tests/CTCFontTestViewController.mm index f252e13d59..266b808d8f 100644 --- a/tests/testapps/CTCatalog/CTCatalog/Tests/CTCFontTestViewController.mm +++ b/tests/testapps/CTCatalog/CTCatalog/Tests/CTCFontTestViewController.mm @@ -93,13 +93,11 @@ - (void)drawRect:(CGRect)rect { // Creates outline CGContextSetLineWidth(context, 2.0); - CGColorSpaceRef colorspace = CGColorSpaceCreateDeviceRGB(); CGContextSetStrokeColorWithColor(context, color.CGColor); CGContextMoveToPoint(context, 0, 0); CGContextAddRect(context, rect); CGContextStrokePath(context); - CGColorSpaceRelease(colorspace); CGPathRelease(path); } @end diff --git a/tests/testapps/CTCatalog/CTCatalog/Tests/CTCFrameTestViewController.mm b/tests/testapps/CTCatalog/CTCatalog/Tests/CTCFrameTestViewController.mm index 834e8da7d9..4f4e371890 100644 --- a/tests/testapps/CTCatalog/CTCatalog/Tests/CTCFrameTestViewController.mm +++ b/tests/testapps/CTCatalog/CTCatalog/Tests/CTCFrameTestViewController.mm @@ -119,14 +119,12 @@ - (void)drawRect:(CGRect)rect { // Creates outline CGContextSetLineWidth(context, 2.0); - CGColorSpaceRef colorspace = CGColorSpaceCreateDeviceRGB(); CGContextSetStrokeColorWithColor(context, color.CGColor); CGContextMoveToPoint(context, 0, 0); CGContextAddRect(context, rect); CGContextStrokePath(context); CGPathRelease(path); - CGColorSpaceRelease(colorspace); [_drawDelegate refreshValuesForFrame:frame]; } diff --git a/tests/testapps/CTCatalog/CTCatalog/Tests/CTCFramesetterTestViewController.mm b/tests/testapps/CTCatalog/CTCatalog/Tests/CTCFramesetterTestViewController.mm index 480698268d..1fa9e13892 100644 --- a/tests/testapps/CTCatalog/CTCatalog/Tests/CTCFramesetterTestViewController.mm +++ b/tests/testapps/CTCatalog/CTCatalog/Tests/CTCFramesetterTestViewController.mm @@ -80,14 +80,12 @@ - (void)drawRect:(CGRect)rect { // Creates outline CGContextSetLineWidth(context, 2.0); - CGColorSpaceRef colorspace = CGColorSpaceCreateDeviceRGB(); CGContextSetStrokeColorWithColor(context, color.CGColor); CGContextMoveToPoint(context, 0, 0); CGContextAddRect(context, rect); CGContextStrokePath(context); CGPathRelease(path); - CGColorSpaceRelease(colorspace); [_drawDelegate refreshValuesForFramesetter:framesetter]; } diff --git a/tests/testapps/CTCatalog/CTCatalog/Tests/CTCLineTestViewController.mm b/tests/testapps/CTCatalog/CTCatalog/Tests/CTCLineTestViewController.mm index 0c92ef0769..63af8d45c2 100644 --- a/tests/testapps/CTCatalog/CTCatalog/Tests/CTCLineTestViewController.mm +++ b/tests/testapps/CTCatalog/CTCatalog/Tests/CTCLineTestViewController.mm @@ -109,13 +109,11 @@ - (void)drawRect:(CGRect)rect { // Creates outline CGContextSetLineWidth(context, 2.0); - CGColorSpaceRef colorspace = CGColorSpaceCreateDeviceRGB(); CGContextSetStrokeColorWithColor(context, color.CGColor); CGContextMoveToPoint(context, 0, 0); CGContextAddRect(context, rect); CGContextStrokePath(context); - CGColorSpaceRelease(colorspace); [_drawDelegate refreshValuesForLine:line]; } diff --git a/tests/testapps/CTCatalog/CTCatalog/Tests/CTCParagraphStyleTestViewController.mm b/tests/testapps/CTCatalog/CTCatalog/Tests/CTCParagraphStyleTestViewController.mm index 904e8013b7..5b9b01b577 100644 --- a/tests/testapps/CTCatalog/CTCatalog/Tests/CTCParagraphStyleTestViewController.mm +++ b/tests/testapps/CTCatalog/CTCatalog/Tests/CTCParagraphStyleTestViewController.mm @@ -95,14 +95,12 @@ - (void)drawRect:(CGRect)rect { // Creates outline CGContextSetLineWidth(context, 2.0); - CGColorSpaceRef colorspace = CGColorSpaceCreateDeviceRGB(); CGContextSetStrokeColorWithColor(context, color.CGColor); CGContextMoveToPoint(context, 0, 0); CGContextAddRect(context, rect); CGContextStrokePath(context); CGPathRelease(path); - CGColorSpaceRelease(colorspace); CFRelease(_paragraphStyle); } diff --git a/tests/testapps/CTCatalog/CTCatalog/Tests/CTCRunTestViewController.mm b/tests/testapps/CTCatalog/CTCatalog/Tests/CTCRunTestViewController.mm index 474b6ae97c..768853f85f 100644 --- a/tests/testapps/CTCatalog/CTCatalog/Tests/CTCRunTestViewController.mm +++ b/tests/testapps/CTCatalog/CTCatalog/Tests/CTCRunTestViewController.mm @@ -89,14 +89,11 @@ - (void)drawRect:(CGRect)rect { // Creates outline CGContextSetLineWidth(context, 2.0); - CGColorSpaceRef colorspace = CGColorSpaceCreateDeviceRGB(); CGContextSetStrokeColorWithColor(context, color.CGColor); CGContextMoveToPoint(context, 0, 0); CGContextAddRect(context, rect); CGContextStrokePath(context); - CGColorSpaceRelease(colorspace); - [_drawDelegate refreshValuesForRun:run]; } From 3d0223c056e70da5f0911033083604715f6575e4 Mon Sep 17 00:00:00 2001 From: Raj Seshasankaran Date: Wed, 2 Nov 2016 11:59:44 -0700 Subject: [PATCH 2/6] Fixing heap corruption when using core text to draw into opaque layers. (#1295) The heap corruption happened in an internal test app. The fix is to use BGRX pixel format for opaque layers - this is supported by WIC render target. --- Frameworks/CoreGraphics/CGBitmapImage.mm | 9 +++++---- Frameworks/QuartzCore/CALayer.mm | 2 +- Frameworks/include/CGIWICBitmap.h | 3 ++- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/Frameworks/CoreGraphics/CGBitmapImage.mm b/Frameworks/CoreGraphics/CGBitmapImage.mm index 7bb265bc29..914ceefa16 100644 --- a/Frameworks/CoreGraphics/CGBitmapImage.mm +++ b/Frameworks/CoreGraphics/CGBitmapImage.mm @@ -389,6 +389,11 @@ } CGBitmapImageBacking::~CGBitmapImageBacking() { + // release the render target first as it may hold locks on the image + if (_renderTarget != nullptr) { + _renderTarget->Release(); + } + if (_cairoLocks != 0 || _imageLocks != 0) { TraceWarning(TAG, L"Warning: Image data not unlocked (refcnt=%d, %d)", _cairoLocks, _imageLocks); @@ -400,10 +405,6 @@ } } - if (_renderTarget != nullptr) { - _renderTarget->Release(); - } - _data->_refCount--; if (_data->_refCount == 0) { diff --git a/Frameworks/QuartzCore/CALayer.mm b/Frameworks/QuartzCore/CALayer.mm index c65f9b93b3..56f520d30f 100644 --- a/Frameworks/QuartzCore/CALayer.mm +++ b/Frameworks/QuartzCore/CALayer.mm @@ -574,7 +574,7 @@ - (void)display { if (useVector) { // target = new CGVectorImage(width, height, _ColorBGR); } else { - drawContext = _CGBitmapContextCreateWithFormat(width, height, _ColorBGR); + drawContext = _CGBitmapContextCreateWithFormat(width, height, _ColorBGRX); } priv->drewOpaque = TRUE; } else { diff --git a/Frameworks/include/CGIWICBitmap.h b/Frameworks/include/CGIWICBitmap.h index e4ec9e8587..37a4eef038 100644 --- a/Frameworks/include/CGIWICBitmap.h +++ b/Frameworks/include/CGIWICBitmap.h @@ -81,12 +81,13 @@ inline WICPixelFormatGUID SurfaceFormatToWICPixelFormat(__CGSurfaceFormat format case _ColorARGB: return GUID_WICPixelFormat32bppPBGRA; case _ColorBGRX: - case _ColorBGR: return GUID_WICPixelFormat32bppBGR; case _ColorGrayscale: case _ColorA8: return GUID_WICPixelFormat8bppAlpha; + case _ColorBGR: default: + UNIMPLEMENTED_WITH_MSG("Unsupported pixel format (%d) used in CGIWICBitmap"); break; } // our default format is alpha premultiplied BGRA From 9599c47a3149773eaf21faef2d45dd36cfe17a66 Mon Sep 17 00:00:00 2001 From: AJ Ballway Date: Wed, 2 Nov 2016 15:12:14 -0700 Subject: [PATCH 3/6] Fix CoreText Drawing (#1297) It seems our drawing implementation was overcomplicated, doing unnecessary transformations which made assumptions about the state of the context which were not always true. Removes now unnecessary private drawing functions and removes ascent as well as simplifying CGContextDrawGlyphRun. Fixes #1290 --- Frameworks/CoreGraphics/CGContext.mm | 4 +-- Frameworks/CoreGraphics/CGContextCairo.mm | 37 ++++++++------------- Frameworks/CoreGraphics/CGContextImpl.mm | 2 +- Frameworks/CoreText/CTFrame.mm | 3 +- Frameworks/CoreText/CTLine.mm | 26 ++++----------- Frameworks/CoreText/CTRun.mm | 27 ++++----------- Frameworks/UIKit/NSLayoutManager.mm | 6 +--- Frameworks/UIKit/NSString+UIKitAdditions.mm | 5 +-- Frameworks/include/CGContextCairo.h | 2 +- Frameworks/include/CGContextImpl.h | 2 +- Frameworks/include/CGContextInternal.h | 2 +- Frameworks/include/CoreTextInternal.h | 6 ---- 12 files changed, 38 insertions(+), 84 deletions(-) diff --git a/Frameworks/CoreGraphics/CGContext.mm b/Frameworks/CoreGraphics/CGContext.mm index 288041dc4f..1274921423 100644 --- a/Frameworks/CoreGraphics/CGContext.mm +++ b/Frameworks/CoreGraphics/CGContext.mm @@ -1239,8 +1239,8 @@ bool CGContextIsPointInPath(CGContextRef c, bool eoFill, CGFloat x, CGFloat y) { return c->Backing()->CGContextIsPointInPath(eoFill, x, y); } -void CGContextDrawGlyphRun(CGContextRef ctx, const DWRITE_GLYPH_RUN* glyphRun, float lineAscent) { - ctx->Backing()->CGContextDrawGlyphRun(glyphRun, lineAscent); +void CGContextDrawGlyphRun(CGContextRef ctx, const DWRITE_GLYPH_RUN* glyphRun) { + ctx->Backing()->CGContextDrawGlyphRun(glyphRun); } // TODO 1077:: Remove once D2D render target is implemented void _CGContextSetScaleFactor(CGContextRef ctx, float scale) { diff --git a/Frameworks/CoreGraphics/CGContextCairo.mm b/Frameworks/CoreGraphics/CGContextCairo.mm index 335b61006f..b5596396ba 100644 --- a/Frameworks/CoreGraphics/CGContextCairo.mm +++ b/Frameworks/CoreGraphics/CGContextCairo.mm @@ -1832,7 +1832,7 @@ * * @parameter glyphRun DWRITE_GLYPH_RUN object to render */ -void CGContextCairo::CGContextDrawGlyphRun(const DWRITE_GLYPH_RUN* glyphRun, float lineAscent) { +void CGContextCairo::CGContextDrawGlyphRun(const DWRITE_GLYPH_RUN* glyphRun) { ObtainLock(); CGContextStrokePath(); @@ -1853,34 +1853,25 @@ // Apply the text transformation (text position, text matrix) in text space rather than user space // This means flipping the coordinate system, - // and applying the transformation about the center of the glyph run rather than about the baseline - // Flip and translate by the difference between the center and the baseline, apply text transforms, then flip and translate back + // Apply text position, where it will be translated to correct position given text matrix value + CGAffineTransform textTransform = + CGAffineTransformTranslate(curState->curTextMatrix, curState->curTextPosition.x, curState->curTextPosition.y); - // Transform to text space - // Technically there should be a horizontal translation to the center as well, - // but it's to the center of _each individual glyph_, as the reference platform applies the text matrix to each glyph individually - // Uncertain whether it's ever going to be worth it to support this using DWrite, so just ignore it for now - CGAffineTransform transform = CGAffineTransformMake(1, 0, 0, -1, 0, -lineAscent / 2.0f); - - // Apply text transforms - transform = CGAffineTransformConcat(curState->curTextMatrix, transform); - - // Undo transform to text space - transform = CGAffineTransformConcat(CGAffineTransformMake(1, 0, 0, -1, 0, lineAscent / 2.0f), transform); - - // Translate to the drawing point - transform = CGAffineTransformTranslate(transform, curState->curTextPosition.x, curState->curTextPosition.y); + // Undo assumed inversion about Y axis + textTransform = CGAffineTransformConcat(CGAffineTransformMake(1, 0, 0, -1, 0, 0), textTransform); // Find transform that user created by multiplying given transform by necessary transforms to draw with CoreText + // First multiply by inverse scale to get properly scaled values + // Then undo assumed inversion about Y axis + // Finally inverse translate by height + // All of which are rolled into one concatenation float height = _imgDest->Backing()->Height(); CGAffineTransform userTransform = - CGAffineTransformConcat(curState->curTransform, CGAffineTransformMake(1.0f / _scale, 0, 0, 1.0f / _scale, 0, -height / _scale)); - - // Apply the context CTM to get total matrix - transform = CGAffineTransformConcat(transform, userTransform); + CGAffineTransformConcat(curState->curTransform, CGAffineTransformMake(1.0f / _scale, 0, 0, -1.0f / _scale, 0, height / _scale)); - // Perform anti-clockwise rotation required to match the reference platform. - imgRenderTarget->SetTransform(D2D1::Matrix3x2F(transform.a, -transform.b, -transform.c, transform.d, transform.tx, -transform.ty)); + // Apply the two transforms giving us the final result + CGAffineTransform transform = CGAffineTransformConcat(textTransform, userTransform); + imgRenderTarget->SetTransform(D2D1::Matrix3x2F(transform.a, transform.b, transform.c, transform.d, transform.tx, transform.ty)); // Draw the glyph using ID2D1RenderTarget ComPtr brush; diff --git a/Frameworks/CoreGraphics/CGContextImpl.mm b/Frameworks/CoreGraphics/CGContextImpl.mm index f5bc27aed9..a5e402607c 100644 --- a/Frameworks/CoreGraphics/CGContextImpl.mm +++ b/Frameworks/CoreGraphics/CGContextImpl.mm @@ -779,7 +779,7 @@ return NULL; } -void CGContextImpl::CGContextDrawGlyphRun(const DWRITE_GLYPH_RUN* glyphRun, float lineAscent) { +void CGContextImpl::CGContextDrawGlyphRun(const DWRITE_GLYPH_RUN* glyphRun) { } // TODO 1077:: Remove once D2D render target is implemented diff --git a/Frameworks/CoreText/CTFrame.mm b/Frameworks/CoreText/CTFrame.mm index 17dddafda3..785102e056 100644 --- a/Frameworks/CoreText/CTFrame.mm +++ b/Frameworks/CoreText/CTFrame.mm @@ -122,8 +122,9 @@ void CTFrameDraw(CTFrameRef frameRef, CGContextRef ctx) { CGContextScaleCTM(ctx, 1.0f, -1.0f); for (_CTLine* line in static_cast>(frame->_lines)) { + // Y position must be negative because the context is inverted CGContextSetTextPosition(ctx, line->_lineOrigin.x, -line->_lineOrigin.y); - _CTLineDraw(static_cast(line), ctx, false); + CTLineDraw(static_cast(line), ctx); } // Restore CTM and Text Matrix to values before we modified them diff --git a/Frameworks/CoreText/CTLine.mm b/Frameworks/CoreText/CTLine.mm index c9e6cce18b..a69d72ec04 100644 --- a/Frameworks/CoreText/CTLine.mm +++ b/Frameworks/CoreText/CTLine.mm @@ -215,40 +215,28 @@ CTLineRef CTLineCreateJustifiedLine(CTLineRef line, CGFloat justificationFactor, return StubReturn(); } -void _CTLineDraw(CTLineRef lineRef, CGContextRef ctx, bool adjustTextPosition) { - if (!lineRef) { +/** + @Status Interoperable +*/ +void CTLineDraw(CTLineRef lineRef, CGContextRef ctx) { + if (lineRef == nil || ctx == nil) { return; } _CTLine* line = static_cast<_CTLine*>(lineRef); - CGPoint curTextPos = {}; - if (adjustTextPosition) { - curTextPos = CGContextGetTextPosition(ctx); - CGContextSetTextPosition(ctx, curTextPos.x, curTextPos.y + line->_relativeYOffset); - } for (size_t i = 0; i < [line->_runs count]; ++i) { _CTRun* curRun = [line->_runs objectAtIndex:i]; if (i > 0) { // Adjusts x position relative to the last run drawn - curTextPos = CGContextGetTextPosition(ctx); + CGPoint curTextPos = CGContextGetTextPosition(ctx); CGContextSetTextPosition(ctx, curTextPos.x + curRun->_relativeXOffset, curTextPos.y); } - // Get height of the line so we draw with the correct baseline for each run - CGFloat ascent; - CTLineGetTypographicBounds(lineRef, &ascent, nullptr, nullptr); - _CTRunDraw(static_cast(curRun), ctx, CFRange{}, false, ascent); + CTRunDraw(static_cast(curRun), ctx, CFRange{}); } } -/** - @Status Interoperable -*/ -void CTLineDraw(CTLineRef lineRef, CGContextRef ctx) { - _CTLineDraw(lineRef, ctx, true); -} - /** @Status Interoperable */ diff --git a/Frameworks/CoreText/CTRun.mm b/Frameworks/CoreText/CTRun.mm index 539c64cab8..e12126b98c 100644 --- a/Frameworks/CoreText/CTRun.mm +++ b/Frameworks/CoreText/CTRun.mm @@ -268,18 +268,17 @@ CGRect CTRunGetImageBounds(CTRunRef run, CGContextRef context, CFRange range) { return StubReturn(); } -void _CTRunDraw(CTRunRef run, CGContextRef ctx, CFRange textRange, bool adjustTextPosition, CGFloat lineAscent) { +/** + @Status Interoperable + @Notes +*/ +void CTRunDraw(CTRunRef run, CGContextRef ctx, CFRange textRange) { _CTRun* curRun = static_cast<_CTRun*>(run); if (!curRun || textRange.length < 0L || textRange.location < 0L || textRange.location + textRange.length > curRun->_dwriteGlyphRun.glyphCount) { return; } - if (adjustTextPosition) { - CGPoint curTextPos = CGContextGetTextPosition(ctx); - CGContextSetTextPosition(ctx, curTextPos.x, curTextPos.y + curRun->_relativeYOffset); - } - id fontColor = [curRun->_attributes objectForKey:(id)kCTForegroundColorAttributeName]; if (fontColor == nil) { CFBooleanRef useContextColor = @@ -294,7 +293,7 @@ void _CTRunDraw(CTRunRef run, CGContextRef ctx, CFRange textRange, bool adjustTe if (textRange.location == 0L && (textRange.length == 0L || textRange.length == curRun->_dwriteGlyphRun.glyphCount)) { // Print the whole glyph run - CGContextDrawGlyphRun(ctx, &curRun->_dwriteGlyphRun, lineAscent); + CGContextDrawGlyphRun(ctx, &curRun->_dwriteGlyphRun); } else { if (textRange.length == 0L) { textRange.length = curRun->_dwriteGlyphRun.glyphCount - textRange.location; @@ -302,19 +301,7 @@ void _CTRunDraw(CTRunRef run, CGContextRef ctx, CFRange textRange, bool adjustTe // Only print glyphs in range DWRITE_GLYPH_RUN runInRange = __GetGlyphRunForDrawingInRange(curRun->_dwriteGlyphRun, textRange); - CGContextDrawGlyphRun(ctx, &runInRange, lineAscent); - } -} - -/** - @Status Interoperable - @Notes -*/ -void CTRunDraw(CTRunRef run, CGContextRef ctx, CFRange textRange) { - if (run && ctx) { - CGFloat ascent; - CTRunGetTypographicBounds(run, {}, &ascent, nullptr, nullptr); - _CTRunDraw(run, ctx, textRange, true, ascent); + CGContextDrawGlyphRun(ctx, &runInRange); } } diff --git a/Frameworks/UIKit/NSLayoutManager.mm b/Frameworks/UIKit/NSLayoutManager.mm index 821b73e3fb..780db64234 100644 --- a/Frameworks/UIKit/NSLayoutManager.mm +++ b/Frameworks/UIKit/NSLayoutManager.mm @@ -246,11 +246,7 @@ - (void)drawGlyphsForGlyphRange:(NSRange)range atPoint:(CGPoint)position { for (int curLine = 0; curLine < count; curLine++) { CTLineRef line = (CTLineRef)_ctLines[curLine]; CGContextSaveGState(curCtx); - - // Ignore descent to keep from drawing descending characters from drawing into exclusion zones - CGFloat ascent, leading; - CTLineGetTypographicBounds(line, &ascent, nullptr, &leading); - CGContextSetTextPosition(curCtx, _lineOrigins[curLine].x, -(_lineOrigins[curLine].y + ascent + leading)); + CGContextSetTextPosition(curCtx, _lineOrigins[curLine].x, -_lineOrigins[curLine].y); CTLineDraw(line, curCtx); CGContextRestoreGState(curCtx); } diff --git a/Frameworks/UIKit/NSString+UIKitAdditions.mm b/Frameworks/UIKit/NSString+UIKitAdditions.mm index 5ade2825fd..fa0b387513 100644 --- a/Frameworks/UIKit/NSString+UIKitAdditions.mm +++ b/Frameworks/UIKit/NSString+UIKitAdditions.mm @@ -104,10 +104,7 @@ static void drawString(UIFont* font, for (size_t i = 0; i < origins.size(); ++i) { // Need to set text position so each line will be drawn in the correct position relative to each other // Y positions will be negative because we are drawing with the coordinate system flipped to what CoreText is expecting - // Translated down by lineheight (ascent - descent + leading) to set origin in the correct position - CGFloat ascent, descent, leading; - CTLineGetTypographicBounds(static_cast(lines[i]), &ascent, &descent, &leading); - CGContextSetTextPosition(context, rect.origin.x + origins[i].x, -(rect.origin.y + origins[i].y + ascent - descent + leading)); + CGContextSetTextPosition(context, rect.origin.x + origins[i].x, -(rect.origin.y + origins[i].y)); CTLineDraw(static_cast(lines[i]), context); } diff --git a/Frameworks/include/CGContextCairo.h b/Frameworks/include/CGContextCairo.h index 1b03dc0e7b..a632507cb0 100644 --- a/Frameworks/include/CGContextCairo.h +++ b/Frameworks/include/CGContextCairo.h @@ -138,7 +138,7 @@ class CGContextCairo : public CGContextImpl { virtual bool CGContextIsPointInPath(bool eoFill, float x, float y); virtual CGPathRef CGContextCopyPath(void); - virtual void CGContextDrawGlyphRun(const DWRITE_GLYPH_RUN* glyphRun, float lineAscent); + virtual void CGContextDrawGlyphRun(const DWRITE_GLYPH_RUN* glyphRun); // TODO 1077:: Remove once D2D render target is implemented virtual void _CGContextSetScaleFactor(float scale); diff --git a/Frameworks/include/CGContextImpl.h b/Frameworks/include/CGContextImpl.h index d0de3ba0c0..f3bf93cf95 100644 --- a/Frameworks/include/CGContextImpl.h +++ b/Frameworks/include/CGContextImpl.h @@ -179,7 +179,7 @@ class CGContextImpl { virtual bool CGContextIsPointInPath(bool eoFill, float x, float y); virtual CGPathRef CGContextCopyPath(void); - virtual void CGContextDrawGlyphRun(const DWRITE_GLYPH_RUN* glyphRun, float lineAscent); + virtual void CGContextDrawGlyphRun(const DWRITE_GLYPH_RUN* glyphRun); // TODO 1077:: Remove once D2D render target is implemented virtual void _CGContextSetScaleFactor(float scale); diff --git a/Frameworks/include/CGContextInternal.h b/Frameworks/include/CGContextInternal.h index 25279f6fde..053575a7ec 100644 --- a/Frameworks/include/CGContextInternal.h +++ b/Frameworks/include/CGContextInternal.h @@ -50,7 +50,7 @@ COREGRAPHICS_EXPORT CGImageRef CGJPEGImageCreateFromFile(NSString* path); COREGRAPHICS_EXPORT CGImageRef CGJPEGImageCreateFromData(NSData* data); COREGRAPHICS_EXPORT bool CGContextIsPointInPath(CGContextRef c, bool eoFill, float x, float y); -COREGRAPHICS_EXPORT void CGContextDrawGlyphRun(CGContextRef ctx, const DWRITE_GLYPH_RUN* glyphRun, float lineAscent); +COREGRAPHICS_EXPORT void CGContextDrawGlyphRun(CGContextRef ctx, const DWRITE_GLYPH_RUN* glyphRun); // TODO 1077:: Remove once D2D render target is implemented COREGRAPHICS_EXPORT void _CGContextSetScaleFactor(CGContextRef ctx, float scale); diff --git a/Frameworks/include/CoreTextInternal.h b/Frameworks/include/CoreTextInternal.h index df7923a77f..29d0406f88 100644 --- a/Frameworks/include/CoreTextInternal.h +++ b/Frameworks/include/CoreTextInternal.h @@ -142,11 +142,5 @@ struct _CTParagraphStyleProperties { } @end -// Note: For some reason namemangling does not happen for these functions causing a linker error. Bug?? -CORETEXT_EXTERNC_BEGIN -void _CTLineDraw(CTLineRef line, CGContextRef ctx, bool adjustTextPosition); -void _CTRunDraw(CTRunRef run, CGContextRef ctx, CFRange textRange, bool adjustTextPosition, CGFloat lineAscent); -CORETEXT_EXTERNC_END - // Private helper methods for UIKit CORETEXT_EXPORT CGSize _CTFrameGetSize(CTFrameRef frame); \ No newline at end of file From f69ac799e3bcd191a19a0f62061e188c03659d63 Mon Sep 17 00:00:00 2001 From: ms-jihua Date: Wed, 2 Nov 2016 15:48:17 -0700 Subject: [PATCH 4/6] Properly pass font weight, stretch, style, to XamlCompositor (#1302) Update from using simplistic mapping using the font's symbolic traits, which ignored stretch, to getting the weight/stretch/style directly from the font. UILabel-rendered narrow or condensed fonts were previously clipped at the sides because the CoreText-calculated width would be for a narrow font, but UILabel would attempt to render a normal font. --- Frameworks/CoreText/CTFont.mm | 27 +++++++++++++++++++ .../UIKit/StarboardXaml/CompositorInterface.h | 15 +++++++++-- .../StarboardXaml/CompositorInterface.mm | 7 ++--- .../UIKit/StarboardXaml/XamlCompositor.cpp | 14 +++------- Frameworks/UIKit/UIFont.mm | 19 +++++++++++++ Frameworks/include/CoreTextInternal.h | 5 +++- Frameworks/include/UIFontInternal.h | 7 +++++ build/CoreText/dll/CoreText.def | 3 +++ 8 files changed, 80 insertions(+), 17 deletions(-) diff --git a/Frameworks/CoreText/CTFont.mm b/Frameworks/CoreText/CTFont.mm index b2b7b82b03..bb34bb6bcd 100644 --- a/Frameworks/CoreText/CTFont.mm +++ b/Frameworks/CoreText/CTFont.mm @@ -902,4 +902,31 @@ CFDataRef CTFontCopyTable(CTFontRef font, CTFontTableTag table, CTFontTableOptio CFTypeID CTFontGetTypeID() { static CFTypeID __kCTFontTypeID = _CFRuntimeRegisterClass(&__CTFontClass); return __kCTFontTypeID; +} + +// Private function for getting font weight for XAML +DWRITE_FONT_WEIGHT _CTFontGetDWriteWeight(CTFontRef font) { + ComPtr fontFace3; + if (font && SUCCEEDED(font->_dwriteFontFace.As(&fontFace3))) { + return fontFace3->GetWeight(); + } + return DWRITE_FONT_WEIGHT_NORMAL; +} + +// Private function for getting font stretch for XAML +DWRITE_FONT_STRETCH _CTFontGetDWriteStretch(CTFontRef font) { + ComPtr fontFace3; + if (font && SUCCEEDED(font->_dwriteFontFace.As(&fontFace3))) { + return fontFace3->GetStretch(); + } + return DWRITE_FONT_STRETCH_NORMAL; +} + +// Private function for getting font style for XAML +DWRITE_FONT_STYLE _CTFontGetDWriteStyle(CTFontRef font) { + ComPtr fontFace3; + if (font && SUCCEEDED(font->_dwriteFontFace.As(&fontFace3))) { + return fontFace3->GetStyle(); + } + return DWRITE_FONT_STYLE_NORMAL; } \ No newline at end of file diff --git a/Frameworks/UIKit/StarboardXaml/CompositorInterface.h b/Frameworks/UIKit/StarboardXaml/CompositorInterface.h index 1a9af8f956..920ddbc2bd 100644 --- a/Frameworks/UIKit/StarboardXaml/CompositorInterface.h +++ b/Frameworks/UIKit/StarboardXaml/CompositorInterface.h @@ -21,6 +21,16 @@ #include "winobjc\winobjc.h" #include +#ifdef __clang__ +#include +#endif + +#include + +#ifdef __clang__ +#include +#endif + class DisplayNode; class DisplayTexture; class DisplayAnimation; @@ -159,8 +169,9 @@ class DisplayTextureXamlGlyphs : public DisplayTexture { float _fontSize; float _lineHeight; bool _centerVertically; - bool _isBold = false; - bool _isItalic = false; + DWRITE_FONT_WEIGHT _fontWeight; + DWRITE_FONT_STRETCH _fontStretch; + DWRITE_FONT_STYLE _fontStyle; DisplayTextureXamlGlyphs(); ~DisplayTextureXamlGlyphs(); diff --git a/Frameworks/UIKit/StarboardXaml/CompositorInterface.mm b/Frameworks/UIKit/StarboardXaml/CompositorInterface.mm index 65feec4fd3..e3acad40eb 100644 --- a/Frameworks/UIKit/StarboardXaml/CompositorInterface.mm +++ b/Frameworks/UIKit/StarboardXaml/CompositorInterface.mm @@ -329,9 +329,10 @@ void SetParams(UIFont* font, _centerVertically = centerVertically; _lineHeight = [font ascender] - [font descender]; - int mask = [font fontDescriptor].symbolicTraits; - _isBold = (mask & UIFontDescriptorTraitBold) > 0; - _isItalic = (mask & UIFontDescriptorTraitItalic) > 0; + _fontWeight = [font _fontWeight]; + _fontStretch = [font _fontStretch]; + _fontStyle = [font _fontStyle]; + std::wstring wideBuffer = Strings::NarrowToWide(text); // The Font Family names DWrite will return are not always compatible with Xaml diff --git a/Frameworks/UIKit/StarboardXaml/XamlCompositor.cpp b/Frameworks/UIKit/StarboardXaml/XamlCompositor.cpp index 3af9511e6b..7b6569a484 100644 --- a/Frameworks/UIKit/StarboardXaml/XamlCompositor.cpp +++ b/Frameworks/UIKit/StarboardXaml/XamlCompositor.cpp @@ -561,17 +561,9 @@ void DisplayTextureXamlGlyphs::ConstructGlyphs(const Microsoft::WRL::Wrappers::H textControl->Foreground = ref new Windows::UI::Xaml::Media::SolidColorBrush(textColor); textControl->FontFamily = ref new Windows::UI::Xaml::Media::FontFamily(reinterpret_cast(fontFamilyName.Get())); - if (_isBold) { - textControl->FontWeight = Windows::UI::Text::FontWeights::Bold; - } else { - textControl->FontWeight = Windows::UI::Text::FontWeights::Normal; - } - - if (_isItalic) { - textControl->FontStyle = Windows::UI::Text::FontStyle::Italic; - } else { - textControl->FontStyle = Windows::UI::Text::FontStyle::Normal; - } + textControl->FontWeight = Windows::UI::Text::FontWeight{ static_cast(_fontWeight) }; + textControl->FontStretch = static_cast(_fontStretch); + textControl->FontStyle = static_cast(_fontStyle); switch (_horzAlignment) { case alignLeft: diff --git a/Frameworks/UIKit/UIFont.mm b/Frameworks/UIKit/UIFont.mm index ceeba65c6d..7a2352a933 100644 --- a/Frameworks/UIKit/UIFont.mm +++ b/Frameworks/UIKit/UIFont.mm @@ -34,6 +34,7 @@ #import "LoggingNative.h" #import "UICTFont.h" #import "UIFontInternal.h" +#import #include #import @@ -296,4 +297,22 @@ - (NSString*)_compatibleFamilyName { return static_cast(_DWriteGetFamilyNameForFontName(static_cast([self fontName]))); } +// WinObjC-only extension for compatibility issues between DWrite and Xaml +// Returns the weight of the font Xaml can use +- (DWRITE_FONT_WEIGHT)_fontWeight { + return _CTFontGetDWriteWeight(static_cast(self)); +} + +// WinObjC-only extension for compatibility issues between DWrite and Xaml +// Returns the stretch of the font Xaml can use +- (DWRITE_FONT_STRETCH)_fontStretch { + return _CTFontGetDWriteStretch(static_cast(self)); +} + +// WinObjC-only extension for compatibility issues between DWrite and Xaml +// Returns the style of the font Xaml can use +- (DWRITE_FONT_STYLE)_fontStyle { + return _CTFontGetDWriteStyle(static_cast(self)); +} + @end \ No newline at end of file diff --git a/Frameworks/include/CoreTextInternal.h b/Frameworks/include/CoreTextInternal.h index 29d0406f88..268437f982 100644 --- a/Frameworks/include/CoreTextInternal.h +++ b/Frameworks/include/CoreTextInternal.h @@ -143,4 +143,7 @@ struct _CTParagraphStyleProperties { @end // Private helper methods for UIKit -CORETEXT_EXPORT CGSize _CTFrameGetSize(CTFrameRef frame); \ No newline at end of file +CORETEXT_EXPORT CGSize _CTFrameGetSize(CTFrameRef frame); +CORETEXT_EXPORT DWRITE_FONT_WEIGHT _CTFontGetDWriteWeight(CTFontRef font); +CORETEXT_EXPORT DWRITE_FONT_STRETCH _CTFontGetDWriteStretch(CTFontRef font); +CORETEXT_EXPORT DWRITE_FONT_STYLE _CTFontGetDWriteStyle(CTFontRef font); \ No newline at end of file diff --git a/Frameworks/include/UIFontInternal.h b/Frameworks/include/UIFontInternal.h index d5ecd67720..6545ea3f71 100644 --- a/Frameworks/include/UIFontInternal.h +++ b/Frameworks/include/UIFontInternal.h @@ -18,10 +18,17 @@ #import #import +#include +#import +#include + @interface UIFont () + (UIFont*)defaultFont; + (UIFont*)titleFont; + (UIFont*)messageFont; + (UIFont*)buttonFont; - (NSString*)_compatibleFamilyName; +- (DWRITE_FONT_WEIGHT)_fontWeight; +- (DWRITE_FONT_STRETCH)_fontStretch; +- (DWRITE_FONT_STYLE)_fontStyle; @end \ No newline at end of file diff --git a/build/CoreText/dll/CoreText.def b/build/CoreText/dll/CoreText.def index e5ffce8ae8..73aeb333e3 100644 --- a/build/CoreText/dll/CoreText.def +++ b/build/CoreText/dll/CoreText.def @@ -136,6 +136,9 @@ LIBRARY CoreText CTFontCopyAvailableTables CTFontCopyTable CTFontGetTypeID + _CTFontGetDWriteWeight + _CTFontGetDWriteStretch + _CTFontGetDWriteStyle ; CTFontCollection Reference.mm kCTFontCollectionRemoveDuplicatesOption DATA From 21a3f61c5fcf1f5d4e192efaa2bd32720cf6808a Mon Sep 17 00:00:00 2001 From: AJ Ballway Date: Wed, 2 Nov 2016 19:44:09 -0700 Subject: [PATCH 5/6] Fix infinite loop in __CreateDWriteTextLayout (#1306) * Fix infinite loop in __CreateDWriteTextLayout * Clang format acting up --- Frameworks/CoreText/DWriteWrapper_CoreText.mm | 2 +- .../unittests/CoreText/CTFramesetterTests.mm | 26 ++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/Frameworks/CoreText/DWriteWrapper_CoreText.mm b/Frameworks/CoreText/DWriteWrapper_CoreText.mm index 60a8c81bbb..492f99eeee 100644 --- a/Frameworks/CoreText/DWriteWrapper_CoreText.mm +++ b/Frameworks/CoreText/DWriteWrapper_CoreText.mm @@ -204,7 +204,7 @@ bool _CloneDWriteGlyphRun(_In_ DWRITE_GLYPH_RUN const* src, _Out_ DWRITE_GLYPH_R for (size_t i = 0; i < [subString length]; i += attributeRange.length) { NSDictionary* attribs = [static_cast(string) attributesAtIndex:i + range.location longestEffectiveRange:&attributeRange - inRange:{ i, [subString length] }]; + inRange:{ i + range.location, [subString length] }]; const DWRITE_TEXT_RANGE dwriteRange = { attributeRange.location, attributeRange.length }; diff --git a/tests/unittests/CoreText/CTFramesetterTests.mm b/tests/unittests/CoreText/CTFramesetterTests.mm index bff2922a9d..8ef672ca97 100644 --- a/tests/unittests/CoreText/CTFramesetterTests.mm +++ b/tests/unittests/CoreText/CTFramesetterTests.mm @@ -22,7 +22,7 @@ static const float c_errorDelta = 0.0005f; -static NSAttributedString* getAttributedString(NSString* str) { +static NSMutableAttributedString* getAttributedString(NSString* str) { UIFontDescriptor* fontDescriptor = [UIFontDescriptor fontDescriptorWithName:@"Times New Roman" size:40]; UIFont* font = [UIFont fontWithDescriptor:fontDescriptor size:40]; @@ -103,4 +103,28 @@ EXPECT_EQ(5L, CFArrayGetCount(CTFrameGetLines(frame))); CFRelease(frame); CGPathRelease(path); +} + +TEST(CTFramesetter, ShouldBeAbleToCreateMultipleFramesFromSameFramesetter) { + NSMutableAttributedString* attrString = getAttributedString(@"ABCDEFGHIJ"); + [attrString addAttribute:NSForegroundColorAttributeName value:[UIColor blueColor] range:NSMakeRange(0, 7)]; + CFAttributedStringRef string = (__bridge CFAttributedStringRef)attrString; + CTFramesetterRef framesetter = CTFramesetterCreateWithAttributedString(string); + CFAutorelease(framesetter); + CGPathRef path = CGPathCreateWithRect(CGRectMake(0, 0, FLT_MAX, FLT_MAX), nullptr); + CTFrameRef firstFrame = CTFramesetterCreateFrame(framesetter, { 0, 5 }, path, nullptr); + EXPECT_NE(nil, firstFrame); + + CTFrameRef secondFrame = CTFramesetterCreateFrame(framesetter, { 1, 8 }, path, nullptr); + EXPECT_NE(nil, secondFrame); + + CTFrameRef thirdFrame = CTFramesetterCreateFrame(framesetter, { 8, 2 }, path, nullptr); + EXPECT_NE(nil, thirdFrame); + + CTFrameRef fullFrame = CTFramesetterCreateFrame(framesetter, {}, path, nullptr); + EXPECT_NE(nil, fullFrame); + CFRelease(firstFrame); + CFRelease(secondFrame); + CFRelease(thirdFrame); + CFRelease(fullFrame); } \ No newline at end of file From 4a5dbd63f03698492c2f9bca1ac487972842c3cd Mon Sep 17 00:00:00 2001 From: Raj Seshasankaran Date: Wed, 2 Nov 2016 20:26:16 -0700 Subject: [PATCH 6/6] Update build version --- build/winobjc.version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/winobjc.version b/build/winobjc.version index d835ab6797..9c7b0b5adb 100644 --- a/build/winobjc.version +++ b/build/winobjc.version @@ -1,4 +1,4 @@ # MAJOR.MINOR.BUILD.REVISION # BUILD = YYMM # REVISION = DD -0.2.1610.24 +0.2.1611.02