<div dir="ltr">Thanks Kevin!<div><div>That's disappointing (if also understandable) to hear that user-supplied shaders may never happen, but yeah that's a separate discussion.</div><div><br></div><div><div><b>> I would need to be convinced that multiple applications would benefit from such a feature</b></div><div>Regarding other applications, I wrote about this in my initial email. I know there's a lot of discussion in this thread so it's easy for stuff like that to get lost, but I'd be happy to discuss in more detail if you're not convinced yet.</div><div><br></div><div><div><b>> , and that your proposed solution -- as documented and exposed by the public API -- is the best way to go.</b></div><div>Great! I'll prepare a draft PR, and a comprehensive detail of the design choices and my thoughts on each.</div><div>It might be a little while before I send such a thing, since I'll take my time, but I'm glad to have the OK to get started on the second step outlined in the contribution guide.</div></div><div><br></div><div><b>> The next step, then, is to get feedback from other application developers as to whether and how they would use this.</b></div><div>Will do. There aren't many projects I've found that seem to use the 3D portion of JavaFX, so I don't know how much feedback I'll get from this, but I'll get feedback from the handful of projects which do use it, including the ones I highlighted previously.</div><div>I was hoping for some of that feedback here, but as nobody has commented on whether/how they would use it, I'll start reaching out.</div><div><br></div><div><b>> 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]</b></div><div>Yep!</div><div><div>I'll prepare a draft PR, as well as a comprehensive detail of the factors I've considered behind the API design choices.</div></div><div>Was holding off until I got your approval on the discussion.</div><div><br></div><div><b>> As Michael mentioned, it is helpful to discus how this might fit in with other possible future improvements.</b></div><div>From my perspective, there aren't very many other features which are impacted by/conflict with vertex colors.</div><div>I've detailed pretty much every feature I think could be added and how vertex colors do/don't impact them.</div><div><br></div><div><u>More realistic features (And how vertex colors impacts them):</u><br></div><div> - Supporting tri-strips, quads, or other primitives. (There are no implications that aren't there from existing design choices)</div><div> - Allowing the user to disable mip-mapping. (Feature not related)</div><div> - Allowing the user to change coordinate system. (Feature not related)</div><div> - Fog (Feature not related)</div><div> - New built-in primitives, similar to Cylinder.java, Sphere.java, etc: Cone.java, Capsule.java. (In theory we could support vertex colors on these, but I'm not sure what purpose vertex colors would serve here or how we'd expose this to the end-user.)</div><div> - Built-in Quaternion & Matrix data structures. (Feature not related)</div><div> - Allow setting texture wrapping mode. (REPEAT / CLAMP / etc, Feature not related)</div><div> - Allow setting texture filtering mode. (LINEAR / NEAREST / etc, Feature not related)</div><div> - Choosing polygon winding order. (Vertex colors not related)<br></div><div> - Cube maps (Vertex colors not related)</div><div> - Performance Improvement: Draw Call instancing (Feature not related)</div><div> - More backends (Vulkan, Metal, DX12, etc). Vertex colors will be supported with minimal performance cost </div><div><br></div><div><u>Less realistic features (</u><u>And how vertex colors impacts them)</u><u>:</u></div><div> - User-supplied shaders. (Unlikely to be implemented due to the reasons Kevin provided) This feature does come with some design implications for vertex colors, but they have already been fleshed out & addressed earlier in the thread.</div><div> - Deferred shading & support of > 3 lights. (Vertex colors don't add any new complexities to such a feature, but they might require an extra render pass, as would all the existing vertex buffers.)</div><div><div> - Stencil Testing (Feature not related, I don't think this will be added due to the scene-graph design of JavaFX not working well with this feature conceptually)</div></div><div><div> - Some method of creating an outline of a particular mesh. (A discussion would need to occur regarding how the outline would visually appear, which could include vertex data in theory.)</div><div><div> - Automatic sorting of polygons by distance to camera for better handling of transparency. (Feature not related)</div></div><div> - Non-phong lighting such as PBR. (Feature not related beyond the fact that vertex colors would be accounted for in the shaders)</div><div> - Bloom (I don't believe this will be added as this feature is more niche and I don't think JavaFX has many people who'd benefit from this. Feature not related)</div></div><div> - Normal Mapping (Feature not related)</div><div> - Realtime shadows (Feature not related)</div><div><br></div><div><b>> 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</b></div><div><div><b>> All of this is by way of saying that even if this feature proceeds, it won't happen quickly.</b></div><div>Of course. It should be no big deal to make the changes to support this feature in Metal (assuming I'm not responsible for the non-vertex color portions)</div><div>However, I have a decent concern which is that I lack any Apple hardware to test with. I could work with others I know who have Apple hardware, but this testing would likely be remote. (Unless I can find someone nearby who's willing to let me borrow machine(s) to test.)</div><div>Does Apple have anything akin to RenderDoc available for Metal? I'd assume so, but I don't know.</div><div>Does JavaFX support 3D on iOS, or would this be looking to just support Mac OS X? Would be an extra device I don't have that would need to be tested. (And doesn't Apple charge in order to develop for iOS?)</div><div>Maybe the person/group working on the Metal implementation right now would be able to assist when it comes to testing too?</div></div></div></div><div>Do you have a contact I can reach out to, so I can get these questions answered? Thanks!</div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Aug 26, 2024 at 6:25 PM Kevin Rushforth <<a href="mailto:kevin.rushforth@oracle.com">kevin.rushforth@oracle.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><u></u>
<div>
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.<br>
<br>
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.<br>
<br>
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]<br>
<br>
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].<br>
<br>
All of this is by way of saying that even if this feature proceeds,
it won't happen quickly.<br>
<br>
-- Kevin<br>
<br>
[0] <a href="https://github.com/openjdk/jfx/blob/master/CONTRIBUTING.md" target="_blank">https://github.com/openjdk/jfx/blob/master/CONTRIBUTING.md</a><br>
[1]
<a href="https://github.com/openjdk/jfx/blob/master/CONTRIBUTING.md#new-features--api-additions" target="_blank">https://github.com/openjdk/jfx/blob/master/CONTRIBUTING.md#new-features--api-additions</a><br>
[2] <a href="https://github.com/openjdk/jfx-sandbox/tree/metal" target="_blank">https://github.com/openjdk/jfx-sandbox/tree/metal</a><br>
<br>
<br>
<div>On 8/25/2024 5:45 PM, Knee Snap wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div dir="ltr">Hoping for further feedback from Michael and
others on this feature proposal, as I'm hoping to work on a
draft PR soon.</div>
<div dir="ltr"><br>
</div>
<div>Thanks!</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Fri, Aug 23, 2024 at
2:03 PM Knee Snap <<a href="mailto:kneester77@gmail.com" target="_blank">kneester77@gmail.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="ltr">
<div>Thanks for the clarification on how the design would
work.</div>
<div><br>
</div>
<div>
<div>However, this design is separate/unrelated to the
goal of this feature proposal.</div>
</div>
<div>
<div>Instead of extending TriangleMesh, you imagine a
new separate mesh which can eventually be used to
support user-supplied shaders.</div>
</div>
<div>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.<br>
</div>
<div><br>
</div>
<div><u>The future we both see for the future of working
with meshes is a scenario with two (or potentially
more) mesh classes:<br>
</u></div>
<div><b>#1) </b>TriangleMesh (No dealing with shaders,
buffers, and other advanced capabilities)</div>
<div><b>#2) </b>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.</div>
<div><br>
</div>
<div>But critical to this design is understanding that
only TriangleMesh needs explicit vertex color support.</div>
<div>
<div>VertexMesh/ShaderMesh/etc would be able to support
vertex colors implicitly due to its ability to have
the user supply arbitrary buffers and shaders.</div>
</div>
<div>
<div>So the whole purpose of my proposal is that
this feature belongs in TriangleMesh (or an extension
of TriangleMesh), but is currently missing.</div>
<div>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. </div>
<div><br>
</div>
<div>I hope this helps clarify!</div>
</div>
<div><br>
</div>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Fri, Aug 23, 2024
at 12:53 AM Michael Strauß <<a href="mailto:michaelstrau2@gmail.com" target="_blank">michaelstrau2@gmail.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">I've
created a rough prototype to illustrate what I mean:<br>
<a href="https://github.com/mstr2/jfx/tree/experiments/vertexmesh" rel="noreferrer" target="_blank">https://github.com/mstr2/jfx/tree/experiments/vertexmesh</a><br>
<br>
This is how you would use VertexMesh in an
application:<br>
<br>
var mesh = new
VertexMesh<>(Vertex.PositionTexCoord.class);<br>
<br>
mesh.getVertices().addAll(<br>
new Vertex.PositionTexCoord(<br>
new Point3D(0, 0, 0),<br>
new Point2D(0, 0)),<br>
<br>
new Vertex.PositionTexCoord(<br>
new Point3D(100, 0, 0),<br>
new Point2D(1, 0)),<br>
<br>
new Vertex.PositionTexCoord(<br>
new Point3D(0, 100, 0),<br>
new Point2D(0, 1))<br>
);<br>
<br>
mesh.getIndices().addAll(0, 2, 1);<br>
<br>
var meshView = new MeshView(mesh);<br>
meshView.setMaterial(new
PhongMaterial(Color.RED));<br>
meshView.setCullFace(CullFace.NONE);<br>
<br>
stage.setScene(new Scene(new Group(meshView)));<br>
stage.show();<br>
<br>
<br>
In addition to PositionTexCoord, we could then also
offer<br>
PositionNormal, PositionNormalTexCoord, PositionColor,<br>
PositionNormalColor, and PositionNormalColorTexCoord.
These objects<br>
are supposed to be data carriers for vertices, and
could be<br>
user-definable in the future.<br>
<br>
Note that this is by no means a well thought-out
proposal, it's just a<br>
rough sketch to get the basic idea across. Most
likely, this API is<br>
deficient in many ways, so take it as a discussion
point rather than a<br>
serious API proposal.<br>
<br>
<br>
<br>
On Fri, Aug 23, 2024 at 6:49 AM Knee Snap <<a href="mailto:kneester77@gmail.com" target="_blank">kneester77@gmail.com</a>>
wrote:<br>
><br>
> Gottcha,<br>
><br>
> 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.<br>
><br>
> 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.<br>
><br>
> 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.<br>
><br>
> 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.<br>
><br>
> 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.<br>
><br>
> 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.<br>
><br>
> 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.<br>
><br>
> 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.<br>
><br>
> 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.<br>
><br>
> Thanks again for the feedback, I look forward to
hearing back again 😁<br>
><br>
> On Thu, Aug 22, 2024, 8:08 PM Michael Strauß <<a href="mailto:michaelstrau2@gmail.com" target="_blank">michaelstrau2@gmail.com</a>>
wrote:<br>
>><br>
>> I understand that you propose to add a
special-purpose mesh<br>
>> (GouraudShadedTriangleMesh) instead of adding
yet another buffer to<br>
>> the existing TriangleMesh. That might be a
valid idea if the goal is<br>
>> to not overload the TriangleMesh class with
special-purpose stuff.<br>
>><br>
>> However, I still feel that the solution space
in terms of API isn't<br>
>> explored in enough detail here. It might be
the case that<br>
>> CustomMesh<TVertex> is not
implementable (and it might also be the<br>
>> case that CustomMesh<TVertex> isn't a
good idea to begin with). But at<br>
>> this point, none of this is obvious to me.<br>
>><br>
>> Usually, when you propose a new feature, you
should explain the<br>
>> motivation, goals and non-goals,
alternatives, and so on (you can use<br>
>> a JEP template for that if you like). You
adequately addressed the<br>
>> motivation for your proposed enhancement, but
I feel that the<br>
>> discussion of different approaches should be
expanded upon. I'm not<br>
>> convinced that CustomMesh<TVertex> is
impossible to implement: if<br>
>> TVertex can only ever be PositionTexCoord,
PositionNormalTexCoord,<br>
>> PositionColorTexCoord, and
PositionNormalColorTexCoord (and this is<br>
>> enforced, for example using sealed
interfaces), then why wouldn't we<br>
>> be able to connect this to our existing
shaders?<br>
>><br>
>> Again, I'm not saying that this is a good
idea; it might not work for<br>
>> any number of reasons. But I think these
alternative approaches should<br>
>> at least be explored a little bit before
dismissing them. Maybe it<br>
>> will be GouraudShadedTriangleMesh in the end.<br>
>><br>
>><br>
>> On Fri, Aug 23, 2024 at 4:45 AM Knee Snap
<<a href="mailto:kneester77@gmail.com" target="_blank">kneester77@gmail.com</a>>
wrote:<br>
>> ><br>
>> > Was hoping to get feedback on my
suggestion instead, but another suggestion doesn't
work.<br>
>> ><br>
>> > 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.<br>
</blockquote>
</div>
</div>
</blockquote>
</div>
</div>
</blockquote>
<br>
</div>
</blockquote></div>