Extend viewer3 d #1259
Extend viewer3 d #1259
Conversation
|
For codacy/pr, I think the issue raised are stupid (methods are not called because it is event programming). |
|
agree... the code coverage tests in codacy are a bit crappy |
|
I don't understand why travis fails ? Is there a configuration problem ? |
|
there is a brew issue when installing the deps on macOS bots. Just try to relaunch the build (there is nothing we can do I think) |
|
Travis ok now. |
|
yes nice I look it! for codacy agree we can désactivante some detection i think ;) |
| ( rand() % d[ 1 ] ) + p[ 1 ], | ||
| ( rand() % d[ 2 ] ) + p[ 2 ] ); | ||
| viewer << a; | ||
| viewer << Viewer::updateDisplay; |
There was a problem hiding this comment.
In this example, the updateDisplay resets the camera settings .. which is a bit odd..
Maybe this could be fixed in the Viewer3D, @kerautret, any idea?
There was a problem hiding this comment.
yes, updateDisplay call updateList with the update of the boundingbox (looks natural in classical use) but with this nice extension is should be better to add a new something like Viewer::updateDisplayFix which is equivalent but with no update.
Else you can simply equivalently call viewer.updateList(false); instead Viewer::updateDisplay; and you will have it ;)
There was a problem hiding this comment.
I applied an improvement to avoid to add new display options, @JacquesOlivierLachaud I PR it on your branch: it checked if the BB has changed and it changed camera position only if the BB changed.
There was a problem hiding this comment.
@kerautret With your PR, do I have now to call viewer.updateList(false); instead of Viewer::updateDisplay ?
There was a problem hiding this comment.
not necessary in your example the BB don't change so the camera will not move. And in other cases it is looks fine if the BB scene change.
There was a problem hiding this comment.
Thanks, I fix David's comment and the PR will be ready.
| * | ||
| * This part of the manual describes how to extend the standard | ||
| * Viewer3D interface of DGtal. For instance, you wish to assign new | ||
| * reactions to keys or to mouse event, or you wish to display or |
There was a problem hiding this comment.
Ok, if you prefer.
| * - Viewer3D::Extension::keyPressEvent. This method may be | ||
| * overloaded to capture other key events. | ||
| * | ||
| * - Viewer3D::Extension::drawWithNames. This method is useful for |
There was a problem hiding this comment.
interesting.. would you have an example of that ?
There was a problem hiding this comment.
It is used indirectly in the mini-interaction method, see for instance
https://github.com/DGtal-team/DGtal/blob/master/examples/io/viewers/viewer3D-10-interaction.cpp
I do not have a useful example here for overriding it, but it was natural to provide ways to override all the standard methods provided by libQGLViewer.
improve auto camera changed
|
👌 |
|
fine also for me ;) |
|
I think so ! |
|
go... ;) |
PR Description
This PR provides a reasonnable and easy way to extend the QGLViewer-based viewer of DGtal. Indeed, one cannot derive directly from the Viewer3D class and override methods. We provide here an Extension base class, that users have to derive in order to create a new interface. A simple example is provided.
Note also that this PR fixes some typos in class Viewer3D.
Checklist
cmakemode (otherwise, Travis C.I. will fail).