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

3D Tiles - Point Cloud color compression #4315

Merged
merged 1 commit into from
Sep 20, 2016
Merged

Conversation

lilleyse
Copy link
Contributor

For #3241 and CesiumGS/3d-tiles#22 (comment)

I tried out two different ways of compressing point cloud colors

  • RGB with 5/6/5 bits per channel (16 bit total, instead of 24 bit). G is given an extra bit in line with other compression techniques that recognize changes in green being more perceptible to our eyes.
  • YCoCg with 6/5/5 bits per channel (16 bit total). I'm not totally sure which component deserves the extra bit, but I gave it to the luminance instead of picking between the chroma channels.

Results:
http://cesium-dev.s3-website-us-east-1.amazonaws.com/cesium/color-compression/Apps/Sandcastle/?src=Hello%20World.html&label=Showcases&gist=29c700b58600b669fbbf9e30aeeac914

Overall I don't think YCoCg offers much benefit and may not be suited for this purpose. The RGB565 and YCoCg results are pretty similar, and conceptually RGB565 is simpler.

Note: once people have a chance to check out the demo and review the code I'm going to remove the Temp folder and add the point clouds to the main folder, as well as write a simple test. I'll squash the changes so we don't have the large point clouds still in the repo afterwards.

@pjcozzi
Copy link
Contributor

pjcozzi commented Sep 16, 2016

Overall I don't think YCoCg offers much benefit and may not be suited for this purpose. The RGB565 and YCoCg results are pretty similar, and conceptually RGB565 is simpler.

+1 for staying with the simple approach, especially given they are the same size and that RGB565 is a common WebGL format (texture, not attribute AFAIK; regardless, it is still familiar).

Did you just eyeball the difference or did you diff them or find any reading on perception to compare the two?

@pjcozzi
Copy link
Contributor

pjcozzi commented Sep 16, 2016

@hobu @connormanning RGB565 is an optional lossy compressed per-point color format that you may want to check out to store colors in 16 bits.

@@ -520,6 +532,16 @@ define([
if (hasColors) {
if (isTranslucent) {
vs += 'attribute vec4 a_color; \n';
} else if (isYCoCg || isRGB565) {
vs += 'attribute float a_color; \n' +
'const float SHIFT_RIGHT_11 = 1.0 / 2048.0; \n' +
Copy link
Contributor

Choose a reason for hiding this comment

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

@lilleyse @bagnell should we introduce built-in czm_ GLSL functions for shifts to basically extract a set of bits from a float?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel like passing on this for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, added to the roadmap. Good beginner low-hanging fruit.

@pjcozzi
Copy link
Contributor

pjcozzi commented Sep 16, 2016

Just those comments.

Let me know when this is ready with tests for review. Can you please also open a PR for the spec?

@lilleyse
Copy link
Contributor Author

Did you just eyeball the difference or did you diff them or find any reading on perception to compare the two?

Yeah, just ran my demo and compared the results visually. They both have banding, just in different ways.

@lilleyse
Copy link
Contributor Author

Removed ycocg and added a test. This is ready now. I'll update the spec.

@pjcozzi
Copy link
Contributor

pjcozzi commented Sep 20, 2016

Did you just eyeball the difference or did you diff them or find any reading on perception to compare the two?

Yeah, just ran my demo and compared the results visually. They both have banding, just in different ways.

I was hoping for something more scientific, but given that RGB565 is widely used in GL, I think it is appropriate. Other options can be added later, of course.

@pjcozzi
Copy link
Contributor

pjcozzi commented Sep 20, 2016

Spec update: CesiumGS/3d-tiles#130

@pjcozzi pjcozzi merged commit 41d472c into 3d-tiles Sep 20, 2016
@pjcozzi pjcozzi deleted the color-compression branch September 20, 2016 00:29
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.

2 participants