TransformStat::get_mat() returns singular matrix on macOS

[el-dee]

Description

When converting a TransformState into a matrix by using get_mat() I noticed recently that sometimes I get a singular matrix even if the transform state is perfectly valid. This seems to only occur on macOS, on Linux I never had the issue.
This happens on latest master but also with much older version but build now, so I suspect a problem with XCode or the third-party code. This happens with Panda3D build locally but also with the latest snapshot build.

Steps to Reproduce

I haven't been able to create a small reproducible example, but here is the dump of a NodePath that cause the problem :

 PandaNode root S:(AntialiasAttrib DepthTestAttrib ShaderAttrib)
   PandaNode scene-anchor T:(pos -24.9877 4.77565 -1.2761e-06 scale 0.001)
     PandaNode shifted-anchor
       PandaNode shape T:q(hpr 25.9029 -21.3056 10.007 scale 6367) S:(CullFaceAttrib CullBinAttrib DepthOffsetAttrib DepthWriteAttrib) (per-camera hidden)
         PandaNode root [] S:(ShaderAttrib)

The result of print(self.instance.get_net_transform()) gives :

T:q(pos -24.9877 4.77565 -1.2761e-06 hpr 25.9029 -21.3056 10.007 scale 6.367)

And print(self.instance.get_net_transform().get_mat()) returns a singular matrix :

0 0 0 0
0 0 0 0
0 0 0 0
0 0 0 0

Environment

• Operating system: macOS 14.1.1
• System architecture: x86_64
• Panda3D version: d9052ba
• Installation method: From source and http://buildbot.panda3d.org/downloads/d9052bae763ac42d0bf1d678be7206b20f7e46f8/Panda3D-SDK-1.11.0-d9052ba.dmg
• Python version (if using Python): 3.9
• Compiler (if using C++): clang 13.0.0

[el-dee]

After further testing, it seems it could be a reference count issue, when directly retrieving the transform matrix, the received matrix is singular :

print(self.instance.get_net_transform().get_mat())

0 0 0 0
0 0 0 0
0 0 0 0
0 0 0 0

But if i store the net transform state in a temporary variable, the matrix is fine :

transform = self.instance.get_net_transform()
print(transform.get_mat())

2.38188 5.41052 -2.34575 0
-5.89714 2.18533 -0.947458 0
6.36e-09 2.52986 5.83519 0
-24.9978 4.72253 -1.26191e-06 1

[rdb]

Is this with the transform cache disabled?

The problem is that TransformState accessors return a const reference to the fields therein, which is bound to the lifetime of the TransformState, which may go out of scope. This is a general issue in the Python API at the moment.

(A related issue was with MouseWatcher::get_mouse(), which used to return a reference to a Point2 that would "magically update" when the mouse moved, causing a lot of user confusion. This was finally changed in 76b9df0.)

Possible ways to solve this include:

1. Interrogate always makes a copy of any const reference return value.
2. We add some clever mechanism to interrogate so that a reference to the TransformState is stored on the returned PyObject, so it is kept alive.
3. We change the return value to an LMatrix4 copy, just for this one method.

I think 3 means we're just going to see this issue pop up somewhere else soon.

While 2 is clever, interrogate can't strictly speaking know for sure what the lifetime of a return value is meant to be bound to, without . We could add a special keyword to add to a method that tells Interrogate to do this, but it sounds difficult.

I'd be inclined to go with option 1, even though this could break existing code. We'll have to analyze whether there are cases where this doesn't make sense, but generally, I think this is what Python users expect.

[el-dee]

Is this with the transform cache disabled?

Yes indeed, I forgot to mention that crucial fact

While 2 is clever, interrogate can't strictly speaking know for sure what the lifetime of a return value is meant to be bound to, without . We could add a special keyword to add to a method that tells Interrogate to do this, but it sounds difficult.

This probably sounds naive, but can't the temporary returned PyObject keep a reference to the TransformState and automatically release that reference when it is garbage collected by the Python engine ?

[rdb]

That's what I suggested with 2, but how does interrogate know that the returned reference is supposed to be bound to the lifetime of the TransformState, and not bound to the lifetime of some other object? It would need to look into the method and see that it's returning a member rather than a member of something else. Or we would need to tag the method with some special keyword.

Or, maybe we just assume this is the case, and accept that it's not a perfect solution? Still, I feel that having objects being able to arbitrarily preserve the lifetime of other objects could also cause unintuitive behaviour for users, perhaps even memory leaks and reference cycles (in the case of classes like NodePath).

[rdb]

I pushed an interim fix that makes interrogate think that these methods return temporaries, so that it is forced to make a copy. This is not breaking compatibility for this specific case since TransformStates are immutable anyway.

We still need a good fix for the general case.