Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TerrainLoader skirting #712

Closed
kylebarron opened this issue Mar 31, 2020 · 17 comments
Closed

TerrainLoader skirting #712

kylebarron opened this issue Mar 31, 2020 · 17 comments
Assignees

Comments

@kylebarron
Copy link
Collaborator

kylebarron commented Mar 31, 2020

cc @chrisgervang

This is a continuation of visgl/deck.gl#4236, but I think it's a good fit to be added directly to the TerrainLoader.

When tiled, there can be a seam between terrain mesh tiles due to the mesh borders having different heights (the dark blue wedge here).

image

From what I've read, the simplest solution is to create a vertical skirt around the edges of the mesh:

image

That image is a little simplistic, because instead of flat edges, the side of each tile varies in height. Because of the varying edge height, I think you would need more than one skirt for each tile edge, so as to match the existing heights.

My idea is:

  • Keep track of x, y, z positions along tile edge

    E.g. here add the position to an array if x or y === 0 or 1. Or better, keep 4 arrays, one for each tile edge.

    https://github.com/uber-web/loaders.gl/blob/d61afe1b04f1374c98ddf0b260c0ce3c8f3276f3/modules/terrain/src/lib/parse-terrain.js#L49-L51

  • Loop over neighboring positions on each tile edge, creating two new triangles for each pair of positions. For example, on the edge x=0, where y1, y2 are neighboring

     # along existing vertical edge
     [[0, y1, z1], [0, y2, z2], [0, y1, z1 - skirtHeight]]
     # along edge formed by previous triangle
     [[0, y1, z1 - skirtHeight], [0, y2, z2], [0, y2, z2 - skirtHeight]]

    For a 256px tile, there are a max of 15 pairs of neighboring positions on each edge, so this skirt process would add a max of 120 additional triangles (but usually less with martini's optimizations). A 256px tile is 256 pixels on each side, not total 😅, so a max of 255 pairs of neighboring positions on each edge, or a max of 25524 = 2040 extra triangles, but usually less.

Thoughts? One question I had is whether the SimpleMeshLayer would accurately apply a texture to a vertical triangle, which doesn't have width in either the x or y dimension. Would it render vertical bars along the edge like the example skirt in the screenshot above?

@ibgreen
Copy link
Collaborator

ibgreen commented Apr 1, 2020

Thoughts?

Makes sense. Add options.skirt?

That image is a little simplistic, because instead of flat edges, the side of each tile varies in height. Because of the varying edge height, I think you would need more than one skirt for each tile edge, so as to match the existing heights.

Just make the skirt triangles go all the way down (or up in a few cases) to sea level? You are just duplicating the border pixels so it doesn't matter how tall the triangles are. Depending on draw order it can generate a bit of "overdraw", but I doubt that we are GPU bound in this case.

One question I had is whether the SimpleMeshLayer would accurately apply a texture to a vertical triangle, which doesn't have width in either the x or y dimension. Would it render vertical bars along the edge like the example skirt in the screenshot above?

Yes, you control this with your UV coordinates for those vertices, just lock them to the same value.

@kylebarron
Copy link
Collaborator Author

Just make the skirt triangles go all the way down (or up in a few cases) to sea level?

I don't think you'd ever want the skirt triangles to go up to sea level. If you're visualizing Death Valley, ~300ft below sea level, the skirt would look weird. But "if z > 0 ? down to sea level : z - 10 meters" would probably be fine.

Yes, you control this with your UV coordinates

Makes sense. Since UV coordinates are 2D and the only new coordinates would vary in z, I don't think I'd have to touch these at all.

@ibgreen
Copy link
Collaborator

ibgreen commented Apr 1, 2020

I don't think you'd ever want the skirt triangles to go up to sea level. If you're visualizing Death Valley, ~300ft below sea level, the skirt would look weird. But "if z > 0 ? down to sea level : z - 10 meters" would probably be fine.

Yes, very true. Maybe have the skirts go down to -100 meters. There is also the rare but very cool oceanographic (sea bottom) visualization case, so maybe just make the skirt bottom depth configurable with an option?

Makes sense. Since UV coordinates are 2D and the only new coordinates would vary in z, I don't think I'd have to touch these at all.

Yes. You just need to duplicate them for the "extruded skirt bottom vertices", using the same value as their on-the-terrain-edge counterparts..

@kylebarron kylebarron mentioned this issue May 4, 2020
1 task
@TimGurnett
Copy link

I had a go at implementing this and the results look very promising!

Before:

After:

@kylebarron I set out with your pseudocode above and while it worked there was a lot of copying to larger arrays like you mention here #729 (comment)

So I created a getMeshWithSkirt function for Martini that generates the skirt vertices and triangles in the initial allocation. It returns those arrays as well as an index into the vertices array where the skirts start.. so TerrainLoader sets the z on any vertices above that index to skirtHeight ( or z - skirtHeight ).

Seems to work best with material: false or _lighting: 'flat' as the skirt is usually perpendicular to the mesh.. guess we could fix this by copying the normal of the "on-the-terrain-edge" counterpart too?

@kylebarron
Copy link
Collaborator Author

This looks really cool! Would you be interested in submitting a Pull Request?

@TimGurnett
Copy link

Yep, most of the changes for this are in martini so I've opened a pull request there mapbox/martini#12

The minor changes needed in loaders.gl are relying on that pull request but the small change to the simple-mesh-layer in deck.gl could be opened now.

@kylebarron
Copy link
Collaborator Author

Awesome! There was interest in making skirts as part of the Martini API (mapbox/martini#7 (comment)). Looking forward to following that issue. I looked through the Martini code to attempt a PR but mourner's code is black magic, and I got lost.

@Ardweaden
Copy link

Ardweaden commented Oct 13, 2020

@TimGurnett
I applied your PR in a project and the skirts are generated nicely. Thanks for that!

However, the seams are still visible even though material: false is set. This is because the skirts aren't always of correct colour. Do you have any advice how to fix this problem?

Here is a video showing an example of that. The lake is clearly dark blue, but the skirts are greenish:
https://www.youtube.com/watch?v=R8kMOD8JPAs

@kylebarron
Copy link
Collaborator Author

kylebarron commented Oct 13, 2020

That video is really dark and I can barely see the terrain at all. Could you include some screenshots, or better yet a CodeSandbox?

@Ardweaden
Copy link

Ardweaden commented Oct 14, 2020

@kylebarron
Sorry about that.

Not sure what code should I include. I'm just using a modified slightly TerrainLayer:

return new TileLayer({
      material: false,
      renderSubLayers: this.renderSubLayers.bind(this),
      getTileData: this.getTiledTerrainData.bind(this),
      minZoom: this.props.minZoom,
      maxZoom: this.props.maxZoom,
      onTileError: () => {},
    });

renderSubLayers.js

return new SimpleMeshLayer(props, {
      mesh,
      texture,
      coordinateSystem: COORDINATE_SYSTEM.CARTESIAN,
      getPosition: d => [0, 0, 0],
      getColor: texture ? [0, 0, 0, 0] : [100, 100, 100],
      material: !texture,
    });

Regarding changes for skirting, I just used @TimGurnett Martini MR and added the following lines to
getMeshAttributes method used by TerrainLoader:

for (let i = 0; i < numOfVerticies; i++) {
    const x = vertices[i * 2];
    const y = vertices[i * 2 + 1];
    const pixelIdx = y * gridSize + x;

    positions[3 * i + 0] = x * xScale + minX;
    positions[3 * i + 1] = -y * yScale + maxY;
    // Changed to create skirts
    if (i > numVerticesWithoutSkirts) {
      positions[3 * i + 2] = terrain[pixelIdx] - skirtHeight;
    } else {
      positions[3 * i + 2] = terrain[pixelIdx];
    }
    // Changes stop here
    texCoords[2 * i + 0] = x / tileSize;
    texCoords[2 * i + 1] = y / tileSize;
  }

I made an image to hopefully illustrate my problem better:
image

@ickeB
Copy link

ickeB commented Oct 21, 2021

Yep, most of the changes for this are in martini so I've opened a pull request there mapbox/martini#12

The minor changes needed in loaders.gl are relying on that pull request but the small change to the simple-mesh-layer in deck.gl could be opened now.

@TimGurnett can you please describe which changes in loaders.gl and deck.gl are necessary besides your PR for Martini to get your result (#712 (comment)).
For us a blocker if the problem can not be solved we can not use deck.gl

@kylebarron
Copy link
Collaborator Author

@ickeB this should be fixed by #1662. Can you try the TerrainLoader or QuantizedMeshLoader with the skirtHeight option?

@ickeB
Copy link

ickeB commented Oct 22, 2021

thx @kylebarron for your answer. From our starting point:
image
we have set the skirtHeight in the Terrainloader loadOptions , this looks like this:
image

Seams are still visible, or do I miss something :)

@TimGurnett solution with modifying the mesh at creation point in martini sounds promising and on his screenhot it also looks like the seams are not visible anymore.

We use loaders.gl 3.0.12

@kylebarron
Copy link
Collaborator Author

That actually looks like it works decently well.

From #712 (comment):

Seems to work best with material: false or _lighting: 'flat' as the skirt is usually perpendicular to the mesh.. guess we could fix this by copying the normal of the "on-the-terrain-edge" counterpart too?

The issue you're seeing is that the seams are perpendicular to the mesh, so the lighting is still noticeable there.

@ikemarv
Copy link

ikemarv commented Nov 1, 2021

hello @ all,
I have noticed that at the zoom level 19.50 the gaps or edges (with skirtHeight) of the tiles disappear.
Is it possible to apply this behavior of rendering or generating the mesh to all zoom levels? The performance will certainly be affected, but it might be acceptable. For us the elimination of these gaps or edges is very important.

Here is an example with zoom level 19.49
zoom_19-49

And here same example with 19,51
zoom_19-51

@ickeB
Copy link

ickeB commented Nov 2, 2021

That actually looks like it works decently well.

From #712 (comment):

Seems to work best with material: false or _lighting: 'flat' as the skirt is usually perpendicular to the mesh.. guess we could fix this by copying the normal of the "on-the-terrain-edge" counterpart too?

The issue you're seeing is that the seams are perpendicular to the mesh, so the lighting is still noticeable there.

@kylebarron the big problem is not only visuel or lighting one, but the fact that actual height differences can be measured at seams to adjacent tiles where there should be no differences. This is a problem for simulations that need to be correct across tiles, e.g. flood simulations.

image
The heights of the two points:
White: 50.63 m
Black: 51.13 m

@kylebarron
Copy link
Collaborator Author

kylebarron commented Nov 2, 2021

the big problem is not only visuel or lighting one, but the fact that actual height differences can be measured at seams to adjacent tiles where there should be no differences

What you're describing is entirely expected for most tiled mesh generators.

You haven't said how you're creating the meshes. But if you're creating meshes using a tool like Martini (Which is used within the TerrainLoader) a mesh will be created independently for every tile. By default, mesh generators like these have a maximum error tolerance. With any error tolerance > 0, it's a mathematical necessity that there will be differences in height at the tile boundaries. The only real way to get around this is to do a post processing step that forces edges to be aligned. But that's a computationally intensive step because for every tile you also need to know the meshes of all 8 surrounding tiles.

Alternatively you can create your tiles ahead of time, save them to the Quantized Mesh format, and use the QuantizedMeshLoader.

@ibgreen ibgreen closed this as completed Dec 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants