Fix colormap picker preview & show it for depth point clouds as well #3373
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Wumpf
added
🪳 bug
emilk
reviewed
Sep 20, 2023
emilk
reviewed
Sep 20, 2023
|
(Speaking as a color n00b) shouldn't we fix the gamma here? That grayscale isn't much of a scale 😬 |
3 tasks
Wumpf
added a commit
that referenced
this pull request
Sep 21, 2023
### What Pointed out in #3373 that our grey colormap looked broken in the preview. When diving into it I got at first a scare about our color space pipeline being broken, but eventually I concluded that it was only the gray colormap which was broken. Grey before/after with reference gradient from gimp:  It's not perfectly equal to the reference, but then again the test wasn't executed with maximum vigor either. Some slight differences are to be expected due to the amount of conversions we need to go through to arrive at the final image. (unchanged) turbo colormap gradient compared with the render from Matplot webpage (a bit sloppy but shows that we're definitely not some wild gamma converstion away from it!): <img width="1248" alt="image" src="https://github.com/rerun-io/rerun/assets/1220815/1440e775-7b7c-4327-a6b0-dadbf8e78a6f"> This affects all usages of the greyscale colormap. ### Checklist * [x] I have read and agree to [Contributor Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and the [Code of Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md) * [x] I've included a screenshot or gif (if applicable) * [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/3391) (if applicable) - [PR Build Summary](https://build.rerun.io/pr/3391) - [Docs preview](https://rerun.io/preview/3eee011742a208f6f3d07353c67fb0f3059c6851/docs) <!--DOCS-PREVIEW--> - [Examples preview](https://rerun.io/preview/3eee011742a208f6f3d07353c67fb0f3059c6851/examples) <!--EXAMPLES-PREVIEW--> - [Recent benchmark results](https://ref.rerun.io/dev/bench/) - [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
Wumpf
commented
Sep 21, 2023
| @@ -116,7 +116,7 @@ impl egui_wgpu::CallbackTrait for ReRendererCallback { | |||
| return; | |||
| }; | |||
|
|
|||
| let screen_position = (info.clip_rect.min.to_vec2() * info.pixels_per_point).round(); | |||
| let screen_position = (info.viewport.min.to_vec2() * info.pixels_per_point).round(); | |||
There was a problem hiding this comment.
this is the actual bugfix!
emilk
approved these changes
Sep 21, 2023
| @@ -116,7 +116,7 @@ impl egui_wgpu::CallbackTrait for ReRendererCallback { | |||
| return; | |||
| }; | |||
|
|
|||
| let screen_position = (info.clip_rect.min.to_vec2() * info.pixels_per_point).round(); | |||
| let screen_position = (info.viewport.min.to_vec2() * info.pixels_per_point).round(); | |||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Checklist