PIPELINE: GLB files have inverted faces after simplification

nielsboecker commented 5 years ago

Description

When in the pipelines a 3D model goes through simplify, the obj -> obj conversion seems to go fine and the result looks normal:

However, when this OBJ is then transformed to a GLB file by obj2gltf, the output looks weird. It seems like the textures outside are transparent, and only on the inside of surfaces, are visible:

Reproduction with other OBJ file

I reproduced the steps with an OBJ file from the internet (teddy.obj) and everything looks fine. Hence, the OBJ files our system produces are the issue.

Analysis

The problem seems to be that the faces are flipped/inverted. In other words, the norms of these faces point in the exactly wrong direction. Apparently, similar issues can be caused by coordinate system mismatches. glTF uses a right-handed coordinate system. When you follow this link and similar Google hits, you will also find that they talk about invertY function for 3D loaders as a possible fix.

So, given that the OBJ teddy from the internet works fine, I don't think simplifier actually breaks the file. I think the output from the pipeline may already be broken. I find it very suspicious that the guys in the link above talk about invertY, just now that you guys flip the Y axis for one of the pipelines (#96).

Are you 100% sure the problem was there even before you made that flip change?
Are you completely sure that it really happens for all pipelines, as @ansonwong9695 said?

Furthermore, all files go through the flip_numpy_array_dimensions method, which transposes the input and flips one axis. I always found this kind of weird, as you say that it is mirrored, but that would mean that all DICOM files are wrong, or that the default loading method by pydicom is broken. Are you really sure we need this??

def flip_numpy_array_dimensions(array: np.ndarray) -> np.ndarray:
    """
    Transposes numpy axes, i.e. (z, y, x) ---> (x, y, z), and flips x axis. This is
    needed as the default data when we read it is "mirrored" and therefore not accurate
    and, e.g., not valid as input for pre-trained NN models.
    :param array: ndarray from pydicom.read_file()
    :return: array with flipped axis to represent the accurate DICOM data
    """
    array = array.transpose((2, 1, 0))
    array = np.flip(array, 0)
    return array

I think you need to rethink the whole numpy flipping you guys are currently doing. Or at least go back and verify it properly. This really sounds like it could be the issue that eventually leads to a wrong coordinate system in the GLB, right?

Quick fix 1

When you change in the GLTF file, doubleSided: false to true. (see GLTF schema definition) What this will do is it will just show the one texture to both sides. It looks normal with this fix.

also verified that this doesn't break anything if you run it on a model that is actually not inversed (teddy bear)
but: this would require further steps. Turn to GLTF first. Then manipulate file. Then transform to GLB. Could be done, e.g. with gltf-pipeline, but this is not great.

Quick fix 2

Disable simplifier 😞

Data for reproduction

data_for_reproduction.zip

nielsboecker commented 5 years ago

@UdomkarnBoonyaprasert, @ansonwong9695: How are you planning to resolve this issue in the few days we have left?

nielsboecker commented 5 years ago

Simplify was disabled in all pipelines in #101

nielsboecker / HoloRepository-Core