VRMLLoader: Support normals, creaseAngle, and index

[Glinkis]

When importing this vrml file: test.wrl.zip, I expect to get this:

But instead I get this:

When inspecting the VRML file, I can see that there is an attribute called creaseAngle, which I suspect is the normal definition angle.

However, in the source code for the VRMLLoader, I can find no such attribute check.

[WestLangley]

Your file specifies no normals, correct? Therefore the loader can't import them.

However, the loader does compute them. The loader generates non-indexed BufferGeometry, a collection of independent triangles, and the computed normals are 'flat' ones orthogonal to each face.

You are correct that the loader does not honor creaseAngle when computing normals.

[Glinkis]

I just want to say I am currently working on this. I'll issue a pull request as soon as the following is done:

• Import normals. - Done
• Import instanced geometry?
• Import vertex colors. - Done
• Use creaseAngle to build normals if none exists.
• Create an indexed geometry. - Done.

[Glinkis]

I intend to also add the other things on my list, so we could keep this open for now.

[WestLangley]

Questions:

1. It is correct to say that this loader should be expected to create normals if normals are not specified in the file?

2. I never liked calling computeVertexNormals() in loaders. IMO, the loaders should just load the data in the file, not create data. It is OK if this loader is an exception, if that is what users expect when creaseAngle is specified.

/ping @donmccurdy /ping @Glinkis

[donmccurdy]

I don't know much about the VRML spec, but for what it's worth the glTF specification says When normals are not specified, client implementations should calculate flat normals.. That's as simple as setting material.flatShading=true for us, but conceptually it doesn't feel so different from calling .computeVertexNormals(). Mainly it is a question of (a) whether VRML specifies any such behavior when normals are omitted, and if not, (b) whether other VRML renderers have consistent behavior we should imitate.

[Glinkis]

If normals exist, they should be imported. If they don't, it should fall back to creaseAngle. And lastly, if neither exist it can simply call computeVertexNormals().

I think this is a good solution?

[WestLangley]

If you insist on supporting crease angle, I would do this:

If normals exist, they should be imported. If they don't, it should fall back to creaseAngle, if specified And lastly, it should use the same code with creaseAngle = DefaultCreaseAngle, whatever that is.

This code you are proposing to write is going to be complicated. Consider doing something simple.

[Glinkis]

It seems that the VRMLLoader currently does not import instanced geometry. The VRML spec supports instances, and so does three. So I could probably enable this, if everyone is okay with it?

[WestLangley]

You can support instancing if you want, but instanced buffer geometry is not the way to do it.

Personally, I would wait until there is demand for it, but it is up to you.

[mrdoob]

Our current support for instanced geometry is very basic. You still need to build you own materials...

[Glinkis]

What about having the same geometry on multiple meshes, when the vrml defines an instance?

This would at least reduce memory usage drastically for vrml models that contains a lot of instances.

[mrdoob]

What about having the same geometry on multiple meshes, when the vrml defines an instance?

Yes, that works and it's definitely encouraged 👌

[Glinkis]

Me and my colleague have been trying to implement crease angle support for a while now, and we've almost got it working.

However it's not what I'd call the most elegant solution, and putting this logic in VRMLLoader seems kind of pointless.

Iit would be way better to just have a computeVertexNormals( creaseAngle ) method in BufferGeometry, like #14859.

The loader could then just call this method and be done with it. it would also enable other loaders to do angle creasing, without a bunch of repeated logic.