amethyst / bracket-lib

The Roguelike Toolkit (RLTK), implemented for Rust.
MIT License
1.53k stars 111 forks source link

Rltk::mouse_pos() is incorrect with HiDPI #40

Closed bofh69 closed 3 years ago

bofh69 commented 4 years ago

I'm using version 0.5.15 and is calling Rltk::set_bg with Rltk::mouse_pos() as position. At first this follows the mouse pointer, but after I resize the window the mouse_pos position is no longer translated to the right position in console coordinate space.

bofh69 commented 4 years ago

The issue remains with 0.5.17.

My window is 80 characters wide. The font has tile_size (8,8). Rltk::mouse_pos contains values in the range 0..1280 since I'm using HiDPI. Rltk::width_pixels is 1280 as well.

Perhaps a better implementation of mouse_pos() would be: mouse_pos * get_char_size() / (width_pixels, height_pixels) ?

bofh69 commented 4 years ago

I was a bit too fast there. No, the issue doesn't remain, it is now always incorrect even before resizing.

thebracket commented 4 years ago

I merged in some HiDPI fixes that included the mouse, this morning. It works on my Win10 laptop when I manually set scaling (I don't have anything that natively uses it). Any chance you could give it a test, please?

NielsRenard commented 4 years ago

Tested it out on Ubuntu 18.4, x11, 2560x1440 resolution When display scale is set to 200% (which is what I usually use) this behaviour is shown: high-dpi-screen

With display scale at 100% the mouse position is correct. high-dpi-screen-unscaled

The mouse position issue I was having on a regular screen is fixed with last update it seems.

thebracket commented 4 years ago

I'll give this another go. Tagging as "help wanted", because I don't seem to have a lot of luck with it!

thebracket commented 4 years ago

Here's a fun bug, and I'm not sure it's my fault! On my Ubuntu VM, I launch Ubuntu on Wayland. Set the display scale to 150%. Querying wc.window().scale_factor() from winit gives an answer of 2.0. That would be a problem.

thebracket commented 4 years ago

This is actually turning into quite the bundle of bugs, so I'm opening a branch to work on it (issue-40-hidpi). I think I'll be sending some bug reports upstream, also.

On Ubuntu-Wayland with 200% scaling set:

On Ubuntu-Wayland, multiplying the x and y by the scale factor works. Doing that on Windows 10 breaks everything if your scale factor isn't 1.0, because the mouse position is actually scaled on that platform. I haven't tried a Mac.

NielsRenard commented 4 years ago

I can help by testing things on my high dpi screen. No idea how hard this stuff is to get up to speed with from zero, but I'll check out that branch.

thebracket commented 4 years ago

I've currently got it scaling properly on Linux, which broke scaling on Windows 10. Ugh.

thebracket commented 4 years ago

It looks like there's a bug report referencing this behavior: https://github.com/rust-windowing/winit/issues/1406

NielsRenard commented 4 years ago

Can confirm that issue-40-hidpi branch works on wayland, with both 200% and 300% scaling.

However: On X11 the behaviour is now the opposite of yesterdays gif's. New gif incoming, 1 sec.

NielsRenard commented 4 years ago

x11-40-high-dpi-200scaled

thebracket commented 4 years ago

It's broken on Windows 10, too - I pushed that to see what happens if I make it work on Wayland, and the answer is "everything else breaks". :-|

thebracket commented 4 years ago

The answer from the winit folks is that it's fixed in their Github master (in issue https://github.com/rust-windowing/winit/pull/1385 ). So I may just have to wait for Glutin to pick that up.

thebracket commented 4 years ago

Apparently, I can patch the transitive dependency - so down that rabbit hole I go.

thebracket commented 4 years ago

The most recent push to the branch seems to be working on Win10 and Wayland (via VM) for me.

NielsRenard commented 4 years ago

x11-40-high-dpi-200scaled-working bingo!

NielsRenard commented 4 years ago

That was X11, let me double check (non VM) wayland as well...

thebracket commented 4 years ago

Thanks! (Awesome gif recorder, by the way). Looks like I have an ice storm incoming, so I'm going offline for a bit.

NielsRenard commented 4 years ago

Hate to tell you, but wayland with 200% scaling is now 'off' again for me. Tried it on two different ubuntu machines.

Behaviour is like yesterday: https://github.com/thebracket/rltk_rs/issues/40#issuecomment-576938169

NielsRenard commented 4 years ago

Otsukaresama desu! Thanks for working on this so far. I run X11 so I'll stick on this branch for a bit. (The gif recorder is called Peek https://github.com/phw/peek)

thebracket commented 4 years ago

So I've got it working reasonably well on my X11 and Win10 setups, but Wayland is causing nothing but headaches.

Ugh. "Use X11" is my best advice currently.

NielsRenard commented 4 years ago

New ubuntu LTS is shipping in a couple months, has X11 by default still. No idea if many people are using Wayland.

I'm curious if it works on macOS as well now. (No way to test though).

thebracket commented 4 years ago

I have a Mac VM (it's extremely slow), I'll look. I just found out that if you disable window decorations, Wayland works ok - but I'm not sure I'd want to tell people "you don't need a minimize button".

I forgot that my virtual Mac doesn't do accelerated OpenGL (I've used it for WASM testing). So I'm none the wiser.

NielsRenard commented 4 years ago

The changes on this branch fix the hidpi issues I have on X11, but I'd like to get off since it's getting stale compared to master. Would it be smart to merge and then open a new issue specifically for Wayland?

edit We don't know yet if this wrecks things for mac users do we?

thebracket commented 4 years ago

I've merged Master back into the issue-40 branch, which should make it "un-stale". Since Wayland seems to be the only blocking issue on that branch, I'll see about ensuring everything else is in the master branch today, then we can bring this down to "Wayland is broken".

thebracket commented 4 years ago

Alright, if you aren't using Wayland, MASTER should now have the fixes from the issue_40 branch in it - OTHER than the physical/logical snafu that is awaiting a winit update.

thebracket commented 4 years ago

Ignore that last commit. I typed the wrong issue number in my commit message and pushed before I realized my error. Sorry.

NielsRenard commented 4 years ago

Alright, if you aren't using Wayland, MASTER should now have the fixes from the issue_40 branch in it - OTHER than the physical/logical snafu that is awaiting a winit update.

👍 I'm back on master, works like a charm on ubuntu 18.4, x11, 2560x1440 resolution with scaling 200%. If I can help out with more testing in the future just @ me.

ghost commented 4 years ago

I'm having this issue on a 2019 MacBook Pro running 10.15.2. Changing display scaling doesn't seem to affect it in any way.

thebracket commented 4 years ago

That's not good (Mac is the platform I have the hardest time testing); it's working on Windows and X11 for me, even changing display scaling while running. Would you mind trying on master/head, and seeing if cargo run --example ex06-astar-mouse gives you correct mouse positions, please?

ghost commented 4 years ago

I actually tried this the other day and it's a weird one.

BIG EDIT: That was actually on the mousefix branch, my bad, on master, it's just got the goofed out mouse issue, it doesn't start out small like that, apologies.

stuff

ghost commented 4 years ago

Update, started goofing around, just changed from Glutin 0.22.0 to 0.23.0 and it completely fixed it. I assume you'll want to make sure nothing changed that will make anything buggy, but it 100% solves the mouse issue on MacOS.

hammerandtongs commented 4 years ago

So on X11 AMD gpu(and Intel gpu) Ubuntu 18.04 without hi-dpi monitor I have the same behavior as shown in the gif

https://github.com/thebracket/rltk_rs/issues/40#issuecomment-577297384

I'm pulling from master atm but 0.6.2 has same behavior now. This is odd as it was not occurring a few days ago and now...

thebracket commented 3 years ago

The most recent fixes should take care of this. I'm currently getting correct mouse_pos() on all the platforms I have available to test, on various DPI settings. Tested on: Windows (with 100%, 125%, 150%, 200%), Ubuntu/X11 (with 100% and 200%), Ubuntu/Wayland (100% and 200%). I'm hoping someone with a Mac can verify this for me.

thebracket commented 3 years ago

A friend with a Mac tells me that this is resolved. :-)

thebracket commented 3 years ago

I've now had two reports that this is ok, so I'm closing this issue. Please feel free to re-open if it bites you again.