Skip to content

re_viewer welcome screen code cleanup#8784

Closed
brody2consult wants to merge 4 commits intorerun-io:mainfrom
brody2consult:re_viewer-welcome-code-cleanup
Closed

re_viewer welcome screen code cleanup#8784
brody2consult wants to merge 4 commits intorerun-io:mainfrom
brody2consult:re_viewer-welcome-code-cleanup

Conversation

@brody2consult
Copy link
Copy Markdown
Contributor

Related

What

refactoring to extract welcome doc button text & color into constants

probably should have been part of PR #8745 - I didn't want to slow it down any further

OPEN QUESTION:

I considered proposing this kind of cleanup throughout the re_viewer code. From a quick look through this module gives me a serious doubt: crates/viewer/re_viewer/src/ui/memory_panel.rs

I would love to find a way to make a more data-driven approach for functions like MemoryPanel::gpu_stats & MemoryPanel::caches_stats.

Maybe best to close & reject this one for now.

Comment thread crates/viewer/re_viewer/src/ui/welcome_screen/welcome_section.rs Outdated
@brody2consult brody2consult force-pushed the re_viewer-welcome-code-cleanup branch from 182ec42 to 70c60ce Compare January 24, 2025 17:11
Comment thread crates/viewer/re_viewer/src/ui/welcome_screen/welcome_section.rs Outdated
…S NOT COMPILE as hex_color macro offers RGBA option that requires non-const Color32::from_rgba_unmultiplied fn
Comment on lines +14 to +21
macro_rules! color_from_rgb_hex {
($hex:literal) => {
match color_hex::color_from_hex!($hex).as_slice() {
[a, b, c] => Color32::from_rgb(*a, *b, *c),
_ => panic!("XXX"),
}
};
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think this should be defined in ecolor, I should be able to propose this over the weekend.

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.

There already is an ecolor::hex_color! macro: https://docs.rs/ecolor/latest/ecolor/macro.hex_color.html

Comment on lines +48 to +49
# XXX TODO FIX THIS IMPORT:
color-hex = { version = "*", default-features = false }
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I would like to remove this in favor of defining the needed macro in ecolor. As I said in the other comment, I should be able to do this over the weekend.

@emilk
Copy link
Copy Markdown
Member

emilk commented Jan 31, 2025

Closed by #8745

@emilk emilk closed this Jan 31, 2025
@brody2consult brody2consult deleted the re_viewer-welcome-code-cleanup branch January 31, 2025 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants