needsHillshadePrepare Cannot read properties of undefined

[acalcutt]

I am testing a terrain demotiles example page here with the latest terrain3d branch code.
https://stackblitz.com/edit/web-platform-x1xjty?file=index.html

I keep seeing this error pop once once and a while

Error in /maplibre-gl.js (7187:19)
Cannot read properties of undefined (reading 'needsHillshadePrepare')

Most of the time it is when I am browsing around quickly looking at terrain... maybe I am not giving it time to load completely before I move to another location. I don't have a good way to reliably reproduce the issue other than click and dragging around with the mouse.

[HarelM]

I think i saw it too, probably related to the fact that it is being checked inside a worker which is more strict.
I'll check where the code now uses it and change the check to facilitate for worker "strict" mode.

[HarelM]

@acalcutt I've added a typeof guard check, can you see if latest code still has this issue?

[acalcutt]

I updated the example, cleared my browser cache, and tried it again but I still managed to get that error a few times. seems like when I was zooming in and out when browsing around.

here's a more detailed error from the chrome console.

TypeError: Cannot read properties of undefined (reading 'needsHillshadePrepare')
at Object.hillshade (maplibre-gl.js:7187:30)
at io.renderLayer (maplibre-gl.js:7286:197)
at to.renderLayer (maplibre-gl.js:7078:162)
at io.render (maplibre-gl.js:7278:28)
at Map._render (maplibre-gl.js:8345:455)
at maplibre-gl.js:8355:160

[HarelM]

I'll re-check the code I wrote, I might have not written the right check.

[HarelM]

Thanks for looking into it.
I'm able to reproduce it.
I'm not sure why the tile is missing in the source cache though.
I added some code to say that if a tile is missing, don't render the hillshade, and it make the map more stable and avoid these error, but I think it's not fixing the root cause of this issue.
I'll look into the code and see where renderLayer is called and what has changed in the branch.
I'll also add a blocker label since this totally crashes the browser.

[HarelM]

Ok, I think I'm on to something.
The following code assumes that every coordinate passed here exists in the sourceCache:

maplibre-gl-js/src/render/draw_hillshade.ts

Line 29 in c92f7a4

const tile = sourceCache.getTile(coord);

This doesn't seem to be the case in the following invocation:

maplibre-gl-js/src/render/render_to_texture.ts

Line 142 in c92f7a4

painter.renderLayer(painter, painter.style.sourceCaches[layer.source], layer, coords);

If I needed to guess, I would guess that the _coordsDescendingInv is not initialized or not in sync with the source cache.
@prozessor13 I'm not sure I know how to fix this though.
But I was able to reproduce the issue relatively easily by trying to zoom in fast (using the scroll button) into a mountain that has terrain on.
Let me know if there's anything else I can help with as I'm not sure how to fix this code... :-(

[prozessor13]

Hmm. I think it is a fight between the terrain-sourcecache and the hillshading-sourcecache based on the same source. For terrain there is a different usecase onto the source then in rendering hillshading.

• double the tileSize, which results in 1/4 loaded tiles
• instead load parent tiles to improve zooming, and calculate visibility in tilted maps
• a zoomlevel of 12 is sufficient

you see this fight in very poor-qualitiy hillshadings in 3d, and i think terrain parallel unload tiles while hillshading wants to access it.

But i think, these differences are mandatory, and i spent a lot of work into this to improve performacne!

In short, it is not a good idea to use the same source for hillshading & terrain. I did not think about this earlier, because i always loaded pre-rendered hillshading tiles, and so i did not noticed this problem in my tests.

To use two different sources is in my opinion no (technical) problem, because they would not load the same tiles twice, but i agree it looks some kind of ugly to use two sources for the same thing.

So there a two solutions:

• let source-caches know of each other using the same source, and change the update method to not delete tiles, which are needed in the connected source-cache. I think this is the complicated way.
• When adding terrain, make sure the connected source is used exclusively, and throw an understandable error otherwhise.

[acalcutt]

I tested adding a second source just for hillshade and it does seem like it helped. I haven't been able to make that issue occur in this test page https://stackblitz.com/edit/web-platform-xyojdp

I also notice the hillshade looks a lot better with its own source (it comes out much darker and smoothed when it shared the same source as terrain)

[acalcutt]

I'm fine with saying this needs separate source for now. I'm going to close this.

[HarelM]

I think we should address this, but there's a valid "workaround" so this is not a blocker I believe.