Image in raster layer shows only partially on certain zoom levels when 3D terrain is enabled.

danielfaust commented 1 year ago

CEA884BC-D1C4-49DB-9611-8BBC39B1C392

maplibre-gl-js version: v2.4.0 (already present in v2.2.0-pre.2)

browser: Google Chrome Version 104.0.5112.102 (Offizieller Build) (64-Bit)

No errors or warnings appear in the console

Steps to Trigger Behavior

Zoom in and out into the image to see the problem, then zoom in to prepare to toggle elevation layer
Toggle elevation layer a couple of times to see the problem.

Link to Demonstration

~https://jsbin.com/qeverim/edit?html,output~

https://jsbin.com/yowixah/1/edit?html,output

Expected Behavior

The image should always fully render into the layer

Actual Behavior

The image is cropped at high zooming levels.

prozessor13 commented 1 year ago

Thx for bugreport, i looks like that only the center-coordinate of the image is rendered onto terrain. When looking into draw_raster for ImageSource a boundsBuffer object is transfered to the shader. But because terrain-tile rendering stops on tile-borders this looks not sufficient. In this case i think a list of terrain-tiles, calculated by the source.coordinates, must be rendernd. Currently i not have the time to look further onto this, anyone else?

HarelM commented 1 year ago

I'll see if this is something I can try and tackle, sounds fairly straight forward from your deception.

HarelM commented 1 year ago

Unfortunately I'm not making any progress :-( I've created the following debug page in order to be able to better debug this issue: Clicking zoom one time shows the issue and allows changing the code using npm run start-debug to see the changes in the code take effect on the page. I looked at the draw_raster tileIDs parameter and it is the same between with and without terrain (it's probably related to boundsBuffer as you said, but I don't know how to make some thing out of it). I guess I just am not familiar enough with the code and where to look for the issue... :-( Leaving it to someone else, if someone is up for it.

<!DOCTYPE html>
<html>

  <head>
    <meta charset="UTF-8">
    <meta http-equiv="X-UA-Compatible" content="IE=edge">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>3D Map with image source clipped</title>
    <script src="../../dist/maplibre-gl-dev.js"></script>
    <link href="../../dist/maplibre-gl.css" rel="stylesheet" />
    <style>
      body { margin: 0; padding: 0; }
      #map { position: absolute; top: 0; bottom: 0; width: 100%; }
    </style>
  </head>

  <body>
    <div id="map"></div>
    <script>
      const center = [9.3948749, 48.4968956];
      var la_t_tile = center[1] + 0.025;
      var lo_l_tile = center[0] - 0.035;
      var la_b_tile = center[1] - 0.025;
      var lo_r_tile = center[0] + 0.035;
      const map = new maplibregl.Map({
        container: 'map',
        hash: true,
        style: {
            version: 8,
            sources: {
                image: {
                    type: 'image',
                    url: 'https://i.imgur.com/3IyX7CK.png',
                    coordinates: [
                        [lo_l_tile, la_t_tile],
                        [lo_r_tile, la_t_tile],
                        [lo_r_tile, la_b_tile],
                        [lo_l_tile, la_b_tile]
                    ]
                },
                terrain: {
                    "type": "raster-dem",
                    "url": "https://api.maptiler.com/tiles/terrain-rgb/tiles.json?key=get_your_own_OpIi9ZULNHzrESv6T2vL",
                    "tileSize": 256,
                }
            },
            layers: [{
                'id': 'image',
                'type': 'raster',
                'source': 'image',
            }],
            terrain: {
                source: "terrain",
                exaggeration: 1,
            }
        },
        center: center,
        zoom: 11.5,
      });

      map.addControl(new maplibregl.NavigationControl({
        visualizePitch: true,
        showZoom: true,
        showCompass: true
      }));

      map.on('load', () => {
        map.showTileBoundaries = true;
      });
    </script>    
  </body>
</html>

navilaufm commented 1 year ago

Seems I am having the same issue, so I will post my demo links here and close the issued #1979 I opened.

and the following appears in console: 83209271-e541-4d31-9f74-52a1663491ad:1 Canvas2D: Multiple readback operations using getImageData are faster with the willReadFrequently attribute set to true. See: https://html.spec.whatwg.org/multipage/canvas.html#concept-canvas-will-read-frequently

Link to Demonstration without 3D https://climaya.com/maplibre/mapl.html?url=https://nube1.on.gt/icon/min_temp_72.png&lon=-98&lat=17&z=4.6&p=28.5&x1=-120.125&x2=-49.875&y1=-0.125&y2=35.125&opacity=0.5

With 3D enabled image overlay does not show complete and opacity is not respected. https://climaya.com/maplibre/mapl.html?url=https://nube1.on.gt/icon/min_temp_72.png&lon=-98&lat=17&z=4.6&p=28.5&x1=-120.125&x2=-49.875&y1=-0.125&y2=35.125&opacity=0.5&d3=1

HarelM commented 1 year ago

Assigned L bounty. Link to parent Bounty: https://github.com/maplibre/maplibre/issues/189

manhcuongincusar1 commented 1 year ago

Hi @HarelM. Can I assign this issue?

HarelM commented 1 year ago

Sure, please do.

manhcuongincusar1 commented 1 year ago

Thanks

manhcuongincusar1 commented 1 year ago

Hi @HarelM after investigating, it seems that the partial rendering problem occurs when the image spans multiple tiles but only one tile's terrain data is utilized, leading to incorrect rendering: https://github.com/maplibre/maplibre-gl-js/blob/1cbb496f5cdf6751475986562eb0067c3ace7c4b/src/render/draw_raster.ts#L64

I think this issue is quite complicated. Can you consider to extend the bounty of this issue?

(In this screenshot, the photo belongs to tile 11/1088/718. If terrain is enabled, only the partial area inside the red rectangle will be rendered. Only at lower zoom levels (higher elevation) where the image fits into a single terrain tile. At those levels, the image will be rendered fully.)

danielfaust commented 1 year ago

I upgraded the demo to v3.1.0 and replaced the defunct Maptiler tiles & terrain with the demotiles from this project

https://jsbin.com/yowixah/1/edit?html,output

manhcuongincusar1 commented 1 year ago

@danielfaust I think this problem will happen with all of un-tiled data sources like Image, Heat maps #1081, 3D meshes (which not loaded from a tiled dataset). Mapbox solved this problem by utilizing ProxiedTileId class to map actual tiles to others terrain tiles to solve the problem in source cache level. That is promising idea other way is update function getTerrainData() to return postMatrix from all spanned tiles.

danielfaust commented 1 year ago

Only at lower zoom levels (higher elevation) where the image fits into a single terrain tile.

I wanted to say: "I don't think that this is correct. All tiles have a size of 256 and this is visualized by the red grid". But then I navigated to the border of the terrain tiles ( https://output.jsbin.com/yowixah/1#12.39/46.99612/11.39178/62.4/29 ) and noticed that these terrain tiles apparently do not match the 256 grid of OSM raster, as if they were offset. Or maybe the terrain tiles are just cut in an odd way.

Here's another view which might be helpful: https://output.jsbin.com/yowixah/1#11.38/47.1349/11.9827/-9.6/60 if you zoom out and in a bit it's odd that the terrain apparently does not seem to change in resolution when the OSM raster is changed. Maybe this could help.

I added a second image at the right border of the terrain tile boundary ( https://output.jsbin.com/yowixah#10.79/47.2697/11.7186 ) , maybe analyzing that tile could help ( https://output.jsbin.com/yowixah#10.13/47.1797/11.8557/-19.2/57 )

manhcuongincusar1 commented 1 year ago

Thank you for the sample code @danielfaust, I will ping you here when there's progress

manhcuongincusar1 commented 1 year ago

@danielfaust Do have any ideas about the problem? I would love to here from you?

danielfaust commented 1 year ago

@manhcuongincusar1 I have absolutely no idea what the problem is.

Here's a modified demo with 2 animated source images.

https://jsbin.com/juwikuq/4/edit?html,output

danielfaust commented 1 year ago

With a bigger radius: https://jsbin.com/juwikuq/7/edit?html,output

It reminds me of another issue regarding of which scale of tiles is shown at which zoom level, apparently there is a mismatch. And since here every second tile is affected, it seems to suggest that the image is living in another zoom level or something like that.

I'm unable to find the issue number, but I think that it was 2d-related as well.

Off Topic, but maybe related: https://github.com/maplibre/maplibre-gl-js/issues/1077 because at the moment when the image center crosses a tile boundary (of the non-visible zoom level), it flickers.

manhcuongincusar1 commented 1 year ago

Thank for the reference @danielfaust

HarelM commented 1 year ago

What bounty did you had in mind?

manhcuongincusar1 commented 1 year ago

What bounty did you had in mind?

I suggest XL @HarelM

HarelM commented 1 year ago

Done.

manhcuongincusar1 commented 1 year ago

@HarelM thank you very much

danielfaust commented 1 year ago

Added some controls to modify size and other things: https://jsbin.com/bucosel/2/edit?html,output

acalcutt commented 1 year ago

I wanted to say: "I don't think that this is correct. All tiles have a size of 256 and this is visualized by the red grid". But then I navigated to the border of the terrain tiles ( https://output.jsbin.com/yowixah/1#12.39/46.99612/11.39178/62.4/29 ) and noticed that these terrain tiles apparently do not match the 256 grid of OSM raster, as if they were offset. Or maybe the terrain tiles are just cut in an odd way.

I was looking at this , and you are using the demo terrain tiles I made at https://github.com/maplibre/demotiles/tree/gh-pages/terrain-tiles .

One thing to note is the demo terrain are 512x512 sized tiles, so if you are setting them to 256 it could cause issues like over zooming I think. rio rgbify, which is used there, makes 512px tiles.

The second thing is this extract is based on a single DEM image of the N047E01 area. The edges are just based off the source file used from JAXA AW3D30, not based on tiles edge, so it is possible the tile could be bigger than the image

HarelM commented 2 months ago

Is this a question about the behavior of image source or this bug? It is unclear from what you wrote. If this is not related to this bug please open a different discussion or issue.

HarelM commented 1 month ago

This doesn't answer my question. Please open a different issue or discussion and delete the comments from this big report.

dennemark commented 3 weeks ago

Hi, added a PR with failing test and example for the repo. But I am stuck, too. I do not understand how the wrap of OverscaledTileID works as described in the PR. Currently the tiles with wrap -1 and 1 fail in my case. But actually, moving the coordinate of my cat image example from 11.43085 to 11.45085 returns the same tiles, but the cat is visible. So even the wrap part might not be the issue. What I know though, getting the source of the terrain for the image goes up zoom levels from 11 to 5 if it finds a source, otherwise up to 0 with the terrain tile being undefined.

grafik

https://github.com/maplibre/maplibre-gl-js/blob/1cbb496f5cdf6751475986562eb0067c3ace7c4b/src/source/terrain_source_cache.ts#L175

My guess is, that the tileID.scaledTo(z--) function in the while loop is not getting the correct parent tile when the tileID is wrapped. I am wondering if the parent for an image should be different, than for normal layers. But I am stuck, since I do not really understand, what wrap is used for. I only can see that it leads to different key generation for OverscaledTileID.

I would also expect the correct terrain tile data to be available, since it is rendered in hillshading.

Please correct me on this assumption or explain, how to understand wrap, but might this be happening?

dennemark commented 3 weeks ago

Image source bounds are bigger than its canonical tile. But it seems only the canonical tile is loaded when reading from its source_cache. Therefore only that tile will be rendered.

Image_source has this tile loaded with wrapped tiles ( 1 and -1 wrapped tiles are used on the edges of the map, to continue rendering it left and right side of a world map, i believe). And it stores it on its this.tiles[0/-1/1]. The loadTile function is called from source_cache from here

https://github.com/maplibre/maplibre-gl-js/blob/9b139a8e158224597661f4cf43f9963e62203a99/src/source/source_cache.ts#L857

And it should be called only once, otherwise the url will be loaded multiple times. That's why it would be great to have a proxy to this tile, since we can then acces the tile with a different tileID. We would need to do this for all tiles that are within the image_source bounds. I tried to do it hardcoded but did not succeed.

 if (!tile) {
            tile = new Tile(tileID, this._source.tileSize * tileID.overscaleFactor());

            this._loadTile(tile, tileID.key, tile.state);
        }

        tile.uses++;
        this._tiles[tileID.key] = tile;
// new code
 if(this._source.type === 'image' && tileID.key !== 'vhvkbb'){
            const clone = tileID.clone()
            clone.canonical.x--
            clone.key = 'vhvkbb' // key of the missing tile with x: 1088 in my example file

            const proxy = new Proxy(tile, {
                get(target, prop, receiver) {
                    if(prop === "tileID"){
                        return clone
                    }
                    return target[prop]
                }
            })
            this._tiles['vhvkbb'] = proxy
        }

If I check the coords arriving in draw_raster, the 'vhvkbb' tile id is missing. And the reason seems to be somewhere in render_to_texture prepareForRender function. The terrain tries to get all renderable tiles, but somehow the 'vhvkbb' tile is missing.

I have to pause my exploration here. Hope someone can pick it up.

HarelM commented 3 weeks ago

@manhcuongincusar1 are you still working on this issue or can I remove your assignment?

manhcuongincusar1 commented 3 weeks ago

@HarelM, please remove me from this issue. I don’t have time for it right now. Sorry about that.

danielfaust commented 1 week ago

In https://jsbin.com/yutuyes/2/edit?html,output the map is updated to 4.5.0. The image oddly sticks ("bakes in") to some tiles, the entire 4.x-range has this problem.

maplibre / maplibre-gl-js