Skip to content

Update egui to latest master #9103

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

Merged
merged 33 commits into from
May 8, 2025
Merged

Conversation

lucasmerlin
Copy link
Contributor

@lucasmerlin lucasmerlin commented Feb 21, 2025

Related

What

I was curious how my work on egui menus and popups would work in rerun, so I gave it a try.

Findings so far

  • The new tooltip positioning works well unless we scroll in the scroll area. Maybe we should intersect the widget rect with the clip rect. Fixed!
Screen.Recording.2025-02-21.at.09.30.39.mov

Copy link

github-actions bot commented Feb 21, 2025

Web viewer built successfully. If applicable, you should also test it:

  • I have tested the web viewer
Result Commit Link Manifest
c0e5a14 https://rerun.io/viewer/pr/9103 +nightly +main

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

@lucasmerlin lucasmerlin force-pushed the lucas/egui-popup-modal-update branch from 9b40cb7 to 200768b Compare February 25, 2025 09:33
emilk added a commit to emilk/egui that referenced this pull request Feb 25, 2025
In Rerun, pressing `Cmd+S` brings up a save dialog using `rfd`, but we
get not key-up event for the `S` key (in winit).
This leads to `S` being mistakenly marked as down when we switch back to
the app.

This PR takes the safe route and marks all keys as up when an egui app
loses focus.

* Tested with rerun-io/rerun#9103
@lucasmerlin lucasmerlin force-pushed the lucas/egui-popup-modal-update branch 2 times, most recently from 2ce720e to 7284f6b Compare February 28, 2025 08:54
@lucasmerlin
Copy link
Contributor Author

lucasmerlin commented Feb 28, 2025

This seems to break resizing of panels, probably an egui bug?

Fixed!

@lucasmerlin lucasmerlin added dependencies concerning crates, pip packages etc egui Requires egui/eframe work labels Mar 3, 2025
@lucasmerlin lucasmerlin force-pushed the lucas/egui-popup-modal-update branch from b52f513 to 98266f5 Compare March 13, 2025 17:18
@lucasmerlin
Copy link
Contributor Author

# Conflicts:
#	crates/viewer/re_redap_browser/src/add_server_modal.rs
#	crates/viewer/re_selection_panel/src/view_entity_picker.rs
#	crates/viewer/re_ui/src/modal.rs
#	crates/viewer/re_view_dataframe/src/view_query/ui.rs
@lucasmerlin
Copy link
Contributor Author

The image loader now being async makes tests that have images (so basically all rerun tests) flakey 🙁 Maybe Harness::run could somehow check if there are any in-progress images and automatically do the run_realtime thing (where it calls thread::sleep between each frame)?

@lucasmerlin lucasmerlin added exclude from changelog PRs with this won't show up in CHANGELOG.md include in changelog and removed include in changelog labels Apr 24, 2025
@lucasmerlin
Copy link
Contributor Author

Also, basically all snapshots containing text have changed. Seems like text is slightly less strong now. Wondering if this was caused by this? emilk/egui#5824

@emilk
Copy link
Member

emilk commented Apr 24, 2025

Text rendering changed in

So it is expected that all screenshots with text changes.

The text should be more crisp and clear now though

@emilk
Copy link
Member

emilk commented Apr 24, 2025

The image loader now being async makes tests that have images (so basically all rerun tests) flakey 🙁 Maybe Harness::run could somehow check if there are any in-progress images and automatically do the run_realtime thing (where it calls thread::sleep between each frame)?

Or we disable the background loading in tests, somehow (making them blocking instead)

Copy link
Member

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Nice

@@ -59,7 +59,7 @@ fn test_help() {

harness.get_by_label("❓").hover();

harness.run();
harness.try_run_realtime().ok();
Copy link
Member

Choose a reason for hiding this comment

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

Why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Snapshot wouldn't include the icons otherwise. Not sure why this is necessary after making images load blockingly in tests, maybe that doesn't work as expected 🤔

@lucasmerlin
Copy link
Contributor Author

lucasmerlin commented Apr 30, 2025

Clicking links in egui_commonmark is broken 😔:

Screen.Recording.2025-04-30.at.14.46.56.mov

Edit: Fix: emilk/egui#6905

@lucasmerlin
Copy link
Contributor Author

I've added snapshot_with_broken_pixels which takes a count of broken pixels, with the following benefits over snapshot_with_broken_pixels_threshold

  • no need to calculate the num_pixels
  • can directly use the diff value from a failed ci job
  • if even a single pixel more changes in a future update the test is ensured to fail

# Conflicts:
#	crates/viewer/re_view_dataframe/src/view_query/ui.rs
lucasmerlin added 10 commits May 6, 2025 17:40
# Conflicts:
#	crates/viewer/re_time_panel/tests/snapshots/focused_item_is_focused.png
#	crates/viewer/re_time_panel/tests/snapshots/various_entity_kinds_timeline_a_0.png
#	crates/viewer/re_time_panel/tests/snapshots/various_entity_kinds_timeline_a_5.png
#	crates/viewer/re_time_panel/tests/snapshots/various_entity_kinds_timeline_a_9223372036854775807.png
#	crates/viewer/re_time_panel/tests/snapshots/various_entity_kinds_timeline_b_0.png
#	crates/viewer/re_time_panel/tests/snapshots/various_entity_kinds_timeline_b_5.png
#	crates/viewer/re_time_panel/tests/snapshots/various_entity_kinds_timeline_b_9223372036854775807.png
#	crates/viewer/re_time_panel/tests/snapshots/various_filters-_path_to,_rig.png
#	crates/viewer/re_time_panel/tests/snapshots/various_filters-_to_the.png
#	crates/viewer/re_time_panel/tests/snapshots/various_filters-_to_the_.png
#	crates/viewer/re_time_panel/tests/snapshots/various_filters-ath,left.png
#	crates/viewer/re_time_panel/tests/snapshots/various_filters-ath,t.png
#	crates/viewer/re_time_panel/tests/snapshots/various_filters-none.png
#	crates/viewer/re_time_panel/tests/snapshots/various_filters-path.png
#	crates/viewer/re_time_panel/tests/snapshots/various_filters-t.png
#	crates/viewer/re_time_panel/tests/snapshots/various_filters-to_the,oid.png
#	crates/viewer/re_time_panel/tests/snapshots/various_filters-to_the.png
#	crates/viewer/re_time_panel/tests/snapshots/various_filters-to_the_.png
#	crates/viewer/re_time_panel/tests/snapshots/various_filters-void.png
# Conflicts:
#	crates/viewer/re_selection_panel/src/defaults_ui.rs
#	crates/viewer/re_selection_panel/src/visualizer_ui.rs
#	crates/viewer/re_view/src/view_property_ui.rs
@lucasmerlin lucasmerlin merged commit 3644768 into main May 8, 2025
37 checks passed
@lucasmerlin lucasmerlin deleted the lucas/egui-popup-modal-update branch May 8, 2025 08:17
lucasmerlin added a commit that referenced this pull request May 8, 2025
### What

#9103 broke the notification popup, this is the fix. Seems like
`view_space_origin_widget_editing_ui` was also broken, but I'm not sure
where that ui is.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies concerning crates, pip packages etc egui Requires egui/eframe work exclude from changelog PRs with this won't show up in CHANGELOG.md
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants