Skip to content
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

Jump to bottom

[iOS] Image re-rendering with potential memory leak #2909

Open
1 task done
DImuthuUpe opened this issue Jan 23, 2025 · 15 comments
Open
1 task done

[iOS] Image re-rendering with potential memory leak #2909

DImuthuUpe opened this issue Jan 23, 2025 · 15 comments
Labels
bug Something isn't working

Comments

@DImuthuUpe
Copy link

Description

I am trying to build a pdf document viewer using react native skia and reanimated. This requires rendering bitmap tiles of a pdf page at different resolution levels when we peform pinch zoom on canvas. As a result of that, I have to regenerat the Skia Image multiple times and assign to the same image component. Please have a look at following simplified code sample. The issue I am struggling is, when I zoom in and generate images of 3000 x 3000 pixels, memory usage goes up as expected but when I zoom out and generate 300x300 bitmaps, memory does not go back down. I initially though that this is related to delayed garbage collection but it seems like not the case as the memory is occupied permanantly for a long period of time. I lookd at the memory profile and there is a lot of objects with stuck at SkData::MakeWithCopy and I think those are the images I created on demad when the pinch zoom ended. Can you help me to figure out why this memory leak is happening and what could be the potential resolution for this?

import React, { useEffect, useState } from 'react';
import { View, StyleSheet } from 'react-native';
import { Canvas, Skia, AlphaType, ColorType, Fill, Image, SkImage } from '@shopify/react-native-skia';
import { GestureDetector, Gesture, GestureHandlerRootView } from 'react-native-gesture-handler';
import Animated, {
  useSharedValue,
  useDerivedValue,
  runOnJS,
} from 'react-native-reanimated';

function App(): React.JSX.Element {

  const tileSize = 512;
  const scale = useSharedValue(1);
  const translationX = useSharedValue(0);
  const translationY = useSharedValue(0);
  const [scaleEnd, setScaleEnd] = useState(1);
  const [skiaImage, setSkiaImage] = useState<SkImage>();

  const createTile = (tileSize: number) => {
      const pixels = new Uint8Array(tileSize * tileSize * 4);
      pixels.fill(255);
      let i = 0;
      for (let x = 0; x < tileSize; x++) {
        for (let y = 0; y < tileSize; y++) {
          pixels[i++] = (x * y) % 255;
        }
      }
      const data = Skia.Data.fromBytes(pixels);
      const img = Skia.Image.MakeImage(
        {
          width: tileSize,
          height: tileSize,
          alphaType: AlphaType.Opaque,
          colorType: ColorType.RGBA_8888,
        },
        data,
        tileSize * 4
      );
  
      return img;
    }


  if (skiaImage === null) {
    return <View style={styles.container} />;
  }

  const panGesture = Gesture.Pan().onChange((e) => {
    translationX.value += e.changeX;
    translationY.value += e.changeY;
  });
                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                   
  const scaleGesture = Gesture.Pinch().onChange((e) => {
    scale.value +=  e.scaleChange - 1;
  }).onEnd(() => { 
    runOnJS(() => {
      setScaleEnd(scale.value);
    })();
  }).runOnJS(true);

  useEffect(() => { 
    const skImg = createTile(tileSize * scale.value);
    console.log("Skia image calculating for tile size " + tileSize * scale.value);
    if (skImg !== null) {
      setSkiaImage(skImg);
    } 
  }, [scaleEnd]);

  return (
    <GestureHandlerRootView>
      <View style={{ flex: 1 }}>
        <Canvas style={{ flex: 1 }}>
          <Fill color="pink" />
          <Image image={skiaImage} x={translationX} y={translationY} width={useDerivedValue(() => tileSize * scale.value)} height={useDerivedValue(() => tileSize * scale.value)} />
        </Canvas>
        <GestureDetector gesture={Gesture.Race(panGesture, scaleGesture)}>
          <Animated.View style={StyleSheet.absoluteFill} />
        </GestureDetector>
      </View>
    </GestureHandlerRootView>
  );
};

const styles = StyleSheet.create({
  container: {
    flex: 1,
    backgroundColor: "pink",
  },
  canvasContainer: {
    flex: 1,
    justifyContent: "center",
    alignItems: "center",
  },
  canvas: {
    flex: 1,
  },
  animatedCanvas: {
    flex: 1,
  },
});

export default App;

React Native Skia Version

1.9.0

React Native Version

0.76.5

Using New Architecture

  • Enabled

Steps to Reproduce

Run the attached code and try multiple pinch zoom on iOS

Snack, Code Example, Screenshot, or Link to Repository

Image Image
@DImuthuUpe DImuthuUpe added the bug Something isn't working label Jan 23, 2025
@wcandillon
Copy link
Contributor

wcandillon commented Jan 23, 2025 via email

@DImuthuUpe
Copy link
Author

@wcandillon I tried to dispose the old image but that raised some EXEC_BAD_ACCESS errors on C++. Please let me whether following way is correct?

useEffect(() => { 
    const skImg = createTile(tileSize * scale.value);
    console.log("Skia image calculating for tile size " + tileSize * scale.value);
    if (skImg !== null) {
      const oldSkiaImage = skiaImage;
      setSkiaImage(skImg);
      if (oldSkiaImage) {
        oldSkiaImage.dispose();
      }
    } 
  }, [scaleEnd]);
Image

@ronickg
Copy link

ronickg commented Feb 5, 2025

Had a similiar issue only on iOS when re-rendering the skia image from a base64 png it would cause a crash. Can also provide some logs if it helps.

@wcandillon
Copy link
Contributor

wcandillon commented Feb 5, 2025 via email

@ronickg
Copy link

ronickg commented Feb 6, 2025

@wcandillon

Skia version: "@shopify/react-native-skia": "^1.11.4"
RN version: "react-native": "0.76.6"

Image

mobilizeV2(57863,0x16c2bf000) malloc: *** error for object 0xc: pointer being freed was not allocated
mobilizeV2(57863,0x16c2bf000) malloc: *** set a breakpoint in malloc_error_break to debug

Issue occurs when creating an image like this:

 const skiaImage = Skia.Image.MakeImageFromEncoded(
    Skia.Data.fromBase64(
      "base64 string is in file below"
    )
  );

Base64 string of the image image.txt

@wcandillon
Copy link
Contributor

@ronickg thanks a lot, this error seems consistent with some other reports we've seen. Just to make sure, it's not as simple as running the snippet you kindly sent to reproduce the error right? I have to assume that it used in another runtime context (reanimated worklet)?
Let me know if you have a strong lead on how to reproduce the crash.

@ronickg
Copy link

ronickg commented Feb 6, 2025

@wcandillon It should be as simple as just running the code without any worklets. To reproduce the error I just created a screen using expo router and just navigated to it. When wrapping it in a memo without any dependencies it works fine as it doesn't re-render, as the issue happens as soon as it needs to run the code a second time. And only had this issue on iOS, on android it works.

Image

@ronickg
Copy link

ronickg commented Feb 6, 2025

I'll make a new project repo to mimic the error which should make debugging easier.

@wcandillon
Copy link
Contributor

Thanks a lot 🫶🏻

@ronickg
Copy link

ronickg commented Feb 6, 2025

Okay, i wasn't able to reproduce the issue in a new repo, so I went back to testing my initial one. And initially I though it was crashing even with just the simple code snippet above, but I think i may have not correctly copied the base64 string. So from all that yes I do think its due to the worklets. I am using the latest react native worklets core and set the base64 string like so:


  const saveImagesAndStop = Worklets.createRunOnJS(
    async (imagesString: string) => {
     setSnapShotBase64(imagesString);
    }
  );

This function is called from inside the frameProcessor in vision camera. Which I am sure is somehow causing the crash.

So I don't think there is anything wrong on the skia side. Maybe this helps someone else who has the same issue. Thx

@wcandillon
Copy link
Contributor

wcandillon commented Feb 6, 2025 via email

@DImuthuUpe
Copy link
Author

DImuthuUpe commented Feb 8, 2025 •
edited
Loading

@wcandillon here it is. For some reasosn, this does not crash for me now but still the memory does not come down when I zoom out. Try to zoom in until you see memory usage goes in beyond 1 gb and try to zoom out. You might notice that the memory does not scale back to 500MBs

import React, { useEffect, useState } from 'react';
import { View, StyleSheet } from 'react-native';
import { Canvas, Skia, AlphaType, ColorType, Fill, Image, SkImage } from '@shopify/react-native-skia';
import { GestureDetector, Gesture, GestureHandlerRootView } from 'react-native-gesture-handler';
import Animated, {
  useSharedValue,
  useDerivedValue,
  runOnJS,
} from 'react-native-reanimated';

function App(): React.JSX.Element {

  const tileSize = 512;
  const scale = useSharedValue(1);
  const translationX = useSharedValue(0);
  const translationY = useSharedValue(0);
  const [scaleEnd, setScaleEnd] = useState(1);
  const [skiaImage, setSkiaImage] = useState<SkImage>();

  const createTile = (tileSize: number) => {
      const pixels = new Uint8Array(tileSize * tileSize * 4);
      pixels.fill(255);
      let i = 0;
      for (let x = 0; x < tileSize; x++) {
        for (let y = 0; y < tileSize; y++) {
          pixels[i++] = (x * y) % 255;
        }
      }
      const data = Skia.Data.fromBytes(pixels);
      const img = Skia.Image.MakeImage(
        {
          width: tileSize,
          height: tileSize,
          alphaType: AlphaType.Opaque,
          colorType: ColorType.RGBA_8888,
        },
        data,
        tileSize * 4
      );
  
      return img;
    }


  if (skiaImage === null) {
    return <View style={styles.container} />;
  }

  const panGesture = Gesture.Pan().onChange((e) => {
    translationX.value += e.changeX;
    translationY.value += e.changeY;
  });
                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                   
  const scaleGesture = Gesture.Pinch().onChange((e) => {
    scale.value +=  e.scaleChange - 1;
  }).onEnd(() => { 
    runOnJS(() => {
      setScaleEnd(scale.value);
    })();
  }).runOnJS(true);

  useEffect(() => { 
    const skImg = createTile(tileSize * scale.value);
    console.log("Skia image calculating for tile size " + tileSize * scale.value);
    if (skImg !== null) {
      const oldSkiaImage = skiaImage;
      setSkiaImage(skImg);
      if (oldSkiaImage) {
        oldSkiaImage.dispose();
      }
    } 
  }, [scaleEnd]);
  

  return (
    <GestureHandlerRootView>
      <View style={{ flex: 1 }}>
        <Canvas style={{ flex: 1 }}>
          <Fill color="pink" />
          <Image image={skiaImage} x={translationX} y={translationY} width={useDerivedValue(() => tileSize * scale.value)} height={useDerivedValue(() => tileSize * scale.value)} />
        </Canvas>
        <GestureDetector gesture={Gesture.Race(panGesture, scaleGesture)}>
          <Animated.View style={StyleSheet.absoluteFill} />
        </GestureDetector>
      </View>
    </GestureHandlerRootView>
  );
};

const styles = StyleSheet.create({
  container: {
    flex: 1,
    backgroundColor: "pink",
  },
  canvasContainer: {
    flex: 1,
    justifyContent: "center",
    alignItems: "center",
  },
  canvas: {
    flex: 1,
  },
  animatedCanvas: {
    flex: 1,
  },
});

export default App;

@wcandillon
Copy link
Contributor

The topic of the memory not coming down is one I need to be more educated on.
We've ran some benchmarks where we could measure that all objects were deleted (and the gc running at regular intervals), running some drawings for hours.
However the allocated memory we would see on the iOS perf monitor of the app would never come down.

I would like to learn more about Hermes behaviours there and understand better this setup and I asked the question at facebook/hermes#1518 but didn't get any feedback on it yet.
Any information that would help me understand this topic better would be useful.

@DImuthuUpe
Copy link
Author

DImuthuUpe commented Feb 11, 2025 •
edited
Loading

I did some customizations to Skia C++ code to monitor the reference count of SKData shared pointer and in some cases, that does not go beyond 1 which causes the memory leak in my case (even if I call dispose of SkData and SkImage). I changed the logic to manually delete the pointer SkData when I call dispose from JS side and then the memory came down but this is causing crashes on long run. I am trying to come up with the minimal code to repreoduce this without gesture handlers. I think for this particular problem, I can clearly see a memory leak and SkData objects are not getting deleted when I did profiling on Xcode

@maxximee
Copy link

maxximee commented Feb 13, 2025 •
edited
Loading

I seem to have memory leak issues with skia as well lately. Can't tell if it's related at all, not sure how I can debug this. When I activate skiaFrameProcessor, you can see the memory going up until app crashes.
video

all the flag does is activate

    const frameProcessor = useSkiaFrameProcessor(frame => {
        'worklet'
            frame.render(defaultPaint)
    }, [paint, defaultPaint, negativeColors])

where: const defaultPaint = Skia.Paint()

Image

Update: Able to fix the issue be downgrading version of:
skia from 1.11.5 -> 1.10.2
worklet-core from 1.5.0 -> 1.3.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@wcandillon @DImuthuUpe @ronickg @maxximee