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

Automatically change hardware scaling based on browser zoom level #12711

Closed
PatrickRyanMS opened this issue Jul 6, 2022 · 10 comments · Fixed by #12737
Closed

Automatically change hardware scaling based on browser zoom level #12711

PatrickRyanMS opened this issue Jul 6, 2022 · 10 comments · Fixed by #12737
Assignees
Milestone

Comments

@PatrickRyanMS
Copy link
Member

When the user sets the browser zoom to anything other than 100%, especially when larger than 100%, we can see artifacts in the render due to the canvas thinking it's a different size than it actually is. Zooming in causes the canvas to render at a lower resolution and shows pixelation of all elements. Any ADT will scale with the zoom level, but still renders at the lower resolution of the canvas. Here is a repro:

  1. Open any sandbox/PG... this one will do Interaction Hint | Babylon.js Playground (babylonjs.com)
  2. Use the browser zoom to zoom in (ctrl+ to zoom in ctrl- to zoom out)
  3. Notice that the canvas is losing resolution as you zoom in and while the ADT is also scaling with the zoom level, it is also losing resolution

If we can detect the zoom level of the browser can we dynamically adjust hardware scaling on the scene so that the resolution of the canvas does not change even if the zoom level does?

@RaananW
Copy link
Member

RaananW commented Jul 11, 2022

We already have a mechanism to adjust according to the zoom ratio, it is simply turned off by default. it can be set when constructing an engine as the 4th variable (adaptToDeviceRatio), and is being used on every resize call to the engine. A zoom call in the browser triggers a window.onresize event. Maybe allowing a better configuration of this value will be enough here?

The position in code - https://github.com/BabylonJS/Babylon.js/blob/master/packages/dev/core/src/Engines/thinEngine.ts#L1731 . We are taking devicePixelRatio in account in different areas (mostly during construction). Maybe we should revisit how we handle this and what we want the defaults to be.

@RaananW
Copy link
Member

RaananW commented Jul 11, 2022

Oh, and also wanted to state - this will only work if the developer registers the resize event of window. We always recommend doing that, but the dev can, of course, skip this step. In this case maybe we can force registering the resize event just in case of a window resize. But that will change the default behavior.

@deltakosh
Copy link
Contributor

I think the resize is not trigger during the zoom change. Or is it?

@RaananW
Copy link
Member

RaananW commented Jul 11, 2022

Checked, it does.

@RaananW
Copy link
Member

RaananW commented Jul 11, 2022

Sorry! just making sure we are talking about the same thing :-)
babylon resize - not necessarily (as it needs to be added by the dev). window.onresize - this one is triggered when you zoom in your browser. at least in chrome.

@RaananW
Copy link
Member

RaananW commented Jul 11, 2022

Zoom in and out here - https://playground.babylonjs.com/#7FBWRV , quality will stay the same.

@deltakosh
Copy link
Contributor

so we are good because then we have everything we need right @PatrickRyanMS ?

@RaananW
Copy link
Member

RaananW commented Jul 11, 2022

I think the only thing we don't have and might be missing is a way to adjust this flag during a session. I don't see a reason why we don't expose the adjust ratio flag and let devs change it when needed (i.e. - why is it only in the constructor).

@deltakosh
Copy link
Contributor

no problem to change it

@PatrickRyanMS PatrickRyanMS assigned RaananW and unassigned thomlucc Jul 11, 2022
@PatrickRyanMS
Copy link
Member Author

@RaananW and I chatted and we think we need to add an additional parameter in the engine constructor for the maximum hardware scaling factor. We have a parameter for the minimum hardware scaling factor already with limitDeviceRatio but adding a parameter for a maximum hardware scaling factor will prevent perf issues on retina displays when zooming out. Otherwise enabling adaptToDeviceRatio in the constructor should prevent the pixelation issues when zooming in.

deltakosh pushed a commit that referenced this issue Jul 12, 2022
…2737)

Fixes #12711
This does 2 things:
1) If adapt to device ratio is turned on we are using the delta between the last recorded device pixel ratio and the current one to change the hardware scaling rate.
2) adaptToDeviceRatio is not public and can be changed not only in the constructor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants