PShape.getVertexCount() always outputs 0

[dtplsongithub]

Most appropriate sub-area of Processing 4?

Core/Environment/Rendering

Processing version

4.3.1

Operating system

Windows 11 (24H2)

Steps to reproduce this

PShape.getVertexCount() always seems to output 0. (tried 1 .svg file and 4 .obj files, even included the .mtl files)

snippet

PShape shape;
  
void setup() {
  size(640, 360, P3D);
    
  shape = loadShape("cuboctahedron.obj");
  println(shape.getVertexCount()); // zero???
}

void draw() {
  background(0);
  lights();
  translate(width/2, height/2, -200);
  scale(50);
  rotateY((float)millis()/2000);
  shape(shape);
}

Additional context

No response

[hx2A]

@dtplsongithub , can you check if your shape is a group shape?

I just tested this on the Processing example "LoadDisplayOBJ", which shows a rotating rocket. The vertex count was zero but I could use the below code to get the vertices for the object's children:

  rocket = loadShape("rocket.obj");
  println(rocket.getVertexCount());
  int count = 0;
  for (PShape shape : rocket.getChildren()) {
    count += shape.getVertexCount();
  }
  println(count);

The output is:

0
1101

I would argue that a call to getVertexCount() on a group shape should return the total vertex count of its children, and that all renderers (JAVA2D, P2D, P3D) should behave in a similar fashion for loaded shapes.

[dtplsongithub]

the code you provided me works, i guess i was using the method wrong...

[hx2A]

the code you provided me works, i guess i was using the method wrong...

No, I think the code you originally wrote should work. getVertexCount() is confusing if it returns zero for group shapes. Also, consider that the 1101 vertex count of my example could also be wrong. What if the group shape contains a mixture of mesh shapes and other group shapes? Group shapes can be nested. Some of the calls to getVertexCount() for the shape's children could also be zero. The 1101 value could be an under-count of the correct number.

Let's keep this open so we can use this as an opportunity to improve Processing. It would likely be an easy PR, so I'll add some tags so a first-time contributor can fix it.

[hx2A]

And @dtplsongithub , if you, or anyone else reading this would like some help working through the PR, I'm available to assist!

[Stefterv]

Adding a warning message, that groups are ignored in the count would also be helpful. Maybe even a countGroups = false parameter.

[micycle1]

@hx2A
I think the PShape data structure is due an overhaul (perhaps one for Processing 5😄).
Beyond the confusion you raise, one even bigger problem is how holes are encoded. Vertices of holes are not directly accessible and must be computed from the PShape's vertex codes using some fairly involved logic.

For 2D shapes the best approach would be follow the model described by Well-known text representation for geometries.

[hx2A]

@hx2A I think the PShape data structure is due an overhaul (perhaps one for Processing 5😄). Beyond the confusion you raise, one even bigger problem is how holes are encoded. Vertices of holes are not directly accessible and must be computed from the PShape's vertex codes using some fairly involved logic.

For 2D shapes the best approach would be follow the model described by Well-known text representation for geometries.

Well, much of PShape isn't that accessible. It's very much a custom data structure that has evolved over time to work well with Processing.

Overhauling PShape would probably need to wait for #881 as PShapeOpenGL is fairly complex and nobody should touch it if it is going to go away eventually.

I like the idea of using WKT for 2D geometry. There is a Java library called JTS that Processing could incorporate or at least learn from, and I believe it uses WKT. This library is closely related to GEOS, which is used by the Python library shapely.

[micycle1]

I like the idea of using WKT for 2D geometry. There is a Java library called JTS that Processing could incorporate or at least learn from, and I believe it uses WKT. This library is closely related to GEOS, which is used by the Python library shapely.

It was within JTS that I came across that format!
I use JTS extensively in my geometry library for Processing and have routines for converting between PShapes and JTS geometries (if you're curious see here). It was also through this exercise that I came across the poor design of PShape as a geometry data structure (notable issues are holes and affine transformations -- the effects of scale() etc that still aren't publicly accessible!).

[hx2A]

Hmmm, if PShape was more closely aligned with JTS, there are a lot of nice computational geometry features we could add to Processing.

[inteqam]

Hi @hx2A , I want to fix this bug ,how should i approach this problem ? Any insights or suggestions on where to start would be really helpful!

[SableRaf]

Hi @inteqam and thanks for your interest in contributing to Processing 💙

Here are a few things you can do to get started:

1. Download Processing and try to reproduce this issue.
2. Set up the development environment by following the build instructions.
3. Find where PShape.getVertexCount() is implemented and consider the problem and discussion above.

@hx2A suggested:

I would argue that a call to getVertexCount() on a group shape should return the total vertex count of its children, and that all renderers (JAVA2D, P2D, P3D) should behave in a similar fashion for loaded shapes.

How would you go about this?

Once you've done this, feel free to ask if you have specific questions. We're happy to help!

[hx2A]

Hi @inteqam ! Thank you for your interest in pursing this bug.

I've been swamped and can't reply to things as quickly.

The basic problem with getVertexCount() is that it returns 0 for GROUP shapes. What it can do instead is first check to see what shape type it is. If it is not a GROUP, it can get the count as it does now. If it is a GROUP shape, it should iterate through the children and return the sum of the childrens' getVertexCount() values. Of course some of those children can also be GROUP shapes. If that's the case, recursion will happen and the user should still get the correct result.

You might need to test and implement this fix in more than one place. The OpenGL renderers P2D and P3D have a PShapeOpenGL class. There's also a PShapeJava2D class, PShapeSVG class, and PShapeOBJ class, in addition to the PShape class they all ultimately inherit from. Trace through the code for each these classes to look for getVertexCount() methods that need to be changed.

I suggest that after you get the development environment working, as @SableRaf suggested, you build a few test Sketches to verify and explore the problem. Then you can start fiddling with the code to understand how it works and explore solutions.

[inteqam]

Thanks @hx2A and @SableRaf for taking the time to explain!

After reproducing this issue I have traced the code for each classes and I found getVertexCount() method in PShape and PShapeOpenGL only . I have updated getVertexCount() method to count Vertex of Group Shapes using recursion ensuring that no Group is missed in Nested Group case
Updated getVertexCount() :
PShape :

and PShapeOpenGL :

After Updating I have tested getVertexCount() for svg and obj files , which are now succesfully counting the vertex count of its children :

Is there anything important I should consider or might have missed? For now, I'm adding it to the PR.

[Junology]

Sorry for interruption, but I am afraid that some people may expect getVertexCount() to return the valid upper bound for the indices passed to vertex query functions such as getVertex().
Namely, the change may cause ArrayIndexOutOfBoundsException if one accidentally pass a GROUP shape to a vertex modifying function like

void disturbShape(PShape shape, float amp) {
  int vcount = shape.getVertexCount();
  PVector vec = new PVector();
  for(int i = 0; i < vcount; ++i) {
    shape.getVertex(i, vec);
    vec.x += randomGaussian() * amp;
    vec.y += randomGaussian() * amp;
    vec.z += randomGaussian() * amp;
    shape.setVertex(i, vec);
  }
}

Also, be aware that the meaning of "vertices" changes in the "Tessellation Update Mode"; see my comments in #902.

[Junology]

I would rather suggest implementing the feature of counting all the vertices as a new function, e.g. as getVertexTotalCount(), instead of refactoring existing functions.
In that case, it will be enough to write the function only in PShape class; if it is written using getVertexCount(), it would work correctly in any derived classes.

[hx2A]

but I am afraid that some people may expect getVertexCount() to return the valid upper bound for the indices passed to vertex query functions such as getVertex().

@Junology , this is an excellent point. A common use case of getVertexCount() is for loops. This PR would change the method so it can't be used that way.

I would rather suggest implementing the feature of counting all the vertices as a new function, e.g. as getVertexTotalCount(), instead of refactoring existing functions.
In that case, it will be enough to write the function only in PShape class; if it is written using getVertexCount(), it would work correctly in any derived classes.

I do like this suggestion. We can add a new method that recurses through the PShape's children to get the total number of vertices in the shape. @SableRaf and @Stefterv , what is the process for proposing new methods? Do you think it is worth it in this case?

[Stefterv]

I agree that it should not be changing existing functionality, especially regarding the use case @Junology provided. I wonder if we can instead add an extra optional parameter to the function, e.g. getVertex(boolean includeChildren = false) that will make sure the reference of getVertexCount shows the option to also count the children

[SableRaf]

Thanks everyone for your thoughtfulness 😃 Overall, I think the solution proposed by @Stefterv makes the most sense for its simplicity, and discoverability of the feature.

Bonus points for requiring only minor changes to @inteqam's existing PR 😉

@hx2A Thanks for your question. I don’t think we have a strict process for adding functions, methods, or classes to the API. It’s mostly based on discussion and community consensus. Backwards compatibility is a big priority, so anything that breaks or changes existing functionality would need more justification. We also try to keep the core small and maintainable, so small, local changes are easier to approve than larger ones, or even small ones that have a broader impact on functionality.

[hx2A]

I wonder if we can instead add an extra optional parameter to the function, e.g. getVertex(boolean includeChildren = false) that will make sure the reference of getVertexCount shows the option to also count the children

Excellent idea, @Stefterv ! That's perfect!

It’s mostly based on discussion and community consensus.

I do like these discussions and taking the time to think through changes. It's so important for the long term success of the library.