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

GLTFLoader.js : Update WebP Texture Loader #23823

Merged
merged 1 commit into from
Mar 31, 2022
Merged

Conversation

gernotziegler
Copy link
Contributor

@gernotziegler gernotziegler commented Mar 31, 2022

Fixed #23822.

Description

Replace parameter 2 to parser.loadTextureImage with extension.source (it must be an index, not an object)

Replace parameter 2 to parser.loadTextureImage with extension.source (it must be an index, not an object)
@Mugen87 Mugen87 added this to the r140 milestone Mar 31, 2022
@mrdoob mrdoob merged commit b1321b1 into mrdoob:dev Mar 31, 2022
@mrdoob
Copy link
Owner

mrdoob commented Mar 31, 2022

Thanks!

@mrdoob
Copy link
Owner

mrdoob commented Mar 31, 2022

Does this deserve a 0.139.2?

@donmccurdy
Copy link
Collaborator

donmccurdy commented Mar 31, 2022

The regression was originally introduced in r138. I'd be fine with either patching it or waiting for r140. 👍🏻

Thank you @gernotziegler!

mrdoob pushed a commit that referenced this pull request Mar 31, 2022
Replace parameter 2 to parser.loadTextureImage with extension.source (it must be an index, not an object)
@mrdoob
Copy link
Owner

mrdoob commented Mar 31, 2022

Just published 0.139.2 with it. #23802 (comment)

@Mugen87 Mugen87 modified the milestones: r140, r139 Apr 1, 2022
@gernotziegler
Copy link
Contributor Author

Thanks for your efforts!
Do you also need a WebP-GLTF for continued testing? I can hand-convert one for you (if it only has 1 or 2 textures, otherwise it is too much work). Our 3D modeler can also supply a 3D model if you like, see www.sketchfab.com/geofront :-)

mrdoob referenced this pull request Apr 1, 2022
@mrdoob
Copy link
Owner

mrdoob commented Apr 2, 2022

Out of curiosity... How come you're using WebP with GLTF? What's your use case?

@LeviPesin
Copy link
Contributor

I think that because WebP compresses images better than both PNG and JPG.

@gernotziegler gernotziegler deleted the patch-1 branch April 2, 2022 08:19
@gernotziegler
Copy link
Contributor Author

Out of curiosity... How come you're using WebP with GLTF? What's your use case?

Levi guessed it already - download sizes. A 4K texture can go from 4 MB in JPEG to 1 MB in WebP. Relevant e.g. for the project ironmen.khm.at :)

@gernotziegler
Copy link
Contributor Author

FWIW, here is my latest node.js script to add the WebP-extension to an existing GLTF file. :-)

`console.info("gltfwebp: Extends a GLTF JSON with webp image references, as defined by ");
console.info("https://github.com/KhronosGroup/glTF/blob/main/extensions/2.0/Vendor/EXT_texture_webp/README.md ");
console.info("This code follows the MIT license. Contact gz@geofront.eu for more information.");
console.info("V1.0 - April 2022. Copyrighted by Dr-Ing. Gernot Ziegler ( geofront e.U. ). \n\n");

var fs = require("fs");

const infilename = process.argv[2];
const outfilename = process.argv[3];

console.log("Reading input GLTF " + infilename);

// Read the file of your original JavaScript Code as text

var gltf = JSON.parse(fs.readFileSync(infilename, 'utf8'));

gltf.extensionsUsed.push("EXT_texture_webp");

// add WebP extension that points to image source indices that dont exist yet, but will be created in the next step.
gltf.textures.forEach(
function(T, index)
{
let webp_sourceindex = T.source + gltf.images.length;
gltf.textures[index].extensions = {"EXT_texture_webp": { "source": webp_sourceindex } }

}
);

// create new images by copying the old ones and changing them to WebP-URIs and acc. mime type
gltf.images.forEach(
function(I, index)
{
let W = {};
W.mimeType = "image/webp";
W.name = I.name;
// W.uri = I.uri.slice(0, -5) + ".webp";
W.uri = I.uri.split('.jp')[0] + ".webp"; // split('.jp')[0] should handle .jpeg and .jpg
gltf.images.push(W);
}
);

fs.writeFileSync(outfilename, JSON.stringify(gltf, null, 2), 'utf8');
console.log("Output file " + outfilename + " written.");
`

@donmccurdy
Copy link
Collaborator

@gernotziegler can make this easier with last week's gltf-transform release:

npm install --global @gltf-transform/cli

# Optimize .jpeg textures
gltf-transform mozjpeg input.glb output.glb

# Optimize .png textures
gltf-transform oxipng input.glb output.glb

# Convert all textures to .webp
gltf-transform webp input.glb output.glb

# Convert specific textures to .webp
gltf-transform webp input.glb output.glb --slots "baseColor*"

I would agree the download sizes tend to be better with WebP, and browser support is now pretty good. Once you start having a lot of textures — or needing to load new textures without freezing the page — KTX2 compression may be a better tradeoff (despite more JPEG-like sizes), given the memory and performance benefits. Or you could mix/match, e.g. using WebP for the normal maps and KTX2 for base color.

@gernotziegler
Copy link
Contributor Author

@donmccurdy :
Thanks! I have used gltf-transform before (to convert some KTX2) - thank you for this tool!

But I thought that WebP was required-usage in GLTFs generated by gltf-transform?
https://gltf-transform.donmccurdy.com/classes/extensions.texturewebp.html

"When the EXT_texture_webp extension is added to a file by glTF-Transform, the extension should always be required. This tool does not support writing assets that "fall back" to optional PNG or JPEG image data."
Is that still the case? Because for my current projects, I was still aiming for a JPEG fallback (large audience for museum). Maybe it is more common to implement this fallback outside the GLTF loader.

I have understood that there is a trade-off between transmission size (Webp) and GPU memory needs (KTX2 better). In our case, transmission size was more important; therefore we used a bumpmap instead of a normal map and side-load it outside the GLTF of the 3D model. (Question: Is there a reason why they havent been standardised for GLTF?)

@donmccurdy
Copy link
Collaborator

But I thought that WebP was required-usage in GLTFs generated by gltf-transform?

That's true, it converts the files and doesn't leave JPEG or PNG images behind as fallbacks. Personally I think I prefer hosting alternate versions of the whole model, but your approach sounds good too.

Bump maps have been discussed a bit (e.g. see KhronosGroup/glTF#948). I think I tend to see bump maps as more of a burden to require on client/engine implementations than the transmission size would justify. Something like RG normal maps could be a possibility.

@gernotziegler
Copy link
Contributor Author

gernotziegler commented Apr 18, 2022

@donmccurdy
I hear you :-) but I don´t think that bump maps are much of a burden for three.js (that actually has bump map functionality built-in). And from a GPU perspective, I see it actually as an advantage that bump maps only need to be single channel versus triple channels for normal maps (or double channel if you reconstruct the third channel) - this is because nowadays, GPU shader run-times suffer more from bandwidth limits than compute limits during rendering.

I also made a comparison of bump map vs. normal map for good measure, using the model data for https://www.geofront.eu/demos/faust (double-click to open the book). For 8192x8192, the bump map requires 9MB, while the normal map takes 21 MB. That makes a difference in user patience.

But this all just a personal opinion, no intention to convince you - we will create a custom GLTF extension eventually, anyways, if Khronos doesn´t want to standardize it for the moment. Just haven´t had the time to modify the Blender exporter plus three.js´ GLTFLoader. Will let you all know if something happens on this front :-)

@donmccurdy
Copy link
Collaborator

Yeah, I don't see bump maps as a particular burden in three.js. I think it was more concerns about Khronos requiring all current and future glTF implementations to support both bump and normal maps. I believe normal maps are also just more consistent – you can specify MikkTSpace tangents and be sure that all software will display it exactly the same, whereas bump maps are going to vary more across implementations. But I understand that's all pretty theoretical and 9MB vs 21MB is a real difference in user experience — point taken! :)

@gernotziegler
Copy link
Contributor Author

@donmccurdy Any forthcoming Khronos specification would of course have to contain a mathematical definition of what a "bumpmap" comprises in GLTF (*), and ideally even provide some add-on shader GLSL code for the PBR shader.

Let´s see how it goes! And thank you both for the discussion :-)

(*) in my old-school teaching from 2002, a bump map does not modify or create vertices, just affects the pixel shading.

abernier pushed a commit to abernier/three.js that referenced this pull request Sep 16, 2022
Replace parameter 2 to parser.loadTextureImage with extension.source (it must be an index, not an object)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GLTF WebP Image Loading support broken because of error in class GLTFTextureWebPExtension.loadTexture
5 participants