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

Show spinner icon while waiting for video decoder #7541

Merged
merged 5 commits into from
Sep 27, 2024

Conversation

emilk
Copy link
Member

@emilk emilk commented Sep 27, 2024

What

video-spinner-3

This uses the default egui::Spinner loading animation.

It plays it directly, even if there is only a single frame of delay in decoding. I thought this could be annoying, but I think it feels great, and it is a lot simpler than keeping track of for how long we've been waiting.

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 exclude from changelog PRs with this won't show up in CHANGELOG.md 🎞️ video labels Sep 27, 2024
@emilk emilk marked this pull request as ready for review September 27, 2024 12:53
@Wumpf Wumpf self-requested a review September 27, 2024 12:56
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

It plays it directly, even if there is only a single frame of delay in decoding. I thought this could be annoying, but I think it feels great, and it is a lot simpler than keeping track of for how long we've been waiting.

I concur, this is a lot better than expected. I even like it after playing around with it a bit!

Follow-up ticket for testing this (and .. everything else!) with 3D views: #7543
looking at the way we insert the 2d spinner here I'm skeptical this will work out of the box, but let's take this separately

@@ -174,4 +174,15 @@ impl VisualizerCollection {
) -> impl Iterator<Item = (ViewSystemIdentifier, &dyn VisualizerSystem)> {
self.systems.iter().map(|s| (*s.0, s.1.as_ref()))
}

/// Iterate over all visualizer data that can be downcast to the given type.
pub fn iter_visualizer_data<SpecificData: 'static>(
Copy link
Member

Choose a reason for hiding this comment

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

neat, liking this much better than the global method

let min = top_left_corner_position;
let max = top_left_corner_position + extent_u + extent_v;
let center = 0.5 * (min + max);
let diameter = (max - min).truncate().abs().min_element();
Copy link
Member

Choose a reason for hiding this comment

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

this likely breaks in 3D #7543

.. but I'm actually not even sure, gotta check how we place labels on 3D points. Likely we'll have to do a similar thing!

@emilk emilk merged commit d32aa44 into main Sep 27, 2024
36 of 37 checks passed
@emilk emilk deleted the emilk/video-load-animation branch September 27, 2024 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude from changelog PRs with this won't show up in CHANGELOG.md ui concerns graphical user interface 🎞️ video
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wait indicator for long video seeking operations
2 participants