rust-windowing / winit

Window handling library in pure Rust
https://docs.rs/winit/
Apache License 2.0
4.88k stars 912 forks source link

Coordinate systems #3891

Open madsmtm opened 3 months ago

madsmtm commented 3 months ago

While doing https://github.com/rust-windowing/winit/pull/3890, I reviewed all our instances of [Physical|Logical|]Position, and categorized them relative to their origin point:

Desktop coordinates:

Window coordinates:

Surface coordinates:

MouseScrollDelta::PixelDelta is a delta, so it has no origin and hence no coordinate system.

I believe that we should make set_cursor_position and the pointer APIs use surface coordinates, since that's the easier coordinate system for users to work with.

kchibisov commented 3 months ago

I believe that we should make set_cursor_position and the pointer APIs use surface coordinates, since that's the easier coordinate system for users to work with.

it's not, it's easy as long as it matches pointer events, otherwise you'd have to convert back and forth all the time.

So, generally, coordinates in mouse events should be surface local, thus making all the set position stuff a surface local.

Window::show_window_menu is window coordinates, since it's a hint for system where to place menu over the window.

madsmtm commented 3 months ago

I believe that we should make set_cursor_position and the pointer APIs use surface coordinates, since that's the easier coordinate system for users to work with.

it's not, it's easy as long as it matches pointer events, otherwise you'd have to convert back and forth all the time.

It's more cumbersome to do things like hit-testing if the pointer position is in window coordinates instead of surface coordinates.

So, generally, coordinates in mouse events should be surface local, thus making all the set position stuff a surface local.

That's what I'm saying? As in, I agree that mouse/pointer events should be relative to the surface's top-left corner, not the window's top-left corner.

Window::show_window_menu is window coordinates, since it's a hint for system where to place menu over the window.

Idk what we are doing here, it's only implemented on Windows and Wayland, and I understand neither of those, I just took a guess from the Windows impl using ClientToScreen internally.

I think that, if we make everything else use surface coordinates, it might make sense to also make this use surface coordinates for consistency, regardless of how it works internally.

kchibisov commented 3 months ago

I think that, if we make everything else use surface coordinates, it might make sense to also make this use surface coordinates for consistency, regardless of how it works internally.

No, this API is only present for the top-level, top-level is a window coordinates, so any other coordinates don't make any sense.

You can pretty much figure out which coordinate system should be used based on whether subview/subsurfaces have them or based on the wording in the protocols.

In general, it would mean for both windows/wayland backend to convert from local to window coordinates which is not really trivial sometimes, and it doesn't make much sense...

Though if majority backends do surface local, then maybe we can use surface local as well...

daxpedda commented 1 month ago

I believe that we should make set_cursor_position and the pointer APIs use surface coordinates, since that's the easier coordinate system for users to work with.

Agreed. This is already the case for Web.

We should also make sure that this is somehow clearly documented.