Points Support

[camnewnham]

I'm interested in doing some work to extend support of POINTS geometry and would like to scope it and determine whether it's a welcome extension to this library beforehand.

The current behaviour is:

Gltf

• GLTFast.GLTFast:PreparePrimitiveIndices logs an error (Primitive mode Points is untested!)
• Points are visible (in editor) but vertex colors do not appear

With Draco

• Fails to load with "Failed: Decoding error."

Technically it's not in the spec (see spec and discussion KhronosGroup/glTF#1809), but the default three.js GltfLoader/DracoLoader will load a point cloud compressed with Draco without issue and the file size reduction is of course significant.

From what I can tell, the main changes would appear to be:

1. Modifying draco_unity_plugin.cc to allow points. Currently:

if (geom_type != draco::TRIANGULAR_MESH) {
    return -2;
  }

2. Investigation of vertex colors & shaders, which could be in response to either:
   a. Creation/addition of a point rendering shader including vertex colors, used when topology == points
   b. VertexColorUnlit is showing jumbled colors on the points, so this may need more investigation.

For clarity of 2b, here's in Unity with glTFast and the VertexColors material:

And here's the same with three.js:

@atteneder at a cursory glance are there any other parts that would need modification, or do you have any other tips?

[atteneder]

Hi @camnewnham ,

Thanks for the great suggestion and putting in the effort.

You're right about the Draco limitation. Currently I refactor the Draco integration (re-base on recent version plus use the advanced Mesh API). It would make sense to wait for this new version or start working on the according branch( draco and DracoUnity).

I'm not experienced in point cloud rendering in Unity. I'm pretty sure we can make sure that the vertex colors are imported correctly. It is likely the material/shader that's not using the vertex colors correctly. This assumption would have to be put to test. Otherwise I see no obvious blockers for this feature.

Regards,
Andi

[camnewnham]

Cool. I'll do some testing with the older version and just to isolate any issues and determine what needs to be done and then move it over to the new one.

I'll post some updates here as I work out what's going on. Starting with a simple colored point cloud:

three.js:

Left is glTFast pts, using VertexColors shader
Right is an exported/importer .ply using the same shader. Subsequently I suspect that the issue is IO related rather than shader related.

The same test using colored meshes:

three.js:

Left is glTFast, right is .ply using VertexColors shader

Given the close-up I suspect the color order is getting shuffled somewhere?

[camnewnham]

It looks like the color ordering is a result of the indexing in the jobs. i.e. GetColorsVec4UInt8Job:

Rather than

        public void Execute(int i) {
            byte* src = input + (i * inputByteStride);
            result[i] = new Color {
                r = src[i] / (float) byte.MaxValue,
                g = src[i+1] / (float) byte.MaxValue,
                b = src[i+2] / (float) byte.MaxValue,
                a = src[i+3] / (float) byte.MaxValue
            };
        }

we need:

        public void Execute(int i)
        {
            byte* src = input + (i * inputByteStride);
            result[i] = new Color
            {
                r = src[0] / (float)byte.MaxValue,
                g = src[1] / (float)byte.MaxValue,
                b = src[2] / (float)byte.MaxValue,
                a = src[3] / (float)byte.MaxValue
            };
        }

This likely also resolves #138

I'll make a PR accordingly before moving onto the draco component.

[atteneder]

Awesome finding!

[camnewnham]

Here's a first go at allow draco to work with the point clouds.

Draco changes: atteneder/draco@unity...camnewnham:unity_point_clouds

• Add DRACO_POINT_CLOUD_COMPRESSION_SUPPORTED flag for BUILD_FOR_GLTF in CMakeLists
• Decode either TRIANGULAR_MESH or POINT_CLOUD based on the EncodedGeometryType

(note I am a C++ amateur so any advice is most welcome - there's a bunch of code duplication here for mesh vs pt processing)

DracoUnity changes: atteneder/DracoUnity@main...camnewnham:unity_point_clouds

• Check if any triangles are in the DracoToUnityMesh, and use this as a flag to either set indices (points) or triangles (mesh)

Originally I had thought to pass a value based on the topology declared in the glTF - but this logic doesn't work when using the draco file importer (and could lead to conflicts for poorly formatted files).

glTFast changes: main...camnewnham:unity_point_clouds

• RecalculateNormals doesn't work with MeshTopology.Points

Last remaining is to include and map an appropriate vertex color material/shader when topology is points - I think it's a reasonable assumption that point clouds should prioritize vertex colors over GLTF assigned material and select a vertex color shader instead (i.e. they might have a PBR assigned due to their creation process).

[atteneder]

Thanks, much appreciated!
I'll try to have a look early this week.

[camnewnham]

Addition of default points shader IMaterialGenerator.GetDefaultPointsMaterial (gltf/Unlit) has been added to the glTFast fork too.

[atteneder]

Sorry for the delay. I had a look across it (didn't test so far). glTFast and DracoUnity look good overall.

Some thoughts about the native part:

• It is based on the current version (my unity branch). I couldn't finish the more up to date work due to an open blocking issue, but that has to be ported soonish
• I'd like to have a look at the impact on the build size first. I don't expect it to be big/a blocker, but none the less
• Code seems a bit redundant. Can we leverage synergies between DecodeMesh and DecodePointCloud a bit better?

Thank you very much!

[camnewnham]

Thanks for having a look. I'll move the changes up to the latest once there's a stable glTFast/DracoUnity/draco combination (or is this the case now?) so that I can tell if breaking things are my issue or pre-existing.

I ran the CI here: https://github.com/camnewnham/draco/actions/runs/587740664 so comparing to the prior builds: https://github.com/atteneder/draco/actions/runs/525167332 we get:

Existing

Including point clouds

So that seems like a pretty negligible change in size for the feature?

Agree on the redundant code - my C++ is a bit lacking. I've updated again (atteneder/draco@unity...camnewnham:unity_point_clouds) using templates which seems to work well and removes the majority of the duplication.

[atteneder]

Now that points support is tested (both regular and Draco compressed) I am closing this issue.

For improved/dedicated point cloud materials I created issue #246

Thanks!