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

feat(CanvasContext): Added optional devicePixelRatio argument to resize #1572

Merged
merged 1 commit into from
Jun 1, 2023

Conversation

saitonakamura
Copy link
Contributor

It's mainly useful in context of rendering in web worker, because web worker doesn't have access to devicePixelRatio, so resize code will be split between

  1. main thread asking for a render, providing devicePixelRatio
  2. Worker rendering and resizing CanvasContext w/ provided ratio
  3. Worker giving back main thread the resize target
  4. Main thread applying css width/height provided by the worker

@rvilarl
Copy link
Collaborator

rvilarl commented May 31, 2023

Do you use the SVGContext? Would something similar be required there?

@ronyeh
Copy link
Collaborator

ronyeh commented May 31, 2023

Thanks!

I think we have been slowly going away from checks against null.
Instead we try to use the ?? operator where possible.

Does this code work for you?

  resize(width: number, height: number, devicePixelRatio?: number): this {
    const canvas = this.context2D.canvas;
    const dpr: number = devicePixelRatio ?? globalObject().devicePixelRatio ?? 1;

    // Scale the canvas size by the device pixel ratio clamping to the maximum supported size.
    [width, height] = CanvasContext.sanitizeCanvasDims(width * dpr, height * dpr);

    // Divide back down by the pixel ratio and convert to integers.
    width = (width / dpr) | 0;
    height = (height / dpr) | 0;

    canvas.width = width * dpr;
    canvas.height = height * dpr;

    // The canvas could be an instance of either HTMLCanvasElement or an OffscreenCanvas.
    // Only HTMLCanvasElement has a style attribute.
    if (isHTMLCanvas(canvas)) {
      canvas.style.width = width + 'px';
      canvas.style.height = height + 'px';
    }

    return this.scale(dpr, dpr);
  }

If it works for you, can you resubmit the PR with this approach? I'll be happy to merge it.

Thanks for your contribution!

@ronyeh ronyeh added the 4.2 label May 31, 2023
@saitonakamura saitonakamura force-pushed the feat-device-pixel-ratio-override branch from 1bfe2cf to a5ab925 Compare June 1, 2023 09:11
@saitonakamura
Copy link
Contributor Author

Do you use the SVGContext? Would something similar be required there?

Thanks for the feedback! I personally don't, but I don't think SVGContext takes any advantage of the devicePixelRatio, it's usually only the canvas thing in my experience. Also don't see it in the svgcontext code

I think we have been slowly going away from checks against null. Instead we try to use the ?? operator where possible.

Thanks for the feedback too! Works for me, updated

It's mainly useful in context of rendering in web worker,
because web worker doesn't have access to devicePixelRatio,
so resize code will be split between
1. main thread asking for a render, providing devicePixelRatio
2. Worker rendering and resizing CanvasContext w/ provided ratio
3. Worker giving back main thread the resize target
4. Main thread applying css width/height provided by the worker
@saitonakamura saitonakamura force-pushed the feat-device-pixel-ratio-override branch from a5ab925 to 4fe9a6b Compare June 1, 2023 10:49
Copy link
Collaborator

@rvilarl rvilarl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks.

@ronyeh ronyeh merged commit 253f769 into 0xfe:master Jun 1, 2023
@ronyeh
Copy link
Collaborator

ronyeh commented Jun 1, 2023

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants