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