Triangulate/Triangulate3

Compiling or Editing the Panda Source
1

Back to working through things another covariant issue has turned up.

In panda/src/mathutil/triangulator3.h we see it define the sequence ‘vertices’ based on the return type of get_vertex (LPoint3d). Good. Cool.

However, Triangulator3 derives from Triangulator. Triangulator also defines the sequence ‘vertices’, also based on the return type of get_vertex. Though in this case get_vertex gives us LPoint2d. Now the sadness begins.

I haven’t dug too deeply into the implementation. From the comments it is my understanding that Triangulator3 uses Triangulator internally, projecting the points onto a plane to compute the result. What version of get_vertex gets called will, in C++, depend entirely on what type you are interacting with. If you have a Triangulator3 then you will see the 3d points. If you have a Triangulator (possibly because you took that as a parameter and a Triangulator3 was implicitly cast to that, or because you called Triangulator::get_vertex on a Triangulator3) it will try to use the 2d version. Without looking at the code I will guess that gives you the points projected to the plane from the 3d version.

Though I may be wrong about what happens if you call the 2d version from Triangulator3.

In any event, this is a problem for the JNI binding. Java sees the LPoint2d returning version of the function and then gets very crabby when there is a LPoint3d returning version. I have a few suggestions on how I would imagine moving forward. As always I am interested in input from the assembled.

  1. Separate the interfaces for Triangulator and Triangulator3 a bit more. Move the common implementation to something like TriangulatorBase, then have the 2d/3d access specific to Triangulator/Triangulator3. This assumes that no one using Triangulator3 actually meaningfully calls the 2d version of get_vertex (and related). I suggest this rather than renaming Triangulator to Triangulator2 as there seem to be quite a few usages of Triangulator around. Though if this approach is desired, I would not be against making the ‘Triangulator2’ change as well if wanted.
  2. Rename the relevant interfaces in Triangulator3 to indicate that they are specifically talking about 3d points. Leaving the 2d interfaces still available.

In terms of the amount of work, I’d lean towards #2. But I think doing things right is more important than doing things expediently.

2

I’d be perfectly happy with either solution, depending on how much extra work you want to do. :slight_smile:

3

I’m thinking about the semantic relationship between Triangulator and Triangulator3. From what I can see the derivation (thus is-a) exists more as an implementation convenience. Not as an expression of semantic intent.

This implies that option (1) would be the more correct. I’ve made a note to go back and do this in my cleanup pass.

Should it turn out that someone out there is using Triangulator3 and accessing both the 3d and 2d points, I think there will be a fairly natural path to providing that.