search

heremaps / tin-terrain

A command-line tool for converting heightmaps in GeoTIFF format into tiled optimized meshes.
MIT License
585 stars 127 forks source link

Terrain shadow #48

Closed frodrigo closed 5 years ago

frodrigo commented 5 years ago

Unless I'm wrong. The Cesium.SceneMode.SCENE3D is not supported because the normal are not included in the tiles? Do you plan to add normal / allow shadow / SCENE3D?

fester commented 5 years ago

Hi @frodrigo,

Indeed, vertex normals computation is not implemented. I will read the format spec and proposed normal encoding algorithm and see how complicated it is to extend the implementation.

BTolputt commented 5 years ago

Adding the vertex normals will make this the "go to" Cesium tile processing tool. Currently the only ones that do it are proprietary. As I understand it, the code "should" be relatively simple once the normals are calculated. The format is simply to add a block of vertex normals (same size as the vertex array) at the end with a type value ("1") and the normals compressed by oct-encoding them.

fester commented 5 years ago

To give an update, we have a consensus to add normals generation to tin-terrain. However I need to clear some other commitments before giving it a go, ETA is roughly this or next week.

BTolputt commented 5 years ago

Sorry to pester, just working out timing details for code I'm working on too - is this still looking for a release this week?

delfrrr commented 5 years ago

@BTolputt tin-terrain is experimental tool which we released to explore the landscape and use cases of Surface Data and 3D Maps.

We put this issue on top of the backlog but cannot commit to specific timeline for it.

However it will help us prioritise the issue if you elaborate your use case of using tin-terrain: What is the industry you are in? What is your final deliverable you are trying to achieve?

BTolputt commented 5 years ago

Is there a way to contact you privately? Whilst not "super secret" by any means, I don't yet have permission from my bosses to share details to the public, but I can talk to you directly about it. @delfrrr @fester

delfrrr commented 5 years ago

@BTolputt of course, we will just create a little anonymous survey and I will send you a link. We do not expect/require any secret details!

BTolputt commented 5 years ago

@delfrrr No problems... though I haven't seen it yet, happy to fill one out and talk to you privately about all the technical details :)

BTolputt commented 5 years ago

@delfrrr @fester - has the survey been sent (& I've missed it) or is it still to be done?

fester commented 5 years ago

Hey @BTolputt and @frodrigo , thank you for your patience and finally I'm glad to inform you that there is a way to add normals to the terrain. However, there are some shortcomings:

  1. I added it and tried to follow spec as close as possible but due to various reasons never checked whether written values make sense. In best case this will happen somewhen next week.
  2. There is a well grounded suspiction that surface normals will not be smooth across tile edges, therefore there will be an edge visible. However, because of #1 I could not check it myself.

Because this feature begs for testing, I didn't merge it into master yet but you can check it out from terrain-normals.fester branch. If you have some spare time and a will to help the project, I kindly ask you to give it a go and share whether it works well (i.e. whether the orientation of normals is correct, are there shading problem across tile borders, etc).

In order to try this feature, you need to check out the aforementioned branch, follow normal build instructions and add '--normals' CLI switch to dem2tintiles subcommand.

frodrigo commented 5 years ago

Just starting to look at. I see you have indentation problem in you new code.

fester commented 5 years ago

Whoa, my Emacs rebelled against my will. Fixed, thanks for pointing it out.

BTolputt commented 5 years ago

@fester - thanks for that. Missed it over our long weekend down here but will test tomorrow.

BTolputt commented 5 years ago

@fester - Tried the new code, but cannot tell whether it works or not as I am having the issue referred to here. Once I get that cleared up, I'll report back with details on the normals. Apologies.

delfrrr commented 5 years ago

@BTolputt hey! finally put survey together. It would help us if you check it out (it is fully anonymous) https://survey.research-feedback.com/jfe/form/SV_6qVft7VrQfwsDml

BTolputt commented 5 years ago

@delfrrr - thanks mate. Hope my input helps :)

frodrigo commented 5 years ago

My test fails. But not sure what wrong, if it's about the top level tiles, bad normals or something else. My tests result in black only.

fester commented 5 years ago

Thanks for trying, we will investigate the problem ourselves and try to find an issue. It will be very helpful for us if you could publish your code or describe your setup.

On Fri 8 Feb 2019, 16:09 Frédéric Rodrigo, notifications@github.com wrote:

My test fails. But not sure what wrong, if it's about the top level tiles, bad normals or something else. My tests result in black only.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/heremaps/tin-terrain/issues/48#issuecomment-461833132, or mute the thread https://github.com/notifications/unsubscribe-auth/AAANeNZOJ01IXKCn0_HMZFxVU2XBVTaTks5vLZMZgaJpZM4ZmYMW .

fester commented 5 years ago

@frodrigo We checked it on our side and it works as expected. Probably you see nothing because Cesium can't handle missing tiles all zoomlevels. You can find more details in this comment.

Unfortunately, normals generated with tin-terrain are not smooth because of how we perform tiling and therefore tile borders are visible. Fixing this requires a different approach which I am thinking about now. It's outside this issue and will close it for now.