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

Canvas blur after rescale page #7502

Closed
vitonsky opened this issue Nov 18, 2021 · 17 comments
Closed

Canvas blur after rescale page #7502

vitonsky opened this issue Nov 18, 2021 · 17 comments

Comments

@vitonsky
Copy link
Contributor

vitonsky commented Nov 18, 2021

Canvas is not rescale content after resize page.
When we load web page with 50% page zoom and then set zoom to 100% canvas looks very bad like low resolution pucture.

I did try to manually rescale fabricjs inner objects while resize page like that:

export const fixCanvasSize = (canvas: fabric.Canvas) => {
    const lowerCanvasEl = (canvas as any).lowerCanvasEl;

    const width = (canvas as any).width;
    const height = (canvas as any).height;

    lowerCanvasEl.width = width;
    lowerCanvasEl.height = height;

    canvas.renderAll();
};

It's just resize inner canvases to consider actual size.

And this fix problem, but controls to resize objects are render wrong and i can't fix it.
If you have solution, tell about it.

Seems it impossible to fix on library user side.

As i think, we have some variable inside fabricjs which is not update after rescale page and always draw picture as while initiation. Maybe we should update this variables on the library level?

Version

3.6.0

Test Case

It reproduce on any page with fabricjs, try it on the http://fabricjs.com/

Information about environment

All browsers

Steps to reproduce

  • Scale page with canvas to 50%
  • Reload page
  • Scale page to 100%

Expected Behavior

  • Fine canvas picture

Actual Behavior

  • Blur on the canvas, because it was render as 50% size and then resized without rescale
@rockerBOO
Copy link
Contributor

Try using setDimensions

   canvas.setDimensions({
      width: window.innerWidth,
      height: window.innerHeight
    });

@vitonsky
Copy link
Contributor Author

@rockerBOO i've try it, but it not work for me.

I did try use only setDimensions, also tried set size for lowerCanvasEl and then call setDimensions and vice versa.
I also tried all options combinations. It did not fix this bug.

After call setDimensions canvas resize without fix of scale factor.

@rockerBOO
Copy link
Contributor

  canvas.setWidth(window.innerWidth);
  canvas.setHeight(window.innerHeight);

  canvas.calcOffset();
  canvas.requestRenderAll();

I have also used this option. Don't try to adjust the lowerCanvasEl or upperCanvasEl. They are generally abstracted away where you shouldn't need to access them. You can infer things from them but manually adjusting is probably going to cause some conflict.

@vitonsky
Copy link
Contributor Author

vitonsky commented Dec 2, 2021

@rockerBOO unfortunately calcOffset is not fix canvas too. Is it work for you?
Btw, i not resize canvas object, i only rescale web page (but i set size explicitly for test it as you say)

I adjust lowerCanvasEl because this method is fix a image, but after this fix an object controls render with incorrect position.
I think that fabric isn't rescale lowerCanvasEl itself and have not hook to do it manual.
Under rescale i mean not only change size, but change size such a way to consider resolution, display size and page scale.

I've tried calcViewportBoundaries to fix controls, but it not work too.

@rockerBOO
Copy link
Contributor

In my example I run that code on window.onresize = () => { ... } seriously only when the screen size changes and set it to the window inner values. This is all I do to resize it and no special css or other changes. Keeps the canvas at 100% width/height. Again don't touch lowerCanvasEl as that is probably messing up things, even if it "fixes" the size of the canvas.

@vitonsky
Copy link
Contributor Author

vitonsky commented Dec 2, 2021

@rockerBOO is you just set new size while resize event and it work for you? Did you have high resolution image after this manipulation? And if you will not do it you have low resolution image on canvas after rescale page?

@rockerBOO
Copy link
Contributor

Slapped this example together.

<!DOCTYPE html>
<html>
<head>
<script src="https://unpkg.com/fabric@4.6.0/dist/fabric.min.js"></script>
<style>
body { margin: 0; }
</style>
</head>
	<body>
		<canvas id="canvas1"></canvas>
<script>
		window.onload = () => {
	const canvas = new fabric.Canvas("canvas1");

canvas.add(new fabric.Circle({ radius: 100, fill: 'red', top: 100, left: 100 }))
const onResize = () => {
		canvas.setWidth(window.innerWidth)
		canvas.setHeight(window.innerHeight)
}
window.addEventListener('resize', onResize)

		onResize()
}
</script>
	</body>
</html>

@vitonsky
Copy link
Contributor Author

vitonsky commented Dec 2, 2021

@rockerBOO i tried your example

I have blured image:

Reproduce steps:

  • scale page to 50%
  • reload page
  • reset scale of page to 100%

@rockerBOO
Copy link
Contributor

Ahh yes I see what you're saying now. Sorry about that. I have noticed this as long as I have used it but just thought it was a canvas thing.

@vitonsky
Copy link
Contributor Author

vitonsky commented Dec 2, 2021

@rockerBOO it's fine. What you think about it? It seems like bug?
What we can do to fix it?

@asturur
Copy link
Member

asturur commented Dec 3, 2021

hi!
The issue here is that the pagezoom is mixed with retina scaling.
to be honest retina scale values that are lesser than 1 should be ignored to avoid blurryness.

@vitonsky
Copy link
Contributor Author

vitonsky commented Dec 3, 2021

@asturur hi. Can we implement some knob to manual update values which affect on this problem while render?
Or maybe we can fix this bug automatically inside fabricjs, if it possible.

I think little things like this is very important for high quality projects.

Thanks for attention

@asturur
Copy link
Member

asturur commented Dec 5, 2021

we should avoid setting retina scaling to less than 1 in fabricjs, since there are no browsers that have more css pixels than real pixels.
The bug would persist anyway, if you are a page zoom 200, and then set back to 100% you would have a larger canvas.

i think you can open a pr by yourself, the function is called getRetinaScaling and should never return a number smaller than 1.

@vitonsky
Copy link
Contributor Author

vitonsky commented Dec 5, 2021

@asturur thanks for explaining bug mechanic.

I've create PR #7537 with fix

Also i want to leave here solution for people from future who have this bug.

As told @asturur cause in devicePixelRatio property of fabricjs which is not equal with actual devicePixelRatio of window, so you should manually update this value, reinitialize retina scaling and render all like that:

const onResize = () => {
  // update devicePixelRatio
  fabric.devicePixelRatio = fabric.window.devicePixelRatio ||
                          fabric.window.webkitDevicePixelRatio ||
                          fabric.window.mozDevicePixelRatio ||
                          1;
  canvas._initRetinaScaling();

  // re-render canvas with new quality
  canvas.requestRenderAll();
}

window.addEventListener('resize', onResize)

It's should fix problem. Also, fabric is provide a setDimensions method which run _initRetinaScaling and recalc other dynamic properties itself, so maybe it will better to call

@asturur
Copy link
Member

asturur commented Dec 5, 2021

on my projects i attach a resize event on the canvas container and then i use setDimension to adapt the canvas.

@vitonsky
Copy link
Contributor Author

vitonsky commented Dec 5, 2021

@asturur yes, call setDimension is better than call inner method _initRetinaScaling on fabricjs user side.

Btw, what you think about some hook for fabric to update devicePixelRatio ?
Maybe something more abstract, like reinitializeInnerState which execute logic like in first initialization of fabric object.

It may be useful for cases like this bug, when some inner variable is more not actual after some changes which we can't predict.
We will just call something like fabric.reinitializeInnerState() and variables like fabric.devicePixelRatio will recalculate explicitly.

@ShaMan123
Copy link
Contributor

Thanks guys, I think this solves an issue in my project.
+1 to update device pixel ratio on resize internally, _onResize

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

No branches or pull requests

4 participants