mrdoob / three.js

JavaScript 3D Library.
https://threejs.org/
MIT License
102.58k stars 35.36k forks source link

GLTFLoader misses UV map with Points #25697

Closed Beilinson closed 1 year ago

Beilinson commented 1 year ago

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 commented 1 year ago

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

Beilinson commented 1 year ago

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

WestLangley commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

I've hacked something in for testing. It actually does not require much changes in the points shader, see https://github.com/Mugen87/three.js/commit/a138894f6e29c72ce722a8bf7d0e8b24d823a51f.

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

Mugen87 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@Mugen87 Would you be up for implementing InstancedSprite?

Mugen87 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

mrdoob commented 1 year ago

@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 commented 1 year ago

Yep, lines with UV mapping works in other viewers

Mugen87 commented 1 year ago

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 https://github.com/mrdoob/three.js/issues/25078.

Mugen87 commented 1 year ago

@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.

mrdoob commented 1 year ago

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?