Fix RTL start-side text clipping on Android 15+ - #58073
Conversation
Wire StaticLayout.Builder.setUseBoundsForWidth and setShiftDrawingOffsetForStartOverhang through reflection so Arabic glyphs that overhang their advance box are drawn at RTL line starts instead of clipped. Apply the same TextView setters for Paper. Fixes react#58064
|
Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
please suggest me any changes which i have to make if there is any you would suggest |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
Summary:
On Android 15+, Arabic glyphs whose ink sticks out past the advance box (alef-madda, alef-wasla, lam-alef) get clipped at the start of an RTL line. Mid-line instances render fine, and iOS is unaffected. That's #58064.
Android already has APIs for this exact case:
StaticLayout.Builder.setUseBoundsForWidth(true)StaticLayout.Builder.setShiftDrawingOffsetForStartOverhang(true)TextLayoutManager.buildLayout()is the only place we constructStaticLayouts, and it wasn't calling either one. There was even a leftover reflection handle forsetUseBoundsForWidthwith zero call sites.This wires those setters up:
StaticLayout.Builderbeforebuild().TextViewmethods fromReactTextView.initView(), because Paper still letsTextViewmake its own layout.I used reflection plus
AndroidVersion.VERSION_CODE_VANILLA_ICE_CREAMinstead of calling the methods directly. Some internal targets still compile against SDK 34, which is why that unused handle existed in the first place.I did not bring back the two-pass visual-bounds measurement from #54721 / #8347cc4. That path grew AT_MOST/UNDEFINED layouts and was later removed after wrapping regressions. Full-width (EXACTLY) RTL paragraphs never went through it anyway — which is the repro in this issue.
setUseBoundsForWidth(true)can still change wrapping a little for glyphs whose visual bounds are wider than their advances. That's the same tradeoff Android's ownTextViewmade on API 35. If you'd rather have a kill switch, I'm happy to put this behind a feature flag.Changelog:
[ANDROID] [FIXED] - Fix RTL Arabic start-side glyph clipping on Android 15+
Test Plan:
Added Robolectric coverage in
TextLayoutManagerStartOverhangTest:StaticLayout.Builderexposes the start-overhang settersuseBoundsForWidthandshiftDrawingOffsetForStartOverhangenabledTextViewgets the same setters (Paper path)I wasn't able to run
./gradlew :packages:react-native:ReactAndroid:testDebugUnitTestlocally — this checkout doesn't have Yarn autolink set up, and the machine only hasandroid-36.1installed (RN currently compiles against 37). I also haven't visually re-checked the clip on an Android 15/16 device against this branch.The issue has a complete repro: https://github.com/muhasabahhub/android-react-native-arabic
If someone can run that repro on this branch, that's the real visual confirmation. CI should still run the new unit tests.