Shopify / react-native-skia

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

Android EGL_BAD_MATCH error on first frame #1904 #1954

Closed sumitmanral closed 1 year ago

sumitmanral commented 1 year ago

Description

Got more finding on #1904

The android issue is only reproducible on android devices with Mali GPU (eg . Google Pixel, Oppo)and works fine on android phones on Adreno GPU like Samsung Galaxy , One plus etc

The SkiaView freezes and on scrolling below error comes -

E/libEGL (16722): eglMakeCurrentImpl:1014 error 3009 (EGL_BAD_MATCH)11-06 08:51:19.301 E/libEGL (16722): eglMakeCurrentImpl:1014 error 3009 (EGL_BAD_MATCH)

Version

214

Steps to reproduce

Plz refer #1904 SkiaView freezes on first frame on android devices with Mali GPU

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

Plz refer #1904

wcandillon commented 1 year ago

Thanks for reaching out Sumit and I would like to help you fix this issue hopefully. Here are some of the comments from #1904:

Closing for now but if you have new elements to provide, we will reopen it immediately.

sumitmanral commented 1 year ago

Thanks for your reply . Is there a document I can refer to for moving from SKiaview to Canvas? What is the alternative for onDraw of SkiaView in Canvas?

wcandillon commented 1 year ago

We are not documenting SkiaView and rather Canvas so I'm almost more curious on how you manage to build your code base using SkiaView? Sorry for the confusion there.

jcgertig commented 1 year ago

We use typescript, so it is just based on the RNSkiaDrawCallback. Where we get canvas: SkCanvas. We pass this into draw methods to control the draw directly.

An example method

export const drawNoData = (
  baseYOffset: number,
  textColor: string,
  backgroundColor: string,
  ctx: DrawingContext,
  typeface: SkTypeface | null,
  text = "No Data",
  width = 67
) => {
  ctx.canvas.save();

  const y = baseYOffset + (ctx.height - baseYOffset) / 3;
  const x = ctx.width / 2 - width / 2 - NO_DATA_PADDING;

  const font = getFont(typeface, NO_DATA_FONT_SIZE);

  const strokePaint = ctx.paint.copy();
  strokePaint.setStyle(PaintStyle.Fill);
  strokePaint.setColor(Skia.Color(backgroundColor));
  ctx.canvas.drawRRect(
    Skia.RRectXY(
      Skia.XYWHRect(
        x,
        y,
        width + NO_DATA_PADDING * 2,
        NO_DATA_FONT_SIZE + NO_DATA_PADDING * 2.5
      ),
      NO_DATA_PADDING,
      NO_DATA_PADDING
    ),
    strokePaint
  );

  const textPaint = ctx.paint.copy();
  textPaint.setStyle(PaintStyle.Fill);
  textPaint.setColor(Skia.Color(textColor));
  ctx.canvas.drawText(
    text,
    x + NO_DATA_PADDING,
    y + NO_DATA_FONT_SIZE + NO_DATA_PADDING,
    textPaint,
    font
  );

  ctx.canvas.restore();
};
jcgertig commented 1 year ago

We do this because we are rendering a high-update data grid with many columns and rows, so we are being very specific about what is drawn and when.

jcgertig commented 1 year ago

We started with this about a year and a half ago when there was no drawing component. We could do something like this potentially:

<Canvas style={{ flex: 1 }} mode="continuous">
  <Drawing
    drawing={({ canvas, paint }) => {
      // methods
    }}
  />
</Canvas>
sumitmanral commented 12 months ago

@wcandillon I tried using below code .


<Canvas
        fsClass="fs-unmask"
        ref={ref}
        style={style}
        mode={mode}
        debug={debug}
        //onTouchEnd={handleTouchEnd}
        onLayout={onLayout}
        //onSize={size}
      >
        <Drawing
          drawing={(context) => {
            handleDrawing(context);
          }}
        />
      </Canvas>

I don't get the freeze issue and EGL_BAD_MATCH error now . But the app starts hanging after a while and is also crashing after some time in android and IOS

wcandillon commented 12 months ago

EGL_BAD_MATCH indicates a thread issue which makes sense if you are using SkiaView or Drawing the code you have in handleDrawing you can use the same code using this API: https://shopify.github.io/react-native-skia/docs/shapes/pictures/ this will get rid of the error. I may also recommend to use regular canvas components if possible

sumitmanral commented 12 months ago

Thanks for your reply. I am not sure if Picture is right choice for us as we have a constantly updating data grid where rows and columns needs to be redrawn . I don't see a draw or drawing function in Picture which I can use for drawing .

jcgertig commented 12 months ago

Maybe we can use but will need to move all of the offset cords to shared values. I'm worried about perf though

On Tue, Nov 7, 2023, 11:10 PM Sumit Manral @.***> wrote:

Thanks for your reply. I am not sure if Picture is right choice for us as we have a constantly updating data grid where rows and columns needs to be redrawn . I don't see a draw or drawing function in Picture which I can use for drawing .

— Reply to this email directly, view it on GitHub https://github.com/Shopify/react-native-skia/issues/1954#issuecomment-1801095887, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADPFO7TEL364IB2JV4MPXDYDMH57AVCNFSM6AAAAAA67EP5ASVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBRGA4TKOBYG4 . You are receiving this because you commented.Message ID: @.***>

wcandillon commented 12 months ago

SkiaView and Drawing are actually slower APIs (due to their locking of the JS thread), this is one of the reason we are deprecating them.

sumitmanral commented 12 months ago

Tried using Picture, used sharedValue to update the Picture but useDerivedValue didn't work so tried useDerivedValueonJS . In the below code both drawValue and layout values are getting updated, but the picture isn't getting updated according to the updated value .

If I don't use sharedValue , and use just a state instead of sharedValue and put that on useMemo dependency for createPicture it works fine but is causing some slowness.

const picture = useDerivedValueOnJS(
      () =>
        createPicture(
          {
            x: layout.x,
            y: layout.y,
            width: layout.width,
            height: layout.height,
          },
          (canvas) => {
            console.log("updated values - ", drawValue.value, layout);
            const paint = Skia.Paint();
            paint.setColor(Skia.Color("pink"));
            canvas.drawRect(
              { x: 0, y: 0, width: 100, height: 100 + drawValue.value },
              paint
            );

            const circlePaint = Skia.Paint();
            circlePaint.setColor(Skia.Color("orange"));
            canvas.drawCircle(
              50 + drawValue.value,
              50 + drawValue.value,
              50,
              circlePaint
            );
          }
        ),

      [drawValue, layout]
    );

    return (
      <GestureDetector gesture={Gesture.Race(tap, longPress, scroll)}>
        <Canvas
          ref={canvasRef}
          style={{ flex: 1 }}
          debug={debug}
          onLayout={handleLayout}
        >
          <Picture picture={picture} />
        </Canvas>
      </GestureDetector>
    );
wcandillon commented 12 months ago

the example app uses Reanimated + picture API, his this definitely the recommended API.

sumitmanral commented 12 months ago

In the above code both drawValue and layout values are getting updated, but the picture isn't getting updated according to the updated value because of useDerivedValueOnJS .

Tried looking for Picture + Reanimated Example app, was not able to find it

wcandillon commented 12 months ago

here it is: https://github.com/Shopify/react-native-skia/blob/main/example/src/Examples/API/Touch.tsx#L41

On Thu, Nov 9, 2023 at 3:37 PM Sumit Manral @.***> wrote:

In the above code both drawValue and layout values are getting updated, but the picture isn't getting updated according to the updated value because of useDerivedValueOnJS .

Tried looking for Picture + Reanimated Example app, was not able to find it

— 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.

sumitmanral commented 12 months ago

I tried Picture recording example , it seems to be working initially but sometimes it crashes

-- Thread Performance Checker: Thread running at QOS_CLASS_USER_INTERACTIVE waiting on a lower QoS thread running at QOS_CLASS_DEFAULT. Investigate ways to avoid priority inversions PID: 83862, TID: 5257673

JSI_HOST_FUNCTION(finishRecordingAsPicture) { auto picture = getObject()->finishRecordingAsPicture(); // com.facebook.react.JavaScript (11): EXC_BAD_ACCESS (code=1, address=0xc)

Code below -

 const scroll = Gesture.Pan()
      .runOnJS(true)
      .onStart((pos) => {
        recorder.current = Skia.PictureRecorder();
        const canvas = recorder.current.beginRecording(
          Skia.XYWHRect(0, 0, layout.width, layout.height)
        );
        const ctx: DrawingContext = {
          canvas,
          paint: Skia.Paint(),
          width: layout.width,
          height: layout.height,
        };
        ctxRef.current = ctx;
        onStart(pos);
      })
      .onChange((pos) => {
        onActive(pos);
      })
      .onEnd((pos) => {
        onEnd();
        touchEndY(pos.velocityY);
        touchEndUnpinnedX(pos.velocityX);
        touchEndPinnedX(pos.velocityX);   
      })
      .onFinalize(() => {
        picture.value = recorder.current.finishRecordingAsPicture();
      });

    return (
      <GestureDetector gesture={Gesture.Race(tap, longPress, scroll)}>
        <Canvas
          ref={canvasRef}
          style={{ flex: 1 }}
          debug={debug}
          onLayout={handleLayout}
        >
          <Picture picture={picture} />
        </Canvas>
      </GestureDetector>
    );
  }
wcandillon commented 12 months ago

can you provide a small reproducible example that works in standalone so we can try it on our side?

sumitmanral commented 12 months ago

Example below. Works for around a minute on continuous Pan Gestures and then crashes with the error mentioned above.

const rec = Skia.PictureRecorder();
rec.beginRecording(Skia.XYWHRect(0, 0, 1, 1));
const emptyPicture = rec.finishRecordingAsPicture();

export const Example = 
  () => {
    const [layout, setLayout] = React.useState<LayoutRectangle>({
      x: 0,
      y: 0,
      width: 100,
      height: 100,
    });
    const picture = useSharedValue<SkPicture>(emptyPicture);
    const ctxRef = React.useRef<DrawingContext | null>(null);
    const recorder = React.useRef<SkPictureRecorder>(Skia.PictureRecorder());

    const handleLayout = useCallback(
      (event: LayoutChangeEvent) => {
        setLayout(event.nativeEvent.layout);
      },
      []
    );

    const onActive = useRefCallback(
      (pos: GestureUpdateEvent<PanGestureHandlerEventPayload>) => {
        if (ctxRef.current) {
          const strokePaint = ctxRef.current.paint.copy();
          strokePaint.setStyle(PaintStyle.Fill);
          strokePaint.setColor(Skia.Color("red"));
          ctxRef.current.canvas.drawRect(
            Skia.XYWHRect(pos.x, pos.y, layout.width, layout.height),
            strokePaint
          );
        }

      },
      [layout]
    );

    const scroll = Gesture.Pan()
      .runOnJS(true)
      .onStart((pos) => {
        recorder.current = Skia.PictureRecorder();
        const canvas = recorder.current.beginRecording(
          Skia.XYWHRect(0, 0, layout.width, layout.height)
        );
        const ctx: DrawingContext = {
          canvas,
          paint: Skia.Paint(),
          width: layout.width,
          height: layout.height,
        };
        ctxRef.current = ctx;

      })
      .onChange((pos) => {
        onActive(pos);
      })
      .onEnd((pos) => {

      })
      .onFinalize(() => {
        picture.value = recorder.current.finishRecordingAsPicture();
      });

    return (
      <GestureDetector gesture={scroll}>
        <Canvas
          style={{ flex: 1 }}
          debug={debug}
          onLayout={handleLayout}
        >
          <Picture picture={picture} />
        </Canvas>
      </GestureDetector>
    );
  }
);
wcandillon commented 12 months ago

the code above it too error-prone, you shouldn't save the canvas into a ref and have it created/deleted in depending on the gesture state, it's way too easy to get into an invalid state here.

sumitmanral commented 12 months ago

In the example here we are doing same thing on gestures - https://github.com/Shopify/react-native-skia/blob/main/example/src/Examples/API/Touch.tsx#L41

only thing I have added is creating Recorder ref and canvas ref .

wcandillon commented 12 months ago

Your code makes it so the canvas can be in a wrong state.

jcgertig commented 12 months ago

Yeah @sumitmanral I would not have gone this direction shoot me the poc and ill review

sumitmanral commented 12 months ago

Sure I will send it over . The reason I made it a ref was the incoming data that should also cause redraw of the Picture not just Gestures