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

Touch input offset on secondary monitor #15

Closed
esnaultdev opened this issue Mar 9, 2018 · 29 comments
Closed

Touch input offset on secondary monitor #15

esnaultdev opened this issue Mar 9, 2018 · 29 comments

Comments

@esnaultdev
Copy link

Hello,
First of all, thank you for this tool.

I know I don't have an usual setup, but here's a bug with it.

This is my setup, I'm on macOS Sierra 10.12.4 and I have two screens:
screens

  • The landscape one is the main display for the OS and is 2880x1800
  • The portrait one is a secondary display and is 1080x1920 (This is where my IDE lives)

Now, on the main display, everything works fine with scrcpy, but when the window is on the secondary display, there is an offset for the touch input:
gif

@rom1v
Copy link
Collaborator

rom1v commented Mar 9, 2018

Thank you for reporting this.

I am aware of the problem, but for now, I didn't find a way to fix it without breaking the one-screen scenario.

For now, you can disable hidpi support:

meson x --buildtype release -Dhidpi_support=false

But the video quality will be bad (pixels will be 2×2).

@rom1v
Copy link
Collaborator

rom1v commented Nov 11, 2018

Does SDL 2.0.9 fix the issue on MacOS?

There is a commit from Oct 12th: metal: Fix high dpi and resizing on macOS, and clean up iOS code. Fixes bug #4250..

@eurekaqq
Copy link

eurekaqq commented Dec 8, 2018

@rom1v I have tested latest scrcpy with SDL 2.0.9 on macos 10.14.2, but this problem still exists.

@DougNg
Copy link

DougNg commented Feb 22, 2019

I can also confirm the issue still exists

scrcpy --version
scrcpy 1.7

dependencies:

  • SDL 2.0.9
  • libavcodec 58.35.100
  • libavformat 58.20.100
  • libavutil 56.22.100

@lpkruger
Copy link
Contributor

lpkruger commented Oct 7, 2019

This issue was bothering me as well, I still think it's a SDL upstream bug but this workaround is simple and safe. If SDL fixes it upstream this workaround won't break, but maybe could be removed eventually.

Normally in HiDPI mode, the Renderer resolution is exactly double the window resolution, which is easy to see by comparing the following:

SDL_GetWindowSize(screen.window, &w, &h);
LOGD("Window is %d %d\n", w, h);
SDL_GetRendererOutputSize(screen.renderer, &w, &h);
LOGD("Renderer is %d %d\n", w, h);

But in the mixed-monitor configuration it gets confused after a window resize and they become the same. This leads to an inconsistency where some part of the SDL code still wants to double the X,Y coordinates of the mouse events.

This fix brute-forces things by reinitializing the rendering state after a window resize. I've tested it in a multi-monitor config with every combination of dragging, resizing, fullscreening, and using control-keys to make the app do resizing I can think of, I think it's harmless. Maybe a little inefficient but it only happens in response to resizes.

@lpkruger
Copy link
Contributor

lpkruger commented Oct 7, 2019

PR #848

@lpkruger
Copy link
Contributor

Hi @rom1v I tested the workaround using SDL_RenderSetLogicalSize() on resize events - unfortunately it does not help when I tested it, that call doesn't fix whatever part of the SDL state gets corrupted.

I was able to update my existing patch so that the re-creation of the rendering and texture state never occurs when only a single monitor is connected - it does this by testing whether that ratio of window "size" to rendering pixel size has unexpectedly changed and only then resets the state. I believe if the SDL bug only affects the multi-monitor case, then this reset call also would only happen when multiple monitors have been connected.

Please let me know your thoughts when you have a chance to look it over.

@rom1v
Copy link
Collaborator

rom1v commented Oct 22, 2019

Could you please test logical_size.3? All sizes and location are computed manually, so maybe it makes the hidpi issue obsolete.

rom1v pushed a commit that referenced this issue Nov 6, 2019
Moving a window between hidpi and non-hidpi monitors is not correctly
handled by SDL.

Detect when this happens, and recreate the renderer.

Fixes <#15>.
rom1v pushed a commit that referenced this issue Nov 6, 2019
Moving a window between hidpi and non-hidpi monitors is not correctly
handled by SDL.

Detect when this happens, and recreate the renderer.

Fixes <#15>.

Signed-off-by: Romain Vimont <rom@rom1v.com>
rom1v pushed a commit that referenced this issue Nov 6, 2019
Moving a window between hidpi and non-hidpi monitors is not correctly
handled by SDL.

Detect when this happens, and recreate the renderer.

Fixes <#15>

Signed-off-by: Romain Vimont <rom@rom1v.com>
@maggu2810
Copy link

AFAIK to use multiple monitors with different scale is not possible with X11, but possible using Wayland.
For me this is a really great feature as you can still use old (non high DPI) monitors with more recent laptops.

I will test a few things and comment here later.

@maggu2810
Copy link

maggu2810 commented Dec 21, 2019

Laptop:

  • native resolution (scale 1.0): 3840x2160
  • resolution using scale 2.0: 1920x1080

External monitor:

  • native resolution (scale 1.0): 1920x1080

Environment:

  • Wayland
  • KDE Plasma
  • scrcpy 1.21.1
Laptop External Monitor Positioning Comment
1920x1080, 2.0 1920x1080, 1.0 Laptop and external one both starts at 0,0 (both use the same visible area) All works as expected
1920x1080, 2.0 1920x1080, 1.0 Laptop is left of the external one (but at the same y-pos) All works as expected
3840x2160, 1.0 1920x1080, 1.0 Laptop and external one both starts at 0,0 (the laptops area exceed the external one on the right and bottom side) All fine (for sure, if the window is moved to the right or bottom side of the laptop monitor it is not visible because both monitors starts at the same position and the laptop one has a much more area that can be shown -- expected behavior. Just note it to make it clear.)
3840x2160, 1.0 1920x1080, 1.0 Laptop left of external If scrcpy starts on the laptop monitor and the window is kept there, all is fine. As soon as the window is moved to the external monitor and the first touch / mouse event is received the content of the window is resized and only a small part is visible. So, not usable. If the window is moved back to the laptop monitor it can be used normalle (I need to press ctrl + x once because a automatically resize is to small).

@carstenhag
Copy link

Hello, I have tried out the mentioned branch and commented everything I knew on #848. It would be awesome if you'd have a look on my comment. Thanks in advance!

@rom1v
Copy link
Collaborator

rom1v commented May 18, 2020

@carstenhag Could you please test the vranch logical_size.14?

@carstenhag
Copy link

carstenhag commented May 18, 2020

Sure. Here's the output:

carstenh  ~  git  random  scrcpy  %  ./run x                                             logical_size.14
2020-05-18 14:08:10.866 scrcpy[9026:275612] INFO: scrcpy 1.13 <https://github.com/Genymobile/scrcpy>
x/server/scrcpy-server: 1 file pushed. 3.3 MB/s (30558 bytes in 0.009s)
[server] INFO: Device: samsung SM-A705FN (Android 10)
2020-05-18 14:08:11.335 scrcpy[9026:275612] INFO: Renderer: metal
2020-05-18 14:08:11.335 scrcpy[9026:275612] WARN: Trilinear filtering disabled (not an OpenGL renderer)
2020-05-18 14:08:11.338 scrcpy[9026:275612] INFO: Initial texture: 1080x2400

Screenshot just after starting scrcpy. After resizing the window, it's fine again:
Bildschirmfoto 2020-05-18 um 14 08 17

When I press command + g the window is being resized, but there are black borders around it. This exists on the latest version and on this branch, so it's probably a different thing.

The main issue is fixed though, the mouse clicks are accurate again 🎉 . Now only the initial size has to be fixed.

@rom1v
Copy link
Collaborator

rom1v commented May 18, 2020

Screenshot just after starting scrcpy. After resizing the window, it's fine again:

OK, (almost) perfect 👍

If you start scrcpy on your primary screen, then moves the window to the secondary screen, does it happen too? (or does it happen only if you start scrcpy directly on the secondary screen?)

Are you available on IRC or somewhere to do some tests to fix the problem? (preferably some day in the evening)

When I press command + g the window is being resized, but there are black borders around it. This exists on the latest version and on this branch, so it's probably a different thing.

Yes, this is different: Ctrl+g resizes the window to the device size, but on some platforms (some window managers) if the window is bigger than the computer screen, the size is clamped (so it does not work as expected).

The main issue is fixed though, the mouse clicks are accurate again 🎉

🎉

@carstenhag
Copy link

Yes, let's talk on irc, I have time today or the next days, I don't mind. Just tell me which irc server/channel I should join :).

@carstenhag
Copy link

But yeah, doesn't matter where my terminal window is or where I last closed scrcpy, it always opens scrcpy on my main screen.

@rom1v
Copy link
Collaborator

rom1v commented May 18, 2020

Cool, thx :)

Just tell me which irc server/channel I should join :)

My nickname is rom1v on freenode.

rom1v added a commit that referenced this issue May 18, 2020
Position and scale the content "manually" instead of relying on the
renderer "logical size".

This avoids possible rounding differences between the computed window
size and the content size, causing one row or column of black pixels on
the bottom or on the right.

This also avoids HiDPI scale issues, by computing the scaling manually.

This will also enable to draw items at their expected size on the screen
(unscaled).

Fixes #15 <#15>
rom1v added a commit that referenced this issue May 18, 2020
On macOS with renderer "metal", HiDPI scaling may be incorrect on
initialization when several displays are connected.

Resetting the window size fixes the problem.

Refs #15 <#15>
@rom1v
Copy link
Collaborator

rom1v commented May 18, 2020

@carstenhag Thank you for your help. So it is fixed now 🎉

Before merging, I would like more tests from users. If you had this problem (or even if you had not), please test #1408.

Thank you for your feedback :)

rom1v added a commit that referenced this issue May 23, 2020
Position and scale the content "manually" instead of relying on the
renderer "logical size".

This avoids possible rounding differences between the computed window
size and the content size, causing one row or column of black pixels on
the bottom or on the right.

This also avoids HiDPI scale issues, by computing the scaling manually.

This will also enable to draw items at their expected size on the screen
(unscaled).

Fixes #15 <#15>
rom1v added a commit that referenced this issue May 23, 2020
On macOS with renderer "metal", HiDPI scaling may be incorrect on
initialization when several displays are connected.

Resetting the window size fixes the problem.

Refs #15 <#15>
@rom1v
Copy link
Collaborator

rom1v commented May 23, 2020

Fixed by #1408.

@rom1v rom1v closed this as completed May 23, 2020
rom1v added a commit that referenced this issue Jun 25, 2020
Touch events were HiDPI-scaled twice:
 - once because the position (provided as floats between 0 and 1) were
   converted in pixels using the drawable size (not the window size)
 - once due to screen_convert_to_frame_coords()

One possible fix could be to compute the position in pixels from the
window size instead, but this would unnecessarily round the event
position to the nearest window coordinates (instead of drawable
coordinates).

Instead, expose two separate functions to convert to frame coordinates
from either window or drawable coordinates.

Fixes #1536 <#1536>
Refs #15 <#15>
Refs e40532a
rom1v added a commit that referenced this issue Jun 27, 2020
Touch events were HiDPI-scaled twice:
 - once because the position (provided as floats between 0 and 1) were
   converted in pixels using the drawable size (not the window size)
 - once due to screen_convert_to_frame_coords()

One possible fix could be to compute the position in pixels from the
window size instead, but this would unnecessarily round the event
position to the nearest window coordinates (instead of drawable
coordinates).

Instead, expose two separate functions to convert to frame coordinates
from either window or drawable coordinates.

Fixes #1536 <#1536>
Refs #15 <#15>
Refs e40532a
rom1v added a commit that referenced this issue Mar 6, 2021
Issue #15 had been fixed in v1.14 by
e40532a.
@rom1v
Copy link
Collaborator

rom1v commented Aug 24, 2021

Ref libsdl-org/SDL@4077f7a

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.

7 participants