[Feature Proposal] Vertex Colors on TriangleMesh

[Knee Snap]

Gottcha,

That helps give the context I need to better elaborate. And to be clear I'm
not suggesting you've done anything wrong, I realized maybe I had implied
that I was upset, so I just wanted to say explicitly that is not the case.

Anywho, regarding CustomMesh<TVertex> it would be impossible to inherit
from TriangleMesh.java without breaking the existing API specification. At
least, when I assume TVertex is the representation of a single vertex. If
this assumption was wrong, and it intended to be the definition of the
vertex, that scenario will also be addressed.

TriangleMesh.java does not currently use vertex objects, and making such a
TVertex to represent each individual vertex is incompatible without
changing the current public TriangleMesh API specification. If the idea of
a vertex object only exists within CustomMesh<TVertex> and not
TriangleMesh, then it's a second-class way of writing to mesh data since it
would only work on a subset of available mesh types, whereas writing
directly to the buffers (as it works now) would have worked in all cases.
If I (someone using JavaFX) want to make utilities for creating meshes, it
rules out using TVertex unless I commit to never using the base
TriangleMesh.

Additionally, using individual vertex objects provides no utility, but
requires a  decent amount of added code complexity, as now there needs to
be a way to correlate vertex objects with buffer positions, and keep them
up to date as the buffer also changes. (What does it mean when a vertex is
moved in the buffer, but the ObservableFloatArray wasn't told that and it
was just given a new full array replacement? This is currently the only way
to update an ObservableFloatArray. Let's consider this vertex object for a
second. What is it? Is it a wrapper around the underlying buffer? If so,
every single time the array changes, all vertex objects would become
invalidated as there's no way to ensure the objects point to the correct
data in the array, or even to know if that data even exists anymore. If
it's not a wrapper around the array then we'd need to make changes to the
array backport to the object. Which has the same problem since the main way
to update the array is to provide a fully new array, meaning we would have
no way to associate the new array contents with the old objects. The only
solution would be to break the API spec and make these new vertex objects
the authoritative data source and not the arrays, which breaks existing
code.

But I'd like to drive home the final nail. There's pretty much no benefit
to be had by having vertices as objects anyways. The 3D/GPU paradigm is
easiest to work with when treating vertex data as arrays and not individual
vertex objects. (Can refer to OpenInventor, Ogre, etc, to confirm this
design choice is standard across other object-oriented 3D frameworks). This
is because at the end of the day, this is what gets passed directly to the
GPU. Adding layers of abstraction is helpful for creating/modifying the
array, but not for representing it in memory.

In other words, while TVertex might intuitively make sense from a general
object-oriented perspective, array buffers are almost always preferable to
vertex objects, even in object-oriented projects. And when individual
objects are desired, they can exist / act as wrappers in user-code, which
benefits of objects we cannot provide automatically, as it requires
information only the user knows about the organization of the arrays.

But what about if TVertex is not a vertex,  but instead a definition of
what buffers the mesh has? Well, we already have that, and it's called
VertexFormat. Making it a generic parameter also wouldn't really provide
any benefit anyways. Instead of making CustomMesh<TriangleMesh>, I propose
expanding VertexFormat to allow for additional arbitrarily defined buffers.
However, I do not think we need to expose this functionality publicly yet,
which is why I've not documented it after the suggestion. We can keep it as
internal implementation details until it's time to add user-supplied
shaders. Doing so will give us maximum flexibility when it is time to make
it public.

This way also has the benefit of us being able to retroactively include
TriangleMesh's points/texCoords/normals arrays in the shader system with
very little complexity, as they are already part of VertexFormat.

Also thanks for the suggestion about the JEPs, I'll keep this in mind
making future proposals, and it sounds like I should follow-up discussing
various different implementation options and why I've chosen the one I've
chosen instead. I suspect the reason this feels somewhat underdeveloped
from the API perspective is because it's the simplest option I came up with
that had the best API outcome and I didn't elaborate as much as I could
have on why others I thought about weren't satisfactory.

Thanks again for the feedback, I look forward to hearing back again 😁

On Thu, Aug 22, 2024, 8:08 PM Michael Strauß <michaelstrau2 at gmail.com>
wrote:

> I understand that you propose to add a special-purpose mesh
> (GouraudShadedTriangleMesh) instead of adding yet another buffer to
> the existing TriangleMesh. That might be a valid idea if the goal is
> to not overload the TriangleMesh class with special-purpose stuff.
>
> However, I still feel that the solution space in terms of API isn't
> explored in enough detail here. It might be the case that
> CustomMesh<TVertex> is not implementable (and it might also be the
> case that CustomMesh<TVertex> isn't a good idea to begin with). But at
> this point, none of this is obvious to me.
>
> Usually, when you propose a new feature, you should explain the
> motivation, goals and non-goals, alternatives, and so on (you can use
> a JEP template for that if you like). You adequately addressed the
> motivation for your proposed enhancement, but I feel that the
> discussion of different approaches should be expanded upon. I'm not
> convinced that CustomMesh<TVertex> is impossible to implement: if
> TVertex can only ever be PositionTexCoord, PositionNormalTexCoord,
> PositionColorTexCoord, and PositionNormalColorTexCoord (and this is
> enforced, for example using sealed interfaces), then why wouldn't we
> be able to connect this to our existing shaders?
>
> Again, I'm not saying that this is a good idea; it might not work for
> any number of reasons. But I think these alternative approaches should
> at least be explored a little bit before dismissing them. Maybe it
> will be GouraudShadedTriangleMesh in the end.
>
>
> On Fri, Aug 23, 2024 at 4:45 AM Knee Snap <kneester77 at gmail.com> wrote:
> >
> > Was hoping to get feedback on my suggestion instead, but another
> suggestion doesn't work.
> >
> > The idea of a CustomMesh<TVertex> is impossible to implement until after
> we have fully user-supplied shader support, which I've already addressed as
> being not the scope of this change (but instead it is a separate future
> change which is not impacted by this) it also feels like this point may
> have been missed as well.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/openjfx-dev/attachments/20240822/0cacd8ad/attachment-0001.htm>