VC demo flying boxes

[donmccurdy]

Moving this issue over from https://github.com/KhronosGroup/glTF/issues/576 —

We're not sure if the issue is in the underlying COLLADA model or COLLADA2GLTF, but the VC sample model has boxes attached to many nodes:

Issue appears in both three.js and BabylonJS.

[tim-rex]

I’ve encountered this same issue with my own custom renderer, parsing via cgltf.

[cx20]

If I remember correctly, this is a problem with the source COLLADA model of the conversion. I have tried displaying VC.gltf in several glTF Loaders below. However, they all show a white box. https://github.com/cx20/gltf-test#more-complex-models

(If possible, it is hoped that the white box will be modified to be transparent for the official glTF sample, as it is a source of confusion.)

[donmccurdy]

We should probably remove this model from the sample repository until the issue can be fixed, I think. Filed https://github.com/KhronosGroup/glTF-Sample-Assets/issues/71.

[cx20]

I don't want to remove it too much because I like this model, which has a lot of cool cameras inside it.

@javagl Do you have any ideas?

[tim-rex]

I do find this to be a good demonstration model, flying boxes notwithstanding. Perhaps it’s sufficient to add a description to a README of sorts for this model, until such time that the Collada to GLTF conversion issue can be addressed.

On Wed, 21 Oct 2020 at 11:26, cx20 notifications@github.com wrote:

I don't want to remove it too much because I like this model, which has a lot of cool cameras inside it.

@javagl https://github.com/javagl Do you have any ideas?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/KhronosGroup/COLLADA2GLTF/issues/146#issuecomment-713470555, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEV2YYCDTFLIEMM2XWRQKJ3SL2ZNRANCNFSM4ERRT5VA .

[javagl]

This is an ancient issue. Some context: https://github.com/KhronosGroup/glTF/issues/576 and https://github.com/KhronosGroup/glTF-Sample-Models/issues/8

I also like the model for its complexity with geometry, textures, animations, and ... last but not least, because it is one of the few (if not the only) model that has multiple cameras in it. It would be a pity to remove it.

Maybe we can figure out the reason for the boxes. (I'd start with re-generating it from the source if that hasn't been done recently). I wrote up a few points in the second issue linked above, but would definitely need a refresher and a chunk of time to look closer into this.

(Adding a comment in the README like "Yeah, there may be some boxes, we're working on that" could be OK, just to make clear that it has this small issue and it might not be the fault of the rendering engine - but I'm not sure about the latter, it has been too long...)

[cx20]

The idea of adding comments to the README as an interim response sounds like a good one.

[cx20]

I found white [1.0, 1.0, 1.0, 1.0] in the materials of the glTF model. https://github.com/KhronosGroup/glTF-Sample-Models/blob/master/2.0/VC/glTF/VC.gltf#L20702-L20707

    "pbrMetallicRoughness": {
        "baseColorFactor": [
            1.0,
            1.0,
            1.0,
            1.0
        ],
        "metallicFactor": 0.0
    },

I confirmed that the red box is displayed by changing the color of index 27 of materials to red [1.0, 0.0, 0.0, 1.0].

However, I did not know how to make it transparent. I will have to look into it.

(Back to work now that my lunch break is over.)

[donmccurdy]

I found white [1.0, 1.0, 1.0, 1.0] in the materials of the glTF model.

Great hint! 😁 I wrote a script to hunt these meshes down, it looks like any material without a texture is "bad". Here's a fixed version of the model...

VC-fixed.glb.zip

import { Mesh, NodeIO, Primitive, Root } from '@gltf-transform/core';

const io = new NodeIO();
const doc = io.read('../glTF-Sample-Models/2.0/VC/glTF-Binary/VC.glb');
const root = doc.getRoot();

root.listMaterials()
    .filter((mat) => !mat.getBaseColorTexture())
    .forEach((mat) => {

        for (const prim of mat.listParents() as Primitive[]) {
            if (prim instanceof Root) continue;

            prim.listParents()
                .filter((mesh: Mesh) => mesh.listPrimitives().length === 1)
                .forEach((mesh) => mesh.dispose());
            prim.dispose();
        }

        mat.dispose();
    });

root.listAccessors()
    .filter((accessor) => accessor.listParents().length === 1)
    .forEach((accessor) => accessor.dispose());

io.write('/Users/donmccurdy/Desktop/VC-fixed.glb', doc);

It does leave some empty nodes behind, but they're not rendered at least. We could update the sample directly this way, I guess? But ideally we want to be able to easily reproduce it from the source (COLLADA) model...

[cx20]

@donmccurdy Thanks! I have confirmed that the white box has been removed in your modified model. I think this model looks good. Your tools are great too.

BTW, It's not a big deal, but the camera numbers identified by the Viewer seem to be different from the ones I tested before. When I tried VC.glb in the Khronos glTF-Sample-Models repository, the helicopter's camera number was number 1, but the modified model seems to have the camera number 13.

original VC.glb VC-fixed.glb

[emackey]

For what it's worth, I believe that high-quality glTF samples are worth far more than maintaining a collection of raw outputs from this COLLADA2GLTF tool. I'm fine with a Blender project or whatever replacing COLLADA as the source material for this model.

The glTF sample repo should show the best-case glTFs, and if that case doesn't come out of this converter raw, so be it.

[emackey]

(We should still fix the bugs in this converter of course, but the samples for testing that don't need to be in the spotlight over in the sample repo if the output isn't ideal).

[cx20]

I rechecked @donmccurdy' modified VC-fixed.glb model and noticed that in addition to the white box, there are parts of it that have been removed. Now that the fighter's gray windshield has been removed, we probably need to specify white as a condition for its removal.

before	after

[donmccurdy]

Hm, is the jet supposed to have an opaque grey windshield hiding that detailed interior? 😅 I guess ideally the windshields would use alpha blend? Or even transmission, but that seems a bit too high-fidelity for this scene. The model could use some help from a technical artist, perhaps. One more issue raised in https://github.com/KhronosGroup/glTF/issues/1982, the model has an occlusion texture that it probably shouldn't.

[donmccurdy]

^Continuing discussion in https://github.com/KhronosGroup/glTF-Sample-Assets/issues/71.