Text TSpan dy behaves differently on Android and iOS

[chrisbolin]

Notice the relative size difference between the text and the <Rect/> on iOS and Android:

import React from 'react';
import { View } from 'react-native';
import Svg, { Rect, Text, TSpan } from "react-native-svg";

export default class App extends React.Component {
  render() {
    return (
      <View>
      <Svg
          height="160"
          width="200"
      >
          <Rect fill="blue" x="0" y="25" width="5" height="42" />
          <Text y="20" dx="5 5">
              <TSpan x="10" >tspan line 1</TSpan>
              <TSpan x="10" dy="15">tspan line 2</TSpan>
              <TSpan x="10" dx="10" dy="15">tspan line 3</TSpan>
          </Text>
      </Svg>
      </View>
    );
  }
}

[msand]

You could try my new branch I just pushed:
https://github.com/msand/react-native-svg/tree/TextOnAPathLayoutRulesConformance

It fixes quite a few bugs in android, I haven't worked on the ios part at all yet.

[chrisbolin]

@msand nice! would you have a sec to test my snippet on your branch? my machine is a little foobar when it comes to running non-Expo Android. (and I can't use Expo because it ships with it's own version of react-native-svg)

[msand]

I have to go get some sleep right now, but I can do it in the morning :)

[chrisbolin]

haha totally fine. there's no rush at all

[msand]

It still had some problems, but your example helped me fix them. There is still one bug left, the text uses different amounts of width, in comparison to e.g. chrome.

[chrisbolin]

awesome! well done @msand. let me know when you open up a PR on this repo. in the meantime, I may refer people to your branch 👍

[msand]

It seems I managed to make it possible to render almost pixel perfect results now. Beware, it has a magic constant of 1.2 baked into two places, I suspect it might be dpi related, but haven't found the reason yet. Will test different screens/dpis tomorrow. Also the labels on the circle have their x value divided by 1.1 in the react-native version vs the web version, still trying to figure out what's causing this as well.

[msand]

If you want to try out the minimal react-native, next.js, svgs, react-native-svg test repo you can clone from here https://github.com/msand/RNSvgTextBug

[msand]

I just noticed the glyphs in "Choose shame or get war" have their tranforms slightly off

[msand]

Seems to be letterSpacing related, I can notice a similar effect on your example. Might be that kerning isn't working correctly, as setting letterSpacing to a valid numeric value seems to make the output identical.

[msand]

Actually, letter-spacing is completely broken in native.

[msand]

@chrisbolin I have letter-spacing and kerning implemented now as well (even chrome doesn't support the kerning attribute in svg yet). I've removed all magical constant and other strange things.

It is now possible to make it render similar output in both chrome and react-native in android, but native sometimes needs different font-size, letter-spacing, and start-offset values, seemingly depending on the length of the path referenced in the href attribute of the textpath element.

Except for that, it now seems at least possible to get essentially pixel perfect results for text, tspan and textpath, merely requiring scaling of some values for the native rendering.

[gpeal]

@msand @chrisbolin Any update on this? We're upgrading Airbnb to RN 0.45 and this is currently a blocker.

[chrisbolin]

Shoot, I think we're in the same boat then. Maybe we all need to learn Java 😶

…

On Jul 7, 2017 10:04 PM, "Gabriel Peal" ***@***.***> wrote: @msand <https://github.com/msand> @chrisbolin <https://github.com/chrisbolin> Any update on this? We're upgrading Airbnb to RN 0.45 and this is currently a blocker. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#377 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACsPoD_eHlDJvv0s2gymfAfSPwcSG9mGks5sLo8-gaJpZM4N9nxW> .

[msand]

I can make a pull request soon, meanwhile you can just install my fork...

[gpeal]

@msand looks like your branch has a peer dep of RN < 0.45 and react <15.5.0. This is also only an issue for us as a transitive dependency of victory-native so we'll have to fork/update that after this lands.

[msand]

I've merged the changes from master and made sure it works in my test project with the latest stable release of react-native 0.46.3 and the required alpha version 12 of react 16. #402

[msand]

I've also made a compatibility branch for RN 0.45, which only reverts the two merge commit for 0.46 compatibility. https://github.com/msand/react-native-svg/tree/5.2.0-conformance

[msand]

I've also added more examples from the specs, demonstrating some issues mostly with the coordinate systems and more complex real-life svg: msand/RNSvgTextBug@ac20be4

[msand]

The remaining problematic examples

[msand]

I've managed to fix all major remaining bugs with text rendering. There is one bug left which I suspect lies in the result returned by android.graphics.PathMeasure.getLength in https://github.com/msand/react-native-svg/blob/5.3.0-conformance/android/src/main/java/com/horcrux/svg/TSpanShadowNode.java#L124-L125 or paint.getTextWidths in https://github.com/msand/react-native-svg/blob/5.3.0-conformance/android/src/main/java/com/horcrux/svg/TSpanShadowNode.java#L148 which causes the midpoint of the glyph of the last character to sometimes be outside the text path, causing it not to be rendered. I have a special case for this in: https://github.com/msand/react-native-svg/blob/5.3.0-conformance/android/src/main/java/com/horcrux/svg/TSpanShadowNode.java#L178-L183 which uses the tangent from the start position instead, as a temporary workaround.

[msand]

Updated full content scroll view screenshots in https://github.com/msand/RNSvgTextBug
Starting to look quite good, I must say.

[msand]

Fixed the last bug as well, was related to the layout of ligaturized text sequences (e.g. "fi") and the methods I suspected.

[msand]

Now the text rendering on android has my stamp of approval, should be good enough for most purposes, and if any bugs are found I can probably fix them with relative ease at this point. Images seem to have some strange effects related to translate/scale transforms and viewport/box sizes, which I might investigate at some point, but it seems others are looking into that as well right now.

[msand]

@gpeal @chrisbolin can you test it out and verify that it works with your use cases?

[alexcouret]

Hi @msand Is this supposed to be out now or we need to use a fork? I tried the one mentioned above but it doesn't seem to be working on my project.

[msand]

@OzoTek It still hasn't been merged, so you have to use my fork from here: https://github.com/msand/react-native-svg/tree/5.3.0-conformance
so something like:
npm install --save react-native-svg@https://github.com/msand/react-native-svg.git#5.3.0-conformance
Or a specific commit:
npm install --save react-native-svg@https://github.com/msand/react-native-svg.git#36595c9

[msand]

I've realized that the remaining letter-spacing issues are not actual issues with rnsvg, but with chrome and firefox etc. Edge implements it correctly according to spec, as we do. Same goes for rendering text on a path, chrome and firefox have never conformed to the spec, not even close. I've add a midLine prop/attr to TextPath, such that we can at least have the glyph midlines either according to spec, or compatible with the broken behavior of chrome/firefox... The letter-spacing compatibility with chrome and firefox just doesn't make sense, its far too complicated and removed from the language of the spec, much better to use Edge as reference, such that rendering is predictable from the svg spec.

[msand]

Actually it's not even just the letter-spacing, its about calculating any glyph width or advance value, even for a single character its completely off. (In chrome and friefox that is, Edge does it perfectly)

[alexcouret]

Hello, thanks for the answer, but unfortunately I get an error that I don't have using the official version 5.3.2:
Error while updating property 'strokeWidth' in shadow node of type: RNSVGGroup
TypeError: expected dynamic type 'double', but had type 'string'

PS : It may be an compatibility issue with the lib I'm using, which is using this lib.(https://github.com/FormidableLabs/victory-native) so maybe something had been fixed with the 5.3.1 and 5.3.2 of react-native-svg

[msand]

@OzoTek This is because I've changed the proptype of strokeWidth to be string instead of number.
https://github.com/msand/react-native-svg/blob/8892166ea3951e90531bb0533a723df8195f5494/lib/props.js#L42

To make it correspond/conform with the svg spec more:
https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/stroke-width
https://www.w3.org/TR/SVG/painting.html#StrokeWidthProperty
https://www.w3.org/TR/SVG2/painting.html#StrokeWidth

Usage context

Categories: Presentation attribute
Value: < length > | < percentage > | inherit
Animatable: Yes
Normative document: SVG 1.1 (2nd Edition)

The value can be a generic length string with one of the units, or a percentage string. Of course we could allow numbers as well, because in practice, that is now converted to a string and a warning is shown instead. But perhaps it should be changed to a string instead of a number in victory-native instead?

[msand]

@OzoTek I've changed the PropType to numberProp now:
553c177#diff-5f58330644c215957cabd8bafa906d32R42

numberProp is defined as
const numberProp = PropTypes.oneOfType([PropTypes.string, PropTypes.number]);

So, the latest commit in my fork shouldn't produce that warning anymore.

[msand]

@OzoTek Or is your issue on the ios side? I haven't ported my changes to ios yet, and I expect to have done some breaking changes to the api contract from js to native. So, if you only experience this problem in ios, then this probably explains it. Have you tried android?

[alexcouret]

Hey, sorry I was off! I found another way of doing it so I don't need this solution anymore. From what I remember the problem was on both platforms.
Thank you for your help!

[alexcouret]

Nevermind I can't get around it indefinitely haha, I still have the type issue on Android (blocking point) using https://github.com/msand/react-native-svg/tree/5.4.0-conformance 🤔
I also have those kind of issues on other properties on iOS because the first error does not happen. I don't have the hand on those because it's still used through VictoryNative, I guess they will release when svg will be updated with your changes.

[msand]

@OzoTek try this instead: https://github.com/msand/react-native-svg/tree/6.0.0

[msand]

Npm/yarn tend not to get the latest commit for the branch, once it's cached, using the latest commit hash instead of the branch helps against this or just flushing/clearing the cache.

[alexcouret]

Thanks. I don't have the double error on strokeWidth anymore but on lots of others such as:

Invalid prop `fontSize` (same for `dx`, `dy`) of type `number` supplied to `TSpan` (same for `Text`), expected `string`.
JSON value '1' of type NSString cannot be converted to NSNumber
JSON value '(
    250
)' of type NSMutableArray cannot be converted to NSString
JSON value '0' of type NSString cannot be converted to NSNumber
JSON value '1' of type NSString cannot be converted to NSNumber
JSON value '(
    215
)' of type NSMutableArray cannot be converted to NSString
JSON value '13.68' of type NSString cannot be converted to NSNumber
...

PS : I cleared the npm cache

[msand]

Are you sure that you both rebuilt the native bundle and restarted the packager? Otherwise a small reproduction would be very much appreciated :)

[msand]

And try this specific commit f3cd34f

[msand]

Btw, warnings shouldn't show up in the production build. But there seems to be some other errors there which need to be dealt with as well. Any kind of repro would be nice, optimally a fresh react-native init:ed project with only minimal dependencies and smallest amount of code to show the problem. But just source code for an illustrative react component would be sufficient.

[msand]

I've made a fresh new test project for easier testing on ios here: https://github.com/msand/RNSvgTests

So, forking that, and adding another component would be perfect.

[alexcouret]

Here's my fork : https://github.com/OzoTek/RNSvgTests
The warning are displaying on iOS, I'm trying to run it on Android to check as well.

[alexcouret]

On Android I miss an error I have on my project (maybe it comes from another component, as I've put the strict minimum to reproduce it), same warnings as iOS, but the Chart doesn't display.
You can see it at the very bottom on iOS.

EDIT : I've pushed, to run on Android and see the error I'm getting , open node_modules/react-native-view-shot/android/src/main/java/fr/greweb/reactnativeviewshot/RNVIewShotPackage.java and remove the createJSModules() method.

[msand]

@OzoTek I've pushed new commit(s) msand@68eeb5e for all the conformance branches / versions: 6.0.0, 5.4.0, 5.3.0, 5.2.0, 5.1.7.1

They work with your chart example, which I've added to the tests: msand/RNSvgTests@675ed32

Thanks for your help!

[msand]

@chrisbolin @gpeal @OzoTek There is now a new version and pull request, with some of the fixes from android ported to ios: #430

[msand]

@chrisbolin @gpeal @OzoTek I've now ported almost all the fixes from android to ios here: #430
npm i -s react-native-svg@https://github.com/msand/react-native-svg.git#13d5157

[msand]

Everything regarding this should be fixed by now, closing. Reopen or create a new issue if there are remaining issues.