[Feature Proposal]: TriangleMesh - Vertex Color Support

[Nir Lisker]

>
> The code snippet returns true because if the VertexFormat specifies no
> normal data, then the contents of the normals array are valid no matter
> what they are, because they are unused. validateNormals() is called
> regardless of if normals are used by the VertexFormat which is why it
> explicitly includes that check.
>
> validateNormals() is only returning whether the mesh data is valid, and if
> the VertexFormat does not include normal data, then the normal array is
> always treated as valid (as normals are not used and thus the mesh is valid
> regardless of the contents of the array).
>

In that case, returning 'false' for an array of length 0 is wrong since
there are no normals to be validated.

Regarding the last sentence of the previous email, yes, that is the same as
> option #2 in my last email. This would break the existing design choices
> (as you've noticed with normals), but it is one of the paths I can see
> working.
>

 Your option 2 says to make VertexFormat read-only and deprecate the
constructor. My suggestion is to keep it as is (for now), and add a color
array. Determine if colors are used by a length == 0 check. The vertex
format has no impact on this.

On Tue, Oct 1, 2024 at 2:01 AM Knee Snap <kneester77 at gmail.com> wrote:

> The code snippet returns true because if the VertexFormat specifies no
> normal data, then the contents of the normals array are valid no matter
> what they are, because they are unused. validateNormals() is called
> regardless of if normals are used by the VertexFormat which is why it
> explicitly includes that check.
>
> validateNormals() is only returning whether the mesh data is valid, and if
> the VertexFormat does not include normal data, then the normal array is
> always treated as valid (as normals are not used and thus the mesh is valid
> regardless of the contents of the array).
>
> Regarding the last sentence of the previous email, yes, that is the same
> as option #2 in my last email. This would break the existing design choices
> (as you've noticed with normals), but it is one of the paths I can see
> working.
>
>
> On Mon, Sep 30, 2024, 2:58 AM Nir Lisker <nlisker at gmail.com> wrote:
>
>> I would first try to figure out if the code in the snippet I gave is
>> actually correct. If it is, then the vertex format has a functional use. If
>> it's not, then we can think about what to do with it. I didn't look into
>> it, but on the face of it, it doesn't look like the unused normals should
>> be validated.
>>
>> The last sentence in my previous mail suggested a different solution:
>> keep the vertex format as it is (for now) to dictate how points and normals
>> are read, but add color without interaction with the vertex format. The
>> color will be used if it's not empty/null.
>> This keeps everything as it is, avoids multiplications of formats, and
>> doesn't hinder possible future additions that will also not use the vertex
>> format. On the slightly confusing side, it means that vertex format doesn't
>> define the format for *all* the data of the vertex, just for its positional
>> components that are mandatory (at the very least, points should be
>> supplied).
>>
>> On Mon, Sep 30, 2024 at 3:12 AM Knee Snap <kneester77 at gmail.com> wrote:
>>
>>> These are some great points!
>>>
>>> Nir mentioned a builder-style pattern for TriangleMesh, but I struggle
>>> to think of how this could work well, as one critical feature of the
>>> current API is to be able to dynamically update mesh data, which is very
>>> important for any kind of animated mesh. I'd love to see any designs that
>>> get prototyped though (regardless of if they are builder-like).
>>>
>>> Let's discuss the big question though, VertexFormat's design.
>>>
>>> To be honest, light maps and shadow maps seem unrealistic right now.
>>> They are way too intensive to generate in JavaFX (we'd need a LOT of work
>>> to do this efficiently, only for the user to not like the algorithm we
>>> selected). And even if the user gets this far, they'll be faced with a
>>> significant performance penalty. Light maps usually contain data for many
>>> 3D objects, but due to JavaFX rendering order being based on the scene
>>> graph order, which is going to cause a significant amount of large texture
>>> swaps. I'd be open to changing that and do hope to fix the performance
>>> issues as much as we can, but overall it strikes me as a very far-fetched
>>> feature.
>>>
>>> Regarding bones/skeletal rigs, that's a really good idea actually. I'm
>>> kind of shocked I didn't think of that sooner.
>>>
>>> I've got a few ideas for how we can tackle these problems, but none
>>> particularly jump out as the perfect option to me, so I'm hoping for
>>> feedback on what you guys think.
>>>
>>> *1) Why not let users define their own VertexFormats? If our concern is
>>> the explosion of definitions we'd be keeping statically, we could just
>>> define a handful of built-in formats but let users define formats as they
>>> see fit. Similar to the Cesium Project Nir linked.*
>>>
>>> *Pros:*
>>>  - This would avoid the explosion of built-in formats, since we wouldn't
>>> need to explicitly define/support every single format, but instead let the
>>> user define the format themselves.
>>>
>>> *Cons:*
>>>  - Doesn't address the concern of TriangleMesh properties not serving
>>> any purpose depending on what VertexFormat it has.
>>>
>>> *2) Have TriangleMesh automatically determine format based on which
>>> buffers contain/do not contain data.*
>>> This is extremely similar to #1, except it
>>>
>>> We could keep VertexFormat around, but make it read-only, so it only
>>> serves as a reflection of the data in TriangleMesh, instead of something
>>> the user can apply. We'd deprecate the constructor
>>> TriangleMesh(VertexFormat) and TriangleMesh.setFormat(VertexFormat) but
>>> keep them around (they would do nothing) thus keeping code compatibility
>>> (and maybe also binary compatibility?).
>>>
>>> *Pros:*
>>>  - Avoid bloat.
>>>  - Extensible without ever leaving any functionality both unusable and
>>> accessible. (IDEA: What if it's not fully removed, but instead is a
>>> read-only property obtainable inside TriangleMesh, and the constructor
>>> which accepts a VertexFormat deprecated instead? It still feels appropriate
>>> to get vertex format data from VertexFormat instead of the mesh itself)
>>>
>>> *Cons:*
>>>  - Breaking API change. (If we deprecate/remove VertexFormat)
>>>  - The user is still responsible for supplying the faces array in the
>>> correct format, and making the VertexFormat calculated implicitly behind
>>> the scenes (something the user must respond to or the mesh will either not
>>> render or display corrupted if we can't tell), would be hidden from the
>>> user. Currently the user has to explicitly say they're changing the format,
>>> but this automatic behavior is less clear to the user. Not a huge deal but
>>> worth considering.
>>>
>>> The overall outcome of this seems pretty similar to #1, but it loses the
>>> benefit of explicitness in exchange for not ever having buffers which are
>>> unusable but still accessible. I'm personally torn on if this trade-off is
>>> worth it.
>>>
>>> *3) Add Vertex Colors to VertexFormat, but future features use either #1
>>> or #2.*
>>> Bump map tangents/light map uvs/skeleton seem significantly less
>>> ubiquitous than vertex colors even if they're still important. Picking this
>>> option would expand the already questionable normals design, but
>>> considering these future features may never be added, perhaps it makes
>>> sense to keep the existing design until we're sure we're even going to add
>>> the others. And at that point we'd be in exactly the same position as now
>>> with exactly the same options for implementing it. In other words, it buys
>>> us time while still letting us scrap VertexFormat if the time ever comes.
>>>
>>> *Pros:*
>>>  - Keeps all existing features consistent with existing public API
>>> design.
>>>  - Avoids bloat
>>>  - Avoids breaking consistency with the existing public API up until the
>>> moment we start adding less common data fields.
>>>
>>> *Cons:*
>>>  - If Option #1 is picked for future features, the same cons from #1 are
>>> present.
>>>  - If Option #2 is picked for future features, the same cons from #2 are
>>> present.
>>>  - If we're going to deprecate VertexFormat, it might be a valid point
>>> that that we don't actually want to delay that, and would rather do it
>>> earlier rather than later.
>>>
>>> I'm torn between all of the options  I've listed here. What do you guys
>>> think?
>>>
>>> We should also discuss how other primitive topologies (POINT, LINE,
>>> TRIANGLE, TRIANGLE_STRIP) could work and how this might factor into our
>>> VertexFormat discussions. (Should LINE really even be represented by
>>> TriangleMesh and not a new LineMesh? Would it make sense to still use
>>> VertexFormat?)
>>>
>>> On Mon, Sep 23, 2024, 9:23 AM Nir Lisker <nlisker at gmail.com> wrote:
>>>
>>>> Having gone through some sources, I found the additional properties
>>>> that can be added as per-vertex data:
>>>> * Additional texture coordinates (mentioned by Michael). These can be
>>>> used for detailed textures.
>>>> * Bone indices and weights. These are for GPU skinning. Interestingly,
>>>> the D3D vertex shader constants file has some preparation for skinning and
>>>> bones [1].
>>>> * Tangents. Used similarly to normals in some lighting-related
>>>> calculations.
>>>>
>>>> While it's not clear that we'll need these, the proliferation of vertex
>>>> formats will be unmanageable with even one of these added (in addition to
>>>> color).
>>>>
>>>> Looking at the use of VertexFormat in TriangleMesh with regards to
>>>> normals:
>>>>
>>>> private boolean validateNormals() {
>>>>     // Only validate normals if vertex format has normal component
>>>>     if (getVertexFormat() != VertexFormat.POINT_NORMAL_TEXCOORD) return
>>>> true;
>>>>
>>>>     if (normals.size() == 0) { // Valid but meaningless for picking or
>>>> rendering.
>>>>     return false;
>>>>     ...
>>>> }
>>>>
>>>> I'm confused by this. If the normals should be validated only when the
>>>> vertex format defines that normals are used, why does the method return
>>>> true when normals aren't used?
>>>> I would think, along with what Michael said, that either if the vertex
>>>> format doesn't declare normals, or the user doesn't declare normal (size
>>>> ==0), then the normals wouldn't be validated. That would also mean that the
>>>> vertex format is redundant since it's implied by the existence/length of
>>>> the array.
>>>>
>>>> Perhaps it's worth looking at the color implementation this way.
>>>> Additional vertex formats should not be added, and the use of vertex colors
>>>> should be dictated by looking at the array.
>>>>
>>>> [1]
>>>> https://github.com/openjdk/jfx/blob/master/modules/javafx.graphics/src/main/native-prism-d3d/hlsl/vsConstants.h
>>>>
>>>> On Sat, Sep 21, 2024 at 5:17 AM Michael Strauß <michaelstrau2 at gmail.com>
>>>> wrote:
>>>>
>>>>> > I'm not yet convinced that new vertex formats are the way to go.
>>>>>
>>>>> To be fair, I'm not sure why VertexFormat even exists. It serves no
>>>>> purpose when creating a TriangleMesh, as the vertex format could
>>>>> easily be inferred by the presence (or absence) of vertex data. Having
>>>>> users specify the VertexFormat explicitly is probably not a good API,
>>>>> as it now makes the other data components be dependent on the choice
>>>>> of the vertex format.
>>>>>
>>>>> Of course, not all combinations of vertex data are valid, but this
>>>>> could be specified in documentation and be validated by the
>>>>> implementation. I think it might be best to deprecate VertexFormat for
>>>>> removal. This would also prevent a potential explosion of vertex
>>>>> formats in the public API.
>>>>>
>>>>> I have no objection to the choice to add vertex colors as an
>>>>> additional data component to TriangleMesh. It adds to the API, but
>>>>> this is not a slippery slope as there's only a very finite set of
>>>>> features we might be tempted to add next:
>>>>>
>>>>> The most obvious thing that JavaFX 3D really can't do is shadows.
>>>>> These come in static form (light mapping) and dynamic form (shadow
>>>>> mapping). Light mapping requires a second set of texcoords, while
>>>>> shadow mapping does not.
>>>>>
>>>>> Adding light mapping would make JavaFX 3D competitive with late-90's
>>>>> graphics, and adding shadow mapping would make it competitive with
>>>>> early-2000's graphics.
>>>>>
>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/openjfx-dev/attachments/20241010/84cf139e/attachment-0001.htm>