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

How to handle window size, and cursor position on web targets? #1491

Closed
SimonWoodburyForget opened this issue Mar 3, 2020 · 10 comments
Closed
Assignees
Labels
B - bug Dang, that shouldn't have happened C - needs investigation Issue must be confirmed and researched DS - web

Comments

@SimonWoodburyForget
Copy link

SimonWoodburyForget commented Mar 3, 2020

I've ran into some pretty major issues when trying to handle window (aka: canvas) size on web targets. First i can't get window resize events, because CSS is quietly resizing the window to fit the screen, which means I need to call web-sys and see if it was resized every time before drawing, but now this means the cursors position is incorrect, because a canvas can have a higher or lower resolution then the actual window.

@SimonWoodburyForget SimonWoodburyForget changed the title How to handle windowsize, and cursor position on web targets? How to handle window size, and cursor position on web targets? Mar 3, 2020
@ryanisaacg ryanisaacg added DS - web C - needs investigation Issue must be confirmed and researched B - bug Dang, that shouldn't have happened labels Mar 3, 2020
@ryanisaacg
Copy link
Contributor

CSS is quietly resizing the window to fit the screen

I don't think that's default behavior of winit, can you post a snippet of the code you're using?

@SimonWoodburyForget
Copy link
Author

SimonWoodburyForget commented Mar 3, 2020

Sorry there's some context missing. -- My use case is a video game (ensure canvas is the same size as the client) and I'm resizing the canvas with CSS the typical way to achieve that effect:

* {
    margin:0;
    padding:0;
    overflow:hidden;
}

canvas {
    width:100vw!important;
    height:100vh!important;
}

This resizes the canvas relative to the CSS layout, but it doesn't resize the canvases resolution, and Winit also doesn't throw a resize event on window resize, so I'm using .clientWidth to resize the canvas before rendering:

let canvas = window.canvas();
let (w, h) = (canvas.client_width(), canvas.client_height());

canvas.set_width(w.try_into().unwrap());
canvas.set_height(h.try_into().unwrap());

After doing so I need to use the resulting (w, h) to do gl.viewport and gl.scissor with WebGL, because Winit doesn't throw a resize event here either. I also use those coordinates to translate my world accordingly.

The issue is resizing it like this is I'm not "always" using physical coordinates, because a canvas can have a higher or lower resolution then the physical screen, so when I go ahead and try to use the cursor event coordinates with my transforms they can offset incorrectly. It's a simple 2d game, so screen coordinates matching cursor coordinates exactly is important.

The most notable time this occurs is when the user zooms in/out, which is how I discovered the issue:

  • 50% zoom means a 50% canvas resolution
  • 150% zoom means a 150% canvas resolution
  • 100% zoom does not result in any offset on my system

For my use-case, this means that locking the page zoom at 100% and handling the resize from a WebGL transform would probably do the trick, otherwise one may calculate zoom factor, otherwise I'm still not entirely sure whether handling canvas resize this way is the best way to go about it.

@SimonWoodburyForget
Copy link
Author

I looked into trying to get Winit to give me some form of dpi factor through ScaleFactorChanged, but I wasn't able to get it to trigger such an event.

I decided to solve my issue by applying the dpi with Window::scale_factor(), which seems to do the trick to cancel out zoom factor.

#[cfg(feature = "web")]
{
    let canvas = window.canvas();
    let (width, height) = (canvas.client_width(), canvas.client_height());

    let factor = window.scale_factor();
    let logical = LogicalSize { width, height };
    let PhysicalSize { width, height } = logical.to_physical(factor);

    canvas.set_width(width as u32);
    canvas.set_height(height as u32);

    (width, height)
}

Looked into using Window::inner_size(), it says it returns PhysicalSize but it's actually in a different coordinate space then the CursorMoved events which are in PhysicalPosition, meaning that you can't use them to determine where a cursor would be on the canvas.

Window::inner_size() also says that it returns canvas size, which I didn't exactly understand at first. Reading through the source I see that it's using canvas.width() and canvas.height(), which unlike clientWidth doesn't actually measure the canvas relative to screen space, it just returns the canvases arbitrary width and height used by CSS to determine pixel size I believe.

@FuriouZz
Copy link

FuriouZz commented Mar 4, 2020

Why not listening resize event to trigger canvas resize ?

window.addEventListener('resize', function() {
  // Update canvas inner size
})

@ryanisaacg
Copy link
Contributor

It seems like what's happening here is a breakdown of Winit's canvas DPI model. I don't think anyone accounted for using CSS to resize the canvas when we developed the web backend, so I'm not surprised that makes everything fall apart. I don't have the time this second to draw up a model of expected behavior when this occurs, but I think we should have one.

@alvinhochun
Copy link
Contributor

A bit late, but since I just found this issue I should make some notes here:

I'm resizing the canvas with CSS the typical way to achieve that effect

The issue is that currently winit expects exclusive control over the canvas's size. If you override the styles of the canvas in unexpected ways it will break winit. Same with overriding the width and height yourself. If you want to manually control the canvas size, you should use Window::set_inner_size instead.

Having the winit window canvas follow CSS page layout is useful, so I opened #1661 and experimented with a POC. The approach requires a parent container element, because IMO there is no sane way to let CSS directly control the canvas size and still have the pixel-perfect backbuffer size.

Perhaps winit should note in the docs that the canvas must not have any custom CSS styles that affects its sizing.

I looked into trying to get Winit to give me some form of dpi factor through ScaleFactorChanged, but I wasn't able to get it to trigger such an event.

It wasn't handled until recently with #1690 (only for the web-sys backend).

@alephpt
Copy link

alephpt commented Apr 5, 2023

Is there an actual solution to this? Would be nice if there was just an 'object' that can be called from the web/browser side that updates the canvas side based on the rust implementation.. like a simple hook or wrapper 🤔

@daxpedda
Copy link
Member

daxpedda commented Apr 6, 2023

Related:
#2733
gfx-rs/wgpu#3620

@daxpedda
Copy link
Member

daxpedda commented Jun 5, 2023

I think there are bunch of a different issues at play here:

  • It's not documented that Winit takes control of the canvas size via CSS properties and you shouldn't mess with that.
  • Feedback is lacking when the canvas is actually resized. Considering this issue is two years old this isn't the case anymore, as far as I can tell. The canvas only gets resized during initialization, scale factor change, fullscreen and when using Window::set_inner_size(). In all these cases (except initialization) you will get a WindowEvent::Resized event reliably.
  • The rendering size, which is different to the canvas size, is now completely decoupled from Winit. See Don't change the internal canvas size #2778. So this is something users have to take care of themselves now. This should be possible with the WindowEvent::Resized event, as stated above.
  • There seems to be now easy way to make the canvas auto-resize to cover the full window, probably a very common issue. Which is largely addressed by Allow canvas size to be controlled by page layout on web target #1661.

I hope I correctly understood and summarized the problems of this issue. As far as I can tell they are all addressed or covered by other issues now, except the documentation, which I will address now.

Please feel free to comment if I missed or misunderstood something!

@daxpedda
Copy link
Member

daxpedda commented Jun 7, 2023

Following up on #2853, I will close this in favor of #1661 as well.
The remaining issues were already covered as pointed out in #1491 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B - bug Dang, that shouldn't have happened C - needs investigation Issue must be confirmed and researched DS - web
Development

Successfully merging a pull request may close this issue.

6 participants