From aab47634ad99fdec0b50149e82f23d3a6101b636 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Thu, 23 Mar 2023 21:39:20 -0700 Subject: [PATCH] Fix textShadow on Android Fabric (#36585) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/36585 1. We don't add `ShadowStyleSpan` unless an offset is set. This is incorrect, and less permissive than `BaseTextShadowNode` in Paper. 2. We don't serialize textShadowOffset to MapBuffer Changelog: [Android][Fixed] - Fix textShadow on Android Fabric Reviewed By: javache Differential Revision: D44302691 fbshipit-source-id: 2cfd3bff3d0e4f329c98d1ed2defff94ad3b7dc3 --- .../react/views/text/TextAttributeProps.java | 23 +++++++++++++++---- .../react/views/text/TextLayoutManager.java | 6 ++++- .../text/TextLayoutManagerMapBuffer.java | 6 ++++- .../renderer/attributedstring/conversions.h | 16 +++++++++---- 4 files changed, 41 insertions(+), 10 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextAttributeProps.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextAttributeProps.java index d503a641808..306cfee86ff 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextAttributeProps.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextAttributeProps.java @@ -51,9 +51,11 @@ public class TextAttributeProps { public static final short TA_KEY_TEXT_DECORATION_STYLE = 16; public static final short TA_KEY_TEXT_SHADOW_RADIUS = 18; public static final short TA_KEY_TEXT_SHADOW_COLOR = 19; - public static final short TA_KEY_IS_HIGHLIGHTED = 20; - public static final short TA_KEY_LAYOUT_DIRECTION = 21; - public static final short TA_KEY_ACCESSIBILITY_ROLE = 22; + public static final short TA_KEY_TEXT_SHADOW_OFFSET_DX = 20; + public static final short TA_KEY_TEXT_SHADOW_OFFSET_DY = 21; + public static final short TA_KEY_IS_HIGHLIGHTED = 22; + public static final short TA_KEY_LAYOUT_DIRECTION = 23; + public static final short TA_KEY_ACCESSIBILITY_ROLE = 24; public static final int UNSET = -1; @@ -198,6 +200,12 @@ public class TextAttributeProps { case TA_KEY_TEXT_SHADOW_COLOR: result.setTextShadowColor(entry.getIntValue()); break; + case TA_KEY_TEXT_SHADOW_OFFSET_DX: + result.setTextShadowOffsetDx((float) entry.getDoubleValue()); + break; + case TA_KEY_TEXT_SHADOW_OFFSET_DY: + result.setTextShadowOffsetDy((float) entry.getDoubleValue()); + break; case TA_KEY_IS_HIGHLIGHTED: break; case TA_KEY_LAYOUT_DIRECTION: @@ -213,7 +221,6 @@ public class TextAttributeProps { // setNumberOfLines // setColor // setIncludeFontPadding - // setTextShadowOffset // setTextTransform return result; } @@ -556,6 +563,14 @@ public class TextAttributeProps { } } + private void setTextShadowOffsetDx(float dx) { + mTextShadowOffsetDx = PixelUtil.toPixelFromDIP(dx); + } + + private void setTextShadowOffsetDy(float dy) { + mTextShadowOffsetDy = PixelUtil.toPixelFromDIP(dy); + } + public static int getLayoutDirection(@Nullable String layoutDirection) { int androidLayoutDirection; if (layoutDirection == null || "undefined".equals(layoutDirection)) { diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.java index 6dae0fea51d..44195221646 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.java @@ -10,6 +10,7 @@ package com.facebook.react.views.text; import static com.facebook.react.views.text.TextAttributeProps.UNSET; import android.content.Context; +import android.graphics.Color; import android.os.Build; import android.text.BoringLayout; import android.text.Layout; @@ -165,7 +166,10 @@ public class TextLayoutManager { if (textAttributes.mIsLineThroughTextDecorationSet) { ops.add(new SetSpanOperation(start, end, new ReactStrikethroughSpan())); } - if (textAttributes.mTextShadowOffsetDx != 0 || textAttributes.mTextShadowOffsetDy != 0) { + if ((textAttributes.mTextShadowOffsetDx != 0 + || textAttributes.mTextShadowOffsetDy != 0 + || textAttributes.mTextShadowRadius != 0) + && Color.alpha(textAttributes.mTextShadowColor) != 0) { ops.add( new SetSpanOperation( start, diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManagerMapBuffer.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManagerMapBuffer.java index 5cc8b14b3ba..45ca23c7641 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManagerMapBuffer.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManagerMapBuffer.java @@ -10,6 +10,7 @@ package com.facebook.react.views.text; import static com.facebook.react.views.text.TextAttributeProps.UNSET; import android.content.Context; +import android.graphics.Color; import android.os.Build; import android.text.BoringLayout; import android.text.Layout; @@ -179,7 +180,10 @@ public class TextLayoutManagerMapBuffer { if (textAttributes.mIsLineThroughTextDecorationSet) { ops.add(new SetSpanOperation(start, end, new ReactStrikethroughSpan())); } - if (textAttributes.mTextShadowOffsetDx != 0 || textAttributes.mTextShadowOffsetDy != 0) { + if ((textAttributes.mTextShadowOffsetDx != 0 + || textAttributes.mTextShadowOffsetDy != 0 + || textAttributes.mTextShadowRadius != 0) + && Color.alpha(textAttributes.mTextShadowColor) != 0) { ops.add( new SetSpanOperation( start, diff --git a/packages/react-native/ReactCommon/react/renderer/attributedstring/conversions.h b/packages/react-native/ReactCommon/react/renderer/attributedstring/conversions.h index 8687b892204..2a3f7bccc5f 100644 --- a/packages/react-native/ReactCommon/react/renderer/attributedstring/conversions.h +++ b/packages/react-native/ReactCommon/react/renderer/attributedstring/conversions.h @@ -1106,10 +1106,12 @@ constexpr static MapBuffer::Key TA_KEY_TEXT_DECORATION_LINE = 15; constexpr static MapBuffer::Key TA_KEY_TEXT_DECORATION_STYLE = 16; constexpr static MapBuffer::Key TA_KEY_TEXT_SHADOW_RADIUS = 18; constexpr static MapBuffer::Key TA_KEY_TEXT_SHADOW_COLOR = 19; -constexpr static MapBuffer::Key TA_KEY_IS_HIGHLIGHTED = 20; -constexpr static MapBuffer::Key TA_KEY_LAYOUT_DIRECTION = 21; -constexpr static MapBuffer::Key TA_KEY_ACCESSIBILITY_ROLE = 22; -constexpr static MapBuffer::Key TA_KEY_LINE_BREAK_STRATEGY = 23; +constexpr static MapBuffer::Key TA_KEY_TEXT_SHADOW_OFFSET_DX = 20; +constexpr static MapBuffer::Key TA_KEY_TEXT_SHADOW_OFFSET_DY = 21; +constexpr static MapBuffer::Key TA_KEY_IS_HIGHLIGHTED = 22; +constexpr static MapBuffer::Key TA_KEY_LAYOUT_DIRECTION = 23; +constexpr static MapBuffer::Key TA_KEY_ACCESSIBILITY_ROLE = 24; +constexpr static MapBuffer::Key TA_KEY_LINE_BREAK_STRATEGY = 25; // constants for ParagraphAttributes serialization constexpr static MapBuffer::Key PA_KEY_MAX_NUMBER_OF_LINES = 0; @@ -1244,6 +1246,12 @@ inline MapBuffer toMapBuffer(const TextAttributes &textAttributes) { TA_KEY_TEXT_SHADOW_COLOR, toAndroidRepr(textAttributes.textShadowColor)); } + if (textAttributes.textShadowOffset) { + builder.putDouble( + TA_KEY_TEXT_SHADOW_OFFSET_DX, textAttributes.textShadowOffset->width); + builder.putDouble( + TA_KEY_TEXT_SHADOW_OFFSET_DY, textAttributes.textShadowOffset->height); + } // Special if (textAttributes.isHighlighted.has_value()) { builder.putBool(TA_KEY_IS_HIGHLIGHTED, *textAttributes.isHighlighted);