visgl / luma.gl

High-performance Toolkit for WebGL-based Data Visualization
https://luma.gl
Other
2.3k stars 211 forks source link

luma.gl/gltf@9.0.24 breaks the Google 3DTiles rendering (ScenegraphLayer No value for binding u_BaseColorSampler) #2233

Closed vsigno closed 2 weeks ago

vsigno commented 2 weeks ago

Actual Result

Using the latest release of deck.gl@9.0.27, the dependency @luma.gl/gltf@9.0.24 (and onwards, up to @luma.gl/gltf@9.0.26) breaks the rendering of the Google 3DTiles (tiles are not displayed). Error: deck.js:82 deck: drawing ScenegraphLayer No value for binding u_BaseColorSampler.

Complete Error:

deck: drawing ScenegraphLayer({id: 'tiles-scenegraph-https://tile.googleapis.com/v1/3dtiles/datasets/CgA/files/[....].glb'}) to screen: No value for binding u_BaseColorSampler in tiles-scenegraph-https://tile.googleapis.com/v1/3dtiles/datasets/CgA/files/[...].glb-cached Error: No value for binding u_BaseColorSampler in tiles-scenegraph-https://tile.googleapis.com/v1/3dtiles/datasets/CgA/files/[...].glb-cached
    at WEBGLRenderPipeline._applyBindings (webgl-render-pipeline.js:314:23)
    at WEBGLRenderPipeline.draw (webgl-render-pipeline.js:167:14)
    at _Model.draw (model.js:256:41)
    at scenegraph-layer.js:238:23
    at _GroupNode.traverse (group-node.js:80:17)
    at _GroupNode.traverse (group-node.js:77:23)
    at _GroupNode.traverse (group-node.js:77:23)
    at _ScenegraphLayer.draw (scenegraph-layer.js:223:31)
    at layer.js:851:22
    at withGLParameters (with-parameters.js:26:17)

Expected Result

Revert the dependency @luma.gl/gltf@9.0.24 to @luma.gl/gltf@9.0.23 solves the issue and the 3DTiles are displayed as expected

chunlampang commented 2 weeks ago

Not only the Google tile, all MeshLayer ScenegraphLayer are broken. And @luma.gl 9.0.26 still has the same error

ibgreen commented 2 weeks ago

@felixpalmer This is likely due to the fact that luma.gl 9.0.23 included the UBO changes to the PBR module. Is there a deck.gl 9.0 patch that covers this?

chunlampang commented 2 weeks ago

@felixpalmer This is likely due to the fact that luma.gl 9.0.23 included the UBO changes to the PBR module. Is there a deck.gl 9.0 patch that covers this?

Deckgl latest version from yesterday still using luma 9.0.15

Btw if there has a breaking change why the version number is not became 10.0.0

felixpalmer commented 2 weeks ago

This the breaking change. Unlike the lighting module PRs which were constrained to shader code in modules that deck@9.0 isn't using, the change to createGLTFModel() means that deck is pulling in the new UBO-based modules

As deck.gl@9.0 isn't including the UBO changes (that will come in deck@9.1) I would propose we revert https://github.com/visgl/luma.gl/pull/2173 and publish a new luma@9.0 version, @ibgreen do you agree? I think this is safer than patching deck@9.0

ibgreen commented 2 weeks ago

Agreed.

felixpalmer commented 2 weeks ago

@chunlampang @vsigno 9.0.27 should fix the issue, can you confirm?

vsigno commented 2 weeks ago

Yes, @luma.gl/gltf@9.0.27 fixes the issue with the Tile3DLayer. I did also a quick test with a ScenegraphLayer and it looks fine. All models loaded with no errors.