Add overload to `PCLVisualizer::addText3D()` that allows specifying text orientation #2038

frozar · 2017-10-27T13:18:24Z

Add a function addText3D() which allows to specify the orientation of a text.

SergioRAgostinho

Your new method (with orientation) is an exact copy of the method above (without orientation), with the added line in which you set the orientation. In order to reduce code repetition, the method without orientation should be modified to invoke your new one with the rotation set to zeros.

SergioRAgostinho · 2017-10-27T16:42:40Z

visualization/include/pcl/visualization/pcl_visualizer.h

+        /** \brief Add a 3d text to the scene
+          * \param[in] text the text to add
+          * \param[in] position the world position where the text should be added
+          * \param[in] orientation the angles of rotation regarding x-, y-, z-axis for the text


What rotation representation is this exactly? A rodrigues vector? Euler? It's important for that to be clear in the comment.

I am currently trying to figure it out from the VTK documentation and from my experiments but I completly agree, it's not so clear.

Don't forget to update this description here with the latest findings, i.e. a ZXY intrinsic rotation. I'm considering even to include a link to wikipedia just to be sure.

That being said, I don't really understand why would VTK would adopt a rotation representation which suffers from gimbal lock. I also wouldn't know how to construct this ZYX rotation from a Rodrigues vector for instance.

I'll update the documentation soon with the wikipedia link which seems to be essential!

I'm like you about the VTK choice. I learned recently about the gimbal lock issue with Euler angle. I still don't find a canonical way to handle them, but it's the VTK API. I hesitated to send a mail to VTK, but it doesn't worth it for me. (I don't know the Rodrigues vector, but it's surely more pleasant.)

SergioRAgostinho · 2017-10-27T16:46:58Z

By the way can you post some screenshots of the effect of the text rotation.

frozar

VTK API behavious

@SergioRAgostinho : I finally understand the API proposed by VTK. It's subtle and not easy to use (but do I have the choice?!?).

This first example is straightforward. I give as example 4 texts in different orienations in the following animation.

The label no_rot is displayed without any orientation/rotation.
The label x_90 shows an orientation 90° in relation to the X-axis (in red).
The labels y_90 and z_90 show respectively an orientation 90° in relation to the Y-axis (in green) and the Z-axis (in blue).
All these orientations/rotations are done in the direct trigonometric way.

The subtle of the API comes in this second example. There's also 4 labels.

The label no_rot is displayed without any orientation/rotation.
The label z_90 shows an orientation 90° in relation to the Z-axis (in blue).

Then the label z_90_n_x_90 shows an orientation of 90° in the Z and X directions. The VTK documentation specifies that the rotations are chained in the order Z, X and Y. For this label, the rotation of 90° around the Z-axis is effectively done around the blue vector. But the next rotation around the X-axis is not done around the red vector, it is done around the green vector so the Y vector.
Why is it done this way? After a while I finally understand that after the first rotation around the Z axis, the next rotation will be done not in the original/absolute coordinate system, but rather in a relative coordinate system that has been rotated by the first rotation around the Z axis. In this example, after the first rotation, here is the situation:

the new/relative X vector = the original Y vector (the green one)
the new/relative Y vector = the original -X vector (not represented)
the new/relative Z vector = the original Z vector (the blue one)
In this new/relative coordinate system, the rotation around the X-axis is done, so in fact around the original Y vector (in green).

Finally the label z_90_n_x_90_n_y_90 shows an orientation of 90° in the Z, X and Y directions. The situation after the rotation around X-axis can be sum up like:

the relative X vector = the absolute Y vector (the green one)
the relative Y vector = the absolute Z vector (the blue one)
the relative Z vector = the absolute X vector (the red one)
So the final rotation around the relative Y vector mean in fact a rotation around the absolute X vector.

The fact that the rotation are done in successive rotate coordinate system is not said in the VTK documentation...

To be complet, you'll find attached to this message the source files that I use to produce the animation.
pcl_text.tar.gz

What to do?

After all, even if the API is not intuitive, this is the way that VTK provide to give an orientation to a text. This is what I need. I would like to see this feature in the main version of PCL.

frozar · 2017-10-28T09:21:23Z

visualization/include/pcl/visualization/impl/pcl_visualizer.hpp

+    // Should we add the actor to all renderers or just to i-nth renderer?
+    if (viewport == 0 || viewport == i)
+    {
+      vtkSmartPointer<vtkActor> textActor = vtkSmartPointer<vtkActor>::New ();


@SergioRAgostinho : This new method is not exactly the same as the previous one because of the vtkActor type. With this type, the object added to the display doesn't change it's orientation, what ever is the camera point of view.
In the previous method the actor type was vtkFollower, which means, the letters will always face the camera point of view. It's easier to see the tag even if you rotate the scene but it's not what I need.
In my opinion, it is not relevant to merge the two addText3D() methods (but certainly there is something better to do). What's your feeling?

I think I would create a third method in which you supply the vtkActor in the signature, which is invoked by both methods. That way you can specialize the vtkActor for both use cases and the rest is the same. The only thing I don't like is exposing vtk types in a function signature, but I don't think I'm able to hide it from the public API because the methods which would call it are templated on the point type.

If you want, I can try do an attempt in this way. As you say, it explose VTK types... Maybe you can hide this if you propose in the API 2 methods:

addText3D_follow(): follow the view point of the camera

addText3D_oriented(): the text stay oriented independently of the camera

You keep the addText3D() function for historical reason, and this function just call addText3D_follow(). Behind addText3D_follow() and addText3D_oriented() you manage the VTK types internally.

It sounds to me that this is exactly the same situation as now, only that instead of making use of function overload, you're assigning different names to functions.

@taketwo what's your feeling on exposing an overload which allows you to provide the vtkActor directly, in the public API?

I don't like exposing VTK classes in the API and would ideally view the fact that we use VTK backend as an implementation detail. (True, we have some VTK classes in the interface of addModelFromPolyData() family of functions, but that's it.)

Now getting back to this specific function, I don't get why we need to create multiple actors (one per viewport) in the oriented text case. Unlike the follow-camera text, they are all the same, aren't they?

@taketwo : you should be right, so the textActor variable should be created outside the while loop, isn't it?

I don't see the need for the loop at all. Just use addActorToRenderer (actor, viewport) as in other functions that add something to the visualizer.

taketwo · 2017-11-03T23:11:20Z

Thanks for the analysis.

Why is it done this way? After a while I finally understand that ... the next rotation will be done ... in a relative coordinate system that has been rotated by the first rotation

So VTK seem to be accepting orientation as ZXY with intrinsic rotations.

After all, even if the API is not intuitive, this is the way that VTK provide to give an orientation to a text. This is what I need. I would like to see this feature in the main version of PCL.

In principle, we can accept orientation in any other form that we find more intuitive and internally convert to VTK representation. But I do not think that it's worth it. I'd stick with VTK representation and provide a clear documentation.

frozar · 2017-11-04T08:00:06Z

@taketwo Many thank for the link about intrinsic rotations. It will be helpful to set the good orientation for my labels. Also I agree with you, I would keep handling the VTK to set the good orientation.

…tion of a text.

frozar · 2017-11-08T09:11:40Z

I just rebase the 'text3D' branch on top on the 'master' branch.

frozar · 2017-11-08T11:08:50Z

The documentation is up to date. Let me know if you are OK with it.

… text (PointCloudLibrary#2038)

SergioRAgostinho requested changes Oct 27, 2017

View reviewed changes

SergioRAgostinho added the needs: author reply Specify why not closed/merged yet label Oct 27, 2017

frozar commented Nov 3, 2017

View reviewed changes

frozar added 2 commits November 8, 2017 10:08

[VISU] Add a function 'addText3D' which allows to specify the orienta…

e923189

…tion of a text.

[VISU] Use the function 'addActorToRenderer()'.

6bb901b

frozar force-pushed the text3D branch from 99e5bc3 to 6bb901b Compare November 8, 2017 09:09

SergioRAgostinho added needs: code review Specify why not closed/merged yet and removed needs: more work Specify why not closed/merged yet labels Nov 8, 2017

[DOC] Update the 'addText3D' documentation.

929deac

SergioRAgostinho approved these changes Nov 8, 2017

View reviewed changes

SergioRAgostinho merged commit 45cca31 into PointCloudLibrary:master Nov 8, 2017

frozar deleted the text3D branch November 8, 2017 12:35

taketwo removed the needs: code review Specify why not closed/merged yet label Nov 8, 2017

taketwo mentioned this pull request Nov 10, 2017

fix for add/remove TEXT3D in multiport view #2062

Closed

UnaNancyOwen pushed a commit to UnaNancyOwen/pcl that referenced this pull request Nov 24, 2017

Add a function 'addText3D' which allows to specify the orientation of…

e4cc743

… text (PointCloudLibrary#2038)

taketwo mentioned this pull request Dec 3, 2017

Small optimizations and fixes in PCLVisualizer #2112

Merged

SergioRAgostinho mentioned this pull request Sep 2, 2018

Change Log generation tool. Automates change log generation. #2396

Merged

SergioRAgostinho added changelog: enhancement Meta-information for changelog generation module: visualization and removed changelog: new feature Meta-information for changelog generation labels Sep 2, 2018

SergioRAgostinho added changelog: new feature Meta-information for changelog generation and removed changelog: enhancement Meta-information for changelog generation labels Sep 2, 2018

SergioRAgostinho changed the title ~~[VISU] addText3D~~ Add overload to PCLVisualizer::addText3D() that allows specifying text orientation Sep 2, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add overload to `PCLVisualizer::addText3D()` that allows specifying text orientation #2038

Add overload to `PCLVisualizer::addText3D()` that allows specifying text orientation #2038

frozar commented Oct 27, 2017

SergioRAgostinho left a comment

SergioRAgostinho Oct 27, 2017

frozar Oct 28, 2017

SergioRAgostinho Nov 8, 2017

frozar Nov 8, 2017

SergioRAgostinho commented Oct 27, 2017 •

edited

Loading

frozar left a comment

frozar Oct 28, 2017

SergioRAgostinho Nov 6, 2017

frozar Nov 7, 2017

SergioRAgostinho Nov 7, 2017

taketwo Nov 7, 2017

frozar Nov 7, 2017

taketwo Nov 7, 2017

frozar Nov 8, 2017

taketwo commented Nov 3, 2017

frozar commented Nov 4, 2017

frozar commented Nov 8, 2017

frozar commented Nov 8, 2017

Add overload to PCLVisualizer::addText3D() that allows specifying text orientation #2038

Add overload to PCLVisualizer::addText3D() that allows specifying text orientation #2038

Conversation

frozar commented Oct 27, 2017

SergioRAgostinho left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SergioRAgostinho commented Oct 27, 2017 • edited Loading

frozar left a comment

Choose a reason for hiding this comment

VTK API behavious

What to do?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

taketwo commented Nov 3, 2017

frozar commented Nov 4, 2017

frozar commented Nov 8, 2017

frozar commented Nov 8, 2017

Add overload to `PCLVisualizer::addText3D()` that allows specifying text orientation #2038

Add overload to `PCLVisualizer::addText3D()` that allows specifying text orientation #2038

SergioRAgostinho commented Oct 27, 2017 •

edited

Loading