AlexandreRouma / SDRPlusPlus

Cross-Platform SDR Software
GNU General Public License v3.0
4.16k stars 576 forks source link

Scrolling doesn't work on MacOS #1486

Closed floens closed 1 month ago

floens commented 1 month ago

Hardware

Software

Bug Description Scrolling doesn't work right on MacOS and possible other OS's supporting fractional scrolling.

The value returned by ImGui::GetIO().MouseWheel is fractional/smooth. The behaviour on a touchpad like the one on this laptop is that this value is often +- 0.1 or +- 0.2 for a few frames after each other, before returning to 0. In other words, unless you're scrolling very fast, the value is always below 1.0. The code assumes the value is a int and as such scrolling doesn't work.

Steps To Reproduce

  1. Mouse/device/OS with fractional scrolling.
  2. Scroll over the frequency selector/waterfall.
  3. Only works when scrolling very fast.

Additional info I changed the three uses of ImGui::GetIO().MouseWheel in my local build (frequency_select, main_window and waterfall) to a float. For the frequency selector, I changed if (mw != 0.0) { to if (fabs(mw) > 0.2) {, otherwise it would change too fast. But the frequency selector code assumes ints anyway, so that might need some changing.

With this it's all smooth and working as expected.

I have the code ready, do you want me to create a PR?

AlexandreRouma commented 1 month ago

Report this to the ImGui project instead.

floens commented 1 month ago

ImGui is working as intended. MouseWheel is a float in ImGui, but the SDR++ code casts this to a int.

AlexandreRouma commented 1 month ago

No it's not. The magnitude of the value is supposed to be consistent across device. Casting to it should cause no issue besides quantising the velocity. Here on linux with fractional scrolling everything works as expected.

floens commented 1 month ago

To further illustrate the issue, I have a logged the value for each frame and attached it to this comment.

Scrolling does work if you scroll fast enough, but the waterfall moves way too quick. When scrolling normally, the value is not reaching more than 1.0, and because of the casting, nothing happens.

Additionally, all uses of imgui themselves in imgui.cpp and the examples upstream use MouseWheel as a float, not casting it to an int.

That makes me wonder if on Linux the values of MouseWheel are still integers, but there's some magic in the background to still make it "fractional scrolling".

[03/10/2024 15:08:04.820] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:04.825] [INFO] ImGui::GetIO().MouseWheel = -0.100000
[03/10/2024 15:08:04.837] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:04.841] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:04.854] [INFO] ImGui::GetIO().MouseWheel = -0.300000
[03/10/2024 15:08:04.857] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:04.870] [INFO] ImGui::GetIO().MouseWheel = -0.700000
[03/10/2024 15:08:04.874] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:04.887] [INFO] ImGui::GetIO().MouseWheel = -0.700000
[03/10/2024 15:08:04.891] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:04.904] [INFO] ImGui::GetIO().MouseWheel = -0.500000
[03/10/2024 15:08:04.907] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:04.921] [INFO] ImGui::GetIO().MouseWheel = -0.600000
[03/10/2024 15:08:04.925] [INFO] ImGui::GetIO().MouseWheel = -0.500000
[03/10/2024 15:08:04.937] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:04.943] [INFO] ImGui::GetIO().MouseWheel = -0.400000
[03/10/2024 15:08:04.954] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:04.959] [INFO] ImGui::GetIO().MouseWheel = -0.200000
[03/10/2024 15:08:04.970] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:04.975] [INFO] ImGui::GetIO().MouseWheel = -0.200000
[03/10/2024 15:08:04.987] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:04.992] [INFO] ImGui::GetIO().MouseWheel = -0.200000
[03/10/2024 15:08:05.004] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:05.008] [INFO] ImGui::GetIO().MouseWheel = -0.200000
[03/10/2024 15:08:05.020] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:05.025] [INFO] ImGui::GetIO().MouseWheel = -0.300000
[03/10/2024 15:08:05.037] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:05.042] [INFO] ImGui::GetIO().MouseWheel = -0.300000
[03/10/2024 15:08:05.054] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:05.058] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:05.070] [INFO] ImGui::GetIO().MouseWheel = -0.200000
[03/10/2024 15:08:05.074] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:05.087] [INFO] ImGui::GetIO().MouseWheel = -0.200000
[03/10/2024 15:08:05.091] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:05.104] [INFO] ImGui::GetIO().MouseWheel = -0.200000
[03/10/2024 15:08:05.108] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:05.120] [INFO] ImGui::GetIO().MouseWheel = -0.200000
[03/10/2024 15:08:05.124] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:05.137] [INFO] ImGui::GetIO().MouseWheel = -0.300000
[03/10/2024 15:08:05.140] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:05.154] [INFO] ImGui::GetIO().MouseWheel = -0.300000
[03/10/2024 15:08:05.159] [INFO] ImGui::GetIO().MouseWheel = -0.200000
[03/10/2024 15:08:05.170] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:05.176] [INFO] ImGui::GetIO().MouseWheel = -0.200000
[03/10/2024 15:08:05.187] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:05.192] [INFO] ImGui::GetIO().MouseWheel = -0.100000
[03/10/2024 15:08:05.204] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:05.209] [INFO] ImGui::GetIO().MouseWheel = -0.100000
[03/10/2024 15:08:05.220] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:05.225] [INFO] ImGui::GetIO().MouseWheel = -0.100000
[03/10/2024 15:08:05.237] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:05.242] [INFO] ImGui::GetIO().MouseWheel = -0.100000
[03/10/2024 15:08:05.254] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:05.258] [INFO] ImGui::GetIO().MouseWheel = -0.100000
[03/10/2024 15:08:05.270] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:05.275] [INFO] ImGui::GetIO().MouseWheel = -0.100000
[03/10/2024 15:08:05.287] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:05.291] [INFO] ImGui::GetIO().MouseWheel = 0.000000
[03/10/2024 15:08:05.304] [INFO] ImGui::GetIO().MouseWheel = -0.100000
[03/10/2024 15:08:05.308] [INFO] ImGui::GetIO().MouseWheel = 0.000000
AlexandreRouma commented 1 month ago

I don't know but my code shouldn't be the one to be integrating the returned value so it looks like regular scrolling. Scrolling the frequency selector or the waterfall is meant to be in defined frequency steps and not an arbitrary float value.

floens commented 1 month ago

Scrolling the frequency selector or the waterfall is meant to be in defined frequency steps and not an arbitrary float value.

Thanks, this gives some context why you're hesitant to just changing it to a float.

but my code shouldn't be the one to be integrating the returned value so it looks like regular scrolling

The imgui API, examples, and usages in their own scrollable regions implementation refute this. The API is intended to be used as a float.

With the added context that the behaviour should be in steps, I understand that that would be more work. I am offering to do that work and create a PR. If this issue is not accepted I'll take a look at asking the popular fork, for me scrolling at the moment is just not working and I would like other users with these kinds of touchpads to benefit from this fix.

AlexandreRouma commented 1 month ago

I don't accept pull requests so consider this issue completely closed. And I don't appreciate the "accept this or else" attitude.