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

feat(radar): capture mouse wheel for zooming radar #5978

Merged
merged 3 commits into from
Nov 25, 2024

Conversation

mwerle
Copy link
Contributor

@mwerle mwerle commented Nov 19, 2024

By placing all of the radar drawing routines into a window, the mouse wheel is captured and is not passed back to the world view.

The work to add manual zoom to the 3D radar (#5958) failed to get this to function properly as the wrong API was used for setting the center of the 3D radar. Using 'setCursorScreenPops()' instead of 'setCursorPos()' has the 3D radar background being drawn properly when placed inside a window.

TODO?: When zooming the 3D radar with the mouse, the mode is switched from automatic zoom to manual zoom, but there is not way (using the mouse) to switch the radar back to automatic zoom. Should there be? One option could be that zooming past the maximum zoom automatically switches back to auto-zoom, but testing shows that this is too easy to do accidentally.

-> Implemented by making the "Automatic/Manual" mode indicator button switch between manual and automatic mode.

@bszlrd
Copy link
Contributor

bszlrd commented Nov 19, 2024

How about double-clikcing on the radar to switch between auto and manual? Would need to be documented, likely on the tooltip as well, but could be handy.

@mwerle
Copy link
Contributor Author

mwerle commented Nov 19, 2024

How about double-clikcing on the radar to switch between auto and manual? Would need to be documented, likely on the tooltip as well, but could be handy.

Not sure I like double-clicking (can add if people think it's good).

But completely forgot about the zoom mode indicator button - I've enabled clicking on it to toggle zoom modes instead of it simply being an indicator.

@bszlrd
Copy link
Contributor

bszlrd commented Nov 19, 2024

Oh, I thought you want to add another way on top of the mode indicator. I assumed that it is a button

@mwerle
Copy link
Contributor Author

mwerle commented Nov 19, 2024

Oh, I thought you want to add another way on top of the mode indicator. I assumed that it is a button

In the original PR, it was just an indicator; didn't seem to make sense to have a specific button to change modes if there's no buttons for changing the zoom manually.

In this PR I made it a button to go back to Automatic mode after changing to Manual mode with the mouse wheel.

Copy link
Member

@sturnclaw sturnclaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functions well, good work!

A thought for a follow-up PR - when clicking the auto/manual toggle button, the manual zoom level should be set so that the tracked target is still visible on the radar scope.

...Additionally, it'd be nice to have intermediary steps between log10 increments for manual zoom, e.g. 1Mm, 2Mm, 5Mm, 10Mm.

By placing all of the radar drawing routines into a window, the mouse
wheel is captured and is not passed back to the world view.

The original implementation failed to get this to work properly as the
wrong API was used for setting the center of the 3D radar. Using
'setCursorScreenPops()' instead of 'setCursorPos()' has the 3D radar
background showing up in the correct location even when placed inside a
window.
Radar buttons are now aligned properly with the other buttons on the
screen.

Also fix the 2D radar being slightly clipped by taking into account its
line thickness.
Use the zoom mode indicator to also toggle between automatic and manual
zoom modes for the 3D radar.
@mwerle
Copy link
Contributor Author

mwerle commented Nov 25, 2024

Rebased on master ready for merge.

@mwerle mwerle mentioned this pull request Nov 25, 2024
3 tasks
@mwerle
Copy link
Contributor Author

mwerle commented Nov 25, 2024

Functions well, good work!

A thought for a follow-up PR - when clicking the auto/manual toggle button, the manual zoom level should be set so that the tracked target is still visible on the radar scope.

...Additionally, it'd be nice to have intermediary steps between log10 increments for manual zoom, e.g. 1Mm, 2Mm, 5Mm, 10Mm.

#5984 :)

@sturnclaw sturnclaw merged commit d0b8a8b into pioneerspacesim:master Nov 25, 2024
@mwerle mwerle deleted the feat/radar_fix_mouse_zoom branch November 25, 2024 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants