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

Fix failing to preview small images #3520

Merged
merged 6 commits into from
Sep 29, 2023
Merged

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented Sep 28, 2023

What

Changed the way we deal with image previews. They now occupy always the requested size (previously, they'd expand to a max size) and the image displayed within may be stretched in rare situations.
Fixed-sized-ness of the preview image is an improvement in thus far as I think as it makes ui layouting a bit easier and more predictable, but I had to add a flag to not do so on hovering the preview since we would have a lot of empty space there otherwise.

Test code:

import rerun as rr
import numpy as np

rr.init("sliver image", spawn=True)

slow_gradient = np.arange(0, 1.0, 1.0 / 1000, dtype=float)
rr.log("image", rr.Image([slow_gradient]))

slow_gradient_rot = slow_gradient.reshape((slow_gradient.shape[0], 1))
rr.log("image2", rr.Image(slow_gradient_rot))

Before with hovering the preview:
horizontal:
image
vertical:
image
car:
image


After with hovering the preview:
horizontal:
image
vertical:
image
car:
image


I originally also wanted to tackle increasing the default zoom in the viewport. Had some initial success but found getting the zoom state correctly set in all situations too fiddely, so gave up on this for now. This aspect will be need to be re-visited anyways when we drop the scroll bars as long planned.

Checklist

@Wumpf Wumpf added 🪳 bug Something isn't working 📺 re_viewer affects re_viewer itself include in changelog labels Sep 28, 2023
@Wumpf Wumpf force-pushed the andreas/better-small-image-handling branch from 0100ab4 to 7ece3b0 Compare September 28, 2023 14:47
@emilk
Copy link
Member

emilk commented Sep 28, 2023

As visible, the cropping makes the larger previews ofc less truthful as well. The only other way to deal with this is stretching which may be acceptable in these cases

I think stretching to at least on ui point wide/high makes perfect sense, and much better than cropping. You want to see this thin gradients, or whatever they are.

@Wumpf
Copy link
Member Author

Wumpf commented Sep 28, 2023

for gradients it's the obvious thing, but we may also run into this with images that aren't gradient, they just happen to haven an awful aspect ratio

@Wumpf
Copy link
Member Author

Wumpf commented Sep 28, 2023

🤔 but maybe stretching to the min size might be the better call. After all the min size is still tiny, you won't notice that it's stretched no matter the content

@Wumpf
Copy link
Member Author

Wumpf commented Sep 28, 2023

okay let's try that instead. should be simpler code as well :/

@Wumpf Wumpf marked this pull request as draft September 28, 2023 14:53
crates/re_data_ui/src/image.rs Outdated Show resolved Hide resolved
crates/re_data_ui/src/image.rs Outdated Show resolved Hide resolved
@Wumpf Wumpf marked this pull request as ready for review September 28, 2023 15:38
@Wumpf
Copy link
Member Author

Wumpf commented Sep 28, 2023

updated screenshots with stretching behavior. much nicer

@Wumpf Wumpf requested a review from emilk September 28, 2023 15:43
crates/re_data_ui/src/image.rs Outdated Show resolved Hide resolved
crates/re_data_ui/src/image.rs Outdated Show resolved Hide resolved
crates/re_data_ui/src/image.rs Outdated Show resolved Hide resolved
@Wumpf
Copy link
Member Author

Wumpf commented Sep 29, 2023

cargo deny failure is a known issue

@Wumpf Wumpf merged commit b87588b into main Sep 29, 2023
26 of 28 checks passed
@Wumpf Wumpf deleted the andreas/better-small-image-handling branch September 29, 2023 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪳 bug Something isn't working include in changelog 📺 re_viewer affects re_viewer itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Viewer refuses to display images which have too large of an aspect ratio
2 participants