[Feature Proposal] Vertex Colors on TriangleMesh
Kevin Rushforth
kevin.rushforth at oracle.com
Tue Aug 27 01:25:38 UTC 2024
As Michael mentioned, it is helpful to discus how this might fit in with
other possible future improvements. Given the points you raised, I think
we can now decouple the discussion of whether and how to add vertex
colors to 3D TriangleMesh from the more general discussion of
user-defined geometry. Especially since adding support for user-defined
shaders is not going to happen any time soon (if ever). This has been
looked at in the past, but always runs into a couple fundamental
problems -- for one, we do not want to expose the low-level rendering
library that Prism happens to be using on a particular platform (which
could change over time: OpenGL --> Metal on macOS, etc), so we would
need a graphics-language-neutral shading language and figure out how to
wire that up to the renderer without exposing Prism internals.
So back to your proposal to add vertex colors to 3D TriangleMesh, it
seems like a somewhat interesting feature, but not a high priority for
us. I would need to be convinced that multiple applications would
benefit from such a feature, and that your proposed solution -- as
documented and exposed by the public API -- is the best way to go.
The next step, then, is to get feedback from other application
developers as to whether and how they would use this. A Draft PR might
be OK as long as the focus is on the API and the use cases. I presume
you have read the CONTRIBUTING guidelines [0], especially the part about
adding new features? [1]
If it proceeds further, be prepared to come up with a plan to document,
test, implement the new API on all platforms. You will need to modify
the shaders for each of the graphics APIs. We might ask you to provide
at least an initial implementation for the in-progress metal pipeline [2].
All of this is by way of saying that even if this feature proceeds, it
won't happen quickly.
-- Kevin
[0] https://github.com/openjdk/jfx/blob/master/CONTRIBUTING.md
[1]
https://github.com/openjdk/jfx/blob/master/CONTRIBUTING.md#new-features--api-additions
[2] https://github.com/openjdk/jfx-sandbox/tree/metal
On 8/25/2024 5:45 PM, Knee Snap wrote:
> 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/20240826/f486f1e4/attachment-0001.htm>
More information about the openjfx-dev
mailing list