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

Better ui for Blobs, especially those representing images #7128

Merged
merged 13 commits into from
Aug 10, 2024

Conversation

emilk
Copy link
Member

@emilk emilk commented Aug 9, 2024

What

The UI for a Blob now shows the size and guessed MIME type of it.

There is a button to save it to disk (or download, on web).
The file extension is guessed from the guessed MIME type.

If it is an image, it will be shown.

Single-row

image

Tooltip

image

Selection panel

image

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested the web demo (if applicable):
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
  • If applicable, add a new check to the release checklist!
  • If have noted any breaking changes to the log API in CHANGELOG.md and the migration guide

To run all checks from main, comment on the PR with @rerun-bot full-check.

@emilk emilk added ui concerns graphical user interface 📺 re_viewer affects re_viewer itself include in changelog labels Aug 9, 2024
@emilk emilk marked this pull request as ready for review August 9, 2024 14:46
Comment on lines 23 to 25
// We ignore the logged `MediaType` component, because the user is looking
// at the blob specifically, not the entity as a whole!
let media_type = MediaType::guess_from_data(self);
Copy link
Member

Choose a reason for hiding this comment

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

Treating MediaType as an implied data-dependency for Blob and querying it here seems preferable to just guessing. This seems inline with the thing we will need to do with ImageBuffer anyways.

Copy link
Member Author

Choose a reason for hiding this comment

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

Showing the actual mime of the actual blob has the advantage that it is true. That is: it can help a user debug if they accidentally log a .png image with a image/jpeg media-type.

In general I would prefer it if we did as few of these magical lookups as possible, and instead show the user what they select. If they click on specific component, then only show data about that component!

For Blob, the guessed media type should work for all common image formats anyways.


For ImageBuffer we do need the actually logged ImageFormat though, but then we should make that clear that the shown image is inferred from a sibling component, or we show the image when the entity is selected.

@emilk
Copy link
Member Author

emilk commented Aug 10, 2024

Testing this on web, I discovered a dormant bug:

@emilk emilk merged commit 719da26 into main Aug 10, 2024
34 of 35 checks passed
@emilk emilk deleted the emilk/blob-preview branch August 10, 2024 09:37
abey79 added a commit that referenced this pull request Aug 11, 2024
Conflict between #7071 and #7128
@abey79 abey79 mentioned this pull request Aug 11, 2024
6 tasks
abey79 added a commit that referenced this pull request Aug 11, 2024
### What

Main broken by #7128, which missed an updated to code recently
introduced (#7071)

### 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 the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/7139?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/7139?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!
* [x] If have noted any breaking changes to the log API in
`CHANGELOG.md` and the migration guide

- [PR Build Summary](https://build.rerun.io/pr/7139)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
include in changelog 📺 re_viewer affects re_viewer itself ui concerns graphical user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants