create a frame around render-to-texture tiles to avoid stiching on different zoomlevels (2. version)

[prozessor13]

This code fixes https://github.com/maplibre/maplibre-gl-js/issues/1602, by creating a frame some pixels down around the terrain-mesh. The height of the frame is calculated in respect of the current zoomlevel. This code should/do not have any performance issues.

• [x] Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
• [x] Briefly describe the changes in this PR.
• [x] Write tests for all new functionality.
• [x] Document any changes to public APIs.
• [x] Manually test the debug page.
• [ ] Add an entry inside this element for inclusion in the maplibre-gl-js changelog: <changelog></changelog>.

[github-actions[bot]]

Bundle size report:

Size Change: +368 B Total Size Before: 205 kB Total Size After: 205 kB

Output file	Before	After	Change
maplibre-gl.js	196 kB	196 kB	+368 B
maplibre-gl.css	9.09 kB	9.09 kB	0 B

ℹ️ View Details

No major changes

[HarelM]

This still needs a render test - I would even consider adding a render test that has a gap caused by the bug this PR fixes and see how this changes when applying this fix :-)

[prozessor13]

This still needs a render test - I would even consider adding a render test that has a gap caused by the bug this PR fixes and see how this changes when applying this fix :-)

Agree with you, but currently i am afraid this is not easy possible because of lacking all needed tiles in assets/tiles (The gap only comes from different zoomlevels in high pitched maps near the horizon, so a lot of tiles are needed). Currently i do not have time to play with this test, sorry. Beside this test, i think the code is ready for merge.

[HarelM]

Thanks!! I'll create a branch in this repo and merge this to here so I can try and add a render test and use the spy thingy. I don't see major blockers though.

[prozessor13]

Oh great, thanks too!

Side-note: next i will focus on paralell-rendering, because also the stencil-branch depends on it.

[florianf]

Just tested it, it works like expected, screenshot with skirts visible with this PR:

Current main, without skirts:

Great work again, thx!

[HarelM]

@florianf any chance you can help out with a render-test to make sure this will not break in the future?

[florianf]

@HarelM is there a reference render-test from where I can start?

[HarelM]

Yup! You can read about it here: https://github.com/maplibre/maplibre-gl-js/blob/main/test/integration/README.md

[HarelM]

I was able to create a test to show this problem, here is how it looks: But the scene @florianf is presenting in this issue looks a lot better.

The code I used in order to download the missing terrain tiles locally in the render test is as follows: In the catch statement in line 234 I've added this "ugly" code:

                if ((req.url as string).endsWith("terrain.png")) {
                    let s = req.url.split("/");
                    let u = s[s.length - 1].replace(".terrain.png", "");
                    let t = u.split("-");
                    let awsUrl = "https://s3.amazonaws.com/elevation-tiles-prod/terrarium/{z}/{x}/{y}.png"
                        .replace("{z}", t[0])
                        .replace("{x}", t[1])
                        .replace("{y}", t[2]);
                    let fn = path.join(__dirname, '../assets', relativePath);
                    const file = fs.createWriteStream(fn);
                    https.get(awsUrl, (res) => {
                        res.pipe(file);
                    });
                    file.on('finish', () => {
                        file.close();
                        console.log("done writing file " + fn);
                        req.setStatus(404); // file not found
                        req.onload();
                    });
                }

Let me know if you need more help creating this test. I've used the following style.json file for this test:

{
    "version": 8,
    "metadata": {
        "test": {
          "height": 1024,
          "width": 1024
        }
    },
    "center": [-113.2233, 35.97287],
    "zoom": 12.77,
    "pitch": 60,
    "sources": {
        "terrain": {
          "type": "raster-dem",
          "tiles": ["local://tiles/{z}-{x}-{y}.terrain.png"],
          "encoding": "terrarium",
          "maxzoom": 15,
          "tileSize": 256
        }
    },
    "layers": [
        {
            "id": "background",
            "type": "background",
            "paint": {
                "background-color": "orange"
            }
        }
    ],
    "terrain": {
        "source": "terrain",
        "exaggeration": 1
    }
}

[HarelM]

@florianf any updates on the test? This is the main thing that is holding back this PR...

[florianf]

@HarelM sorry, I've currently no time for creating the test. I tested it visually and it works :-)

[HarelM]

Can you share the style you used to create the above image? And the location and pitch?

[florianf]

Link to the satellite debug page: http://localhost:9966/test/debug-pages/terrain-satellite.html#13.98/47.08108/11.7746/175/75

The effect should be clearly visible on the latest master.

[HarelM]

Closing in favor of #1710