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

USDZExporter: Add support for vertex colors. #27943

Merged
merged 2 commits into from
Mar 20, 2024
Merged

Conversation

Bigguysahaj
Copy link
Contributor

@Bigguysahaj Bigguysahaj commented Mar 19, 2024

Description :
This pull request addresses the issue of the USDZ exporter not supporting the export of vertex colors from Three.js geometries.

Fixed #27942

Changes :

  • Modified the buildPrimvars function in the USDZ exporter to handle the color attribute from Three.js geometries.
  const colorAttribute = attributes.color;
  if (colorAttribute !== undefined) {
    const count = colorAttribute.count;
    string += `
      color3f[] primvars:displayColor = [${buildVector3Array(colorAttribute, count)}] (
        interpolation = "vertex"
      )`;
  }
  • Tested the changes to verify that vertex colors are accurately exported to the USDZ file and rendered correctly in USDZ-compatible viewers.

Before :
The USDZ exporter did not support exporting vertex colors from Three.js geometries, resulting in meshes with vertex colors appearing incorrectly or without the expected coloring when viewed in USDZ-compatible viewers.

After :
The USDZ exporter now correctly exports vertex colors from Three.js geometries, ensuring that meshes with vertex colors are rendered accurately in USDZ-compatible viewers.

Add handling for vertex colors
@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 19, 2024

I have imported the following PLY asset (which has vertex colors) into the editor:

https://github.com/mrdoob/three.js/blob/dev/examples/models/ply/ascii/dolphins_colored.ply

The following link leads to a test version of the editor with your change:

https://rawcdn.githack.com/Bigguysahaj/three.js/d3f0c6151d6bb7082f12664767ec39f4c1ff4809/editor/index.html

After the import, you have to add lights to the scene and mark the Vertex Colors checkbox in the material tab. It should look like so:

image

When exporting to USDZ I can see no vertex colors on macOS Preview.

image

Does macOS Preview not support UDSZ assets with vertex colors or is something wrong with the PR?

@Mugen87 Mugen87 added this to the r163 milestone Mar 19, 2024
@Bigguysahaj
Copy link
Contributor Author

Hi Mugen,

Thanks for providing more perspective on the color display issue. Unfortunately, I'm on a Windows system and can't directly test how the USDZ file renders in macOS Preview.
Although for my testing, I resorted to online USDZ viewers.

https://www.usdz-viewer.net/
https://convert3d.org/convert

both were able to show colors accurately on the Dolphin USDZ mesh.
image

Please try it out for yourself, and I'll try getting it tested on apple machines and update you on it.

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 20, 2024

Okay let's give this a try.

I've hesitated first since I've thought uint8 as a data type is maybe required but it seems using color3f is indeed the way to go.

@Mugen87 Mugen87 merged commit 10d1622 into mrdoob:dev Mar 20, 2024
11 checks passed
@GitHubDragonFly
Copy link
Contributor

Whomever might get about updating USDZ Loader to reflect these new changes, here is what worked with my test of the current dev branch:

	// insert at current Line 315
	// Move displayColor to Mesh

	if ( 'color3f[] primvars:displayColor' in data ) {

		object[ 'color3f[] primvars:displayColor' ] = data[ 'color3f[] primvars:displayColor' ];

	}

	// insert at current Line 407
	if ( 'color3f[] primvars:displayColor' in data ) {

		const color = JSON.parse( data[ 'color3f[] primvars:displayColor' ].replace( /[()]*/g, '' ) );
		const attribute = new BufferAttribute( new Float32Array( color ), 3 );

		geometry.setAttribute( 'color', attribute );

	}
@GitHubDragonFly
Copy link
Contributor

One might also consider adding the following line to the loader:

	// insert at current line 768
	if ( geometry && geometry.hasAttribute( 'color' ) ) material.vertexColors = true;
@hybridherbst
Copy link
Contributor

Hey, my 2ct: while this PR adds vertex colors to the exported mesh, that vertex color won’t by default be used for rendering, only when explicitly looking at the geom data (like being in Vertex Paint mode in Blender).

It would be displayed by default if

  • the geom data is bound in the material (works in usdview and VisionOS, displays as white on iOS)
  • or the object does not have a material (works in usdview and VisionOS, but displays as error in iOS)

So just for managing expectations, this PR does not add support for vertex colors to USDZ files displayed on iOS and visionOS with QuickLook, e.g. when used with model-viewer.

@Bigguysahaj
Copy link
Contributor Author

Hey Hybrid, Thank you for raising this point!
So how should one go about fixing/avoiding this behavior on IOS and QuickLook? or is it something not in our control.

So just for managing expectations, this PR does not add support for vertex colors to USDZ files displayed on iOS and visionOS with QuickLook, e.g. when used with model-viewer.

To address this point, should I change the pull request title to "Updated USDZExporter.js : Add partial support for exporting vertex colors to USDZ format" . Would that be appropriate?

@GitHubDragonFly
Copy link
Contributor

@Bigguysahaj it doesn't look like you would need to change anything about this PR since, in my opinion, it is a valuable contribution to the three.js library and can be handled by the library itself.

If there is a lack of support for vertex colors in the QuickLook then it would be up to their developers to eventually and hopefully include it.

It doesn't look like there are any possible changes that could be done on three.js side to trick the QuickLook into showing these models properly.

@hybridherbst
Copy link
Contributor

It can be changed to work on visionOS (by not exporting a material). We do that in Needle Engine’s USDZExporter - when there’s a white material without maps and with vertex colors, we don’t export the material.

If there is a lack of support for vertex colors in the QuickLook then it would be up to their developers to eventually and hopefully include it.

In my opinion it’s not as easy as this. A big production use case of USDZExporter is usage in model-viewer for iOS QuickLook. I’d hope that a PR like this is tested to not break that main use case. (It shouldn’t; just saying that breaking something and then saying Apple should fix it wouldn‘t help actual users)

@GitHubDragonFly
Copy link
Contributor

That all sounds like a rather delicate balancing over what should be included at any time.

@Mugen87 Mugen87 changed the title Updated USDZExporter.js : Add support for exporting vertex colors to USDZ format Mar 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants