Skip to content

fix(ui-windows): DPI-scale layout intrinsics (refs #5884); unblock crate unit tests #6637

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.

Sign up for GitHub

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
proggeramlug merged 2 commits into from
Jul 20, 2026
Merged

fix(ui-windows): DPI-scale layout intrinsics (refs #5884); unblock crate unit tests #6637

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a4e7f1b
fix(ui-windows): DPI-scale layout intrinsics; make crate tests linkable
Jul 18, 2026
eb76567
Merge branch 'main' into fix/win-ui-dpi-intrinsics
proggeramlug Jul 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions crates/perry-ui-windows/src/app.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -889,6 +889,9 @@ pub fn app_set_vibrancy(app_handle: i64, value_ptr: *const u8) {
crate::dwm::set_attr_i32(hwnd, 20 /* DWMWA_USE_IMMERSIVE_DARK_MODE */, 1);

// Extend client area into frame for borderless mica/acrylic
// #[link] so the cargo-test harness link resolves too
// (the final app link adds dwmapi.lib itself).
#[link(name = "dwmapi")]
extern "system" {
fn DwmExtendFrameIntoClientArea(
hwnd: isize,
Expand Down
5 changes: 5 additions & 0 deletions crates/perry-ui-windows/src/dwm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ const DWMWCP_ROUND: i32 = 2;
/// silently ignore that, so requesting it unconditionally is safe.
const DWMSBT_MAINWINDOW: i32 = 2;

// `#[link]` so the import resolves for EVERY link of this crate — the final
// app link always adds dwmapi.lib explicitly, but the in-crate `cargo test`
// harness link did not, failing with LNK2019 and silently blocking the
// crate's whole unit-test suite from ever running.
#[link(name = "dwmapi")]
extern "system" {
fn DwmSetWindowAttribute(hwnd: isize, attr: u32, value: *const i32, size: u32) -> i32;
}
Expand Down
149 changes: 102 additions & 47 deletions crates/perry-ui-windows/src/layout.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ fn layout_stack(handle: i64, width: i32, height: i32, vertical: bool) {
.as_ref()
.is_some_and(|ci| matches!(ci.kind, WidgetKind::Picker | WidgetKind::Combobox))
{
h.max(28) + 200
h.max(dpi(28)) + dpi(200)
} else {
h
};
Expand Down Expand Up @@ -335,6 +335,30 @@ fn layout_navstack(handle: i64, width: i32, height: i32) {
}
}

/// Scale a logical (96-DPI) pixel constant by the current DPI factor.
///
/// The intrinsic control sizes below are authored at 96 DPI. Fonts, the
/// window size, stack spacing, and explicit setSize dimensions are all
/// scaled at their entry points (app.rs / widget_create.rs /
/// widget_layout_extras.rs) — but these layout-engine constants never
/// were, so on a 125–200% display every fixed-height control (TextField,
/// Toggle, Picker, …) clipped its own DPI-scaled font (the #5884 family
/// of bugs). Measured text extents are already physical pixels and must
/// NOT pass through this.
fn dpi(px: i32) -> i32 {
scale_by(px, crate::app::get_dpi_scale())
}

/// Pure scaling core, unit-testable without the process-global DPI state.
/// A scale of exactly 1.0 (or an unset/degenerate reading ≤ 1.0 — Windows
/// never scales below 100%) returns the constant unchanged.
fn scale_by(px: i32, scale: f64) -> i32 {
if scale <= 1.0 {
return px;
}
(px as f64 * scale).round() as i32
}

/// Measure the intrinsic size of a widget along the main axis.
fn measure_intrinsic(handle: i64, kind: &WidgetKind, vertical: bool, cross_size: i32) -> i32 {
// Check fixed dimensions first — they override intrinsic measurement
Expand All @@ -358,52 +382,56 @@ fn measure_intrinsic(handle: i64, kind: &WidgetKind, vertical: bool, cross_size:
}
}
if vertical {
20
dpi(20)
} else {
100
dpi(100)
}
}
WidgetKind::Button => {
#[cfg(target_os = "windows")]
{
if let Some(hwnd) = widgets::get_hwnd_safe(handle) {
let size = measure_text_height(hwnd, cross_size, vertical);
// Add padding: 8px vertical, 16px horizontal
return if vertical { size + 16 } else { size + 32 };
// Add padding: 8px vertical, 16px horizontal (logical)
return if vertical {
size + dpi(16)
} else {
size + dpi(32)
};
}
}
if vertical {
34
dpi(34)
} else {
100
dpi(100)
}
}
Comment on lines 390 to 408

@coderabbitai coderabbitai Bot Jul 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Clarify the padding comment vs. actual values.

The comment states "8px vertical, 16px horizontal (logical)" but the code adds dpi(16) and dpi(32) respectively — double the stated figures. This is likely intentional (per-side padding: 8px × 2 sides = 16, 16px × 2 sides = 32), but as written the comment invites a future contributor to "fix" the code to match the literal 8/16 values, silently halving button padding.

✏️ Suggested comment clarification 
-                    // Add padding: 8px vertical, 16px horizontal (logical)
+                    // Add padding (total, both sides): 16px vertical (8px top + 8px bottom),
+                    // 32px horizontal (16px left + 16px right), logical pixels.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
WidgetKind::Button => {
#[cfg(target_os = "windows")]
{
if let Some(hwnd) = widgets::get_hwnd_safe(handle) {
let size = measure_text_height(hwnd, cross_size, vertical);
// Add padding: 8px vertical, 16px horizontal
return if vertical { size + 16 } else { size + 32 };
// Add padding: 8px vertical, 16px horizontal (logical)
return if vertical {
size + dpi(16)
} else {
size + dpi(32)
};
}
}
if vertical {
34
dpi(34)
} else {
100
dpi(100)
}
}
WidgetKind::Button => {
#[cfg(target_os = "windows")]
{
if let Some(hwnd) = widgets::get_hwnd_safe(handle) {
let size = measure_text_height(hwnd, cross_size, vertical);
// Add padding (total, both sides): 16px vertical (8px top + 8px bottom),
// 32px horizontal (16px left + 16px right), logical pixels.
return if vertical {
size + dpi(16)
} else {
size + dpi(32)
};
}
}
if vertical {
dpi(34)
} else {
dpi(100)
}
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-ui-windows/src/layout.rs` around lines 390 - 408, Update the
padding comment in the WidgetKind::Button sizing branch to clarify that the code
adds 16px total vertical padding and 32px total horizontal padding, representing
8px and 16px per side respectively. Keep the existing dpi(16) and dpi(32)
calculations unchanged.

WidgetKind::TextField => {
if vertical {
30
dpi(30)
} else {
200
dpi(200)
}
}
WidgetKind::Toggle => {
if vertical {
24
dpi(24)
} else {
100
dpi(100)
}
}
WidgetKind::Slider => {
if vertical {
24
dpi(24)
} else {
200
dpi(200)
}
}
WidgetKind::Divider => {
if vertical {
2
dpi(2)
} else {
2
dpi(2)
}
}
WidgetKind::Spacer => 0, // handled separately
Expand All @@ -413,23 +441,23 @@ fn measure_intrinsic(handle: i64, kind: &WidgetKind, vertical: bool, cross_size:
WidgetKind::ScrollView | WidgetKind::LazyVStack => {
// ScrollView/LazyVStack takes all available space
if vertical {
200
dpi(200)
} else {
200
dpi(200)
}
}
WidgetKind::SecureField => {
if vertical {
24
dpi(24)
} else {
200
dpi(200)
}
}
WidgetKind::ProgressView => {
if vertical {
20
dpi(20)
} else {
200
dpi(200)
}
}
WidgetKind::Form | WidgetKind::Section => {
Expand All @@ -438,80 +466,80 @@ fn measure_intrinsic(handle: i64, kind: &WidgetKind, vertical: bool, cross_size:
WidgetKind::ZStack | WidgetKind::NavStack => {
// ZStack/NavStack takes all available space
if vertical {
200
dpi(200)
} else {
200
dpi(200)
}
}
WidgetKind::Picker => {
if vertical {
28
dpi(28)
} else {
200
dpi(200)
}
}
WidgetKind::Canvas => {
if vertical {
200
dpi(200)
} else {
200
dpi(200)
}
}
WidgetKind::Image => {
if vertical {
24
dpi(24)
} else {
24
dpi(24)
}
}
WidgetKind::Calendar => {
if vertical {
240
dpi(240)
} else {
280
dpi(280)
}
}
WidgetKind::DatePicker => {
// Compact date field — a single row, narrow like a textfield.
if vertical {
28
dpi(28)
} else {
160
dpi(160)
}
}
WidgetKind::Combobox => {
if vertical {
28
dpi(28)
} else {
200
dpi(200)
}
}
WidgetKind::TreeView => {
if vertical {
200
dpi(200)
} else {
240
dpi(240)
}
}
WidgetKind::RichText => {
if vertical {
200
dpi(200)
} else {
240
dpi(240)
}
}
WidgetKind::Chart => {
if vertical {
200
dpi(200)
} else {
240
dpi(240)
}
}
WidgetKind::TextArea => {
if vertical {
120
dpi(120)
} else {
280
dpi(280)
}
}
}
Expand Down Expand Up @@ -576,13 +604,13 @@ fn measure_text_height(hwnd: HWND, width: i32, vertical: bool) -> i32 {
unsafe {
let hdc = GetDC(Some(hwnd));
if hdc.is_invalid() {
return if vertical { 20 } else { 100 };
return if vertical { dpi(20) } else { dpi(100) };
}

let text_len = GetWindowTextLengthW(hwnd);
if text_len == 0 {
let _ = ReleaseDC(Some(hwnd), hdc);
return if vertical { 20 } else { 100 };
return if vertical { dpi(20) } else { dpi(100) };
}

let mut buf = vec![0u16; (text_len + 1) as usize];
Expand Down Expand Up @@ -616,7 +644,7 @@ fn measure_text_height(hwnd: HWND, width: i32, vertical: bool) -> i32 {
}
let _ = ReleaseDC(Some(hwnd), hdc);

(rect.bottom - rect.top).max(16)
(rect.bottom - rect.top).max(dpi(16))
} else {
let mut size = SIZE::default();
GetTextExtentPoint32W(hdc, &buf[..text_len as usize], &mut size);
Expand All @@ -626,11 +654,38 @@ fn measure_text_height(hwnd: HWND, width: i32, vertical: bool) -> i32 {
}
let _ = ReleaseDC(Some(hwnd), hdc);

size.cx.max(20)
size.cx.max(dpi(20))
}
}
}

#[cfg(test)]
mod tests {
use super::scale_by;

#[test]
fn identity_at_100_percent() {
assert_eq!(scale_by(30, 1.0), 30);
assert_eq!(scale_by(200, 1.0), 200);
}

#[test]
fn degenerate_scale_is_identity() {
// An unset/zeroed DPI reading must never shrink controls.
assert_eq!(scale_by(30, 0.0), 30);
assert_eq!(scale_by(30, 0.5), 30);
}

#[test]
fn scales_and_rounds_at_common_factors() {
assert_eq!(scale_by(30, 1.25), 38); // 37.5 rounds up
assert_eq!(scale_by(30, 1.5), 45);
assert_eq!(scale_by(30, 2.0), 60);
assert_eq!(scale_by(2, 1.5), 3); // Divider stays visible
assert_eq!(scale_by(28, 1.75), 49);
}
}

/// Force-invalidate all widgets with a background brush so WM_PAINT fires.
pub fn force_paint_backgrounds(handle: i64) {
#[cfg(target_os = "windows")]
Expand Down
Loading