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

Implement FPS navigation / flying mode in the 3D viewer #40893

Merged
merged 15 commits into from
Jan 13, 2021

Conversation

NEDJIMAbelgacem
Copy link
Contributor

@NEDJIMAbelgacem NEDJIMAbelgacem commented Jan 7, 2021

Description

This implements a new camera navigation mode with the WASD / arrow keys and the mouse to control the angles of the camera.
W / Up arrow : move forward
S / Down arrow : move back
A / Left arrow : move left
D / right arrow : move right
Q / Page up : move up
E / Page down : move down

And the clicking and moving the camera cursor changes the angle of the camera

Any feedback is welcome

@github-actions github-actions bot added this to the 3.18.0 milestone Jan 7, 2021
@Saijin-Naib
Copy link

I've got two thoughts:

  1. Ability to set keybinds, maybe in the 3D View settings? For instance, our French friends who use AZERTY might not find QWERTY comfortable. Same goes for QWERTZ or other region-specific layouts.

  2. By convention, space is most commonly "Swim/Move Up" and C/Shift are "Swim/Move Down". Q/E are typically lean or roll, which honestly could be nice to have supported!

Looks like a big improvement either way! Thanks!

@nyalldawson
Copy link
Collaborator

This is great! Thanks! I can finally explore the Ankor Wat point cloud and walk through the building 😄

A couple of points based on some initial testing:

  • the "s" key isn't working for me -- it toggles snapping instead of stepping backward
  • i think a key shortcut to lock cursor movement would be useful. I.e. if "~" is pressed then the cursor gets locked to movement (even without left button pressed), and another ~ releases it. I'd also suggest we use the same shortcut to temporarily switch to fly navigation, even if the previous setting was "basic". (this would give a really quick method to do fine adjustments to a terrain based camera too)
  • I suggest we follow blender and use scroll wheel to change the movement speed when in fly mode, instead of using a hardcoded value or adding a user setting for this step size. Using scroll wheel means you can very very smoothly control the rate of movement.
  • I think Q/E should ALWAYS affect only the vertical position, regardless of the camera direction. currently if you angle the camera up or down then the behavior of q/e becomes somewhat unpredictable.
  • we could consider allowing the right-click-drag smooth zoom in/out even when fly mode is active?
  • I think in future we could add a variant to the fps mode for "walk" navigation -- in this one the camera will stick at a fixed height above the terrain as you walk around, but the rest of the navigation follows the fps mode style. Q/E would adjust that fixed height (ie make the viewer taller/shorter!)

src/3d/qgs3dmapsettings.cpp Outdated Show resolved Hide resolved
src/3d/qgscameracontroller.h Outdated Show resolved Hide resolved
src/3d/qgscameracontroller.h Outdated Show resolved Hide resolved
src/ui/3d/map3dconfigwidget.ui Outdated Show resolved Hide resolved
src/ui/3d/map3dconfigwidget.ui Outdated Show resolved Hide resolved
@nirvn
Copy link
Contributor

nirvn commented Jan 8, 2021

@NEDJIMAbelgacem , what does the UI for toggling/managing this nav mode look like?

By the way, YAY! :)

@NEDJIMAbelgacem
Copy link
Contributor Author

NEDJIMAbelgacem commented Jan 8, 2021

Left mouse button controls:
https://imgur.com/sESZib0

@NEDJIMAbelgacem
Copy link
Contributor Author

NEDJIMAbelgacem commented Jan 8, 2021

I added this right mouse button movement to move the camera up/dowm (not in the z direction but the camera up axis) and left/righ

https://imgur.com/rqJe16D

@NEDJIMAbelgacem
Copy link
Contributor Author

WASD navigation:
https://imgur.com/PMxULu8

@NEDJIMAbelgacem
Copy link
Contributor Author

@nirvn For now I just added a combobox to choose a navigation mode
As you can see here:
https://imgur.com/tDfEjng

@NEDJIMAbelgacem
Copy link
Contributor Author

  • the "s" key isn't working for me -- it toggles snapping instead of stepping backward

Hmmm I don't know why is that happening, for me it works fine..

  • i think a key shortcut to lock cursor movement would be useful. I.e. if "~" is pressed then the cursor gets locked to movement (even without left button pressed), and another ~ releases it. I'd also suggest we use the same shortcut to temporarily switch to fly navigation, even if the previous setting was "basic". (this would give a really quick method to do fine adjustments to a terrain based camera too)

You mean to have the camera cursor being locked to the 3D view on one position like in video games ?

  • I suggest we follow blender and use scroll wheel to change the movement speed when in fly mode, instead of using a hardcoded value or adding a user setting for this step size. Using scroll wheel means you can very very smoothly control the rate of movement.

I kept the wheel to zooming in/out to make it a bit more uniform with the 2D view. I guess we can also have hotkeys for modifying the movement speed like Shift + '+' and Shift + '-'.

  • I think Q/E should ALWAYS affect only the vertical position, regardless of the camera direction. currently if you angle the camera up or down then the behavior of q/e becomes somewhat unpredictable.

I changed to that now, in my head I know it's the camera up vector and that made it behave like flying in a true sense :)

  • we could consider allowing the right-click-drag smooth zoom in/out even when fly mode is active?

That's already implemented with the mouse wheel.

  • I think in future we could add a variant to the fps mode for "walk" navigation -- in this one the camera will stick at a fixed height above the terrain as you walk around, but the rest of the navigation follows the fps mode style. Q/E would adjust that fixed height (ie make the viewer taller/shorter!)

Sounds good!

@nyalldawson
Copy link
Collaborator

Can you cherry-pick nyalldawson@cc73030 and nyalldawson@266c64b here please @NEDJIMAbelgacem ?

@nyalldawson
Copy link
Collaborator

nyalldawson@24d0aa5 is another one for consideration

@nyalldawson
Copy link
Collaborator

(also nyalldawson@eaafcc1 ) 😄

@wonder-sk
Copy link
Member

Nice!

For UX I would suggest to have the navigation mode switch a button right in the toolbar of the 3D view, so it is not necessary to go to settings dialog to switch between the two.

From code perspective, I think ideally the navigation mode should not be a property of the 3D map scene (in Qgs3DMapSettings) - and it would be better to have this property tied to 3D map canvas...

@uclaros
Copy link
Contributor

uclaros commented Jan 8, 2021

The 3d map view should handle the key presses and not forward them to the map canvas / main app actions. The wasd keyboard region is most commonly remapped to useful actions and we don't want them firing randomly while flying in the 3d view!

I kept the wheel to zooming in/out to make it a bit more uniform with the 2D view. I guess we can also have hotkeys for modifying the movement speed like Shift + '+' and Shift + '-'.

There should be some visual feedback about movement speed changes. Setting too high a speed may easily result in a lost camera, since we have no visual feedback about camera position (camera x, y, z, pitch, yaw, roll would be great for both navigation modes!)

we could consider allowing the right-click-drag smooth zoom in/out even when fly mode is active?

A drag-to-move option would be indeed handy since it is fully analog. If right-click is used for elevation change, then maybe with some modifier key?

@timlinux
Copy link
Member

timlinux commented Jan 8, 2021

@NEDJIMAbelgacem for me your branch build fails with

[ 99%] Built target translations
sip: QgsCameraController::NavigationMode is undefined
gmake[2]: *** [python/CMakeFiles/python_module_qgis__3d_autogen.dir/build.make:733: python/3d/sip_3dpart0.cpp] Error 1
gmake[2]: *** Deleting file 'python/3d/sip_3dpart0.cpp'
gmake[2]: Leaving directory '/home/timlinux/dev/cpp/QGIS-Debug-Build'
gmake[1]: *** [CMakeFiles/Makefile2:8627: python/CMakeFiles/python_module_qgis__3d_autogen.dir/all] Error 2
gmake[1]: Leaving directory '/home/timlinux/dev/cpp/QGIS-Debug-Build'
gmake: *** [Makefile:163: all] Error 2
15:25:11: The process "/usr/bin/cmake" exited with code 2.
Error while building/deploying project qgis (kit: QGIS Build Kit)
When executing step "CMake Build"
15:25:11: Elapsed time: 00:21.

@roya0045
Copy link
Contributor

roya0045 commented Jan 8, 2021

@timlinux I doubt you need python to test this, disabling python should avoid sip issues.

@timlinux
Copy link
Member

timlinux commented Jan 8, 2021

@timlinux I doubt you need python to test this, disabling python should avoid sip issues.

Yes doing so now, thanks

@nyalldawson
Copy link
Collaborator

@nirvn
Copy link
Contributor

nirvn commented Jan 13, 2021

@NEDJIMAbelgacem , @nyalldawson , thanks to both of you for working on this, it really elevates 3D support.

@NEDJIMAbelgacem NEDJIMAbelgacem marked this pull request as ready for review January 13, 2021 16:48
@NEDJIMAbelgacem
Copy link
Contributor Author

Hi @wonder-sk

For UX I would suggest to have the navigation mode switch a button right in the toolbar of the 3D view, so it is not necessary to go to settings dialog to switch between the two.

For that I added ctrl + ~ hotkey to switch between the two modes

From code perspective, I think ideally the navigation mode should not be a property of the 3D map scene (in Qgs3DMapSettings) - and it would be better to have this property tied to 3D map canvas...

I added the property in Qgs3DMapSettings so that the user can save the navigation mode he is using

nyalldawson and others added 9 commits January 13, 2021 18:00
Instead of just reponding to key presses/releases (and auto repeat
versions of these), instead track which keys are held down continuously
and use a timer based approach to handle the actual resultant camera
movement.

This allows use of multiple keys when navigating, e.g. moving forward
while strafing left/right/up/down.
@NEDJIMAbelgacem
Copy link
Contributor Author

NEDJIMAbelgacem commented Jan 13, 2021

UPDATE:
So far these are the fly navigation control keys:

  • Combo box in the configuration dialog to change the navigation mode.
  • Hotkey ctrl + ~ to change the navigation mode on the fly
  • WASD or arrow keys to move in the scene in a game like manner.
  • Q/E or Page up/down to move up and down in the vertical axis.
  • Left mouse button or middle mouse button and drag to change the camera orientation.
  • Right mouse button and drag for strafing (moving up/down and left/right smoothly).
  • Pressing the ~ will lock the mouse inside the 3D viewer (press ~ again to unlock).
  • Scrolling the mouse wheel changes the movement speed.

@roya0045
Copy link
Contributor

@NEDJIMAbelgacem Will the bindings be exposed and customizable? For people with different keyboard layout some keys may be more annoying to find/use. I think I use 4 or 5 layout variations and finding some keys like the tilde or the double quotes can be annoying.

@nyalldawson
Copy link
Collaborator

@roya0045 I suggest that customisation is deferred until we have support for customising non-main-window shortcuts through the shortcuts dialog (the same request also affects layouts). I'd rather not see an ad-hoc key customisation added just for these shortcuts alone.

@uclaros
Copy link
Contributor

uclaros commented Jan 13, 2021

@roya0045 I suggest that customisation is deferred until we have support for customising non-main-window shortcuts through the shortcuts dialog (the same request also affects layouts). I'd rather not see an ad-hoc key customisation added just for these shortcuts alone.

If customization is deferred, then we should avoid hard coding wasd and stick with the arrow keys that will work for most people. If wasd or the tilde is assigned in qgis to other actions then they do not work for 3d fly mode and the assigned action fires instead.

* Hotkey `ctrl` + `~` to change the navigation mode on the fly

ctrl+~ is a pretty common shortcut for clipboard managers like ditto or copyq. I think we need something spacebar based here...

* Pressing the `~` will lock the mouse inside the 3D viewer (press `~` again to unlock).

Again, this key may be occupied in qgis blocking its usage. Also, esc should also pop the mouse back or users will get trapped like novices trying to exit vim! Also, the mouse pointer visibility logic is lacking, you can also get trapped without a mouse pointer if you hit the navigation mode shortcut while on free look.

* Scrolling the mouse wheel changes the movement speed.

We need feedback for the movement speed change. If you scroll too much, which is easy to do accidentally if stuck on wrong mode, then trying to move will get you lost in no time!

* Combo box in the configuration dialog to change the navigation mode.

This needs to be in a very visible spot on the toolbar. Like add a fly mode icon next to the hand icon, then make hand be terrain based movement.

When using mouse lock navigation, invert the vertical axis

Mouse pitch is inverted in all 3d view modes (move mouse up, camera looks down) but when free look is enabled this is changed?? This is very confusing, free look mode camera movement should be the same as moving the camera when pressing the mouse button!

  • Free look view is very jumpy on the initial mouse move after ~ is pressed. Sometimes the view looks directly down and moving the mouse up does not move the camera, then the pointer shows up going over the main qgis window.
  • If the 3d view dialog has focus but the 3d canvas does not (eg try resizing the 3d view window) then key presses are not handled.
  • Strafe left-right is pretty unintuitive if not combined with mouse turn left-right. It would be better if left-right keys were turn left-right when the mouse pointer is visible and strafe when the mouse pointer is hidden (free look mode)
  • I think it would be helpful if middle mouse button was terrain based movement pan when on fly mode
  • Z fly sensitivity (either mouse or pgup-pgdn) should probably be smaller than x-y sensitivity. gis projects usually have a much grater range in xy dimensions than in z.

@nyalldawson
Copy link
Collaborator

@uclaros

If customization is deferred, then we should avoid hard coding wasd and stick with the arrow keys that will work for most people. If wasd or the tilde is assigned in qgis to other actions then they do not work for 3d fly mode and the assigned action fires instead.

I've got some plans in how to improve the capture of keystrokes here which should alleviate your concerns, but I'll submit them after this PR is merged during the feature freeze. For now, I'm going to go ahead and merge this prior to freeze with the understanding that if we can't get things resolved in time for final release then the mode can be disabled.

ctrl+~ is a pretty common shortcut for clipboard managers like ditto or copyq. I think we need something spacebar based here...

That won't be an issue if we ensure that the key is only caught when a 3d canvas is focused -- for those widgets theres no free text entry so clipboard handling isn't an issue

We need feedback for the movement speed change. If you scroll too much, which is easy to do accidentally if stuck on wrong mode, then trying to move will get you lost in no time!

Agreed (and will try to address during freeze) -- but keep in mind that this mode was based off blender, and even that (with it's multi million $ / year budget) doesn't give this feedback. I don't see it as a blocker accordingly.

This needs to be in a very visible spot on the toolbar. Like add a fly mode icon next to the hand icon, then make hand be terrain based movement.

+1 - to be addressed during freeze

Mouse pitch is inverted in all 3d view modes (move mouse up, camera looks down) but when free look is enabled this is changed?? This is very confusing, free look mode camera movement should be the same as moving the camera when pressing the mouse button!

I disagree -- the general move to "natural" scrolling of widgets on desktop/mobile means that a click and drag implies the inverted motion, whereas when capturing the natural motion is the reverse. We should consider an inverted y axis setting for users though.

Free look view is very jumpy on the initial mouse move after ~ is pressed. Sometimes the view looks directly down and moving the mouse up does not move the camera, then the pointer shows up going over the main qgis window.

173298e fixes that

If the 3d view dialog has focus but the 3d canvas does not (eg try resizing the 3d view window) then key presses are not handled.

See above -- post merge i'm going to try reworking how these events are captured

Strafe left-right is pretty unintuitive if not combined with mouse turn left-right. It would be better if left-right keys were turn left-right when the mouse pointer is visible and strafe when the mouse pointer is hidden (free look mode)

Not if you grew up on FPS shooters 😆

I think it would be helpful if middle mouse button was terrain based movement pan when on fly mode
Z fly sensitivity (either mouse or pgup-pgdn) should probably be smaller than x-y sensitivity. gis projects usually have a much grater range in xy dimensions than in z.

Good feedback, thanks!

@nyalldawson nyalldawson merged commit 5ebab69 into qgis:master Jan 13, 2021
@uclaros
Copy link
Contributor

uclaros commented Jan 13, 2021

That won't be an issue if we ensure that the key is only caught when a 3d canvas is focused -- for those widgets theres no free text entry so clipboard handling isn't an issue

The standard action for clipboard managers is to invoke their gui with this global shortcut, so they do pop up and steal focus. Is ctrl+~ a blender thing that we try to mimic here?

Agreed (and will try to address during freeze) -- but keep in mind that this mode was based off blender, and even that (with it's multi million $ / year budget) doesn't give this feedback. I don't see it as a blocker accordingly.

I've barely touched blender years ago, but I'd be shocked if in blender it is as easy to get completely lost as in qgis 3d view!

I disagree -- the general move to "natural" scrolling of widgets on desktop/mobile means that a click and drag implies the inverted motion, whereas when capturing the natural motion is the reverse. We should consider an inverted y axis setting for users though.

This is not about scrolling and widgets. Enter fly mode. Move the camera with lmb, pitch is inverted. Press ~ and move the camera, pitch is not inverted. Press ~ and move camera with lmb, pitch is inverted again. This is inconsistent.

Not if you grew up on FPS shooters laughing

Especially if you grew up on fps shooters! In shooters you always have mouse free look on. The perfect analogy is trying to avoid enemies when your wireless mouse suddenly ran out of batteries! You can only run forward and strafe, you can't hide, you're fragged!

@nyalldawson
Copy link
Collaborator

Especially if you grew up on fps shooters! In shooters you always have mouse free look on. The perfect analogy is trying to avoid enemies when your wireless mouse suddenly ran out of batteries! You can only run forward and strafe, you can't hide, you're fragged!

Ahh - I misunderstood your comment. " It would be better if left-right keys were turn left-right when the mouse pointer is visible " -- I thought you were referring to the a/d left right strafe keys here, not the cursor keys. In that case I agree with you.

@nyalldawson
Copy link
Collaborator

@uclaros I'm starting to collate fixes here: #40999

@uclaros
Copy link
Contributor

uclaros commented Jan 13, 2021

In that case I agree with you.

I'm glad you agree, but a is just the same as left key : if ( mDepressedKeys.contains( Qt::Key_Left ) || mDepressedKeys.contains( Qt::Key_A ) ) ;P

Pre-doom shooters used arrow to turn and alt-arrows to strafe. They were built for playing with the keyboard. This is how I see the default fly mode.
With the introduction of mouse on post-doom shooters, the arrows changed to strafe since turning was handled by the mouse now. This is how I see the ~ fly mode.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants