Three.js: TypeError: Cannot read property 'array' of undefined

[waitingcheung]

Description of the problem

I am getting a TypeError when constructing an instance of Geometry from an instance of BufferGeometry.

TypeError: Cannot read property 'array' of undefined
    at Geometry.fromBufferGeometry (/path/three.js:12675:39)

Here is a simple test case to reproduce that

var bufferGeometry = new THREE.BufferGeometry();
bufferGeometry.addAttribute('color', new THREE.BufferAttribute(new Float32Array( [0, 0, 0, 0.5, 0.5, 0.5, 1, 1, 1] ), 3 ) );
bufferGeometry.addAttribute('normal', new THREE.BufferAttribute(new Float32Array( [0, 1, 0, 1, 0, 1, 1, 1, 0] ), 3 ) );
bufferGeometry.addAttribute('uv', new THREE.BufferAttribute(new Float32Array( [0, 0, 0, 1, 1, 1] ), 2 ) );
bufferGeometry.addAttribute('uv2', new THREE.BufferAttribute(new Float32Array( [0, 0, 0, 1, 1, 1] ), 2 ) );
var geometry = new THREE.Geometry().fromBufferGeometry(bufferGeometry);

It seems that the problem occurs when the attribute 'position' is missing. If we change line 231 of Geometry.js to the following, we can create an instance of Geometry successfully.

var positions = attributes.position !== undefined ? attributes.position.array : [];

Three.js version

• [x] Dev
• [ ] r82
• [ ] ...

Browser

• [x] All of them
• [ ] Chrome
• [ ] Firefox
• [ ] Internet Explorer

OS

• [x] All of them
• [ ] Windows
• [ ] Linux
• [ ] Android
• [ ] IOS

Hardware Requirements (graphics card, VR Device, ...)

[Mugen87]

The code expects at least a position attribute that contains the vertex data by now. This is in some way the minimum information for a valid geometry. Why don't you just add the following line in your code?

bufferGeometry.addAttribute( 'position', new THREE.Float32BufferAttribute( [], 3 ) );

[WestLangley]

@waitingcheung What is your use case for creating a BufferGeometry with no position attribute?

[waitingcheung]

The position attribute is computed later so it is not available at the time of creating a BufferGeometry. The suggestion from @Mugen87 could be a workaround for this. It would be great if you can support default positions when the position attribute is not available.

[mrdoob]

Why are you converting THREE.BufferGeometry to THREE.Geometry?

[Mugen87]

Closing the issue since there is no compelling reason for converting THREE.BufferGeometry to THREE.Geometry.

[WestLangley]

there is no compelling reason for converting THREE.BufferGeometry to THREE.Geometry.

If that is true, we should remove the method...

I am not sure it is true. There are issues such as this one:

https://stackoverflow.com/questions/43394385/three-smoothshading-has-no-effect-on-geometry/43395883#43395883

[Mugen87]

Wouldn't it be better to solve such issues when creating/exporting the model? I'm not sure if three.js should be the tool for "fixing" those geometries.

[WestLangley]

there is no compelling reason for converting THREE.BufferGeometry to THREE.Geometry.

Do you propose removing the method, then?

[Mugen87]

Well, we need the method to create the counterparts of BoxBufferGeometry or SphereBufferGeometry. But generally i thought we want to deprecate Geometry, right? I think it's counterproductive if we now construct use cases for Geometry.

[WestLangley]

Users may need Geometry for adjacency information, for example. Who knows...

I also think it would be wise to identify where users use Geometry so we are prepared to offer work-arounds when we deprecate it.

We will stop _rendering_Geometry at some point, I expect. But I would retain the conversion utilities.

And I would not close this PR for the reason you stated -- maybe some other reason, but not that one. JMHO :).

[Mugen87]

Okay, you've convinced me 😊

[mrdoob]

Oh my... Sorry.

[Mugen87]

I have revisited this issue. For clarification: Even if the following code is applied to Geometry.fromBufferGeometry()...

var positions = attributes.position !== undefined ? attributes.position.array : [];

...no attribute data are converted since a position attributes is required for Face3 creation (an index without no position attribute makes no sense).

So we could apply this fix in order to avoid the runtime error however the output would not be usable at all. How about adding this instead:

if ( attributes.position === undefined ) {

    console.error( 'THREE.Geometry.fromBufferGeometry(): Position attribute required for conversion.' );

}

[WestLangley]

I think that is reasonable.