Skip to content

fix: converge physical-output presentation size negotiation #25

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

Open
maryny4 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: converge physical-output presentation size negotiation #25

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
100 changes: 49 additions & 51 deletions src/capture/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,43 +120,19 @@ fn clamp_to_h264_software_limits(width: u32, height: u32) -> (u32, u32) {
(scaled_width, scaled_height)
}

fn normalize_to_source_aspect_bounds(
requested_size: (u32, u32),
source_size: (u32, u32),
) -> (u32, u32) {
let (requested_w, requested_h) =
clamp_to_h264_software_limits(requested_size.0, requested_size.1);
/// Clamp a requested presentation size to the H.264 policy limits and even
/// dimensions. Aspect mismatches are letterboxed by the presentation scaler,
/// so the requested size is otherwise kept as-is: reshaping it here announces
/// a size the client did not ask for, which clients answer with another
/// resize request — the negotiation then walks a shrinking staircase
/// (728x408 → 724x408 → 724x406 → …) instead of converging.
fn normalize_presentation_size(requested_size: (u32, u32), source_size: (u32, u32)) -> (u32, u32) {
let (width, height) = clamp_to_h264_software_limits(requested_size.0, requested_size.1);
let (source_w, source_h) = source_size;
if requested_w == 0 || requested_h == 0 || source_w == 0 || source_h == 0 {
if width == 0 || height == 0 || source_w == 0 || source_h == 0 {
return (0, 0);
}

let requested_w_u64 = u64::from(requested_w);
let requested_h_u64 = u64::from(requested_h);
let source_w_u64 = u64::from(source_w);
let source_h_u64 = u64::from(source_h);

let (width, height) = if requested_w_u64.saturating_mul(source_h_u64)
<= requested_h_u64.saturating_mul(source_w_u64)
{
(
requested_w,
((requested_w_u64 * source_h_u64) / source_w_u64) as u32,
)
} else {
(
((requested_h_u64 * source_w_u64) / source_h_u64) as u32,
requested_h,
)
};

let width = width & !1;
let height = height & !1;
if width == 0 || height == 0 {
(0, 0)
} else {
(width, height)
}
(width, height)
}

#[derive(Clone, Copy, Debug, Eq, PartialEq)]
Expand Down Expand Up @@ -184,7 +160,7 @@ fn startup_presentation_size(
configured_resolution
};
if physical_output {
normalize_to_source_aspect_bounds(requested, source_size)
normalize_presentation_size(requested, source_size)
} else {
requested
}
Expand All @@ -202,7 +178,7 @@ fn initial_size_resize_decision(
}

let (width, height) = if physical_output {
normalize_to_source_aspect_bounds(requested_size, source_size?)
normalize_presentation_size(requested_size, source_size?)
} else {
requested_size
};
Expand Down Expand Up @@ -238,7 +214,7 @@ fn display_control_resize_decision(
headless_display_control_size(layout)?
};
let (width, height) = if physical_output {
normalize_to_source_aspect_bounds((requested_w, requested_h), source_size?)
normalize_presentation_size((requested_w, requested_h), source_size?)
} else {
clamp_to_h264_software_limits(requested_w, requested_h)
};
Expand Down Expand Up @@ -441,7 +417,7 @@ impl HyprDisplay {
source_h = capture_info.height,
applied_w = presentation_resolution.0,
applied_h = presentation_resolution.1,
"Physical output presentation bounds normalized to source aspect"
"Physical output presentation bounds clamped to encoder limits"
);
}
output_layout
Expand Down Expand Up @@ -645,7 +621,7 @@ impl HyprDisplay {
requested_h,
applied_w = decision.width,
applied_h = decision.height,
"DisplayControl presentation bounds normalized to physical output aspect"
"DisplayControl presentation bounds clamped to encoder limits"
);
}
ResizeTarget::ManagedHeadlessOutput => {
Expand Down Expand Up @@ -912,10 +888,10 @@ mod output_downscaling {
}

#[test]
fn physical_output_startup_normalizes_explicit_resolution_bounds_to_source_aspect() {
fn physical_output_startup_keeps_explicit_resolution_for_letterboxing() {
assert_eq!(
startup_presentation_size(true, true, (1920, 1200), (3840, 1080)),
(1920, 540)
(1920, 1200)
);
assert_eq!(
startup_presentation_size(true, true, (1600, 900), (3840, 2160)),
Expand Down Expand Up @@ -955,7 +931,7 @@ mod output_downscaling {
}

#[test]
fn physical_output_initial_size_uses_client_size_as_source_aspect_bounds() {
fn physical_output_initial_size_keeps_client_size_for_letterboxing() {
let decision = initial_size_resize_decision(
true,
false,
Expand All @@ -966,7 +942,29 @@ mod output_downscaling {
.unwrap();

assert_eq!(decision.target, ResizeTarget::PhysicalPresentation);
assert_eq!((decision.width, decision.height), (1920, 540));
assert_eq!((decision.width, decision.height), (1920, 1200));
}

#[test]
fn physical_output_size_negotiation_reaches_fixed_point_immediately() {
// Regression: reshaping the requested size to the source aspect made
// the client re-request every applied size, walking a shrinking
// staircase (728x408 → 724x408 → 724x406 → … → 704x396) with a full
// capture and encoder restart on every step.
let source = Some((3840, 2160));
let decision =
initial_size_resize_decision(true, false, (3840, 2160), (728, 408), source).unwrap();
assert_eq!((decision.width, decision.height), (728, 408));

// The client echoes the applied size back; the negotiation must stop.
let echo = initial_size_resize_decision(
true,
false,
(decision.width, decision.height),
(decision.width, decision.height),
source,
);
assert_eq!(echo, None);
}

#[tokio::test]
Expand All @@ -989,18 +987,18 @@ mod output_downscaling {
size,
DesktopSize {
width: 1920,
height: 540
height: 1200
}
);
assert_eq!(shared.get_surface_size(), (1920, 540));
assert_eq!(shared.get_surface_size(), (1920, 1200));

let inner = display.inner.lock().await;
assert_eq!(inner.resolution, (1920, 540));
assert_eq!(inner.resolution, (1920, 1200));
assert_eq!(
inner.pending_initial_resize,
Some(DesktopSize {
width: 1920,
height: 540
height: 1200
})
);
}
Expand Down Expand Up @@ -1041,7 +1039,7 @@ mod output_downscaling {
.unwrap();

assert_eq!(decision.target, ResizeTarget::PhysicalPresentation);
assert_eq!((decision.width, decision.height), (100, 56));
assert_eq!((decision.width, decision.height), (100, 100));
}

#[test]
Expand All @@ -1058,7 +1056,7 @@ mod output_downscaling {
initial_size_resize_decision(
true,
false,
(1920, 540),
(1920, 1200),
(1920, 1200),
Some((3840, 1080))
),
Expand All @@ -1082,7 +1080,7 @@ mod output_downscaling {
}

#[test]
fn physical_output_displaycontrol_uses_monitor_size_as_source_aspect_bounds() {
fn physical_output_displaycontrol_keeps_monitor_size_for_letterboxing() {
let decision = display_control_resize_decision(
&single_primary(1920, 1200),
true,
Expand All @@ -1093,7 +1091,7 @@ mod output_downscaling {
.unwrap();

assert_eq!(decision.target, ResizeTarget::PhysicalPresentation);
assert_eq!((decision.width, decision.height), (1920, 540));
assert_eq!((decision.width, decision.height), (1920, 1200));
}

#[test]
Expand Down