Skip to content

Remove frame lag when creating loop region#11862

Merged
emilk merged 1 commit into
mainfrom
emilk/remove-frame-selection-frame-lag
Nov 12, 2025
Merged

Remove frame lag when creating loop region#11862
emilk merged 1 commit into
mainfrom
emilk/remove-frame-selection-frame-lag

Conversation

@emilk

@emilk emilk commented Nov 11, 2025

Copy link
Copy Markdown
Member

This was already very noticeable at 60 Hz. My bad for only using 120Hz+ in my day-to-day

@emilk emilk added 📺 re_viewer affects re_viewer itself 📉 performance Optimization, memory use, etc include in changelog labels Nov 11, 2025
@github-actions

github-actions Bot commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

Web viewer built successfully.

Result Commit Link Manifest
cc54b60 https://rerun.io/viewer/pr/11862 +nightly +main

View image diff on kitdiff.

Note: This comment is updated whenever you push a commit.

@emilk emilk added the consider-patch PRs & issues that should be considered to be cherry-picked to a patch release branch. label Nov 12, 2025

@IsseW IsseW left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, also nice to factor out into a function

let selected_range = time_commands
.iter()
.rev()
.find_map(|c| {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That's a neat way to do this!

.iter()
.rev()
.find_map(|c| {
if let TimeControlCommand::SetLoopSelection(range) = c {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could possibly check for a RemoveLoopSelection here to also have that not be frame delayed. But not as important as setting it

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

More of a thought, not something I think needs any actions

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

True, but removal is much less sensitive to frame delays than dragging is, so I rather not add the complexity now

Comment on lines +261 to +272
// Use latest range to avoid frame delay
let selected_range = time_commands
.iter()
.rev()
.find_map(|c| {
if let TimeControlCommand::SetLoopSelection(range) = c {
Some(AbsoluteTimeRangeF::from(*range))
} else {
None
}
})
.or(time_ctrl.loop_selection())?;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wish there was a nicer way to query this out, feels hacky :/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think it's that hacky when you consider that time_commands will almost always be empty. Might be nice to have a function for this if similar pattern pops up more times though.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think the nice way to re-arrange this at some point is to put the time commands into ctx and then have helpers on ctx to get the latest values, including pending commands. Maybe.

Comment on lines +290 to +296
let corner_radius = tokens.normal_corner_radius();
let corner_radius = egui::CornerRadius {
nw: corner_radius,
ne: corner_radius,
sw: 0,
se: 0,
};

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

pre-existing, but personally I found the corner radius here a bit strange

Image Image

🤷 no strong opinion on it, but can't unsee now

@emilk emilk merged commit fe5a3e5 into main Nov 12, 2025
51 checks passed
@emilk emilk deleted the emilk/remove-frame-selection-frame-lag branch November 12, 2025 14:57
IsseW pushed a commit that referenced this pull request Nov 12, 2025
This was already very noticeable at 60 Hz. My bad for only using 120Hz+
in my day-to-day
@IsseW IsseW removed the consider-patch PRs & issues that should be considered to be cherry-picked to a patch release branch. label Nov 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

include in changelog 📉 performance Optimization, memory use, etc 📺 re_viewer affects re_viewer itself

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants