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.

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

Severe performance decrease after 1.10.2 update #2904

Open
1 task done
flowtyone opened this issue Jan 21, 2025 · 4 comments
Open
1 task done

Severe performance decrease after 1.10.2 update #2904

flowtyone opened this issue Jan 21, 2025 · 4 comments
Labels
bug Something isn't working

Comments

@flowtyone
Copy link

flowtyone commented Jan 21, 2025

Description

I'm seeing severe performance decrease, 10 to 20 fps drop when using the newest version of react-native-skia. I saw that a new reconciler was introduced and thought I might try to upgrade, but will roll back for now. Still wanted to report it.
I also upgraded react-native-reanimated and react-native-gesture-handler. These are on versions 3.16.7 and 2.22.0.
My canvas is very simple, a full screen gesture handler with around 100 rectangles in a group. The group can be panned and scaled. I was getting 60+ fps on version 1.8.1.

React Native Skia Version

1.10.2

React Native Version

0.76.5

Using New Architecture

  • Enabled

Steps to Reproduce

Run the following code.

Snack, Code Example, Screenshot, or Link to Repository

import React from "react";
import {Dimensions, Platform, View} from "react-native";
import {
    Canvas,
    Fill,
    Group,
    matchFont,
    Rect,
    rect,
    rrect,
    Skia,
    SkMatrix,
    vec,
    Vector,
    Text
} from "@shopify/react-native-skia";
import {useDerivedValue, useSharedValue} from "react-native-reanimated";
import {Gesture, GestureDetector} from "react-native-gesture-handler";

const translateMatrix = (matrix: SkMatrix, x: number, y: number) => {
    "worklet";
    const m = Skia.Matrix();
    m.translate(x, y);
    m.concat(matrix);
    return m;
};


const scaleMatrix = (matrix: SkMatrix, s: number, origin: Vector) => {
    "worklet";
    const source = Skia.Matrix(matrix.get());
    source.translate(origin.x, origin.y);
    source.scale(s, s);
    source.translate(-origin.x, -origin.y);
    return source;
};

const tileProperties = {
    size: {
        width: 100,
        height: 160
    },
    cornerRadius: 16,
    color: "#202020",
    padding: 10
}

const getTilePositions = () => {
    const positions: { x: number, y: number }[] = [];
    const {width, height} = Dimensions.get('window');

    const minX = 0;
    const minY = 0;
    const maxX = width * 3;
    const maxY = height * 3;

    let x = minX, y = minY;

    while (y < maxY) {
        while (x < maxX) {
            positions.push({x, y})

            x += tileProperties.size.width + tileProperties.padding;
        }

        y += tileProperties.size.height + tileProperties.padding
        x = minX;
    }

    return positions;
}

const tilePositions = getTilePositions();

const debugFont = matchFont({
    fontFamily: Platform.select({ios: "Helvetica", default: "serif"}),
    fontSize: 14
});

const Tile: React.FC<{ position: { x: number, y: number } }> = ({position}) => {

    const clipRect = React.useMemo(() => rrect(
        rect(position.x, position.y, tileProperties.size.width, tileProperties.size.height),
        tileProperties.cornerRadius,
        tileProperties.cornerRadius
    ), [position])

    return (
        <Group clip={clipRect}>
            <Rect
                style="fill"
                color={tileProperties.color}
                width={tileProperties.size.width}
                height={tileProperties.size.height}
                {...position}
            />
            <Text
                text="1"
                x={position.x + 30}
                y={position.y + 60}
                font={debugFont}
                color="white"
            />
        </Group>
    )
}
const Tiles = () => {

    const pivot = useSharedValue(Skia.Point(0, 0));
    const position = useSharedValue(Skia.Point(0, 0));

    const matrix = useSharedValue<SkMatrix>(Skia.Matrix());
    const prevMatrix = useSharedValue(Skia.Matrix());

    const pan = Gesture.Pan()
        .onChange((e) => {
            matrix.value = translateMatrix(
                matrix.value,
                e.changeX,
                e.changeY
            );
        });

    const pinch = Gesture.Pinch()
        .onStart((e) => {
            prevMatrix.value = matrix.value;

            pivot.value = vec(
                e.focalX - position.value.x,
                e.focalY - position.value.y
            );
        })
        .onChange((e) => {
            matrix.value = scaleMatrix(
                prevMatrix.value,
                e.scale,
                pivot.value
            );
        });

    const gesture = Gesture.Race(pinch, pan);

    return (
        <GestureDetector gesture={gesture}>
            <View style={{flex: 1}}>
                <Canvas style={{flex: 1}}>
                    <Fill color="black"/>
                    <Group matrix={matrix}>
                        {tilePositions.map((point) => (
                            <Tile
                                key={`${point.x},${point.y}`}
                                position={point}
                            />
                        ))}
                    </Group>
                </Canvas>
            </View>
        </GestureDetector>
    )
}

export default Tiles;
@flowtyone flowtyone added the bug Something isn't working label Jan 21, 2025
@wcandillon
Copy link
Contributor

wcandillon commented Jan 21, 2025

Thank you for the nice standalone example this looks very interesting, I'm gonna investigate this with the highest priority. You can use CanvasOld to switch back and forth between implementations.

I'm really interested to see what might be happening there.

@flowtyone
Copy link
Author

Thanks! Let me know if you need any additional details. I also noticed that when I install the app in release mode on my android phone, performance is significantly better, but still lower than the previous version.

@wcandillon
Copy link
Contributor

wcandillon commented Jan 21, 2025

Thank you again for the compelling example. Here I rewrote your example in a way that beats the performance of even the old canvas: https://gist.github.com/wcandillon/f315c9c9ec800c8302b33421700085da (no chance to have a frame drop here, I measured ~100x faster).

That being said, this example, written the way you did, shouldn't be so slow.
Let me provide some context on what is happening here.

In its current form, this new reconciler is completely dependent on Hermes/JSI performance. We did a lot of benchmarking and an example like the one you provided completely escaped us (because indeed we would write it as in the gist I provided above). What we would benchmark instead is an example like the one you provided but where each tile is animated. At that scale, the user would have to engage as many reanimated derived values as tiles and therefore the JS overhead Skia would add on each frame would become smaller in comparison to the consumed frame budget (not negligible but definitely smaller).

What we immediately did is to write a C++ version to remove JS out of the equation, but because we need to access the same value from Reanimated, we still need to use the JSI API (to execute sharedValue.value essentially). We have a prototype of this and it's definitely faster but not nearly as fast as we would have hoped.
So we designed a solution in C++ where there is no JS nor JSI involved: Reanimated will soon provide a C++ API that allows us to query for the current value of a shared value without using JSI. And we thought we would wait for this feature to come out as it would completely remove JS and JSI out of the equation and it would just be pure native performance.
However, seeing this issue, it looks like we may need to provide a C++ implementation more urgently than anticipated. I hope this message helps and makes sense. Let me know what you think.

@flowtyone
Copy link
Author

flowtyone commented Jan 21, 2025

@wcandillon I wasn't aware of the drawAsPicture method, that's pretty cool. I wish I could use it, its just that in my actual app code (which is not that far from this example), the text on the tiles needs to change.

Thank you for the in-depth explanation, I will be using the older implementation until you're able to fix the issue, no worries.

I'm also looking for a way to render rectangles conditionally based on a sharedValue, and it seems like you might be able to bridge that gap aswell with the c++ API. I'm currently using runOnJS when a gesture ends instead of updating the values properly every frame. So in short, looking forward to it!

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

No branches or pull requests

2 participants