Shopify / react-native-skia

High-performance React Native Graphics using Skia
https://shopify.github.io/react-native-skia
MIT License
6.95k stars 447 forks source link

text in ImageSVG will not shown #2404

Closed zhiqingchen closed 5 months ago

zhiqingchen commented 6 months ago

Description

https://shopify.github.io/react-native-skia/docs/images-svg#font-family

The document says: When rendering your SVG with Skia, all fonts available in your app are also available to your SVG. However, the way you can set the font-family attribute is as flexible as on the web.

But I can't use the text that uses the system font in svg.

Version

1.2.1

Steps to reproduce

  1. Use ImageSVG components
  2. Use MakeFromString
  3. Add text to svg string
  4. This svg string is displayed normally in svgo

Snack, code example, screenshot, or link to a repository

const svg = Skia.SVG.MakeFromString(
  `<svg viewBox='0 0 200 200' width="200" height="200" xmlns='http://www.w3.org/2000/svg'>
    <text font-family="sans-serif" font-size="12" x="10" y="10" fill="#fff">Hello World</text>
  </svg>`
)!;

const HelloWorld = () => {
  return (
    <Canvas style={{ flex: 1, width: 300, height: 300 }}>
      <ImageSVG
        svg={svg}
        x={110}
        y={110}
        width={290}
        height={500}
      />
    </Canvas>
  );
};
liamjones commented 5 months ago

I've just hit this while doing upgrades too. From what I've been able to discern so far:

The last version this worked in was 0.1.234, it's been broken since the subsequent release - 0.1.236. I've not tested all releases due to the amount of time it takes to do native builds to test each time but I've bisected between 0.1.213 (the version it was working on for us under RN 0.72) and 1.2.3 (the version I tried to upgrade to under RN 0.73).

Works in 0.1.213, 0.1.223, 0.1.233, 0.1.234. Doesn't work in 0.1.236, 0.1.238, 0.1.241, 1.0.1, 1.1.0, 1.2.0, 1.2.3.

From looking at the diff between 0.1.234 and 0.1.236 the first thing that jumps out as possibly related is this commit which changed how fonts were looked up: https://github.com/Shopify/react-native-skia/commit/92e6bc9d57333c634682604378d055cbf3728cd4. The other big thing I see in this release which may have had an effect is skia moving from m119 to m122: https://github.com/Shopify/react-native-skia/commit/f3042c32636d41520fb7902c20b3e4a14e469009

I'm going to try and debug through JsiSkTypefaceFactory.h & <ImageSVG> to work out what's changed but I'm not a C++ programmer so if someone with more knowledge can help it'd be appreciated! 🙏

wcandillon commented 5 months ago

We will have a look at this issue but currently Text support in SVG is likely to not work. I strongly recommend not relying on it.

I will investigate the issue and advise/document accordingly.

On Wed, May 22, 2024 at 10:03 AM Liam Jones @.***> wrote:

I've just hit this while doing upgrades too. From what I've been able to discern so far:

The last version this worked in was 0.1.234, it's been broken since the subsequent release - 0.1.236. I've not tested all releases due to the amount of time it takes to do native builds to test each time but I've bisected between 0.1.213 (the version it was working on for us under RN 0.72) and 1.2.3 (the version I tried to upgrade to under RN 0.73).

Works in 0.1.213, 0.1.223, 0.1.233, 0.1.234. Doesn't work in 0.1.236, 0.1.238, 0.1.241, 1.0.1, 1.1.0, 1.2.0, 1.2.3.

From looking at the diff between 0.1.234 and 0.1.236 the first thing that jumps out as possibly released is this commit which changed how fonts were looked up: 92e6bc9. The other big thing I see in this release which may have had an effect is skia moving from m119 to m122: f3042c3

I'm going to try and debug through JsiSkTypefaceFactory.h & to work out what's changed but I'm not a C++ programmer so if someone with more knowledge can help it'd be appreciated! 🙏

— Reply to this email directly, view it on GitHub or unsubscribe. You are receiving this email because you are subscribed to this thread.

Triage notifications on the go with GitHub Mobile for iOS or Android.

liamjones commented 5 months ago

I will investigate the issue and advise/document accordingly.

Thank you!

We will have a look at this issue but currently Text support in SVG is likely to not work. I strongly recommend not relying on it.

Okay, that's probably more difficult for me. Like zhiqingchen, I'm hitting this issue via react-native-echarts which is passing a SVG string to Skia.SVG.MakeFromString.

wcandillon commented 5 months ago

we offered the svg module has a convenience we didn't anticipate that people would use as a full blown renderer. Here something else would need to be done.

On Wed, May 22, 2024 at 11:31 AM Liam Jones @.***> wrote:

I will investigate the issue and advise/document accordingly.

Thank you!

We will have a look at this issue but currently Text support in SVG is likely to not work. I strongly recommend not relying on it.

Okay, that's probably more difficult for me. Like zhiqingchen, I'm hitting this issue via react-native-echarts which is passing a SVG string to Skia.SVG.MakeFromString.

— Reply to this email directly, view it on GitHub or unsubscribe. You are receiving this email because you commented on the thread.

Triage notifications on the go with GitHub Mobile for iOS or Android.

zhiqingchen commented 5 months ago

Thanks @wcandillon , this function is really useful.

github-actions[bot] commented 5 months ago

:tada: This issue has been resolved in version 1.3.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

troberts-28 commented 3 months ago

I've just hit this while doing upgrades too. From what I've been able to discern so far:

The last version this worked in was 0.1.234, it's been broken since the subsequent release - 0.1.236. I've not tested all releases due to the amount of time it takes to do native builds to test each time but I've bisected between 0.1.213 (the version it was working on for us under RN 0.72) and 1.2.3 (the version I tried to upgrade to under RN 0.73).

Works in 0.1.213, 0.1.223, 0.1.233, 0.1.234. Doesn't work in 0.1.236, 0.1.238, 0.1.241, 1.0.1, 1.1.0, 1.2.0, 1.2.3.

From looking at the diff between 0.1.234 and 0.1.236 the first thing that jumps out as possibly released is this commit which changed how fonts were looked up: 92e6bc9. The other big thing I see in this release which may have had an effect is skia moving from m119 to m122: f3042c3

I'm going to try and debug through JsiSkTypefaceFactory.h & <ImageSVG> to work out what's changed but I'm not a C++ programmer so if someone with more knowledge can help it'd be appreciated! 🙏

Hey @liamjones, did you ever find a solution to this? In the same boat as you with echarts

liamjones commented 3 months ago

@troberts-28 we went with a somewhat crappy workaround for now; downgrading rn-skia to a version that still renders text on Android, disabling auto-linking on iOS (as that version crashes iOS) and then using the Skia renderer on Android and the SVG renderer on iOS. 😕 I fully appreciate this only works if you're not using Skia for things other than eCharts on iOS and that, at some point in the future, this will likely break entirely because it's not being patched.

Details here: https://github.com/wuba/react-native-echarts/issues/174#issuecomment-2129350461

The 'real' solution is react-native-echarts does a deeper integration with rn-skia for rendering but I have no idea if the maintainers have the appetite & bandwidth for doing that work. I've raised a feature request for this: https://github.com/wuba/react-native-echarts/issues/190

wcandillon commented 3 months ago

The SVG module is not something we have control over and we just offered as a convenience because it is indeed useful in some case. Maybe an SVG renderer in RN Skia could be implemented. There are many opportunities to build something cool there.

wcandillon commented 1 month ago

We will be releasing a new version to RN Skia with Skia m130 and it looks like the Skia team has made big improvements to the text handling in SVG. I will follow up on that, let's see.