[Feature Proposal] Vertex Colors on TriangleMesh

[Knee Snap]

Hoping for further feedback from Michael and others on this feature
proposal, as I'm hoping to work on a draft PR soon.

Thanks!

On Fri, Aug 23, 2024 at 2:03 PM Knee Snap <kneester77 at gmail.com> wrote:

> Thanks for the clarification on how the design would work.
>
> However, this design is separate/unrelated to the goal of this feature
> proposal.
> Instead of extending TriangleMesh, you imagine a new separate mesh which
> can eventually be used to support user-supplied shaders.
> I do hope to propose such a feature at a future date (support user-defined
> shaders), but until such a proposal this system isn't super relevant /
> doesn't have much relation to the current proposal.
>
>
> *The future we both see for the future of working with meshes is a
> scenario with two (or potentially more) mesh classes:*
> *#1) *TriangleMesh (No dealing with shaders, buffers, and other advanced
> capabilities)
> *#2) *VertexMesh (or name it ShaderMesh, etc), which allows the user to
> do more advanced capabilities and lets the user define their own
> buffers, which could end up looking like the design you've shown.
>
> But critical to this design is understanding that only TriangleMesh needs
> explicit vertex color support.
> VertexMesh/ShaderMesh/etc would be able to support vertex colors
> implicitly due to its ability to have the user supply arbitrary buffers and
> shaders.
> So the whole purpose of my proposal is that this feature belongs in
> TriangleMesh (or an extension of TriangleMesh), but is currently missing.
> The example you've linked however does not extend TriangleMesh, instead
> it's starting work on the future proposal, ignoring the need for this
> feature in the existing TriangleMesh.
>
> I hope this helps clarify!
>
> On Fri, Aug 23, 2024 at 12:53 AM Michael Strauß <michaelstrau2 at gmail.com>
> wrote:
>
>> I've created a rough prototype to illustrate what I mean:
>> https://github.com/mstr2/jfx/tree/experiments/vertexmesh
>>
>> This is how you would use VertexMesh in an application:
>>
>>     var mesh = new VertexMesh<>(Vertex.PositionTexCoord.class);
>>
>>     mesh.getVertices().addAll(
>>         new Vertex.PositionTexCoord(
>>             new Point3D(0, 0, 0),
>>             new Point2D(0, 0)),
>>
>>         new Vertex.PositionTexCoord(
>>             new Point3D(100, 0, 0),
>>             new Point2D(1, 0)),
>>
>>         new Vertex.PositionTexCoord(
>>             new Point3D(0, 100, 0),
>>             new Point2D(0, 1))
>>     );
>>
>>     mesh.getIndices().addAll(0, 2, 1);
>>
>>     var meshView = new MeshView(mesh);
>>     meshView.setMaterial(new PhongMaterial(Color.RED));
>>     meshView.setCullFace(CullFace.NONE);
>>
>>     stage.setScene(new Scene(new Group(meshView)));
>>     stage.show();
>>
>>
>> In addition to PositionTexCoord, we could then also offer
>> PositionNormal, PositionNormalTexCoord, PositionColor,
>> PositionNormalColor, and PositionNormalColorTexCoord. These objects
>> are supposed to be data carriers for vertices, and could be
>> user-definable in the future.
>>
>> Note that this is by no means a well thought-out proposal, it's just a
>> rough sketch to get the basic idea across. Most likely, this API is
>> deficient in many ways, so take it as a discussion point rather than a
>> serious API proposal.
>>
>>
>>
>> On Fri, Aug 23, 2024 at 6:49 AM Knee Snap <kneester77 at gmail.com> wrote:
>> >
>> > 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/20240825/09671910/attachment-0001.htm>