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

DracoLoader should normalise color data by default #26035

Closed
Gmadges opened this issue May 10, 2023 · 9 comments · Fixed by #26036
Closed

DracoLoader should normalise color data by default #26035

Gmadges opened this issue May 10, 2023 · 9 comments · Fixed by #26036

Comments

@Gmadges
Copy link
Contributor

Gmadges commented May 10, 2023

Description

I would expect color attributes from the loader to default to normalised

Solution

Set the normalized to true in the dracoLoader

Alternatives

This may be undesired behaviour for some users?

Additional context

https://github.com/NASA-AMMOS/3DTilesRendererJS/pull/326/files#r1182293106

@donmccurdy
Copy link
Collaborator

donmccurdy commented May 10, 2023

Are you loading .drc files, glTF files containing Draco compression data, or something else? All glTF files I see using Draco compression have used float32 vertex attributes, in which case normalization is not relevant. I don't know how to create Draco files containing uint8 or uint16 vertex colors, are you able to share examples? I agree that those could be marked as normalized.

@donmccurdy
Copy link
Collaborator

donmccurdy commented May 11, 2023

If possible, it would still be great to have an example of a .drc or .glb file with Draco compression, using normalized 8-bit or 16-bit attributes.

@major-mayer
Copy link

I'm currently struggling with loading my .drc file, which has vertex colors stored in uint8 format.
encoded_mesh.zip

I try to load the file using the following code:

// Load a Draco geometry
loader.load(
	// resource URL
	'encoded_mesh.drc',
	// called when the resource is loaded
	function ( geometry ) {
		const material = new THREE.MeshStandardMaterial( {vertexColors: true} );
		const mesh = new THREE.Mesh( geometry, material );
        mesh.geometry.attributes.color.normalized = true;
        console.log(mesh.geometry.getAttribute("color"))
		scene.add( mesh );

	},

However the colors look very distorted (you can see a lot of clipping):
image

This is no wonder when I look at the console output:
image

Can I somehow tell Three.js that my colors are actually uInt8 values and not Float32?
I guess ThreeJs tries to parse the colors as Float32 values.

@donmccurdy
Copy link
Collaborator

donmccurdy commented Dec 14, 2023

I'm not 100% sure on how Draco handles this, particularly with .drc files, but I believe that when we ask the Draco decoder for f32 data, it is supposed to return f32 data, regardless of what precision the original colors might have had. That it returns these values far out of [0,1] range suggests that the data encoded in the .drc file might not be valid. Is there any other decoder we can check the .drc file's validity with?

This type of issue should be handled automatically when using Draco data within a .gltf/.glb file, if that's an option for you.

There's also this problem:

Presumably your uint8 colors were normalized, and the Draco encoder would not have known that, I'm not sure what implications that might have here.

If you can identify a solution, PRs on DRACOLoader would be welcome too!

@major-mayer
Copy link

major-mayer commented Dec 16, 2023

Thanks for your fast response.
Yeah, it makes sense for the Draco Loader to return Float32 data, but I still think that it parses the original uInt8 values incorrectly.
I've checked the colours in the C++ debugger of my application (before I set them using the Draco API draco::TriangleSoupMeshBuilder::SetAttributeValuesForFace) and they are always values between 0 and 255.
So I assume that they are serialized the same way, even though I don't know of another decoder to check it.
This means that my colours are not normalized, correct?

As a workaround, I converted all the colours to float values and then set the data type of the "color" attribute to draco::DataType::DT_FLOAT32.
Indeed, it showed all the colours correctly after loading them with the Three.js DracoLoader.

The only issue that remains is that for some reason the red and blue values are interchanged, but that's probably a different issue (on my side...?).
In this image you can see the mesh in Three.js on the left and how it should look like on the right.
grafik

From this I assume that in general the colours are serialized correctly.

@donmccurdy
Copy link
Collaborator

I'm not sure how the Draco encoder/decoder works on a low level, but I would expect that any uint8 RGBA values must be normalized. If they're not normalized, that might explain why our retrieving the value as float32 would return much larger values 2000+, as Draco wouldn't know how to decode the values as float32.

@major-mayer
Copy link

major-mayer commented Dec 19, 2023

Sorry for my noob question, but when you are talking about normalized uint8 values, you mean that they must be in the range of 0–255? Because that's definitely the case with my data.
I found this document from the OpenGL wiki, which led me to my conclusion: https://www.khronos.org/opengl/wiki/Normalized_Integer
But I'm unsure if that's also true in the context of Draco's mesh compression.

Or the other way around, how would a denormalized integer (uint8) look like?

@donmccurdy
Copy link
Collaborator

That's correct, but also — I think that Draco's encoder needs to be told that the data is normalized, so it knows they semantically represent 0–1 floats. In the WASM version of the Draco encoder it's not possible to do this correctly, this is the open issue in google/draco#1023, but if you're using the C++ encoder directly you might be able to just set the normalized flag when creating the Draco uint8 attribute, and then the Draco decoder may give the right results when three.js asks it to decode into a float32 array.

@major-mayer
Copy link

Ahh, makes perfectly sense, thanks for the explanation.
I will try to find the required API (the Draco C++ documentation is really the worst/ pretty much not existent) and try this out the next days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants