Finalize attribute API in PShape

[processing-bot]

Created by: codeanticode

The API to set/get generic attributes in PShape is not completely defined in PShape and implemented in PShapeOpenGL. Only the signature of the methods setAttrib(String name, int index, float... values), setAttrib(String name, int index, int... values), and setAttrib(String name, int index, boolean... values) are currently present in PShape. For completeness, the following additional setters should be defined in PShape and implemented in PShapeOpenGL:

• public void setAttribPosition(String name, int index, float x, float y, float z)
• public void setAttribNormal(String name, int index, float nx, float ny, float nz)
• public void setAttribColor(String name, int index, int color)

and all the missing getters:

• public PVector getAttribPosition(String name, int index, PVector vec)
• public float getAttribPositionX(String name, int index)
• public float getAttribPositionY(String name, int index)
• public float getAttribPositionZ(String name, int index)
• public PVector getAttribNormal(String name, int index, PVector vec)
• public float getAttribNormalX(String name, int index)
• public float getAttribNormalY(String name, int index)
• public float getAttribNormalZ(String name, int index)
• public int getAttribColor(String name, int index)
• public float[] getAttrib(String name, int index, float[] values)
• public int[] getAttrib(String name, int index, int[] values)
• public boolean[] getAttrib(String name, int index, boolean[] values)

[processing-bot]

Created by: codeanticode

Implemented here.

[processing-bot]

Created by: benfry

Hm, any ideas for a more elegant way for us to add these without it being 15 new methods in PShape that are specific to OpenGL?

[processing-bot]

Created by: codeanticode

Yes, I completely agree with this point. I also felt it was a little crazy to add all of these methods in one swoop. However, in my view, these methods are just the logical conclusion of the attribute API in PApplet and the setter/getters in PShape, which results in a large number of methods simply to complete the expected definition of the API.

For example, take the vertex() method in PApplet. It has two overloaded signatures (for 2D and 3D, ignoring the ones with the uv params). Those methods are mirrored in PShape, which then adds the setters setVertex(vec), setVertex(x, y), and setVertex(x, y, z), as well as the getters getVertex(vec), getVertexX(), getVertexY(), and getVertexZ(). So you get a whole bunch of methods in PShape just for the built-in vertex attribute alone.

The attribute API already adds several methods in PApplet simply due to the sheer number of combinations: attributes can be float, integer, or boolean, and there are three specialized attributes (positional and normal floats) and color as a special kind of integer attribute: attrib(float), attrib(int), attrib(boolean), attribPosition(float), attribNormal(float), and attribColor(int). As in the case of vertex, all these are already mirrored in PShape, and then you need the corresponding setters/getters, which were missing before. So, I was only "fixing" the incomplete API to match what a user would expect given the current logic of the API. But yes, these are a lot of methods :-)

To reduce the number of attribute methods in PShape while keeping internal consistency in the API we would need to redesign the entire attribute API, including in PApplet, as well as its underlying implementation, which is not trivial and will take longer.

We could simply hold off adding the new methods to PShape, this is a very specialized/advanced API after all and I do not have any idea how many people are actually using it (although I believe a few bugs have been filed about it before). But on the other hand I felt I wanted to see the API to be was truly complete for the 4.0 release :-)

[processing-bot]

Created by: codeanticode

Adding @REAS to this discussion in case he has any thoughts about the attribute API, and how to possibly simplify it.

[processing-bot]

Created by: benfry

Having given it some thought, I think let's just keep these in PShapeOpenGL, and if folks want to use them, they can do a cast.

[processing-bot]

Created by: codeanticode

Yes, makes sense. The attribute functions are only useful when using the OpenGL renderers, and cannot think of a simpler/better solution at the moment. I will update the PR with these changes shortly.

[processing-bot]

Created by: codeanticode

@benfry I think we can close this.

[processing-bot]

Created by: github-actions[bot]

This issue has been automatically locked. To avoid confusion with reports that have already been resolved, closed issues are automatically locked 30 days after the last comment. Please open a new issue for related bugs.