search

Return-To-The-Roots / s25client

Return To The Roots (Settlers II(R) Clone)
http://www.rttr.info
GNU General Public License v2.0
476 stars 75 forks source link

Fix mouse panning issues in SDL2 video driver #1557

Closed siredmar closed 1 year ago

siredmar commented 1 year ago

This should fix #1526 and #1500. The SDL2 video driver got stuck on the logic regarding mouseMoved. I didn't see any reason why to catch the first mouse move only. The problem is, that the Msg_MouseMove is only sent on the first time -> no mouse pan working. So this PR removes the logic around mouseMoved and basically sends every mouse motion event.

Spikeone commented 1 year ago

Guess Flamefire will have a proper review, only thing I could imagine is: this was for smartmoves and maybe the mouse event fired twice in some cases.

maybe this even fixes https://github.com/Return-To-The-Roots/s25client/issues/1102

Flamefire commented 1 year ago

I guess this was for the case where a mouse-warp caused a motion event breaking things, but not sure anymore.
I can't see why this would be the cause: It just means that only the first queued motion event will be handled. Then stuff will be redrawn etc and in the next event handling part the next (new) motion event will be handled

The problem is, that the Msg_MouseMove is only sent on the first time -> no mouse pan working.

But it does work doesn't it? It may ignore some parts of the movements if they happen "to fast" or drawing etc takes to long.

Anyway: @siredmar Have you reproduced the issue from #1526 and this patches fixes it for you? I tried in virtual box and there it is still broken (but I think this is a bug in SDL)

siredmar commented 1 year ago

@Flamefire yes, I can reproduce this issue with my vanilla ubuntu 22.04 installation using Intel graphic drivers. This PR fixes the issue. Since the patch i had no issues anymore.

Edit: To be more clear. It does not work at all on my machine using the upstream source. Only with my fix the mouse panning works on my machine.

siredmar commented 1 year ago

@Flamefire during my debugging i saw that SDL fired MOUSEMOTION events on right click enabled permanently. So once clicked and moved one pixel there were a lot of events. Can you reproduce it with your virtual box environment?

Flamefire commented 1 year ago

@Flamefire during my debugging i saw that SDL fired MOUSEMOTION events on right click enabled permanently. So once clicked and moved one pixel there were a lot of events. Can you reproduce it with your virtual box environment?

In VirtualBox the events were fired correctly but the warp-back doesn't work as it has no effect on future motion events. Reported at: https://github.com/libsdl-org/SDL/issues/5741

This PR fixes the issue. Since the patch i had no issues anymore.

As I wasn't able to produce any issues with this patch (maybe the bug I worked around with that code does no longer exist in SDL2) I'm fine with merging this and see if that also helps others.

siredmar commented 1 year ago

@Flamefire the codecov/patch workflow fails because of missing unit tests. I'm not quite sure how to unit test this. Do you have any idea or simply skip this time?

Flamefire commented 1 year ago

@Flamefire the codecov/patch workflow fails because of missing unit tests. I'm not quite sure how to unit test this. Do you have any idea or simply skip this time?

It was just missing an approving review. Coverage is only informational.

Merging, thanks for this!