Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extensions and bugfixes for the model builder package #103

Merged
merged 25 commits into from
Apr 11, 2024

Conversation

javagl
Copy link
Owner

@javagl javagl commented Mar 20, 2024

This PR summarizes a few extensions and bugfixes, mainly for the jgltf-model-builder package and related classes.

It makes the PR #99 obsolete (it includes all changes from there).

Fixes #98
Fixes #101

A short summary:

  • When a mesh primitive with morph targets was passed to a GltfModelBuilder, then the morph targets had not been taken into account when building the buffer structure ( morph targets aren't saved properly #98 )
    • The MeshPrimitiveModel/DefaultMeshPrimitiveModel had minor changes for handling morph targets. These should not affect clients in terms of behavior.
  • When a material with a normal texture was passed to a GltfModelBuilder, then this texture was not properly added to the model. This was fixed in 36c7875
  • When a GltfModel was read (from a glTF/GLB file), then the meshModel.getWeights() method always returned null ( Mesh weights are not read into model #101 )
  • When an ImageModel with JPG MIME type was created from image data that included an alpha component, then the resulting image was invalid (JPG does not support alpha). This was fixed in 04112ff
  • A "breaking change", in some way - or maybe rather a clarification and early error detection:
    Previously, it was possible to pass an AccessorModel to a GltfModelBuilder even when the AccessorModel already had an associated BufferViewModel. This caused an "invalid" glTF to be created (because the whole point of the GltfModelBuilder is to create the bufferView/buffer structure for the data that is passed to it). Now, attempting to do this will cause a GltfException to be thrown. (The validation and error handling could be extended and improved in many ways and places, but this may be a first step).
    In order to simplify the process of inserting an existing AccessorModel (that was read from another model) to a GltfModelBuilder, there now is an AccessorModels.copy(AccessorModel) method: It extracts the data from the given AccessorModel and creates a new AccessorModel (without a buffer view) that can be passed to the builder, and the builder will eventually create the buffer view as necessary.
  • Additional classes with convenience methods have been added. The names of these classes are created by appending s to the model class name. These classes are:
    • ImageModels to create image models from buffered images
    • TextureModels the create texture models from image models or buffered images
    • MaterialModels to create "default" materials (usually, with metallicFactor=0) from base colors, images, or textures
    • MeshPrimitiveModels to create mesh primitive models from the common input of indices/positions/normals/texCoords (with all except for the positions being optional)
    • SceneModels to create simple scenes from a single mesh primitive, mesh, or node

@javagl
Copy link
Owner Author

javagl commented Mar 20, 2024

During some tests, I noticed another issue that is addressed here.

Fixes #104 : The DefaultBufferBuilderStrategy now takes care of duplicate accessors. This will require some more thorough tests for corner cases, but for now, prevents the main error that was shown in the example of the linked issue.

@javagl javagl linked an issue Apr 9, 2024 that may be closed by this pull request
@javagl
Copy link
Owner Author

javagl commented Apr 11, 2024

I'll merge this for now, because it contains some important and uncontroversial bugfixes.

But I'll not immediately schedule a new release.

There still are open questions and issues that should be addressed before a new release. Some of them are summarized in #107 , and I'll create a (draft) PR with some approaches for addressing them ... soon,

@javagl javagl merged commit ebeb9b9 into master Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant