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

Add ArcBallCamera example #75

Closed
wants to merge 12 commits into from
Closed

Conversation

ttnghia
Copy link
Contributor

@ttnghia ttnghia commented Jan 28, 2020

Here is the arcball camera example :)

image

Brief Usage:

By mouse:

  • Left mouse: rotation
  • Right mouse: translation
  • Middle mouse: zooming

Keyboard shortcuts:

  • 'R': reset view
  • 'L': toggle lagging (smooth navigation)
  • '+'/'-': zooming
  • Up/Down/Left/Right: translation

Enjoy.

@ttnghia ttnghia force-pushed the arcball-camera branch 2 times, most recently from 2b11368 to 85cad80 Compare January 29, 2020 17:45
Copy link
Owner

@mosra mosra left a comment

Choose a reason for hiding this comment

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

Thank you, this is great!

I went a bit more in detail on this, because I think this isn't too far from becoming a builtin feature -- especially when we relax the dependency on SceneGraph a bit :)

src/arcball-camera/ArcBall.cpp Outdated Show resolved Hide resolved
src/arcball-camera/ArcBall.cpp Outdated Show resolved Hide resolved
src/arcball-camera/ArcBall.cpp Outdated Show resolved Hide resolved
src/arcball-camera/ArcBall.cpp Outdated Show resolved Hide resolved
src/arcball-camera/ArcBall.h Outdated Show resolved Hide resolved
src/arcball-camera/ArcBallCameraExample.cpp Outdated Show resolved Hide resolved
src/arcball-camera/ArcBallCameraExample.cpp Outdated Show resolved Hide resolved
src/arcball-camera/ArcBallCameraExample.cpp Outdated Show resolved Hide resolved
src/arcball-camera/ArcBall.h Outdated Show resolved Hide resolved
src/arcball-camera/ArcBall.cpp Outdated Show resolved Hide resolved
@mosra mosra added this to the 2020.0a milestone Jan 29, 2020
@mosra mosra added this to TODO in Project management via automation Jan 29, 2020
@mosra mosra moved this from TODO to In progress in Project management Jan 29, 2020
@ttnghia
Copy link
Contributor Author

ttnghia commented Jan 29, 2020

I would prefer to create just one ArcBallCamera object and nothing else, instead of having 3 objects:

ArcBall* _arcball { nullptr };
Object3D*             _cameraObject { nullptr };
SceneGraph::Camera3D* _camera { nullptr };

By doing so, the last 2 objects will be private members of the ArcBall class. How do you think?

@mosra
Copy link
Owner

mosra commented Jan 29, 2020

Okay, maybe I'm diverging from simplicity of the example a bit too much already, but (to repeat myself) I'd like to have this easy to integrate for people who don't use / want to use the scenegraph.

What about an ArcBall class, containing the algorithmic implementation, and SceneGraphArcBall, being a convenience all-in-one thing containing the camera+object inside? It still feels a bit too rigid for me, as it breaks composability, but as a convenience API why not.

Or if you're fine with the templated update() above, let's do just that, as it's less code overall :)

@ttnghia
Copy link
Contributor Author

ttnghia commented Jan 29, 2020

Hah, two-classes sounds good to me. We can have both of them, ArcBall will implement the update(cameraObj) function above, and ArcBallCamera will have an ArcBall object and 2 scene graph camera objects inside as private members.
I'm going to make that. The second class will just be a short header.

@ttnghia
Copy link
Contributor Author

ttnghia commented Jan 29, 2020

Done!

Copy link
Owner

@mosra mosra left a comment

Choose a reason for hiding this comment

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

If you have time and want to make the merging easier on my side, could you:

  • add a PNG 800x600 screenshot to doc/arcball-camera.png, without window borders please
  • add a doc/arcball-camera.dox doc page, showing the screenshot, listing key controls and having links to all source files? Similarly to what's in e.g. doc/fluidsimulation2d.dox, doesn't need to be perfect, just putting most of the relevant content there :)
  • mention the new WITH_ option in doc/building-examples.dox
  • and enable the new WITH_ option in package/ci/ in all *.bat and *.sh files, except the vulkan, android, emscripten and ios ones, so I can see if it builds correctly on most platforms?

This will help me a lot, thanks much! 🙏

src/arcball-camera/ArcBallCameraExample.cpp Outdated Show resolved Hide resolved
src/arcball-camera/CMakeLists.txt Outdated Show resolved Hide resolved
src/arcball-camera/ArcBallCamera.h Outdated Show resolved Hide resolved
src/arcball-camera/ArcBallCamera.h Outdated Show resolved Hide resolved
src/arcball-camera/ArcBall.h Outdated Show resolved Hide resolved
src/arcball-camera/ArcBall.h Outdated Show resolved Hide resolved
@mosra
Copy link
Owner

mosra commented Feb 4, 2020

Merged as 49a1bbe, thanks a lot! Especially thanks for the docs+CI push, that saved me quite some time. Docs now online here: https://doc.magnum.graphics/magnum/examples-arcball.html

As I mentioned on Gitter, I made the ArcBall class internals use dual quats and as a result ArcBallCamera is independent of used scenegraph transformation implementation. It needed an update on magnum's side to allow this (mosra/magnum@c25b19d), so be sure to pull it before going further :)

@mosra mosra closed this Feb 4, 2020
Project management automation moved this from In progress to Done Feb 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants