Skip to content

Add apple_fontSmoothing style and global default setting #329

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

Merged
merged 17 commits into from
May 1, 2020
Merged

Add apple_fontSmoothing style and global default setting #329

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
685f103
Update scripts to publish react-native-macos-init
tom-un Apr 4, 2020
8ccd26c
Merge remote-tracking branch 'ms/master'
tom-un Apr 4, 2020
45d2ee2
Merge remote-tracking branch 'ms/master'
tom-un Apr 4, 2020
1e2c329
Clean up merge markers
tom-un Apr 4, 2020
408dae3
Merge remote-tracking branch 'ms/master'
tom-un Apr 6, 2020
bdaa8ad
Merge remote-tracking branch 'ms/master'
tom-un Apr 7, 2020
5a67ae0
Restored ios:macos RNTester parity except for InputAccessoryView.
tom-un Apr 13, 2020
e5a6df2
Revert "Restored ios:macos RNTester parity except for InputAccessoryV…
tom-un Apr 13, 2020
4bca23c
Merge remote-tracking branch 'ms/master'
tom-un Apr 16, 2020
d067629
Merge remote-tracking branch 'ms/master'
tom-un Apr 21, 2020
6ed01c8
Merge remote-tracking branch 'ms/master'
tom-un Apr 28, 2020
ae5cc57
Merge remote-tracking branch 'ms/master'
tom-un Apr 29, 2020
07ba414
Added experimental webkitFontSmoothing style.
tom-un Apr 29, 2020
e7faf7a
Rename to fontSmoothing
tom-un Apr 30, 2020
efc5587
Missed some upstream delta comments
tom-un Apr 30, 2020
8cbf04a
Add .command to .gitattributes for lf
tom-un Apr 30, 2020
f158098
Renamed fontSmoothing to apple_fontSmoothing
tom-un Apr 30, 2020
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
1 change: 1 addition & 0 deletions .gitattributes
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@
# that.
gradlew text eol=lf
*.sh text eol=lf
*.command text eol=lf
1 change: 1 addition & 0 deletions Libraries/Components/View/ReactNativeViewViewConfig.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ const ReactNativeViewConfig = {
fontStyle: true,
fontVariant: true,
fontWeight: true,
apple_fontSmoothing: true, // TODO(OSS Candidate ISS#2710739)
height: true,
includeFontPadding: true,
justifyContent: true,
Expand Down
11 changes: 11 additions & 0 deletions Libraries/DeprecatedPropTypes/DeprecatedTextStylePropTypes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,17 @@ const DeprecatedTextStylePropTypes = {
| 'proportional-nums',
>,
>),
/** [TODO(OSS Candidate ISS#2710739)
Copy link

Choose a reason for hiding this comment

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

Where is this deprecated file still used?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I actually don't know. But it contains all the same props as the other files like ViewConfig so I made it parallel.

* @platform ios, macos
*/
apple_fontSmoothing: (ReactPropTypes.oneOf([
'auto' /*default*/,
'none',
'antialiased',
'subpixel-antialiased',
]): React$PropType$Primitive<
'auto' | 'none' | 'antialiased' | 'subpixel-antialiased',
>), // ]TODO(OSS Candidate ISS#2710739)
textShadowOffset: (ReactPropTypes.shape({
width: ReactPropTypes.number,
height: ReactPropTypes.number,
Expand Down
5 changes: 5 additions & 0 deletions Libraries/StyleSheet/StyleSheetTypes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -602,6 +602,11 @@ export type ____TextStyle_Internal = $ReadOnly<{|
| 'tabular-nums'
| 'proportional-nums',
>,
apple_fontSmoothing?:
| 'auto'
| 'none'
| 'antialiased'
| 'subpixel-antialiased', // TODO(OSS Candidate ISS#2710739)
textShadowOffset?: $ReadOnly<{|
width: number,
height: number,
Expand Down
1 change: 1 addition & 0 deletions Libraries/Text/BaseText/RCTBaseTextViewManager.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ - (RCTShadowView *)shadowView
RCT_REMAP_SHADOW_PROPERTY(allowFontScaling, textAttributes.allowFontScaling, BOOL)
RCT_REMAP_SHADOW_PROPERTY(maxFontSizeMultiplier, textAttributes.maxFontSizeMultiplier, CGFloat)
RCT_REMAP_SHADOW_PROPERTY(letterSpacing, textAttributes.letterSpacing, CGFloat)
RCT_REMAP_SHADOW_PROPERTY(apple_fontSmoothing, textAttributes.fontSmoothing, RCTFontSmoothing) // TODO(OSS Candidate ISS#2710739)
// Paragraph Styles
RCT_REMAP_SHADOW_PROPERTY(lineHeight, textAttributes.lineHeight, CGFloat)
RCT_REMAP_SHADOW_PROPERTY(textAlign, textAttributes.alignment, NSTextAlignment)
Expand Down
4 changes: 4 additions & 0 deletions Libraries/Text/RCTTextAttributes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@
#import <React/RCTUIKit.h> // TODO(macOS ISS#2323203)

#import <React/RCTTextDecorationLineType.h>
#import <React/RCTFontSmoothing.h> // TODO(OSS Candidate ISS#2710739)

#import "RCTTextTransform.h"

NS_ASSUME_NONNULL_BEGIN

extern NSString *const RCTTextAttributesIsHighlightedAttributeName;
extern NSString *const RCTTextAttributesFontSmoothingAttributeName; // TODO(OSS Candidate ISS#2710739)
extern NSString *const RCTTextAttributesTagAttributeName;

/**
Expand All @@ -37,6 +39,8 @@ extern NSString *const RCTTextAttributesTagAttributeName;
@property (nonatomic, copy, nullable) NSArray<NSString *> *fontVariant;
@property (nonatomic, assign) BOOL allowFontScaling;
@property (nonatomic, assign) CGFloat letterSpacing;
@property (nonatomic, assign) RCTFontSmoothing fontSmoothing; // TODO(OSS Candidate ISS#2710739)
@property (class, nonatomic, assign) RCTFontSmoothing fontSmoothingDefault; // TODO(OSS Candidate ISS#2710739)
// Paragraph Styles
@property (nonatomic, assign) CGFloat lineHeight;
@property (nonatomic, assign) NSTextAlignment alignment;
Expand Down
21 changes: 21 additions & 0 deletions Libraries/Text/RCTTextAttributes.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#import <React/RCTLog.h>

NSString *const RCTTextAttributesIsHighlightedAttributeName = @"RCTTextAttributesIsHighlightedAttributeName";
NSString *const RCTTextAttributesFontSmoothingAttributeName = @"RCTTextAttributesFontSmoothingAttributeName"; // TODO(OSS Candidate ISS#2710739)
NSString *const RCTTextAttributesTagAttributeName = @"RCTTextAttributesTagAttributeName";

@implementation RCTTextAttributes
Expand Down Expand Up @@ -56,6 +57,7 @@ - (void)applyTextAttributes:(RCTTextAttributes *)textAttributes
_fontVariant = textAttributes->_fontVariant ?: _fontVariant;
_allowFontScaling = textAttributes->_allowFontScaling || _allowFontScaling; // *
_letterSpacing = !isnan(textAttributes->_letterSpacing) ? textAttributes->_letterSpacing : _letterSpacing;
_fontSmoothing = textAttributes->_fontSmoothing != RCTFontSmoothingAuto ? textAttributes->_fontSmoothing : _fontSmoothing; // TODO(OSS Candidate ISS#2710739)

// Paragraph Styles
_lineHeight = !isnan(textAttributes->_lineHeight) ? textAttributes->_lineHeight : _lineHeight;
Expand Down Expand Up @@ -183,6 +185,12 @@ - (NSParagraphStyle *)effectiveParagraphStyle
attributes[RCTTextAttributesIsHighlightedAttributeName] = @YES;
}

// [TODO(macOS ISS#2323203)
if (_fontSmoothing != RCTFontSmoothingAuto) {
Copy link

Choose a reason for hiding this comment

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

Is it possible it could get set to something other than auto and then set back to auto (which would then fail to update here)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, that's a good point. Although in practice I doubt anyone is going to be toggling this style on and off. I'll test some stuff out setting it without the condition

attributes[RCTTextAttributesFontSmoothingAttributeName] = @(_fontSmoothing);
}
// ]TODO(macOS ISS#2323203)

if (_tag) {
attributes[RCTTextAttributesTagAttributeName] = _tag;
}
Expand Down Expand Up @@ -290,6 +298,7 @@ - (BOOL)isEqual:(RCTTextAttributes *)textAttributes
RCTTextAttributesCompareObjects(_fontVariant) &&
RCTTextAttributesCompareOthers(_allowFontScaling) &&
RCTTextAttributesCompareFloats(_letterSpacing) &&
RCTTextAttributesCompareOthers(_fontSmoothing) && // TODO(OSS Candidate ISS#2710739)
// Paragraph Styles
RCTTextAttributesCompareFloats(_lineHeight) &&
RCTTextAttributesCompareFloats(_alignment) &&
Expand All @@ -309,4 +318,16 @@ - (BOOL)isEqual:(RCTTextAttributes *)textAttributes
RCTTextAttributesCompareOthers(_textTransform);
}

// [TODO(OSS Candidate ISS#2710739)
static RCTFontSmoothing _fontSmoothingDefault = RCTFontSmoothingAuto;

+ (RCTFontSmoothing)fontSmoothingDefault {
return _fontSmoothingDefault;
}

+ (void)setFontSmoothingDefault:(RCTFontSmoothing)fontSmoothingDefault {
_fontSmoothingDefault = fontSmoothingDefault;
}
// ]TODO(OSS Candidate ISS#2710739)

@end
36 changes: 32 additions & 4 deletions Libraries/Text/Text/RCTTextView.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,12 +159,40 @@ - (void)drawRect:(CGRect)rect
NSTextContainer *textContainer = layoutManager.textContainers.firstObject;

NSRange glyphRange = [layoutManager glyphRangeForTextContainer:textContainer];
[layoutManager drawBackgroundForGlyphRange:glyphRange atPoint:_contentFrame.origin];
[layoutManager drawGlyphsForGlyphRange:glyphRange atPoint:_contentFrame.origin];

__block UIBezierPath *highlightPath = nil;
NSRange characterRange = [layoutManager characterRangeForGlyphRange:glyphRange
actualGlyphRange:NULL];
// [TODO(OSS Candidate ISS#2710739)
Copy link

Choose a reason for hiding this comment

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

I think that the feature is awesome.

It feels that this property should be part of like ParagraphAttributes (see Fabric's ParagraphAttributes), not TextAttributes.

So, it should be applied to the whole text node. Or, do we have use cases where it should be applied only for some fragments?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, the biggest reason we can't do that is because mac doesn't work with Fabric yet so we have no ParagraphAttributes 😢. Otherwise, just to be parallel with the w3c font-smooth style and webkit --webkit-font-smoothing styles which can be applied at any level of granularity.

[_textStorage enumerateAttribute:RCTTextAttributesFontSmoothingAttributeName
inRange:characterRange
options:0
usingBlock:
^(NSNumber *value, NSRange range, __unused BOOL *stop) {
RCTFontSmoothing smoothing = value.integerValue;
if (smoothing == RCTFontSmoothingAuto) {
smoothing = [RCTTextAttributes fontSmoothingDefault];
}
CGContextRef context = UIGraphicsGetCurrentContext();
CGContextSaveGState(context);
switch (smoothing) {
case RCTFontSmoothingNone:
CGContextSetShouldAntialias(context, false);
break;
case RCTFontSmoothingAntialiased:
CGContextSetAllowsFontSmoothing(context, false);
CGContextSetShouldSmoothFonts(context, false);
break;
case RCTFontSmoothingAuto:
case RCTFontSmoothingSubpixelAntialiased:
break;
}
NSRange subGlyphRange = [layoutManager glyphRangeForCharacterRange:range actualCharacterRange:nil];
[layoutManager drawBackgroundForGlyphRange:subGlyphRange atPoint:_contentFrame.origin];
[layoutManager drawGlyphsForGlyphRange:subGlyphRange atPoint:_contentFrame.origin];
CGContextRestoreGState(context);
}];
// ]TODO(OSS Candidate ISS#2710739)

__block UIBezierPath *highlightPath = nil;
[_textStorage enumerateAttribute:RCTTextAttributesIsHighlightedAttributeName
inRange:characterRange
options:0
Expand Down
Loading