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

Terrain 3D #1022

Merged
merged 175 commits into from
May 12, 2022
Merged

Terrain 3D #1022

merged 175 commits into from
May 12, 2022

Conversation

wipfli
Copy link
Member

@wipfli wipfli commented Feb 24, 2022

@prozessor13 thank you for all the work you did to make 3D happen in MapLibre GL JS.

This pull request is a continuation of #165 with the goal to merge the terrain3d branch into main.

Demo

flyTo example

  • Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!

…ing the

development i did no intermediate commits, so in this first commit all the
following functionality is included:

* allow MTK terrainRGB-tiles encoding with parameters:
  [6553.6, 25.6, 0.03, 10000.0]. In our opinion 0.1 for height-steps is to
  rough.
* create a TerrainSourceCache.js class which is similar to SourceCache.js and
  holds all the terrain-tiles used for the 3D-mesh. If a terrainRGB tile is
  used in both, terrain & hillshading, it is loaded twice. This makes the
  whole process much more easy.
* create a 3d-mesh in raster_dem_tile_worker_source.js with the martini
  library.
* rewrite the draw logic to render all layers, except symbols, into a
  framebuffer. This framebuffer a later used as a texture onto the 3d-mesh.
* rewrite symbol rendering to use 3d-coordinates. This is done with an extra
  a_ele shader parameter, because the z-value of the a_pos variable is already
  used for other things.
* add the third dimension into the collision index.
* create a terrain.html test-page.
This is done with an extra framebuffer in which all tile coordinates are
rendered in an encoded rgb value. Dont know why, but this framebuffer looks
very inperformant. As advantage, grabbing coordinates from screen
is very fast. It may help to render the framebuffer only every 100ms instead
of every requestAnimationFrame?
This logic is currently used in
* map.project
* map.unproject.
Other usecases are:
* elevate camera over terrain
* use in mouse-events
* use in queryRenderedFeatures
* add exaggeration setting in TerrainSourceCache
* fix farZ clipping-plane
* set points: any declaration to Array<Object>
* new more performant encoding of the coords-framebuffer
* decrease the number of framebuffers
* decrease the number of framebuffer/texture switches
* more caching and less re-rendering

the old render-pipeline renders layer by layer,
which is ok to render on display framebuffer, but when
rendering into a textures it is more performant to render all
necessary layers at once.
This checkin is only for backup, so do not test!

* changed elevation calculation from CPU to GPU

This change had a big impact in performacne. To calculate the hight in
CPU sometimes about 100ms per tile was needed. When loading a lot of
tiles the framerate was really bad. This works now much better.
NOTE: Currently only the Mesh elevation is moved to GPU, symbols are
still calculated via the CPU.

* refactor render to texture workflow below ZL 14

Until now, below ZL 14 (e.g. our Vector-Tile maxzoom) the tile-size
increased with ZL, so ZL 1-14 had 1024px, ZL 15 2048px, ZL 16 4096px,
and so on. This was very easy to program, but needed also a lot of GPU
performacne and RAM. So now, the TerrainSourceCache holds tiles for each
zoomlevel of the size 1024, and below ZL 14 in each is renderd a
sub-region of ZL 14.

* refactor the coords-framebuffer.

Also the coords-framebuffer use a textures with 1024px in size. To avoid
bluring the texture below ZL 14 an additional small texture is needed
(u_coords_index) which holds in its RGBA values the tile quadrants of
the sub-regions.
NOTE: This is currently a work in progress!
@HarelM
Copy link
Collaborator

HarelM commented Apr 8, 2022

I've took this branch for a spin in my project to see how it behaves in a complicated (real life) scenario.
Unfortunately, when using the terrain the app breaks and the console has a lot of errors.
I'll see if I can create a minimal reproduction and open the relevant issues.
Bottom line, unfortunately, still not ready to be merged unless we want to create a pre-release version to allow other people to test it and report issues.
What I did in order to test this if anyone is interested:

  1. Clone the repo, checkout this branch and build it (with codegen and everything)
  2. Copy the dist folder to the node_modules/maplibre-gl/dist of the project I'm testing
  3. Build the project and run it
  4. [Optional] I'm also using the patch-package npm package to allow CI to build it or reduce the work for other people

src/render/terrain.ts Show resolved Hide resolved
@prozessor13
Copy link
Collaborator

Unfortunately, when using the terrain the app breaks and the console has a lot of errors.

What do you mean with the app break? Which errors? Mobile or Deskop?

@HarelM
Copy link
Collaborator

HarelM commented Apr 11, 2022

What do you mean with the app break? Which errors? Mobile or Deskop?

The errors in the console are errors related to the two places I commented yesterday.
In some cases when I tested it on desktop chrome, which I need to still identify the exact scenario, the map stops responding to gestures and you can't pan the map using the mouse.

Let's fix the above two errors and I'll test again to see if I can reproduce the last issue where the map stops responding to gestures.

@prozessor13
Copy link
Collaborator

prozessor13 commented Apr 11, 2022

Ok, i will focus on that! Thanks for testing!

@HarelM
Copy link
Collaborator

HarelM commented Apr 11, 2022

Just FYI: I've placed the current build output (relevant to the time I'm writing this) here:
https://maplibre.org/maplibre-gl-js/pull/1022/maplibre-gl.js
If anyone needs it for codepen or whatever.
I'll try and update it from time to time as long as this PR is open.

@prozessor13
Copy link
Collaborator

Hello all,

i created a new issue-label Terrain 3D Blocker. So, for blocking issues (e.g. must fixed before merge) please tag with this label, so i can focus on the right things.

Thanks in advance!

@thunderkid
Copy link

3dMapLibreBrave

I'm finding that the demo link on this page behaves strangely in the Brave browser (V1.37.113 - Apr 12, 2022 running on Windows 10 21H1 19043.1586 and also on a couple of older Brave versions that I checked). The 3d terrain works, but with random spikes all over it as shown in this screen capture. These persist when moving around. On Chrome it's working fine on the same machine.

@HarelM
Copy link
Collaborator

HarelM commented Apr 13, 2022

This is a problem with brave, feel free to open a defect for them. In general I've seen bad behavior of brave with maplibre not only in terrain 3D.

…in branch (#1182)

* Fixes related to console errors and some typings improvement

* Fix lint

* Added -1 for terrain calculation, added a test, fixed public field

* Add typings
@HarelM
Copy link
Collaborator

HarelM commented Apr 14, 2022

#1166 is the only blocking issue according to the labels.
Once fixed I tend to say that this branch can be merged.
@wipfli what do you think?

* Use MapTiler terrain tiles

* Default location Innsbruck, add terrain control

* Remove terrain_control debug page
@wipfli
Copy link
Member Author

wipfli commented Apr 15, 2022

I really like how well terrain 3D works now and can't stop looking at maps of the Alps in 3D mode. Thanks @prozessor13 for all the work you put into this. I think I found another bug related to toggling terrain on and off, see #1185. Once addressed, I think we can merge this to main and create a pre-release.

prozessor13 and others added 9 commits April 16, 2022 09:18
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

Successfully merging this pull request may close these issues.