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

Document the relationship between offscreen canvas and pixel density #2336

Closed
wcandillon opened this issue Apr 4, 2024 · 11 comments
Closed
Labels
bug Something isn't working

Comments

@wcandillon
Copy link
Contributor

Description

As reported in #1811

Version

latest

Steps to reproduce

Draw an offscreen canvas on a high-density screen. Take a screenshot and compare the image to the same drawing on an onscreen canvas.

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

See #1811

@wcandillon wcandillon added the bug Something isn't working label Apr 4, 2024
@daehyeonmun2021
Copy link
Contributor

CleanShot 2024-04-17 at 18 32 02@2x

I found out those two textures shows different results. (RN Skia v.1.0.2, RN v.0.73.6)

Left: drawing on canvas from offscreen, Right: drawing on canvas from PictureRecoder.

Is this because offscreen doesn't respect pixel density?

@wcandillon
Copy link
Contributor Author

yes this is very likely:

// TODO: We're not sure yet why PixelRatio is not needed here.

Could you share the example with me?

@daehyeonmun2021
Copy link
Contributor

daehyeonmun2021 commented Apr 18, 2024

@wcandillon Here is the code. You can see the density issue and paint stroke goes different if setting strokeWidth is skipped on the two textures.

I attached the result image (above: offscreen, below: pictureRecorder) and code.

Thanks :)

Versions:
RN skia: 1.1.0
react-native: 0.73.6

CleanShot 2024-04-18 at 13 03 19@2x

import {
  Canvas,
  Group,
  Image,
  Line,
  PaintStyle,
  Picture,
  rect,
  SkCanvas,
  Skia,
  vec,
} from '@shopify/react-native-skia';
import { Dimensions } from 'react-native';
import { useDerivedValue } from 'react-native-reanimated';

const { width: stageWidth, height: stageHeight } = Dimensions.get('screen');
const stageHeightHalf = stageHeight / 2;

const TOTAL = 1000;
const RADIUS = 10;
const BALL_STYLE = Skia.Paint();
BALL_STYLE.setStyle(PaintStyle.Stroke);
// BALL_STYLE.setStrokeWidth(1); // * If you skip setting strokeWidth, the two textures shows different result.
const BALLS = Array.from({ length: TOTAL }, () => ({
  x: RADIUS + Math.random() * (stageWidth - RADIUS),
  y: RADIUS + Math.random() * (stageHeightHalf - RADIUS),
  r: RADIUS,
}));

const Page = () => {
  const drawBalls = (canvas: SkCanvas) => {
    'worklet';

    canvas.save();
    for (let i = 0; i < BALLS.length; i++) {
      const ball = BALLS[i];
      canvas.drawCircle(ball.x, ball.y, ball.r, BALL_STYLE);
    }
    canvas.restore();
  };
  const offscreen = useDerivedValue(() => {
    const offscreen = Skia.Surface.MakeOffscreen(stageWidth, stageHeightHalf)!;
    const canvas = offscreen.getCanvas();
    drawBalls(canvas);
    return offscreen.makeImageSnapshot();
  }, []);
  const pictureRecorder = useDerivedValue(() => {
    const recorder = Skia.PictureRecorder();
    const canvas = recorder.beginRecording(rect(0, 0, stageWidth, stageHeightHalf));
    drawBalls(canvas);
    return recorder.finishRecordingAsPicture();
  }, []);

  return (
    <Canvas
      style={{
        width: stageWidth,
        height: stageHeight,
        backgroundColor: 'lightblue',
      }}
    >
      <Group transform={[{ translate: [0, 0] }]}>
        <Image image={offscreen} width={stageWidth} height={stageHeightHalf} />
      </Group>
      <Group transform={[{ translate: [0, stageHeightHalf] }]}>
        <Picture picture={pictureRecorder} />
      </Group>
      <Group>
        <Line p1={vec(0, stageHeightHalf)} p2={vec(stageWidth, stageHeightHalf)} strokeWidth={10} color="blue" />
      </Group>
    </Canvas>
  );
};

export default Page;

@wcandillon
Copy link
Contributor Author

I think this is the expected result.
This is how you need to write the code:

const pd = PixelRatio.get();

  const offscreen = useDerivedValue(() => {
    const off = Skia.Surface.MakeOffscreen(
      stageWidth * pd,
      stageHeightHalf * pd
    )!;
    const canvas = off.getCanvas();
    canvas.scale(pd, pd);
    drawBalls(canvas);
    canvas.restore();
    return off.makeImageSnapshot();
  }, []);

@daehyeonmun2021
Copy link
Contributor

daehyeonmun2021 commented Apr 18, 2024

@wcandillon Nice :) Thanks for the insight ! 🥰

@daehyeonmun2021
Copy link
Contributor

daehyeonmun2021 commented Apr 18, 2024

@wcandillon Oh I have a question, so you are planning to fix this offscreen density issue to make users don't need to add the pixel density configuration for offscreen canvas?

@wcandillon
Copy link
Contributor Author

I don't think it makes sense to add it in this API because the offscreen API has nothing to do with the screen.
But now I am trying to see if this causes inconsistencies with higher-level API (e.g useTexture) which are indeed on screen.

@daehyeonmun2021
Copy link
Contributor

Got it! Thanks 🙏🥰

@wcandillon
Copy link
Contributor Author

What we will do as a first step is document this better.

@wcandillon wcandillon changed the title Offscreen Canvas doesn't respect pixel density Document the relationship between offscreen canvas and pixel density Apr 18, 2024
@wcandillon
Copy link
Contributor Author

I'm closing it for now, as the current situation seems to make sense.

@verger-guo
Copy link

I don't think it makes sense to add it in this API because the offscreen API has nothing to do with the screen.我认为在这个 API 中添加它没有意义,因为离屏 API 与屏幕无关。 But now I am trying to see if this causes inconsistencies with higher-level API (e.g useTexture) which are indeed on screen.但现在我想看看这是否会导致与屏幕上确实存在的更高级别 API(例如 useTexture)不一致。

Yes, it does appear when using useTexture

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

3 participants