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 Quantization #3225

Merged
merged 46 commits into from
Dec 2, 2015
Merged

Terrain Quantization #3225

merged 46 commits into from
Dec 2, 2015

Conversation

bagnell
Copy link
Contributor

@bagnell bagnell commented Nov 19, 2015

Opening for early review.There will be a lot of reorganization and generalization for other geometry rendered RTC.

  • Add quantized mesh skirts
  • Fix compression with normals
  • Add compression to meshes created from heightmaps
  • Remove all of the uniforms. Only a matrix multiply is needed to undo the scale and bias.
  • Update picking commands
  • Update ray-terrain mesh intersection tests.
  • Performance
  • Tests
  • Doc

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 19, 2015

@mlimper this work is inspired by our conversation at SIGGRAPH about WEB3D_quantized_attributes. We are also going to implement that glTF extension.

* @namespace
* @alias TerrainCompression
*/
var TerrainCompression = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be @private?

*/
var TerrainMesh = function TerrainMesh(center, vertices, indices, minimumHeight, maximumHeight, boundingSphere3D, occludeePointInScaledSpace, vertexStride, orientedBoundingBox) {
var TerrainMesh = function TerrainMesh(center, vertices, indices, minimumHeight, maximumHeight, boundingSphere3D, occludeePointInScaledSpace, vertexStride, orientedBoundingBox, encoding) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this technically a breaking change since TerrainMesh is public?

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 19, 2015

Looks good so far. Reasonably clean.

@bagnell
Copy link
Contributor Author

bagnell commented Nov 19, 2015

Reasonably clean.

I moved all of the quantization and packing to TerrainEncoding I was hoping it could be more general, but its terrain specific only because of the height component.

@mlimper
Copy link
Contributor

mlimper commented Nov 20, 2015

@mlimper this work is inspired by our conversation at SIGGRAPH about WEB3D_quantized_attributes. We are also going to implement that glTF extension.

Great, happy to hear that. Let me know if you discover things that could be improved in the extension.

@bagnell
Copy link
Contributor Author

bagnell commented Nov 30, 2015

@pjcozzi This is ready for another review.

@bagnell
Copy link
Contributor Author

bagnell commented Nov 30, 2015

For the same views as in the fog Sandcastle example:

memory high alt low alt
compressed CPU 224,368K 231,200K
uncompressed CPU 428,552K 426,096K
compressed GPU 216,600K 189,776K
uncompressed GPU 252,024K 192,020K

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 30, 2015

Update CHANGES.md for 1.17 including the removed classes (that we'll deprecate in 1.16).

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 30, 2015

For the same views as in the fog Sandcastle example:

"uncompressed" is before, and "compressed" is after, right?

The CPU results are, well, amazing.

Do you think the low altitude GPU results are suspect since there is such a small difference?

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 30, 2015

Seems like we get a lot more cracking. Zoom into San Francisco. See the dotted black lines in the water?

image

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 30, 2015

That is with terrain.

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 30, 2015

Here is just off the coast in South Beach without terrain:

ok

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 30, 2015

If we can't track this down, worse case, I guess we can render the boundary (and/or perhaps skirts) with full precision. Or maybe we use double the required precision (if it turns out to still be smaller overall) so that adjacent tile boundaries at the same LOD are guaranteed to match up. Would need to think more about adjacent tiles at different LODs.

@bagnell
Copy link
Contributor Author

bagnell commented Nov 30, 2015

The CPU results are, well, amazing.

They are a little better than I expected but in the same area since the number of bits per vertex was reduced by half.

Do you think the low altitude GPU results are suspect since there is such a small difference?

I think the GPU results are much closer because the textures are stored there and aren't kept on the CPU.

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 30, 2015

I think the GPU results are much closer because the textures are stored there and aren't kept on the CPU.

Ah yes. You could also test with Natural Earth to see, but we would not publish those numbers since they won't be a typical case.

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 30, 2015

The boundary artifact is the only issue. This is ready (post 1.16) otherwise.

@bagnell
Copy link
Contributor Author

bagnell commented Nov 30, 2015

Here is just off the coast in South Beach without terrain

That is just the imagery. The same thing happens in master.

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 30, 2015

Here is just off the coast in South Beach without terrain

That is just the imagery. The same thing happens in master.

Ah, OK. Can you reproduce the issue in SF?

@bagnell
Copy link
Contributor Author

bagnell commented Nov 30, 2015

Fixes #3265.

@bagnell
Copy link
Contributor Author

bagnell commented Dec 2, 2015

@pjcozzi The cracking issue with terrain was fixed. This is ready for another look.

@pjcozzi
Copy link
Contributor

pjcozzi commented Dec 2, 2015

Please don't forget the blog post and final memory numbers.

@pjcozzi
Copy link
Contributor

pjcozzi commented Dec 2, 2015

Looks good!

pjcozzi added a commit that referenced this pull request Dec 2, 2015
@pjcozzi pjcozzi merged commit 2ab93e0 into master Dec 2, 2015
@pjcozzi pjcozzi deleted the quantization branch December 2, 2015 23:53
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.

4 participants