Handle mipmaps and VRAM compression for glTF binary images #62499

fire · 2022-06-28T21:44:09Z

Provide three options:

[ Ignore and Warn (default) | Optimize Loading Embedded as Basisu | Extract ]

Fixes: godotengine/godot-proposals#4271

Relates: #62436

Relates: #62295

Part of the V-Sekai group. https://github.com/v-sekai

fire · 2022-06-28T23:18:11Z

See also #59540 ( Directly compress mip maps from Basis Universal #59540 )

fire · 2022-06-29T18:08:58Z

The change itself is good, but the basis universal compression is not fast enough. Will work on it or find an other approach like detect 3d.

lyuma · 2022-06-29T18:19:38Z

@fire Do you have some data on the performance problems?

I think this is what we were trying to figure out when we were discussing the viability of Basis a few months ago.

Is the problem with import / compression (okay) or runtime / decompression / conversion to native VRAM compression (bad, affects game loading time)

Also, it would be nice if it can reuse the compressed textures if they have the same hash if I reimport the gltf

fire · 2022-06-29T18:35:38Z

I think we need time limits for the import. I tested using a sample non commercial licensed avatar like https://github.com/Miraikomachi/MiraikomachiVRM/blob/master/Miraikomachi.vrm and it was very slow with our vrm addon.

I think we can disable uastc, and live with the worse etc1 quality for a speedup.

fire · 2022-07-07T21:29:04Z

Original use case:

Have a single packed godot scene to store the entire level and expecting the level's texture images to work on android, mac and pc.
Use basis u and portable compressed texture
and the images will be portable compressed texture
if one disables uastc, it will be bad quality

reduz · 2022-08-07T13:30:58Z

I think we need a better way to tell users they exported their GLTF with built-in textures and by default Godot will take no action with them (we could make it optional what to do per scene in the GLTF import options).

fire · 2022-08-07T14:39:54Z

Discussed with @reduz

We concluded with this design.

[ Ignore and Warn (default) | Optimize Loading Embedded as Basisu etc1s via 59540 | Extract ]

fire · 2022-08-07T15:02:03Z

We discussed avoiding creating a folder.

Do filename.gltf -> filename[minus .gltf].texturename.png

I'll try to provide a pull request where the GLTF2 can compress to BasisU (etc1s) and extract to the folder using the above convention.

fire · 2022-12-26T19:21:23Z

A bug is the portable texture requires knowing about if the texture is a normal map or not but when we go through gltf -> portable compressed 2d texture -> basisu we lose that when compressing basisu as if it was a base color map.

clayjohn · 2023-01-18T20:48:45Z

@fire what remains to be done on this PR before it is ready? It would be nice for this to make it in time for 4.0 so we can support .glb files better

fire · 2023-01-18T21:04:50Z

I believe this was blocked on ASTC support (done by one of my prs), which is blocked on ASTC import (reduz expressed interest in doing this) which is blocked on Basisu changes (easy).

fire · 2023-01-19T16:59:16Z

Use the image naming at the same level don't use a folder.

fire · 2023-01-26T22:47:22Z

I am happy with regards to usability.

[ Extract (default) | Ignore and Warn | Optimize Loading Embedded as Basisu ]

modules/gltf/gltf_document.cpp

core/string/ustring.cpp

akien-mga · 2023-01-27T09:53:41Z

core/string/ustring.cpp

+static const char *invalid_filename_characters = ": / \\ ? * \" | % < >";
+
+String String::validate_filename() const {
+	Vector<String> chars = String(invalid_filename_characters).split(" ");


Could be LocalVector maybe.
Edit: Nevermind, I guess split() returns Vector<String> so you can't just assign it to a different container.

The issue is .split() allocates a Vector. validate_node_name does the same. It's called infrequently in imoprt code.
I was originally thinking of doing .replace().replace()...

It can be for a follow-up PR if needed/relevant, but I guess invalid_filename_characters could be made a static LocalVector<CharString> with all the chars already split, so we don't need to redo this computation for what's going to always be the same every time we validate a filename.

core/string/ustring.cpp

[ Ignore and Warn | Extract Textures (default) | Optimize Loading Embedded as Basisu ] Enable compressed mip maps from Basis Universal for faster compressions. Increase the quality of Basis to avoid corruption. To keep compatibility use the first mip of the previous internal Godot format. Because texture names may have invalid filename characters, adds String::validate_filename to sanitize filenames for import pipeline use.

Changes done.

akien-mga · 2023-01-27T10:37:33Z

Thanks!

fire requested a review from a team as a code owner June 28, 2022 21:44

fire force-pushed the gltf-binary-img-compression branch 3 times, most recently from 5f51c69 to 7ecee28 Compare June 28, 2022 22:00

fire changed the title ~~To keep performance, load gltf binary as compressed texture~~ To keep runtime performance, load gltf binary as compressed texture Jun 28, 2022

Calinou added enhancement topic:import topic:3d labels Jun 28, 2022

Calinou added this to the 4.0 milestone Jun 28, 2022

fire marked this pull request as draft June 29, 2022 18:08

fire force-pushed the gltf-binary-img-compression branch 2 times, most recently from 6dad928 to 94b1ef8 Compare July 7, 2022 20:52

fire force-pushed the gltf-binary-img-compression branch from 94b1ef8 to 5efe9e0 Compare July 7, 2022 21:38

fire force-pushed the gltf-binary-img-compression branch from 5efe9e0 to 2903122 Compare July 19, 2022 01:10

fire marked this pull request as ready for review July 19, 2022 01:50

fire mentioned this pull request Aug 7, 2022

Directly compress mip maps from Basis Universal #59540

Closed

fire force-pushed the gltf-binary-img-compression branch 2 times, most recently from 52b708f to b3e9e7e Compare August 7, 2022 21:19

fire changed the title ~~To keep runtime performance, load gltf binary as compressed texture~~ Handle gltf binary Aug 7, 2022

fire force-pushed the gltf-binary-img-compression branch 3 times, most recently from cc86d29 to 30e379b Compare August 7, 2022 21:33

fire mentioned this pull request Dec 21, 2022

3.x: Materials generated from .gltf file are too big in filesize #34641

Open

fire marked this pull request as draft December 26, 2022 07:17

fire force-pushed the gltf-binary-img-compression branch 5 times, most recently from 54c3cb4 to dc8a8dd Compare December 26, 2022 09:34

lyuma force-pushed the gltf-binary-img-compression branch from dc8a8dd to 3962109 Compare January 27, 2023 09:33

lyuma reviewed Jan 27, 2023

View reviewed changes

modules/gltf/gltf_document.cpp Outdated Show resolved Hide resolved

lyuma marked this pull request as ready for review January 27, 2023 09:49

lyuma requested review from a team as code owners January 27, 2023 09:49

lyuma approved these changes Jan 27, 2023

View reviewed changes

akien-mga reviewed Jan 27, 2023

View reviewed changes

core/string/ustring.cpp Outdated Show resolved Hide resolved

akien-mga reviewed Jan 27, 2023

View reviewed changes

core/string/ustring.cpp Outdated Show resolved Hide resolved

lyuma force-pushed the gltf-binary-img-compression branch from 3962109 to 39922d7 Compare January 27, 2023 10:02

akien-mga approved these changes Jan 27, 2023

View reviewed changes

akien-mga merged commit 9d555f5 into godotengine:master Jan 27, 2023

akien-mga changed the title ~~Handle gltf binary images~~ Handle mipmaps and VRAM compression for glTF binary images Jan 27, 2023

fire deleted the gltf-binary-img-compression branch January 27, 2023 15:18

geowarin mentioned this pull request Jan 31, 2023

Update project to Godot 4 beta 17 (animations still not fully updated) gdquest-demos/godot-4-3d-third-person-controller#13

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle mipmaps and VRAM compression for glTF binary images #62499

Handle mipmaps and VRAM compression for glTF binary images #62499

fire commented Jun 28, 2022 •

edited

Loading

fire commented Jun 28, 2022 •

edited

Loading

fire commented Jun 29, 2022 •

edited

Loading

lyuma commented Jun 29, 2022 •

edited

Loading

fire commented Jun 29, 2022 •

edited

Loading

fire commented Jul 7, 2022

reduz commented Aug 7, 2022

fire commented Aug 7, 2022 •

edited

Loading

fire commented Aug 7, 2022 •

edited

Loading

fire commented Dec 26, 2022

clayjohn commented Jan 18, 2023

fire commented Jan 18, 2023 •

edited

Loading

fire commented Jan 19, 2023

fire commented Jan 26, 2023

akien-mga Jan 27, 2023 •

edited

Loading

lyuma Jan 27, 2023

akien-mga Jan 27, 2023

akien-mga commented Jan 27, 2023

Handle mipmaps and VRAM compression for glTF binary images #62499

Handle mipmaps and VRAM compression for glTF binary images #62499

Conversation

fire commented Jun 28, 2022 • edited Loading

fire commented Jun 28, 2022 • edited Loading

fire commented Jun 29, 2022 • edited Loading

lyuma commented Jun 29, 2022 • edited Loading

fire commented Jun 29, 2022 • edited Loading

fire commented Jul 7, 2022

reduz commented Aug 7, 2022

fire commented Aug 7, 2022 • edited Loading

fire commented Aug 7, 2022 • edited Loading

fire commented Dec 26, 2022

clayjohn commented Jan 18, 2023

fire commented Jan 18, 2023 • edited Loading

fire commented Jan 19, 2023

fire commented Jan 26, 2023

akien-mga Jan 27, 2023 • edited Loading

Choose a reason for hiding this comment

lyuma Jan 27, 2023

Choose a reason for hiding this comment

akien-mga Jan 27, 2023

Choose a reason for hiding this comment

akien-mga commented Jan 27, 2023

fire commented Jun 28, 2022 •

edited

Loading

fire commented Jun 28, 2022 •

edited

Loading

fire commented Jun 29, 2022 •

edited

Loading

lyuma commented Jun 29, 2022 •

edited

Loading

fire commented Jun 29, 2022 •

edited

Loading

fire commented Aug 7, 2022 •

edited

Loading

fire commented Aug 7, 2022 •

edited

Loading

fire commented Jan 18, 2023 •

edited

Loading

akien-mga Jan 27, 2023 •

edited

Loading