Skip to content

Commit

Permalink
[HTML5] Fix DPI-handling
Browse files Browse the repository at this point in the history
  • Loading branch information
RobDangerous committed Feb 26, 2022
1 parent 7ffa86c commit a0db16d
Showing 1 changed file with 15 additions and 2 deletions.
17 changes: 15 additions & 2 deletions Backends/HTML5/kha/SystemImpl.hx
Original file line number Diff line number Diff line change
Expand Up @@ -533,9 +533,22 @@ class SystemImpl {

if (canvas.getContext != null) {
if (lastCanvasClientWidth != canvas.clientWidth || lastCanvasClientHeight != canvas.clientHeight) {
// canvas.width is the actual backbuffer-size.
// canvas.clientWidth (which is read-only and equivalent to
// canvas.style.width in pixels) is the output-size
// and by default gets scaled by devicePixelRatio.
// We revert that scale so backbuffer and output-size
// are the same.

var scale = Browser.window.devicePixelRatio;
canvas.width = Std.int(canvas.clientWidth * scale);
canvas.height = Std.int(canvas.clientHeight * scale);
var clientWidth = canvas.clientWidth;
var clientHeight = canvas.clientHeight;
canvas.width = clientWidth;
canvas.height = clientHeight;
if (scale != 1) {
canvas.style.width = Std.int(clientWidth / scale) + "px";
canvas.style.height = Std.int(clientHeight / scale) + "px";
}
lastCanvasClientWidth = canvas.clientWidth;
lastCanvasClientHeight = canvas.clientHeight;
}
Expand Down

12 comments on commit a0db16d

@onelsonic
Copy link

Choose a reason for hiding this comment

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

Hi there, seems there is an error here :
if you want to apply the scale you should multiply it not divide, corrected it should be :
canvas.style.width = Std.int(clientWidth * scale) + "px";
canvas.style.height = Std.int(clientHeight * scale) + "px";

@RblSb
Copy link
Contributor

@RblSb RblSb commented on a0db16d Aug 24, 2022

Choose a reason for hiding this comment

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

Not really, you need to set big canvas size and N times smaller canvas.style size, or canvas will have blur.

@RobDangerous
Copy link
Member Author

Choose a reason for hiding this comment

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

The thing is, we explicitly don't want to apply a scale. In Kha the user is responsible for handling dpi-scaling, therefore the backend should get to the raw pixels aka undo any scaling. See also the comment right above the code.

@onelsonic
Copy link

@onelsonic onelsonic commented on a0db16d Aug 24, 2022

Choose a reason for hiding this comment

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

Sure blur is not good.

For web apps the scale is specify at the top in the meta header :

using : minimum-scale=1.0, initial-scale=1.0, maximum-scale=1.0
the scale will be 1.0 for all canvas.

then you can call the canvas with some specified pixels size: width='800' height='600'
and the canvas will resize to the wanted 800x600 size, that was done so it behave the same across browser and device with different DPI.

1/ If you divide the scale as above, the canvas won't fit the pixels size specified. (or else you have a way to resize the canvas?)
2/ If you divide the scale as above, if we want to render the canvas to a lower resolution and do some up-scaling (the canvas will not resize)

It just does not seems to be the way the Webgl canvas was intended to be used with DPI.

@RobDangerous
Copy link
Member Author

Choose a reason for hiding this comment

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

  1. But it does and seems to work as intended. If something does not work as intended, please create a minimal Kha-sample showing it and open an issue.
  2. If you want to do that, use a render-target - just as you would do for any other Kha-target.

@onelsonic
Copy link

@onelsonic onelsonic commented on a0db16d Aug 24, 2022

Choose a reason for hiding this comment

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

Not really, you need to set big canvas size and N times smaller canvas.style size, or canvas will have blur.

if you want to prevent blur, you should use this style for your content :
image-rendering: pixelated;

see more details here : https://developer.mozilla.org/en-US/docs/Web/CSS/Image-rendering

@RobDangerous
Copy link
Member Author

Choose a reason for hiding this comment

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

That's exactly not what we want. Let's stop this here. As I said, if something doesn't work as intended, create a small sample-app showing it and open an issue.

@onelsonic
Copy link

Choose a reason for hiding this comment

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

just curious on which did you noticed the blurry effect ? on Safari iOS and Mac apple devices or was it on Chrome, or Firefox?

@RobDangerous
Copy link
Member Author

Choose a reason for hiding this comment

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

On all of them. And with "pixelated" things are even worse.

@onelsonic
Copy link

@onelsonic onelsonic commented on a0db16d Aug 24, 2022

Choose a reason for hiding this comment

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

well I can see your blurring issue when up-scaling the scale of the physical display and yes it's bad...

@onelsonic
Copy link

@onelsonic onelsonic commented on a0db16d Aug 26, 2022

Choose a reason for hiding this comment

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

Just so you know,
I found that Blur is only showing when the scale is set above 100% on the physical device and in this case adjusting the scale in the meta header fixes the blur.

and if the scale become too big (like on an iPad), I change the canvas.width and width.height to an bigger value.

This way I have not detected any blur if the scale is correctly specified in the meta headers to 1.0 scale. :
<meta name="viewport" content="width=device-width, height=device-height, minimum-scale=1.0, initial-scale=1.0, user-scalable=no, viewport-fit=cover">

I've run a few tests and with the last changes, Canvas sizes are now inconsistent across browsers as canvas.style sizes are forcibly specified.
I ran some tests on PC (Firefox/Chrome), MAC (Firefox/Chrome/Safari), iOS iPhone/iPad (Safari)

You can correct me if I am wrong here but now we cannot use 100vw or 100vh values for the canvas and the canvas.style
and so correctly center the canvas on all platforms without dedicated code for each platform.
Also the Webgl canvas does not resize according to the scale in the meta headers anymore.
Which from a webdesign standpoint looks like a step backward, removing the flexibility to use the scale.

This point is valid except if you are targeting one platform in particular and don't bother the other platforms.

@RobDangerous
Copy link
Member Author

Choose a reason for hiding this comment

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

As said before, create a minimal sample, open an issue, tell us what happens and what you expect to happen instead. And use the generated html for that.

Please sign in to comment.