feat: add screen color eyedropper and data-URI tray icons#481
Conversation
Adds a cross-platform screen:pick-color capability (NSColorSampler on macOS, click-to-pick GDI on Windows, XDG portal with X11 fallback on Linux) and data:image/png;base64 support for extension tray icons, so extensions like a color picker can sample the screen and render a live swatch in the menu bar.
There was a problem hiding this comment.
Code Review
This pull request introduces a cross-platform screen color sampling (eyedropper) service with native implementations for macOS, Windows, and Linux, alongside support for base64-encoded PNG data URIs for tray icons. Feedback focuses on critical safety improvements for the Windows implementation, where low-level hooks could leak and swallow global input if the async task is cancelled; storing the thread ID and using a Drop guard to cancel the hooks is recommended. Additionally, casting variables to u64 in the X11 channel mask extraction is suggested to prevent potential integer overflow.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| static PICK_IN_PROGRESS: AtomicBool = AtomicBool::new(false); | ||
|
|
||
| thread_local! { | ||
| // LL hook procs run on the thread that installed them (inside its | ||
| // message pump), so a thread_local is enough to hand back the click. | ||
| static PICKED_POINT: Cell<Option<(i32, i32)>> = const { Cell::new(None) }; | ||
| } |
There was a problem hiding this comment.
Low-level Windows hooks (WH_MOUSE_LL and WH_KEYBOARD_LL) are global and run on the thread that installed them. If the calling future is dropped or cancelled, the blocking thread running the message loop will continue to run in the background, permanently swallowing the user's clicks and Esc keypresses globally. To prevent this, we should store the thread ID of the active hook thread and provide a cancel_pick function to post a WM_QUIT message to the thread's message queue.
| static PICK_IN_PROGRESS: AtomicBool = AtomicBool::new(false); | |
| thread_local! { | |
| // LL hook procs run on the thread that installed them (inside its | |
| // message pump), so a thread_local is enough to hand back the click. | |
| static PICKED_POINT: Cell<Option<(i32, i32)>> = const { Cell::new(None) }; | |
| } | |
| static PICK_IN_PROGRESS: AtomicBool = AtomicBool::new(false); | |
| static HOOK_THREAD_ID: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(0); | |
| thread_local! { | |
| // LL hook procs run on the thread that installed them (inside its | |
| // message pump), so a thread_local is enough to hand back the click. | |
| static PICKED_POINT: Cell<Option<(i32, i32)>> = const { Cell::new(None) }; | |
| } |
| fn run_pick_loop() -> Result<Option<PickedColor>, AppError> { | ||
| PICKED_POINT.with(|p| p.set(None)); | ||
|
|
||
| let mouse = unsafe { SetWindowsHookExW(WH_MOUSE_LL, Some(mouse_hook), None, 0) } | ||
| .map_err(|e| AppError::Platform(format!("failed to install mouse hook: {e}")))?; | ||
| let keyboard = match unsafe { SetWindowsHookExW(WH_KEYBOARD_LL, Some(keyboard_hook), None, 0) } | ||
| { | ||
| Ok(h) => h, | ||
| Err(e) => { | ||
| let _ = unsafe { UnhookWindowsHookEx(mouse) }; | ||
| return Err(AppError::Platform(format!( | ||
| "failed to install keyboard hook: {e}" | ||
| ))); | ||
| } | ||
| }; | ||
|
|
||
| unsafe { | ||
| let mut msg = MSG::default(); | ||
| while GetMessageW(&mut msg, None, 0, 0).into() { | ||
| let _ = TranslateMessage(&msg); | ||
| DispatchMessageW(&msg); | ||
| } | ||
| let _ = UnhookWindowsHookEx(mouse); | ||
| let _ = UnhookWindowsHookEx(keyboard); | ||
| } | ||
|
|
||
| match PICKED_POINT.with(|p| p.take()) { | ||
| Some((x, y)) => sample_pixel(x, y).map(Some), | ||
| None => Ok(None), // Esc — user cancelled | ||
| } | ||
| } |
There was a problem hiding this comment.
Update run_pick_loop to store the current thread ID before entering the message loop, clear it on exit, and implement a public cancel_pick function to allow other threads to cleanly interrupt the message loop.
fn run_pick_loop() -> Result<Option<PickedColor>, AppError> {
PICKED_POINT.with(|p| p.set(None));
let mouse = unsafe { SetWindowsHookExW(WH_MOUSE_LL, Some(mouse_hook), None, 0) }
.map_err(|e| AppError::Platform(format!("failed to install mouse hook: {e}")))?;
let keyboard = match unsafe { SetWindowsHookExW(WH_KEYBOARD_LL, Some(keyboard_hook), None, 0) }
{
Ok(h) => h,
Err(e) => {
let _ = unsafe { UnhookWindowsHookEx(mouse) };
return Err(AppError::Platform(format!(
"failed to install keyboard hook: {e}"
)));
}
};
unsafe {
HOOK_THREAD_ID.store(::windows::Win32::System::Threading::GetCurrentThreadId(), Ordering::SeqCst);
let mut msg = MSG::default();
while GetMessageW(&mut msg, None, 0, 0).into() {
let _ = TranslateMessage(&msg);
DispatchMessageW(&msg);
}
HOOK_THREAD_ID.store(0, Ordering::SeqCst);
let _ = UnhookWindowsHookEx(mouse);
let _ = UnhookWindowsHookEx(keyboard);
}
match PICKED_POINT.with(|p| p.take()) {
Some((x, y)) => sample_pixel(x, y).map(Some),
None => Ok(None), // Esc — user cancelled
}
}
pub fn cancel_pick() {
let tid = HOOK_THREAD_ID.swap(0, Ordering::SeqCst);
if tid != 0 {
unsafe {
let _ = PostThreadMessageW(
tid,
::windows::Win32::UI::WindowsAndMessaging::WM_QUIT,
WPARAM(0),
LPARAM(0),
);
}
}
}| #[cfg(target_os = "windows")] | ||
| { | ||
| let _ = app; | ||
| tauri::async_runtime::spawn_blocking(windows::pick_color_blocking) | ||
| .await | ||
| .map_err(|e| AppError::Platform(format!("color pick task failed: {e}")))? | ||
| } |
There was a problem hiding this comment.
Use a Drop guard to ensure that windows::cancel_pick() is called if the future is dropped or cancelled before the blocking task completes. This ensures the low-level Windows hooks are always cleanly uninstalled.
| #[cfg(target_os = "windows")] | |
| { | |
| let _ = app; | |
| tauri::async_runtime::spawn_blocking(windows::pick_color_blocking) | |
| .await | |
| .map_err(|e| AppError::Platform(format!("color pick task failed: {e}")))? | |
| } | |
| #[cfg(target_os = "windows")] | |
| { | |
| let _ = app; | |
| struct CancelGuard; | |
| impl Drop for CancelGuard { | |
| fn drop(&mut self) { | |
| windows::cancel_pick(); | |
| } | |
| } | |
| let _guard = CancelGuard; | |
| tauri::async_runtime::spawn_blocking(windows::pick_color_blocking) | |
| .await | |
| .map_err(|e| AppError::Platform(format!("color pick task failed: {e}")))? | |
| } |
| pub fn channel_from_mask(pixel: u32, mask: u32) -> u8 { | ||
| if mask == 0 { | ||
| return 0; | ||
| } | ||
| let raw = (pixel & mask) >> mask.trailing_zeros(); | ||
| let max = mask >> mask.trailing_zeros(); | ||
| ((raw * 255 + max / 2) / max) as u8 | ||
| } |
There was a problem hiding this comment.
In channel_from_mask, the calculation raw * 255 + max / 2 can overflow a u32 if the mask is large (e.g., a 32-bit mask where raw and max can be close to u32::MAX). To prevent any potential overflow or panic, cast raw and max to u64 before performing the arithmetic.
| pub fn channel_from_mask(pixel: u32, mask: u32) -> u8 { | |
| if mask == 0 { | |
| return 0; | |
| } | |
| let raw = (pixel & mask) >> mask.trailing_zeros(); | |
| let max = mask >> mask.trailing_zeros(); | |
| ((raw * 255 + max / 2) / max) as u8 | |
| } | |
| pub fn channel_from_mask(pixel: u32, mask: u32) -> u8 { | |
| if mask == 0 { | |
| return 0; | |
| } | |
| let raw = ((pixel & mask) >> mask.trailing_zeros()) as u64; | |
| let max = (mask >> mask.trailing_zeros()) as u64; | |
| (((raw * 255) + max / 2) / max) as u8 | |
| } |
Adds a cross-platform screen:pick-color capability (NSColorSampler on
macOS, click-to-pick GDI on Windows, XDG portal with X11 fallback on
Linux) and data:image/png;base64 support for extension tray icons, so
extensions like a color picker can sample the screen and render a
live swatch in the menu bar.