Skip to content

Add Win32 GUI#802

Open
jrmuizel wants to merge 1 commit into
mstange:mainfrom
jrmuizel:gui
Open

Add Win32 GUI#802
jrmuizel wants to merge 1 commit into
mstange:mainfrom
jrmuizel:gui

Conversation

@jrmuizel
Copy link
Copy Markdown
Collaborator

@jrmuizel jrmuizel commented May 1, 2026

No description provided.

Copy link
Copy Markdown
Owner

@mstange mstange left a comment

Choose a reason for hiding this comment

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

Cool stuff, could you put a screenshot in the PR?

Comment thread samply/Cargo.toml Outdated
Comment on lines +123 to +124
[lints.rust]
unexpected_cfgs = { level = "warn", check-cfg = ['cfg(gui)'] }
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

What does this do? Also unnecessary space at line start

Comment thread samply/build.rs Outdated
@@ -0,0 +1,24 @@
fn main() {
#[cfg(windows)]
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This should be scoped to the new feature. Or, could we use https://crates.io/crates/embed-manifest instead?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Oh I misunderstood embed-manifest, that's also something you put in a build.rs. Ok so what you have here (with the feature check added) seems fine.

eprintln!("Profiling process with pid {pid}...");
eprintln!("Press Ctrl+C to stop.");
// TODO: Respect recording_props.time_limit, if specified
// Wait for Ctrl+C.
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The "Wait for Ctrl+C" comment is dropped

Comment thread samply/src/windows/gui.rs Outdated
const COLLAPSED_H: i32 = 140;
const EXPANDED_H: i32 = 315;

unsafe fn create_button(
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'm curious to see a version which uses https://crates.io/crates/winsafe to reduce the amount of unsafe here

Comment thread samply/src/windows/profiler.rs Outdated
recording_mode: RecordingMode,
recording_props: RecordingProps,
profile_creation_props: ProfileCreationProps,
stop_receiver: Option<std::sync::mpsc::Receiver<()>>,
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'd prefer a stop_condition: StopCondition arg which is either StopCondition::CtrlC or StopCondition::ReceiverSignaled(receiver)

@jrmuizel
Copy link
Copy Markdown
Collaborator Author

jrmuizel commented May 2, 2026

@mstange
Copy link
Copy Markdown
Owner

mstange commented May 3, 2026

Winsafe version is here: https://github.com/jrmuizel/samply/blob/winsafe-gui/samply/src/windows/gui.rs

I think that looks a lot better, what do you think?

@jrmuizel jrmuizel force-pushed the gui branch 10 times, most recently from 1256b4c to eb47863 Compare May 4, 2026 19:26
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