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

Support writing Document containing multiple Buffers as GLB #1171

Open
Makio64 opened this issue Nov 14, 2023 · 4 comments
Open

Support writing Document containing multiple Buffers as GLB #1171

Makio64 opened this issue Nov 14, 2023 · 4 comments
Labels

Comments

@Makio64
Copy link
Contributor

Makio64 commented Nov 14, 2023

Describe the bug
On this particular model I cant convert the gltf to glb. but it works perfectly using the gltf-pipeline
It says there is no buffer but I dont understand whats wrong, works fine with other model i tested.

To Reproduce
here the model : https://www.dropbox.com/scl/fi/calx036yfn0h3qt9x67vw/camera_curve.gltf?rlkey=vltp12bbg7s021wmur0wa2kjp&dl=0
here my code for converting :

let io = null
const initNodeIO = async () => {
	let dracoDecoder = await draco3d.createDecoderModule()
	let dracoEncoder = await draco3d.createEncoderModule()
	io = new NodeIO()
		.registerExtensions(ALL_EXTENSIONS)
		.registerDependencies({
			'meshopt.decoder': MeshoptDecoder,
			'meshopt.encoder': MeshoptEncoder,
		})
		.registerExtensions(KHRONOS_EXTENSIONS)
		.registerDependencies({
			'draco3d.decoder': dracoDecoder,
			'draco3d.encoder': dracoEncoder,
		})
}
initNodeIO 

async function convertGltfToGlb(inputPath) {
	const document = await io.read(inputPath)

	// write the glb
	const outputPath = inputPath.replace(path.extname(inputPath), '.glb')
	await io.write(outputPath, document)

	console.log(`Converted ${inputPath} to ${outputPath}`)
	return outputPath
}

Expected behavior
Convert the model with no problem.

Versions:

  • Version: 3.9.0
  • Environment: Nodejs Windows & Mac
@Makio64 Makio64 added the bug Something isn't working label Nov 14, 2023
@donmccurdy
Copy link
Owner

donmccurdy commented Nov 15, 2023

@Makio64 is this the error you're seeing?

GLB must have 0–1 buffers.

Perhaps the error should be more specific here, but the problem is that this model has many (13) buffers. Normally that would mean 13 external .bin files. Cinema4D is exporting with 13 large base 64 Data URIs, which is kind of a strange choice.

In any case, GLB requires the buffers be combined, and that's easy to do. Use unpartition() before exporting to .glb, it's lossless and won't hurt anything if the model doesn't have this problem —

https://gltf-transform.dev/modules/functions/functions/unpartition

@donmccurdy
Copy link
Owner

The CLI does this automatically as well:

if (this._outputFormat === Format.GLB) {
await document.transform(unpartition());
}

@Makio64
Copy link
Contributor Author

Makio64 commented Nov 15, 2023

Thanks, yes the more specific message would have help, especially if it point to the unpartition.
Maybe it could be a good idea to do the unpartition automatically if there is too many buffer to convert ? And add a warning explaining an unpartition had been done on the way ?

@donmccurdy
Copy link
Owner

I think it's important that io.write never modifies the input, so these should give the same result regardless of order:

// (a)
io.write('out.gltf', document);
io.write('out.glb', document);

// (b)
io.write('out.glb', document);
io.write('out.gltf', document);

Perhaps it wouldn't be too difficult, when writing to GLB, to ignore the buffer layout specified by the document without modifying the document. I'll mark this as a feature request if someone would like to look into that here:

json.buffers = [];
root.listBuffers().forEach((buffer, index) => {
const bufferDef = context.createPropertyDef(buffer) as GLTF.IBuffer;
const groupByParent = context.accessorUsageGroupedByParent;
const accessorParents = context.accessorParents;
const bufferAccessors = buffer
.listParents()
.filter((property) => property instanceof Accessor) as Accessor[];
const bufferAccessorsSet = new Set(bufferAccessors);

This may affect implementation of Meshopt and Draco compression, in which case it could turn out to be simpler just to log a more specific warning.

@donmccurdy donmccurdy changed the title Cant convert to glb Support writing Document containing multiple Buffers as GLB Nov 15, 2023
@donmccurdy donmccurdy added feature New enhancement or request package:core needs investigation and removed bug Something isn't working labels Nov 15, 2023
@donmccurdy donmccurdy added this to the 🗄️ Backlog milestone Nov 15, 2023
@donmccurdy donmccurdy modified the milestones: 🗄️ Backlog, v4.0 Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants