xeokit / xeokit-sdk

Open source JavaScript SDK for viewing high-detail, full-precision 3D BIM and AEC models in the Web browser.
https://xeokit.io
Other
715 stars 286 forks source link

Missing objects in large .XKT models #315

Closed lasselaakkonen closed 4 years ago

lasselaakkonen commented 4 years ago

When models have lots of indices, not all indices are rendered.

I have 0 experience with the xeokit codebase, so take everything below with a grain of salt.

Reproducing

Investigation

PerformanceModel.constructor() creates a new BatchingLayer when !this._currentBatchingLayer.canCreatePortion(positions.length).

canCreatePortion() checks for space based on (!this._finalized && this._buffer.lenPositions + lenPositions) < (this._buffer.maxVerts * 3)

That does not seem to be checking for the correct things. lenPositions is not guaranteed to equal lenIndices*3 (= maxVerts*3), there can be fewer positions than that (or maybe there always are?). The number of indices seems to be limiting factor when filling out the BatchingBuffers?

So a new BatchingLayer is not created when the current one is already full.

This does not cause any errors in code, at least because in BatchingLayer when indices are added to the indices buffer in buffer.indices[buffer.lenIndices + i] = indices[i] + vertsIndex;, the Uint32Array seems to do simply a no-op when assigning values to out of range array indices (at least on Chrome).

So the layer is eventually rendered, but model indices after 5 000 000 are ignored.

The 5 000 000 limit comes from BatchingBuffer which defines MAX_VERTS like this:

const bigIndicesSupported = WEBGL_INFO.SUPPORTED_EXTENSIONS["OES_element_index_uint"];
const SLICING = true;
const MAX_VERTS = SLICING ? (bigIndicesSupported ? 5000000 : 65530) : 5000000;

Possible solution

This is where it gets very dicey. I don't know of all the consequences there are from this. But works for me with at least 2 different models.

In BatchingLayer, changing this:

    canCreatePortion(lenPositions) {
        if (this._finalized) {
            throw "Already finalized";
        }
        return (!this._finalized && this._buffer.lenPositions + lenPositions) < (this._buffer.maxVerts * 3);
    }

To for example this:

    canCreatePortion(lenIndices) {
        if (this._finalized) {
            throw "Already finalized";
        }
        return (!this._finalized && (this._buffer.lenIndices + lenIndices < this._buffer.maxVerts));
    }

And in PerformanceModel, in the only usage of canCreatePortion(), change canCreatePortion(positions.length) to canCreatePortion(indices.length).

When using a model with 6 500 000 indices, this ends up creating two layers, which are both rendered, instead of a single layer with 5 000 000 indices.

xeolabs commented 4 years ago

@lasselaakkonen your solution makes sense - since maxVerts is indeed the number of vertices, not the length of storage for vertices.

lasselaakkonen commented 4 years ago

Please correct me if I am wrong, but I don't believe 6c0ae8d would fix the issue entirely.

Have I understood correctly that that in theory there can be an unlimited number of indices for even a small number of positions? So the limiting factor for buffers is the number of indices and indices must be used for checking if the buffer is full.

In 5f02ce3 I switched from using lenPositions for checking if the buffer is full to checking lenIndices.

6c0ae8d happens to fix the original issue for me with at least one model, but it also creates an unnecessary extra BatchingLayer, I guess because of the ratio of positions to indices there happens to be in the model.

Amoki commented 4 years ago

We have a similar issue with http://openifcmodel.cs.auckland.ac.nz/_models/2019030617043%20-%20APHS%205-3-19.ifc. A lot of objects are missing.

It seems to be fixed for this model with the latest commit

xeolabs commented 4 years ago

I just committed a fix in https://github.com/xeokit/xeokit-sdk/commit/1b424a8ababff35fe5202993eea33a3cc3667847

@Amoki do all the missing objects appear in this test model? https://xeokit.github.io/xeokit-sdk/examples/#loading_XKT_APHS

Amoki commented 4 years ago

They all seem to be there! Capture d’écran de 2020-06-02 16-05-15 Capture d’écran de 2020-06-02 16-13-59