Skip to content

Commit

Permalink
Fix video loader spinner in 3D views (rerun-io#7544)
Browse files Browse the repository at this point in the history
### What
* Follow-up to rerun-io#7541 
* Closes rerun-io#7543

<img width="2398" alt="image"
src="https://github.com/user-attachments/assets/6f86614b-5eae-4f6d-a982-18cb877eae5d">
(image via @Wumpf : Wasn't able to catch the spinner because it
disappeared right away)

Test:
```
pixi run -e examples python3 tests/python/release_checklist/check_video.py
```

https://rerun.io/viewer/pr/7544?url=https%3A%2F%2Fstatic.rerun.io%2Frrd%2F0.19.0-alpha.1%2Bdev%2Fdata_dca3624dd9631b0208ab10257ec9495903699237.rrd

### 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/7544?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/7544?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/7544)
- [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`.
  • Loading branch information
emilk authored Oct 1, 2024
1 parent 68ccfcc commit 49fee63
Show file tree
Hide file tree
Showing 11 changed files with 143 additions and 26 deletions.
2 changes: 1 addition & 1 deletion crates/viewer/re_renderer/src/video/decoder/web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ impl VideoDecoder {

/// Reset the video decoder and discard all frames.
fn reset(&mut self) -> Result<(), DecodingError> {
re_log::debug!("Resetting video decoder.");
re_log::trace!("Resetting video decoder.");
if let Err(_err) = self.decoder.reset() {
// At least on Firefox, it can happen that reset on a previous error fails.
// In that case, start over completely and try again!
Expand Down
42 changes: 38 additions & 4 deletions crates/viewer/re_space_view_spatial/src/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,14 +272,48 @@ pub fn create_labels(
pub fn paint_loading_spinners(
ui: &egui::Ui,
ui_from_scene: egui::emath::RectTransform,
eye3d: &Eye,
visualizers: &re_viewer_context::VisualizerCollection,
) {
use glam::Vec3Swizzles as _;
use glam::Vec4Swizzles as _;

let ui_from_world_3d = eye3d.ui_from_world(*ui_from_scene.to());

for data in visualizers.iter_visualizer_data::<SpatialViewVisualizerData>() {
for &rect_in_scene in &data.loading_rects {
let rect_in_ui = ui_from_scene.transform_rect(rect_in_scene);
for &crate::visualizers::LoadingSpinner {
center,
half_extent_u,
half_extent_v,
} in &data.loading_spinners
{
// Transform to ui coordinates:
let center_unprojected = ui_from_world_3d * center.extend(1.0);
if center_unprojected.w < 0.0 {
continue; // behind camera eye
}
let center_in_scene: glam::Vec2 = center_unprojected.xy() / center_unprojected.w;

let mut radius_in_scene = f32::INFINITY;

// Estimate the radius so we are unlikely to exceed the projected box:
for radius_vec in [half_extent_u, -half_extent_u, half_extent_v, -half_extent_v] {
let axis_radius = center_in_scene
.distance(ui_from_world_3d.project_point3(center + radius_vec).xy());
radius_in_scene = radius_in_scene.min(axis_radius);
}

radius_in_scene *= 0.75; // Shrink a bit

let max_radius = 0.5 * ui_from_scene.from().size().min_elem();
radius_in_scene = radius_in_scene.min(max_radius);

let rect = egui::Rect::from_center_size(
egui::pos2(center_in_scene.x, center_in_scene.y),
egui::Vec2::splat(2.0 * radius_in_scene),
);

// Shrink slightly:
let rect = egui::Rect::from_center_size(rect_in_ui.center(), 0.75 * rect_in_ui.size());
let rect = ui_from_scene.transform_rect(rect);

egui::Spinner::new().paint_at(ui, rect);
}
Expand Down
2 changes: 1 addition & 1 deletion crates/viewer/re_space_view_spatial/src/ui_2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ impl SpatialSpaceView2D {
}

// Add egui-rendered spinners/loaders on top of re_renderer content:
crate::ui::paint_loading_spinners(ui, ui_from_scene, &system_output.view_systems);
crate::ui::paint_loading_spinners(ui, ui_from_scene, &eye, &system_output.view_systems);

// Add egui-rendered labels on top of everything else:
painter.extend(label_shapes);
Expand Down
1 change: 1 addition & 0 deletions crates/viewer/re_space_view_spatial/src/ui_3d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,7 @@ impl SpatialSpaceView3D {
crate::ui::paint_loading_spinners(
ui,
RectTransform::from_to(ui_rect, ui_rect),
&eye,
&system_output.view_systems,
);

Expand Down
14 changes: 14 additions & 0 deletions crates/viewer/re_space_view_spatial/src/visualizers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,20 @@ pub use utilities::{
UiLabel, UiLabelTarget,
};

/// Shows a loading animation in a spatial view.
///
/// Represents a 2D rectangle, oriented somewhere in scene coordinates.
#[derive(Clone, Copy, Debug)]
pub struct LoadingSpinner {
pub center: glam::Vec3,

/// The "radius" along one local axis.
pub half_extent_u: glam::Vec3,

/// The "radius" along the other local axis.
pub half_extent_v: glam::Vec3,
}

// ---

use ahash::HashMap;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
use re_log_types::EntityPathHash;

use super::UiLabel;
use crate::{view_kind::SpatialSpaceViewKind, PickableTexturedRect};
use crate::{view_kind::SpatialSpaceViewKind, visualizers::LoadingSpinner, PickableTexturedRect};

/// Common data struct for all spatial scene elements.
///
/// Each spatial scene element is expected to fill an instance of this struct with its data.
pub struct SpatialViewVisualizerData {
/// Loading icons/spinners shown using egui, in world/scene coordinates.
pub loading_rects: Vec<egui::Rect>,
pub loading_spinners: Vec<LoadingSpinner>,

/// Labels that should be shown using egui.
pub ui_labels: Vec<UiLabel>,
Expand All @@ -26,7 +26,7 @@ pub struct SpatialViewVisualizerData {
impl SpatialViewVisualizerData {
pub fn new(preferred_view_kind: Option<SpatialSpaceViewKind>) -> Self {
Self {
loading_rects: Default::default(),
loading_spinners: Default::default(),
ui_labels: Default::default(),
bounding_boxes: Default::default(),
pickable_rects: Default::default(),
Expand Down
15 changes: 6 additions & 9 deletions crates/viewer/re_space_view_spatial/src/visualizers/videos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use crate::{
contexts::SpatialSceneEntityContext,
ui::SpatialSpaceViewState,
view_kind::SpatialSpaceViewKind,
visualizers::{entity_iterator, filter_visualizable_2d_entities},
visualizers::{entity_iterator, filter_visualizable_2d_entities, LoadingSpinner},
PickableRectSourceData, PickableTexturedRect, SpatialSpaceView2D,
};

Expand Down Expand Up @@ -207,14 +207,11 @@ impl VideoFrameReferenceVisualizer {
VideoFrameTexture::Ready(texture) => texture,
VideoFrameTexture::Pending(placeholder) => {
// Show loading rectangle:
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();
self.data.loading_rects.push(egui::Rect::from_center_size(
egui::pos2(center.x, center.y),
egui::Vec2::splat(diameter),
));
self.data.loading_spinners.push(LoadingSpinner {
center: top_left_corner_position + 0.5 * (extent_u + extent_v),
half_extent_u: 0.5 * extent_u,
half_extent_v: 0.5 * extent_v,
});

// Keep polling for the decoded result:
ctx.viewer_ctx.egui_ctx.request_repaint();
Expand Down
2 changes: 1 addition & 1 deletion tests/assets/download_test_assets.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Downloads test assets used by tests.
Usage:
pixi run python ./tests/assets/download_assets.py
pixi run python tests/assets/download_test_assets.py
"""

from __future__ import annotations
Expand Down
9 changes: 5 additions & 4 deletions tests/python/release_checklist/README.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
![Rerun.io](https://user-images.githubusercontent.com/1148717/218142418-1d320929-6b7a-486e-8277-fbeef2432529.png)

# Interactive release checklist

Welcome to the release checklist.

_**⚠ Make sure to clean your blueprints if you want to start from a clean slate ⚠**_
Expand All @@ -10,8 +9,12 @@ _**⚠ Make sure to clean your blueprints if you want to start from a clean slat
pixi run rerun reset
```

### When releasing
Run the testlist with:
```
pixi run -e examples python tests/python/release_checklist/main.py
```

### When releasing
Each check comes in the form a recording that contains:
1. a markdown document specifying the user actions to be tested, and
2. the actual data required to test these actions.
Expand All @@ -23,8 +26,6 @@ If you've closed all of them, then things are in a releasable state.


### When developing


Every time you make a PR to add a new feature or fix a bug that cannot be tested via automated means
for whatever reason, take a moment to think: what actions did I take to manually test this, and should
these actions be added as a new check in the checklist?
Expand Down
62 changes: 62 additions & 0 deletions tests/python/release_checklist/check_video.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
from __future__ import annotations

import os
from argparse import Namespace
from uuid import uuid4

import rerun as rr

README = """\
# Video support
This test only works in the browser!
The video should show up both in 2D, 3D, and in the selection panel.
When moving the time cursor, a "loading" spinner should show briefly, while the video is seeking.
"""


def run(args: Namespace) -> None:
rr.script_setup(args, f"{os.path.basename(__file__)}", recording_id=uuid4())

rr.log("readme", rr.TextDocument(README, media_type=rr.MediaType.MARKDOWN), timeless=True)

# Log video asset which is referred to by frame references.
video_path = os.path.dirname(__file__) + "/../../../tests/assets/video/Big_Buck_Bunny_1080_10s_av1.mp4"
video_asset = rr.AssetVideo(path=video_path)
rr.log("world/cam", video_asset, static=True)

# Send automatically determined video frame timestamps.
frame_timestamps_ns = video_asset.read_frame_timestamps_ns()
rr.send_columns(
"world/cam",
# Note timeline values don't have to be the same as the video timestamps.
times=[rr.TimeNanosColumn("video_time", frame_timestamps_ns)],
components=[rr.VideoFrameReference.indicator(), rr.components.VideoTimestamp.nanoseconds(frame_timestamps_ns)],
)

rr.log(
"world/cam",
rr.Transform3D(
translation=[10, 0, 0],
),
static=True, # Static, so it shows up in the "video_time" timeline!
)

rr.log(
"world/cam",
rr.Pinhole(
resolution=[1920, 1080],
focal_length=1920,
),
static=True, # Static, so it shows up in the "video_time" timeline!
)


if __name__ == "__main__":
import argparse

parser = argparse.ArgumentParser(description="Interactive release checklist")
rr.script_add_args(parser)
args = parser.parse_args()
run(args)
14 changes: 11 additions & 3 deletions tests/python/release_checklist/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,17 @@
from __future__ import annotations

import argparse
import glob
import importlib
import subprocess
import sys
from os.path import basename, dirname, isfile, join
from pathlib import Path

import rerun as rr


def log_checks(args: argparse.Namespace) -> None:
import glob
import importlib

modules = glob.glob(join(dirname(__file__), "*.py"))
modules = [basename(f)[:-3] for f in modules if isfile(f) and basename(f).startswith("check_")]

Expand All @@ -29,6 +31,12 @@ def main() -> None:
rr.script_add_args(parser)
args = parser.parse_args()

# Download test assets:
download_test_assest_path = (
Path(__file__).parent.parent.parent.joinpath("assets/download_test_assets.py").absolute()
)
subprocess.run([sys.executable, download_test_assest_path])

log_checks(args)

# Log instructions last so that's what people see first.
Expand Down

0 comments on commit 49fee63

Please sign in to comment.