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 <kneeste...@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ß
<michaelstr...@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
<kneeste...@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ß
<michaelstr...@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
<kneeste...@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.
