Support kCTParagraphStyleSpecifierLineHeightMultiple#1298
Conversation
| for (_CTLine* line in static_cast<id<NSFastEnumeration>>(framePtr->_lines)) { | ||
| if (line->_lineOrigin.y < framePtr->_frameRect.size.height && line->_lineOrigin.y > 0) { | ||
| for (size_t i = 0; i < framePtr->_lineOrigins.size(); ++i) { | ||
| if (framePtr->_lineOrigins[i].y < framePtr->_frameRect.size.height && framePtr->_lineOrigins[i].y > 0) { |
There was a problem hiding this comment.
framePtr->_lineOrigins[i].y > 0 [](start = 81, length = 32)
switch around and have this check first.
There was a problem hiding this comment.
whoops this check isn't actually needed any longer
| ret->_frameRect.size.height += totalShifted; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Although you don't "prefer" early returns. This code would be much simpler and so much simpler to read if you used early returns.
We have if layers that aren't the greatest looking for code readability.
There was a problem hiding this comment.
I hate to admit it but you're right in this case, too many layers
| sizeof(lineHeightMultiple), | ||
| &lineHeightMultiple) && | ||
| lineHeightMultiple > 0) { | ||
| // The actual ratio we need to change the line height by is multiple - 1 |
There was a problem hiding this comment.
multiple [](start = 76, length = 8)
lineHeightMultiple
| CGPathRef path = CGPathCreateWithRect(CGRectMake(0, 0, FLT_MAX, FLT_MAX), nullptr); | ||
|
|
||
| NSMutableAttributedString* normalString = [[[NSMutableAttributedString alloc] initWithString:@"TEST\nTEST\nTEST"] autorelease]; | ||
| CTFramesetterRef normalFramesetter = CTFramesetterCreateWithAttributedString((CFAttributedStringRef)normalString); |
There was a problem hiding this comment.
CFAttributedStringRef [](start = 82, length = 21)
proper casting?
| static_cast<NSString*>(kCTParagraphStyleAttributeName) : (id)CTParagraphStyleCreate(settings, 1) | ||
| } | ||
| range:NSMakeRange(0, 14)]; | ||
|
|
| CGPoint halfOrigins[3]; | ||
| CTFrameGetLineOrigins(halfFrame, {}, halfOrigins); | ||
| EXPECT_NEAR(half * normalLineSpaceFirst, halfOrigins[1].y - halfOrigins[0].y, c_errorDelta); | ||
| EXPECT_NEAR(half * normalLineSpaceSecond, halfOrigins[2].y - halfOrigins[0].y, c_errorDelta); |
There was a problem hiding this comment.
this is a duplicate of the above with slightly different configuration. Refactor the code so you can use it generically and can add more tests easily.
|
🕐 |
| return [_DWriteGetFrame(static_cast<CFAttributedStringRef>(typesetter->_attributedString.get()), range, frameSize) retain]; | ||
| _CTFrame* ret = _DWriteGetFrame(static_cast<CFAttributedStringRef>(typesetter->_attributedString.get()), range, frameSize); | ||
|
|
||
| // Trying to access attributes without any text with throw an error |
There was a problem hiding this comment.
nit: typo: // Trying to access attributes without any text with will throw an error
|
|
||
| TEST_P(CoreTextLineSpaceMultipleTest, OriginsShouldBeMovedByRatio) { | ||
| NSMutableAttributedString* multipleString = [[[NSMutableAttributedString alloc] initWithString:@"TEST\nTEST\nTEST"] autorelease]; | ||
| CGFloat param = GetParam(); |
There was a problem hiding this comment.
param [](start = 12, length = 5)
please give this a more meaningful name
| CTParagraphStyleRef settings = static_cast<CTParagraphStyleRef>( | ||
| [typesetter->_attributedString attribute:static_cast<NSString*>(kCTParagraphStyleAttributeName) atIndex:0 effectiveRange:nullptr]); | ||
|
|
||
| if (settings == nil) { |
There was a problem hiding this comment.
nil [](start = 20, length = 3)
nit: nullptr, since it's not objc #ByDesign
There was a problem hiding this comment.
It actually is an objc
| } | ||
|
|
||
| return [_DWriteGetFrame(static_cast<CFAttributedStringRef>(typesetter->_attributedString.get()), range, frameSize) retain]; | ||
| _CTFrame* ret = _DWriteGetFrame(static_cast<CFAttributedStringRef>(typesetter->_attributedString.get()), range, frameSize); |
There was a problem hiding this comment.
_CTFrame [](start = 4, length = 8)
If you do a StrongId<_CTFrame> and then .detach() when you return, this will memory manage sanely.
|
|
|
|
Adds support for kCTParagraphStyleSpecifierLineHeightMultiple. DWrite only allows line height multiples if the line height is kept constant through the frame, which is not guaranteed, so we need to manually manipulate the line origins. Also removes the redundant line origin variable in _CTLine.
Fixes #1090
This change is