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

Update to latest Magnum with glTF additions and block Y-flip APIs #2123

Merged
merged 2 commits into from
Jun 8, 2023

Conversation

mosra
Copy link
Collaborator

@mosra mosra commented Jun 5, 2023

Motivation and Context

Another ~two weeks and ~15k lines of changes in Magnum 😅 This PR mainly brings initial support for loading glTFs with Basis-compressed files that aren't pre-Y-flipped, as -- due to unfortunate featurebugs in Basis itself -- that makes them problematic to load in all other software.

This is implemented by Y-flipping the block-compressed data on import. So far this is implemented only for a few formats, more will be gradually added once I figure them out. GPUs that don't support any formats I have Y-flip implemented will fall back to RGBA8.

Compared to other libraries, this means that Magnum is now capable of loading both Y-up and Y-down files (assuming the orientation metadata is correct in the file, which isn't the case for gltfpack -y_flip, for example), so both all existing datasets and new datasets generated using the updated tools will load with correct orientation. For consistency, this Y-flip-on-load change is also made for DDS and KTX file format import, so for example FBX files with DDS textures will load correctly as well.

I promised to add a possibility for PNG fallback to Basis-compressed files, but decided not to because storing the extra image files would be a hurdle. Instead, the way to import Basis-compressed files to Blender and other tools that don't support the KHR_texture_basisu glTF extension, is to first decode them back to PNGs with the following.

magnum-sceneconverter --set BasisImporter:format=RGBA8 output.glb decoded.glb

The output, if modified, can be then encoded back to Basis if needed, although each such roundtrip will result in a slight quality decrease, so it's better to operate directly on the original non-encoded files instead. The format fallback isn't completely out-of-question yet, but I'm postponing it for now as I don't see an immediate need.

Besides the Y-flip, there's the following updates:

  • New Utility::JsonWriter::beginCompactArray() for generalized compact pretty-printing of JSON arrays
  • Full test coverage for AnyImageImporter, AnySceneImporter and AnySceneConverter, a fix for properly propagating mesh LOD levels there
  • Improved DebugTools::CompareImage format detection and matching, allowing to compare pixel views of sRGB, normalized and half-float formats.
  • Significant additions to GltfImporter and GltfSceneConverter for better asset compatibility:
    • Fixed parsing of string node extra fields where different keys could have their values mixed together
    • Parsing also nested objects in node extra fields
    • Buffer views for mesh vertex data are now shared among interleaved attributes and the vertex and index data is aligned and padded if necessary. This fixes the major compatibility issue that prevented the generated files from being openable in 3rd party tools.
    • JSON and BIN chunks in a *.glb file are padded according to the spec now as well.
    • Accessor min / max properties are written for POSITION attributes as the glTF spec demands. The glTF validator however also demands that those properties are written with an excessively large precision and complains if not. This was reported multiple times upstream but no fix in the validator was made yet.
    • Export of skinning mesh attributes. Not skins yet though, that's still a good contribution opportunity.
    • Support for exporting multi-primitive meshes (i.e., more than one mesh attached to a node)
    • Export of string, boolean and array node extra fields
    • The glTF exporter now embeds full commit information in the output file to make it easier to track down the tools revision given file was made with.
  • The OpenExrImporter has a new option that allows importing 32-bit float images as 16-bit, with the conversion performed directly by the OpenEXR library. Used by @jturner65 for creating smaller HDR lightmaps.
  • New BcDecImageConverter and EtcDecImageConverter plugins. Just to be clear, those aren't used to perform Y-flip in any way, but were needed for testing the Y-flip algorithm correctness.
  • New assumeYUp, assumeYUpZBackward and assumeOrientation options in BasisImporter, DdsImporter and KtxImporter plugins to be able to override the orientation information from file metadata (if there's any) for greater control over Y flipping.
  • Python bindings additions:

How Has This Been Tested

📗

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Jun 5, 2023
Copy link
Contributor

@jturner65 jturner65 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Contributor

@aclegg3 aclegg3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks.

@aclegg3 aclegg3 merged commit 6d10cf3 into main Jun 8, 2023
@aclegg3 aclegg3 deleted the update-magnum27 branch June 8, 2023 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants