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 misses UV map with Points #25697

Closed
Beilinson opened this issue Mar 21, 2023 · 18 comments · Fixed by #25707
Closed

GLTFLoader misses UV map with Points #25697

Beilinson opened this issue Mar 21, 2023 · 18 comments · Fixed by #25707

Comments

@Beilinson
Copy link
Contributor

Description

I have a simple GLTF scene with a Points mesh, that uses UV map for coloring of individual points. In both SketchFab and the microsoft 3D Viewer the uv map works correctly:

image

Meanwhile using the GLTFLoader the uv map is completely ignored:

image

I have checked and the loaded PointsMaterial contains the required map with the proper type and the actual geometry buffer contains the proper uvs.

Reproduction steps

  1. Download model from: https://sketchfab.com/3d-models/enzo-star-field-8449bc7dcf964e0aa6e1ba656ea6bf4c#download as converted gltf or glb
  2. Drag model into https://gltf-viewer.donmccurdy.com/
  3. Points are all white

Code

Just using the basic GLTFLoader

Live example

None

Screenshots

No response

Version

r150

Device

Desktop, Mobile

Browser

Chrome, Firefox, Edge

OS

Windows, MacOS

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 21, 2023

This is not necessarily an issue of the loader. The engine does in general not support texture coordinates with point clouds.

@Beilinson
Copy link
Contributor Author

Is there a possibility to support it in the future, and not through custom shaders?

@WestLangley
Copy link
Collaborator

@Beilinson

  1. You need a solution now, no?

  2. The map for PointsMaterial is currently used to define the shape and color of each identical point. Do you intend to use the map with attribute uvs, instead, or do you need two maps -- one for the shape and one for the color?

  3. Can you use vertex colors, instead?

@donmccurdy
Copy link
Collaborator

Quick conversion to vertex colors:

  1. Open the model in https://gltf.report
  2. Run the script below
import { ColorUtils } from '@gltf-transform/core';
import { KHRMaterialsUnlit } from '@gltf-transform/extensions';
import { getPixels } from 'ndarray-pixels';

for (const mesh of document.getRoot().listMeshes()) {
	for (const prim of mesh.listPrimitives()) {
		const material = prim.getMaterial();
		const texture = material.getBaseColorTexture();
		const texCoord = material.getBaseColorTextureInfo().getTexCoord();
		const uv = prim.getAttribute(`TEXCOORD_${texCoord}`)
		const color = document.createAccessor()
			.setArray(new Float32Array(uv.getCount() * 3))
			.setBuffer(uv.getBuffer())
			.setType('VEC3');

		const pixels = await getPixels(texture.getImage(), texture.getMimeType());
		const _uv = [], _color = [];

		// Read color from texture, write to COLOR_0 vertex attribute.
		for (let i = 0, il = uv.getCount(); i < il; i++) {
			uv.getElement(i, _uv);
			const x = Math.floor(_uv[0] * pixels.shape[0]);
			const y = Math.floor((_uv[1]) * pixels.shape[1]);
			
			_color[0] = pixels.get(x, y, 0) / 255;
			_color[1] = pixels.get(x, y, 1) / 255;
			_color[2] = pixels.get(x, y, 2) / 255;

			ColorUtils.convertSRGBToLinear(_color, _color);

			color.setElement(i, _color);
		}

		prim.setAttribute('COLOR_0', color);
		uv.dispose();
	}
}

// Clean up unused textures.
for (const texture of document.getRoot().listTextures()) {
	texture.dispose();
}

// Switch the material to unlit.
const unlitExtension = document.createExtension(KHRMaterialsUnlit);
for (const material of document.getRoot().listMaterials()) {
	material.setExtension('KHR_materials_unlit', unlitExtension.createUnlit());
}
  1. Export GLB

enzo_star_field_vert-v4.glb.zip

Screenshot 2023-03-21 at 8 50 51 PM

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 22, 2023

I wonder if we could update the shader of PointsMaterial such that if an uv attribute is present, it is used to sample the color value from map (and alphaMap) instead of computing the uvs inline based on gl_PointCoord.

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 22, 2023

I've hacked something in for testing. It actually does not require much changes in the points shader, see Mugen87@a138894.

@mrdoob If you like I can file a PR with the feature.

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 22, 2023

BTW: In WebGPU, this should be the default. Points always have a 1px size so using map to define the shape of the point does not work anymore.

So I vote to support this change in WebGLRenderer to provide an equal outcome with WebGPU.

@Beilinson
Copy link
Contributor Author

I can use vertex colors, we decided on using UV's because its Vec2 instead of Vec3 and we work with millions of points so the file size can quickly get out of hand.
I use only attribute uvs for color

Another benefit is quick switching of texture to instantly update colors, rather than looping over all colors individually.

I would love for pointsmaterial to support map.

@mrdoob
Copy link
Owner

mrdoob commented Mar 24, 2023

@Mugen87

BTW: In WebGPU, this should be the default. Points always have a 1px size so using map to define the shape of the point does not work anymore.

So I vote to support this change in WebGLRenderer to provide an equal outcome with WebGPU.

That sounds good to me.

I guess we should then remove size, sizeAttenuation and alphaMap from PointMaterial?

We could then create a InstancedSprite for people that relied on the previous behavior.

InstancedSprite would then finally solve the issues of gl_PointSize being capped in some hardware and not being consistent when changing the renderer's pixel ratio.

@mrdoob
Copy link
Owner

mrdoob commented Mar 25, 2023

@Mugen87 Would you be up for implementing InstancedSprite?

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 25, 2023

Yes, I can do that 👍 . Regarding #25078, do you still think it's okay to start with InstancedSprite? I would derive InstancedSprite from Sprite which means there will be some redundant checks inside the renderer.

@Beilinson
Copy link
Contributor Author

Do you think it makes sense that LineBasicMaterial would also support UV map? That way all three main geometry types could handle both vertex colors and uv maps

@mrdoob
Copy link
Owner

mrdoob commented Mar 27, 2023

It does makes sense to add this to LineBasicMaterial too. GLTF supports that too I guess?

@mrdoob
Copy link
Owner

mrdoob commented Mar 27, 2023

@Mugen87

Yes, I can do that 👍 . Regarding #25078, do you still think it's okay to start with InstancedSprite? I would derive InstancedSprite from Sprite which means there will be some redundant checks inside the renderer.

Not sure I follow...

You mean you rather try to implement Instances( Mesh|SkinnedMesh|Line|Points, count ) instead of InstancedSprite? If so, that sounds good to me! 👍

@Beilinson
Copy link
Contributor Author

Yep, lines with UV mapping works in other viewers

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 27, 2023

GLTF supports that too I guess?

Yes, related issue: #17511

You mean you rather try to implement Instances( Mesh|SkinnedMesh|Line|Points, count ) instead of InstancedSprite? If so, that sounds good to me! 👍

I would start with InstancedSprite since it should be more straightforward to implement. It would also give us the chance to see where redundant code occurs so the implementation of a more generic Instances class becomes hopefully easier. Just wanted to make sure this is okay regarding #25078.

@Mugen87
Copy link
Collaborator

Mugen87 commented Mar 29, 2023

@mrdoob I have started to implement InstancedSprite and encountered a problem.

I wanted to port webgl_points_sprites to InstancedSprite but failed since the entire point cloud is rotated in the animation loop. Sprites however do not honor any changes applied to their rotation or quaternion property. There is SpriteMaterial.rotation for that:

https://jsfiddle.net/vktsy0qr/1/

This is actually an issue since point clouds are frequently transformed via rotation. And since InstancedSprite is derived from Sprite, I wonder how the issue can be solved.

  • Should InstancedSprite not support Object3D.rotation/Object3D.quaternion like Sprite?
  • If InstancedSprite should support rotation, it will still not be possible to rotate individual sprites via instanceMatrix. I guess we need a separate API setRotationAt() and getRotationAt() like with colors (probably with a separate instanced attribute).

@mrdoob
Copy link
Owner

mrdoob commented Mar 29, 2023

Interesting...

Probably better to not derive from Sprite then.

The idea of InstancedSprite was just to be able have something like Points with size support.

Maybe it would be better to try to implement Points using instancing internally instead?

Or maybe a new class with a different name?

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.

5 participants