gkjohnson/three-gpu-pathtracer

SheenWoodLeatherSofa renders incorrectly

mrdoob opened this issue · 10 comments

FYI)it looks fine in my mac

image

i try in three.js editor with royal_esplanade_1k.hdr background, environment

@mrdoob - I'm not seeing the issue, either. Can you reproduce it? What kind of lighting setup were you using?

Here's what it looks like with some directional lights:

image

I check in index(default example), it occasionally reproduces

add this into window.MODEL_LIST

'SheenWoodLeatherSofa': {
    url:'https://raw.githubusercontent.com/KhronosGroup/glTF-Sample-Assets/SheenWoodLeatherSofa/Models/SheenWoodLeatherSofa/glTF-Binary/SheenWoodLeatherSofa.glb',
    credit: 'glTF Sample Model.',
},

Here's the corrected file url that worked for me:

https://raw.githubusercontent.com/KhronosGroup/glTF-Sample-Assets/e51c108880863c3d1ee161341a0d834a9152278c/Models/SheenWoodLeatherSofa/glTF-Binary/SheenWoodLeatherSofa.glb

I check in index(default example), it occasionally reproduces

Interesting - in addition to textures sometimes not working I'm seeing that an error is occasionally hit when building the merged geometry, which is odd. I wonder if there's some kind of loading race condition here? This will need a bit more digging.

ah me too float32array fail when mergeGeometry called Float32array fail because of byteOffset and length over the lenth of arraybuffer
i'll dig it too

FYI) currently i found something. mergeGeometries.js

	// copy all the attribute data over
	const attributes = Object.keys( geometries[ 0 ].attributes );
	console.log('total Keys', attributes);
	for ( let i = 0, l = attributes.length; i < l; i ++ ) {

		let forceUpdateAttr = false;
		const key = attributes[ i ];
		if ( ! targetGeometry.getAttribute( key ) ) {

			console.log('has not key', key)
			const firstAttr = geometries[ 0 ].getAttribute( key );
			const attribute = createAttributeClone( firstAttr, totalAttributeCount );
			console.log('targetGeometry key', key, firstAttr, attribute, totalAttributeCount, geometries[0], geometries);
			targetGeometry.setAttribute( key, attribute );
			forceUpdateAttr = true;

		}

see this part

      const firstAttr = geometries[ 0 ].getAttribute( key );
     const attribute = createAttributeClone( firstAttr, totalAttributeCount );
image

index 0 BakeGeometry color attribute has item size 3 but index 1 bake geometry color attribute has item size 4 so it causes float32 array constructor range error

i'm going to find why BakeGeometry attribute itemSize is not same

Almost BufferGeometries don't have color attributes.
However, this one does have a color attribute with an itemSize 3, which causes a problem.
In the function called setCommonAttributes, if there is no colorAttribute, it defaults to white(1,1,1,1) with itemSize 4.
What do you think about solution for this case, the values that do not have itemSize 4 (maybe alpha) are filled with 1.
@gkjohnson

like this(workaround code)
image

@dongho-shin thank you very much for looking into this! I think that workaround looks good to me if you'd like to make a PR. Does this fix the texture rendering inconsistency, as well?

Very nice find!

@gkjohnson In my case, fix texture rendering inconsistency yes if other edge case happened like geometry issue. let's open issue then. at least initialization fail was fixed I'll open pr for this

Fixed in #657