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

GLTF: Allow specifying export image format including from extensions #79314

Merged
merged 1 commit into from
Sep 16, 2023

Conversation

aaronfranke
Copy link
Member

@aaronfranke aaronfranke commented Jul 11, 2023

This PR adds support for specifying the image format when exporting a GLTF file from Godot. This is similar to #76895, but for export instead of import. The base GLTFDocument code supports "None", "PNG", and "JPEG" image formats. GLTFDocumentExtension classes can add support for new image formats, and the WebP extension class adds support for "Lossless WebP" and "Lossy WebP" formats (friendly names intended to be exposed to the UI later).

Four new methods have been added to GLTFDocumentExtension, a very quick summary:

  • _get_saveable_image_formats: An extension can return an array of user-friendly image formats to use for saving/exporting. If the image format is set to one of these, this extension will be used for image export.
  • _serialize_image_to_bytes: When exporting with embedded textures, write to a buffer.
  • _save_image_at_path: When exporting with separate textures, write to a file.
  • _serialize_texture_json: For setting the texture JSON data that will be present in the GLTF textures array.

Also, this PR means that the export code is no longer hard-coded for PNG.

Q&A:

Q: Why are you adding variables to GLTFDocument? Isn't it supposed to be stateless?
A: GLTFDocument is stateless in terms of not containing any scene data. However, the image format settings used for export is not in the Godot scene, nor in the GLTF file, the settings just change what the Godot scene data is translated into. The export settings are separate from the scene data in GLTFState in the same way that the .import file is separate from the data in the .gltf. From a practical perspective, this also makes it possible to use one GLTFDocument to process multiple scenes with the same settings.

Q: What about exporting multiple texture formats, like WebP + PNG/JPG fallback?
A: The whole point of using WebP is to have a reduced file size, so in order for a fallback to make any sense, the combination would need to be smaller in file size than just storing the full quality as PNG/JPG. This means that the PNG/JPG would need to be reduced in resolution or quality. Doing this automatically feels like too much magic, and would be a lot of code, but we can easily add this in the future if we have a good idea of how it will work.

Q: What about preserving the original file bytes if available? Like a .jpg file when exporting .jpg keeps its exact data instead of being recompressed, losing quality and gaining artifacts.
A: Lyuma suggested this, it would be a good improvement to make, after this PR is merged.

Q: What about an export settings menu to make adjusting this setting user-friendly?
A: Yes, I want that. This PR came as a result of me working on an export settings menu, and splitting off the code that allows configuring the export image setting in GLTFDocument itself.

@aaronfranke aaronfranke added this to the 4.x milestone Jul 11, 2023
@aaronfranke aaronfranke requested a review from a team as a code owner July 11, 2023 07:05
@aaronfranke aaronfranke marked this pull request as draft July 12, 2023 21:05
@aaronfranke aaronfranke force-pushed the gltf-image-format branch 2 times, most recently from 967456a to f961951 Compare July 16, 2023 06:35
@aaronfranke aaronfranke marked this pull request as ready for review July 16, 2023 06:44
@fire
Copy link
Member

fire commented Aug 4, 2023

I support this but haven't had time to review.

Also please rebase.

@aaronfranke
Copy link
Member Author

aaronfranke commented Aug 4, 2023

Rebased and re-tested.

@aaronfranke aaronfranke force-pushed the gltf-image-format branch 2 times, most recently from d76111b to 54f0109 Compare August 4, 2023 20:12
@YuriSizov YuriSizov modified the milestones: 4.x, 4.2 Sep 15, 2023
@akien-mga akien-mga merged commit f985bc9 into godotengine:master Sep 16, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@aaronfranke aaronfranke deleted the gltf-image-format branch September 18, 2023 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants