vtk.vtkTransform.GetOrientation giving wrong results for large scale differences

Development
1

vtk.vtkTransform.GetOrientation produces the incorrect orientation if the difference in scales is more than 100.

Running on
Elements:

40 0 0 0 
0 0 -10 0 
0 0.04 0 0 
0 0 0 1

gives the correct: [90.0, -0.0, 0.0]

But changing the scale slightly

41 0 0 0 
0 0 -10 0 
0 0.04 0 0 
0 0 0 1

results in [0.0, -0.0, 0.0] which is incorrect

The reason why I need this in the first place is because I need to set the transform matrix of an actor. And the only way to do that is using SetScale, SetPosition and SetOrientation separately.

    m0 = mat1.GetElement(0, 0)
    m1 = mat1.GetElement(0, 1)
    m2 = mat1.GetElement(0, 2)
    m4 = mat1.GetElement(1, 0)
    m5 = mat1.GetElement(1, 1)
    m6 = mat1.GetElement(1, 2)
    m8 = mat1.GetElement(2, 0)
    m9 = mat1.GetElement(2, 1)
    m10 = mat1.GetElement(2, 2)

    scale1 = (m0 * m0 + m4 * m4 + m8 * m8) ** 0.5
    scale2 = (m1 * m1 + m5 * m5 + m9 * m9) ** 0.5
    scale3 = (m2 * m2 + m6 * m6 + m10 * m10) ** 0.5

    actor.SetScale(scale1, scale2, scale3)

    x = mat1.GetElement(0, 3)
    y = mat1.GetElement(1, 3)
    z = mat1.GetElement(2, 3)

    actor.SetOrigin(0, 0, 0)
    actor.SetPosition(x, y, z)

    out = [0, 0, 0]
    vtk.vtkTransform.GetOrientation(out, mat1)

    print("=======================================================")

    print(mat1)
    print(out)

    actor.SetOrientation(*out)

A possible work-around is to remove the scaling from the matrix before calculating the orientation:

    mat1.SetElement(0,0, m0 / scale1)
    mat1.SetElement(0,1, m1 / scale2)
    mat1.SetElement(0,2, m2 / scale3)

    mat1.SetElement(1, 0, m4 / scale1)
    mat1.SetElement(1, 1, m5 / scale2)
    mat1.SetElement(1, 2, m6 / scale3)

    mat1.SetElement(2, 0, m8 / scale1)
    mat1.SetElement(2, 1, m9 / scale2)
    mat1.SetElement(2, 2, m10 / scale3)
2

Hi Ruben,

I looked at the vtkTransform source code, and apparently this behavior was intentional. The code defines this constant:

VTK_AXIS_EPSILON = 0.001

If the ratio of the smallest scale to the largest scale is less than VTK_AXIS_EPSILON, then the angle is set to zero. For 0.04 and 41, the ratio is 0.00097561.

When the code was first written, this epsilon check was needed to avoid numerical issues with single-precision floats. But with double-precision, the epsilon should be something like 1e-12.

I can check to see if the VTK test suite succeeds with VTK_AXIS_EPSILON = 1e-12. If it does, then we can permanently change it.

3

Yes, but your method of computing the scale does not work if the scaling occurs along oblique axes. The general method of removing the scaling is to orthogonalize the matrix.

So perhaps GetOrientation() should be changed so that it automatically orthogonalizes the matrix before computing the angles. At the very least, it could check the condition number of the matrix and orthogonalize if the matrix is ill-conditioned like yours. This is also something that I can look into.

4

Euler angles are really bad for representing angles due to singularities and ambiguities when converting to/from axis directions (3x3 matrix). By adjusting tolerances or using more bits we cannot solve the problems, just reduce the frequency of failures.

I would recommend to use SetUserMatrix to position, scale, and orient actors and never use Euler angles (Get/SetOrientation) for representing orientation.

Maybe we should add comments to the API documentation of Get/SetOrientation methods, warning users that they are not recommended for general use.

5

@dgobbi , thanks for looking into the code. Changing the epsilon to 1e-12 would be more than sufficient for my purpose. The epsilon of 1e-3 is already on the edge of occuring cases.

@lassoan , about the UserMatrix, I’ve been trying to avoid UserTransforms due to Are UserTransforms going to be deprecated?.

For me the ultimate solution would be to set the Matrix of the actors transform directly. Would a PR for that be appreciated or is there a reason to disallow setting it directly?

6

I would love to be able to directly set the actor’s transform. Ever since I first started using VTK, the way VTK handled transformations seemed very clumsy. For me, it doesn’t even make sense to have methods like “SetOrientation” or “SetPosition” for actors. The vtkTransform already has all the methods necessary to build a 4x4 matrix, so just passing the 4x4 matrix (or the transform itself) to an actor would be much more flexible. The current API is very limiting.

However, as you can see from Ken Martin’s comment in the issue that you linked, you probably wouldn’t get very far with a PR.

1 Like
7

We need to be able to set user transform/matrix directly. Requiring setting the orientation using Euler angles would be very complicated and fragile when you already have the position+orientation+scaling as a matrix.

I asked for the exact requirements on discourse and submitted the issue in gitlab to sort out this issue proactively, instead of having to push back on a pull request that breaks/removes the essential direct matrix setting API.

@ken-martin could you describe what are the user transform/matrix requirements that must be fulfilled for correct behavior?

For example we could require the user matrix to be a homogeneous transformation matrix composed of rigid transformation + scaling with positive factors, resulting in a:

  • 4x4 matrix with bottom row equal to (0, 0, 0, 1)
  • topeft 3x3 submatrix is orthogonal and has positive determinant

We may also define some soft requirements for accuracy and numerical stability. For example, maximum ratio between smallest and largest absolute value of scaling factor is recommended to be below 1e3 (or 1e6 or 1e12?). It might be also useful to specify a tolerance for the orthogonality of the orientation submatrix and for the (0,0,0,1) values in the last row.

Fully agree. We have been always using SetUserMatrix in 3D Slicer. Require using Euler angles for setting the actor orientation would be completely unacceptable. Setting the orientation using a quaternion would be sufficient and make sense in theory, but it would be impractical.

8

@ken-martin could you describe what are the user transform/matrix requirements that must be fulfilled for correct behavior?

FYI Ken has retired, I doubt you’ll get a response.

1 Like
9

Thanks a lot for the information. This is very important news. I’ve just checked and his profile no longer shows up on Kitware’s website (at least searching by his name did not bring up anything for me). Was there an announcement on the forum that I missed?

Could you recommend people to bring into this discussion who have the experience and authority to make the necessary changes in VTK? It may be just documentation change in the end, but whatever is described in the end should be aligned with long-term plans for THIS API.

1 Like
10

No external announcement that I am aware of, Ken wanted to go quietly. But don’t feel bad for him, I understand he’s having a lot of fun adventures :slight_smile:

As far as moving ahead with this discussion, David G knows more about VTK transforms than probably anyone else. @Sankhesh Jhaveri is moving towards a rendering leadership position, and should be involved in discussions. I know the folks in Kitware’s French office are very talented with lots of great rendering experience, I’ll make sure that they (as well as any others I can think of) are aware of the discussion. And of course there’s a lot of smart people across the VTK community - hopefully they will speak up.

BTW, I’ve never been completely happy with the transformation/matrix implementation and design, I personally would like to see something cleaner. It’s going to be a challenge though with all the code out there. And FYI, much of the API was initially designed in the mid-1980s in an object-oriented system called LYMB (a precursor to VTK). Graphics have changed a little bit since then :slight_smile:

1 Like
11

Thank you @will.schroeder.

This is amazing. Huge enterprises, frameworks, libraries emerged and disappeared during this time but VTK stayed around. I guess it is due to VTK developers being very smart to make many good decisions and being humble to admit and fix the few mistakes.

1 Like
12

You hit the nail on the head: the community is amazing. A great mix of talent, but maybe more importantly very little drama and people trying to do the right thing (even when it means humbly admitting a mistake or less than stellar design :-)).

1 Like
13

@ken-martin will be missed to be sure. He has always been the go-to resource once other people got over their heads.

Ideally we turn this into an opportunity for more people to get active in the rendering pipeline and re-think from today’s graphics perspective. It would be nice to have althernate rendering paths so that the legacy features don’t break but new, cleaner, more flexible code can be developed. Ideally multiple ways of rendering could be composited at the framebuffer/zbuffer level efficiently.

14

If we’re going to rethink rendering, we need to address the eventual EOL of OpenGL. The last I heard is that Dawn was still a path forward, but whatever, this is a good opportunity.

15

Agreed, Dawn is a good option. I’ve also experimented with WGPU which is similar to Dawn but uses Rust instead of C++. WGPU also has some nice python bindings. Having more than one WebGPU compatible implementation option is a nice strength I think, and sharing code/expertise between C++, JavaScript, and Python development environments would be awesome.

Also, just as an aside before people get too concerned, it’s true that OpenGL is deprecated on MacOS and being replaced by Vulkan in many places but there are several ways to continue using the OpenGL API with a compatibility layer. So rethinking the rendering pipeline is more of an opportunity than a crisis.

16

I’m bumping this topic.

My work around for setting the actor transform has been working until I recently updated VTK to 9.3 (from pip). It now starts to throw:

InternalUpdate: doing hack to support legacy code. This is deprecated in VTK 4.2. May be removed in a future version.

when I’m setting the elements of the matrix. Easy to solve by making a copy first, but the stack of work-arounds on work-around get bigger and bigger.

Still looking for a good way to set the transform on an actor.

17

What is your workaround? Have you tried to use SetUserMatrix?

As I wrote above, in 3D Slicer we have been using SetUserMatrix for over 25 years. It’s been working well, except a couple of times during all these years somebody forgot about UserMatrix when implemented some new feature and those issues had to be fixed. There may be still some undiscovered bugs in rare cases (most likely in some special lighting computations) but it should not be too hard to fix those.

18

my work-around is calculating the position, scale and orientation from the transform and then setting those using SetScale, SetOrientation and SetPosition.

On: SetUserMatrix

" @lassoan , about the UserMatrix, I’ve been trying to avoid UserTransforms due to Are UserTransforms going to be deprecated? ."

19

SetOrientation is fragile, so I would never use that. If you don’t want to switch to UserMatrix then I would recommend to send a merge request that adds API to VTK to set the position/orientation/scale of the actor using a vtkMatrix4x4. David, I, and probably several other VTK developers will strongly support your case, so I do not expect huge resistance. Probably the discussion will be limited to what are the requirements for the matrix (orthogonal, positive definite, etc.) and how to enforce them (automatically correct the matrix, add helper function that can be used to correct the matrix, only check the matrix, or do not do any checks or corrections).

1 Like
20

I was assuming that vtkProp3D would internally store its transform as a matrix. That would adding make adding such a function to the API quite straight forward.

But as it turns out it is in fact stored as Position, Scale and Orientation. The matrix is computed from those in the “ComputeMatrix()” method of vtkProp3D.
There are some other parts of vtk (vtkFollower, vtkAxisFollower, vtkProp3DAxisFollower mainly) that read and use position, orientation and scale directly, so simply rewriting position, orientation and scale as matrix is not as trivial as I had hoped. It would make the code a lot cleaner though, so may still be worth the effort if it solves more than just my case.

Without re-writing an added API to set the matrix using a matix would still need to extract the position, orientation and scale from that matrix. which was the problem in the first place.

And there was probably a good way why it was done like this in the first place (although mid-80s it was maybe a memory thing, so it may not be such a good reason anymore).