Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Fix rotation and nslayout line drawing#1274

Merged
aballway merged 6 commits into
microsoft:developfrom
aballway:coretext
Nov 1, 2016
Merged

Fix rotation and nslayout line drawing#1274
aballway merged 6 commits into
microsoft:developfrom
aballway:coretext

Conversation

@aballway

@aballway aballway commented Oct 31, 2016

Copy link
Copy Markdown
Contributor

Fixes incorrect rotation introduced by my recent math changes and adds descent to nslayoutmanager line drawing to make exclusion zones more accurate

Fixes #1275


This change is Reviewable


CGFloat ascent, leading;
CGFloat ascent, descent, leading;
CTLineGetTypographicBounds(line, &ascent, nullptr, &leading);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nullptr, [](start = 50, length = 8)

you're not currently getting the descent though, so it doesn't look like this would've changed anything?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(i think ascent + leading would've been correct here in any case)


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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You're right, added a comment and reverted change

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: I would probably just make the comment be about what the SetTextPosition is actually doing? "because CG origin is the opposite vertically from UIKit origin, flip the y line origin from upper left corner to lower left corner (baseline)", except use better wording than i did


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


// 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));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

-transform.c [](start = 78, length = 12)

i'd been wondering for a while why this was missing...

@aballway aballway Oct 31, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not quite sure how it worked before

@ms-jihua

Copy link
Copy Markdown
Contributor

:shipit:

@rajsesh rajsesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good catch. Please add a page to CTCatalog for rotate. Also I'd like to start adding automated tests that validate the contents of bitmap context to catch regressions that would be easy to miss with untrained eye.

<AppxManifest Include="Package.appxmanifest">
<SubType>Designer</SubType>
</AppxManifest>
<ClangCompile Include="..\..\CTCatalog\Tests\CTCAffineTransformationTestViewController.mm" />

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These files also need to be added to xcodeproject and should compile and run on reference platform.

@aballway aballway Oct 31, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Already done and tested

[_translateYSlider setNeedsDisplay];
}

- (void)didReceiveMemoryWarning {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

these functions can be deleted.


#import "CTCAffineTransformationTestViewController.h"

static const NSString* text = @"The quick brown dog jumps over the lazy fox. THE QUICK BROWN DOG JUMPS OVER THE LAZY FOX.";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would change this text to read what the expected test result should be (changing rotate should cause the text to rotate in which direction?).


setting.spec = kCTParagraphStyleSpecifierAlignment;
setting.valueSize = sizeof(CTTextAlignment);
CTTextAlignment alignment = kCTCenterTextAlignment;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

left alignment here to easily see the pivot point?

@ms-jihua ms-jihua left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: it'd be nice if the sliders had labels, but it's fine as-is


// Creates outline
CGContextSetLineWidth(context, 2.0);
CGColorSpaceRef colorspace = CGColorSpaceCreateDeviceRGB();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This colorspace does not seem to be used.

@aballway aballway merged commit 85ccfb8 into microsoft:develop Nov 1, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants