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.

Sign up for GitHub

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

Jump to bottom

Automatically determine image/tensor color mapping & need for sRGB decoding #2342

Merged
merged 13 commits into from
Jun 12, 2023
Merged

Automatically determine image/tensor color mapping & need for sRGB decoding #2342

merged 13 commits into from
Jun 12, 2023

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented Jun 8, 2023 •
edited by github-actions bot
Loading

What

Fixes #2274

Will need to be further worked on via:

Introduces 3 new things actually:

  • finite range, determined if the range of a tensor was in fact not finite
  • heuristic for color mapping range
  • heuristic for srgb
    • ⚠️ does this break things?

Range & sRGB:

This

import rerun as rr
import numpy as np
import matplotlib.pyplot as plt

rr.init("image color", spawn=True)
img = np.random.random((300, 300, 3)) * 0.2
rr.log_image("image", img)
print(img.min(), img.max())  # to check they are in range [0,1]

plt.imshow(img)
plt.show()

Gives now the expected image:
image

When setting a pixel to inf, we show the "finite range" as well, and behave correctly:
image

Checklist

PR Build Summary: https://build.rerun.io/pr/2342

Docs preview: https://rerun.io/preview/a1011c1/docs
Examples preview: https://rerun.io/preview/a1011c1/examples

@Wumpf Wumpf added enhancement New feature or request 🔺 re_renderer affects re_renderer itself 📺 re_viewer affects re_viewer itself labels Jun 8, 2023
@Wumpf
Copy link
Member Author

Wumpf commented Jun 8, 2023

python ./examples/python/api_demo/main.py --demo image_tensors
before:
image

after:
image

looks the same which is encouraging

@emilk emilk self-requested a review June 9, 2023 06:34
@emilk
Copy link
Member

emilk commented Jun 9, 2023

python ./examples/python/api_demo/main.py --demo image_tensors before: image

after: image

looks the same which is encouraging

I find it surprising and discouraging that the changes in this PR seems to have no effect.

On of the central parts of the heuristic in this PR is that if the color values are in the [0,255] range, we assume they are in gamma-sapce. The image_tensors test takes a u8 image and then cast it to u16, u32, f32, etc. So the range should stay the same, and the heuristic should assume we need to apply the gamma correction, no?

Look at img_rgb_uint8 - it has the gamma-curve applied, giving that darker red look that is correct. Shouldn't we get that same correct color for all the unsigned and floating point types?

emilk
emilk requested changes Jun 9, 2023
View reviewed changes
Copy link
Member

@emilk emilk left a comment

Choose a reason for hiding this comment

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

I think the heuristics are good (except for it being RGB(A) only), but the I would expect it to have some effect on our tests. In particular, I would expect it would make all float and unsigned types in ./examples/python/api_demo/main.py --demo image_tensors have the correct gamma.

crates/re_renderer/shader/rectangle_fs.wgsl Outdated Show resolved Hide resolved
crates/re_renderer/shader/rectangle_fs.wgsl Outdated Show resolved Hide resolved
crates/re_viewer_context/src/gpu_bridge/mod.rs Outdated Show resolved Hide resolved
crates/re_viewer_context/src/gpu_bridge/mod.rs Outdated Show resolved Hide resolved
crates/re_viewer_context/src/gpu_bridge/mod.rs Outdated Show resolved Hide resolved
crates/re_viewer_context/src/tensor/tensor_stats.rs Outdated Show resolved Hide resolved
@Wumpf
Copy link
Member Author

Wumpf commented Jun 9, 2023

Much much better now! Needed quite a few more changes, but I'm happy with those now
image

@Wumpf Wumpf requested a review from emilk June 9, 2023 10:59
emilk
emilk approved these changes Jun 9, 2023
View reviewed changes
Copy link
Member

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Looks good, but please make sure you test it with depth maps and tensor view before merging!

@Wumpf Wumpf added the do-not-merge Do not merge this PR label Jun 9, 2023
@Wumpf
Copy link
Member Author

Wumpf commented Jun 9, 2023 •
edited
Loading

oh. yes. haven't done that again with the latest changes!

@Wumpf Wumpf removed the do-not-merge Do not merge this PR label Jun 12, 2023
@Wumpf
Copy link
Member Author

Wumpf commented Jun 12, 2023 •
edited
Loading

looked all good, didn't find any regressions related to this

@Wumpf Wumpf merged commit 15cb22c into main Jun 12, 2023
@Wumpf Wumpf deleted the andreas/better-image-range-heuristic branch June 12, 2023 11:03
emilk pushed a commit that referenced this pull request Jun 15, 2023
@Wumpf @emilk
Automatically determine image/tensor color mapping & need for srgb de…
2843e9c 
…coding (#2342)

<!--
Open the PR up as a draft until you feel it is ready for a proper
review.

Do not make PR:s from your own `main` branch, as that makes it difficult
for reviewers to add their own fixes.

Add any improvements to the branch as new commits to make it easier for
reviewers to follow the progress. All commits will be squashed to a
single commit once the PR is merged into `main`.

Make sure you mention any issues that this PR closes in the description,
as well as any other related issues.

To get an auto-generated PR description you can put "copilot:summary" or
"copilot:walkthrough" anywhere.
-->

Fixes #2274
* #2274

Will need to be further worked on via:
* #2341

Introduces 3 new things actually:
* finite range, determined if the range of a tensor was in fact not
finite
* heuristic for color mapping range
* heuristic for srgb
   * ⚠️ does this break things?

Range & sRGB:

This
```
import rerun as rr
import numpy as np
import matplotlib.pyplot as plt

rr.init("image color", spawn=True)
img = np.random.random((300, 300, 3)) * 0.2
rr.log_image("image", img)
print(img.min(), img.max())  # to check they are in range [0,1]

plt.imshow(img)
plt.show()
```
Gives now the expected image:

![image](https://github.com/rerun-io/rerun/assets/1220815/9fcda125-f762-4fd4-ae24-ce6dcaaa496a)

When setting a pixel to inf, we show the "finite range" as well, and
behave correctly:
<img width="1026" alt="image"
src="https://github.com/rerun-io/rerun/assets/1220815/7d3037e8-f562-4b6c-8f08-ec4f5627c21a">

* [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)

<!-- This line will get updated when the PR build summary job finishes.
-->
PR Build Summary: https://build.rerun.io/pr/2342

<!-- pr-link-docs:start -->
Docs preview: https://rerun.io/preview/93697e6/docs
Examples preview: https://rerun.io/preview/93697e6/examples
<!-- pr-link-docs:end -->
@emilk emilk mentioned this pull request Jun 15, 2023
Closed
3 tasks
@emilk emilk changed the title Automatically determine image/tensor color mapping & need for srgb decoding Automatically determine image/tensor color mapping & need for sRGB decoding Jun 15, 2023
@emilk emilk mentioned this pull request Jun 15, 2023
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emilk emilk emilk approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request 🔺 re_renderer affects re_renderer itself 📺 re_viewer affects re_viewer itself
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Image viewer normalizes (?) the colors and popup shows unnormalized RGB
2 participants
@Wumpf @emilk