Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
66 changes: 59 additions & 7 deletions crates/rds-cli/src/viewer_main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ async fn main() -> std::process::ExitCode {
}

async fn run(mut cli: Cli, matches: &clap::ArgMatches) -> anyhow::Result<()> {
let mut configured_resolution = None;
let key_path = rds_net::default_key_path();
let directory = match cli.control_dir.take() {
Some(directory) => directory,
Expand Down Expand Up @@ -130,18 +131,13 @@ async fn run(mut cli: Cli, matches: &clap::ArgMatches) -> anyhow::Result<()> {
if cli.grant_file.is_none() {
cli.grant_file = config.grant_file;
}
if let Some(resolution) = config.resolution
&& matches.value_source("resolution") != Some(ValueSource::CommandLine)
{
cli.options.resolution = resolution;
}
configured_resolution = config.resolution;
}
Err(error) if error.kind() == std::io::ErrorKind::NotFound => {}
Err(error) => return Err(error.into()),
}
}
if !cli.options.headless && matches.value_source("resolution") != Some(ValueSource::CommandLine)
{
if configure_resolution(&mut cli.options, matches, configured_resolution) {
let Some(height) = rds_desktop::render::choose_resolution() else {
return Ok(());
};
Expand All @@ -160,3 +156,59 @@ async fn run(mut cli: Cli, matches: &clap::ArgMatches) -> anyhow::Result<()> {
rds_cli::desktop::managed(&client, session, grant, cli.options).await
}
}

/// Apply explicit quality choices before deciding whether native selection is
/// needed. Command-line values override the persisted profile.
fn configure_resolution(
options: &mut rds_cli::desktop::Options,
matches: &clap::ArgMatches,
configured: Option<rds_cli::desktop::Resolution>,
) -> bool {
let explicit_cli = matches.value_source("resolution") == Some(ValueSource::CommandLine);
if !explicit_cli && let Some(resolution) = configured {
options.resolution = resolution;
}
!options.headless && !explicit_cli && configured.is_none()
}

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

fn configured_cli(args: &[&str], configured: Option<&str>) -> (Cli, bool) {
let matches = Cli::command().try_get_matches_from(args).unwrap();
let mut cli = Cli::from_arg_matches(&matches).unwrap();
let config: Config = serde_json::from_str(configured.unwrap_or("{}")).unwrap();
let picker = configure_resolution(&mut cli.options, &matches, config.resolution);
(cli, picker)
}

#[test]
fn persisted_quality_is_applied_without_reopening_the_picker() {
for (profile, height) in [("hd", 720), ("full-hd", 1080), ("native", 0)] {
let json = format!(r#"{{"resolution":"{profile}"}}"#);
let (cli, picker) = configured_cli(&["rds-viewer"], Some(&json));
assert_eq!(cli.options.resolution.height(), height);
assert!(
!picker,
"a persisted profile must not be overwritten by a picker"
);
}
}

#[test]
fn command_line_quality_overrides_the_persisted_profile() {
let (cli, picker) = configured_cli(
&["rds-viewer", "--resolution", "native"],
Some(r#"{"resolution":"hd"}"#),
);
assert_eq!(cli.options.resolution.height(), 0);
assert!(!picker);
}

#[test]
fn only_unconfigured_native_launches_need_the_picker() {
assert!(configured_cli(&["rds-viewer"], None).1);
assert!(!configured_cli(&["rds-viewer", "--headless"], None).1);
}
}
27 changes: 25 additions & 2 deletions crates/rds-discovery/tests/http_framing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,27 @@ async fn client_refuses_ambiguous_framing_even_with_a_valid_record_body() {
#[tokio::test]
async fn fragmented_maximum_binary_body_is_preserved_in_both_directions() {
use tokio::io::AsyncWriteExt;
// Fragment reads independently of transport capacity. A seven-byte
// channel made a maximum body require tens of thousands of writer/reader
// wakeups, accidentally turning this codec assertion into a scheduler
// benchmark on loaded CI runners.
struct Fragments<S>(S);
impl<S: tokio::io::AsyncRead + Unpin> tokio::io::AsyncRead for Fragments<S> {
fn poll_read(
mut self: std::pin::Pin<&mut Self>,
cx: &mut std::task::Context<'_>,
output: &mut tokio::io::ReadBuf<'_>,
) -> std::task::Poll<std::io::Result<()>> {
let mut bytes = [0; 7];
let limit = output.remaining().min(bytes.len());
let mut fragment = tokio::io::ReadBuf::new(&mut bytes[..limit]);
let result = std::pin::Pin::new(&mut self.0).poll_read(cx, &mut fragment);
if let std::task::Poll::Ready(Ok(())) = result {
output.put_slice(fragment.filled());
}
result
}
}
let body: Vec<u8> = (0..MAX_BODY).map(|n| (n % 256) as u8).collect();
for is_request in [true, false] {
let mut wire = Vec::new();
Expand All @@ -341,8 +362,10 @@ async fn fragmented_maximum_binary_body_is_preserved_in_both_directions() {
.await
.unwrap();
}
// Seven bytes forces splits inside header names, CRLF and binary data.
let (mut tx, mut rx) = tokio::io::duplex(7);
// Reads still split header names, CRLF and binary data at seven bytes.
// The bounded transport can queue one small 4 KiB block at a time.
let (mut tx, rx) = tokio::io::duplex(4096);
let mut rx = Fragments(rx);
let ((), received) = tokio::time::timeout(std::time::Duration::from_secs(5), async {
tokio::join!(
async {
Expand Down
9 changes: 5 additions & 4 deletions docs/native-viewer.md
Original file line number Diff line number Diff line change
Expand Up @@ -346,10 +346,11 @@ contains no endpoint identity; the normal private viewer configuration applies.

## Quality selection

The macOS application presents a native quality chooser before opening its
remote session. Full HD (1080p) is the default; HD (720p) and Original are also
available. `--resolution full-hd|hd|native` skips the chooser for explicit CLI
or automated launches. Output preserves aspect ratio and does not upscale a
The macOS application presents a native quality chooser when no explicit
profile is configured. Full HD (1080p) is the default; HD (720p) and Original are
also available. `--resolution full-hd|hd|native` or a `resolution` value in
`viewer.json` skips the chooser; the command-line value overrides the file.
Headless runs never open the chooser. Output preserves aspect ratio and does not upscale a
smaller source. This is encoded video geometry, not the local window's size.

The additive `DesktopV3` greeting and `DesktopProfile` manager command carry the
Expand Down
21 changes: 21 additions & 0 deletions docs/reports/rds-persisted-viewer-quality-20261007.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Persisted native viewer quality — 2026-10-07

`viewer.json` already supported a typed `resolution` value. Startup applied it,
then still opened the native picker because only the CLI value source was
checked. A subsequent picker response could overwrite the persisted choice.

Apply the CLI/file precedence before deciding whether a picker is needed.
An explicit CLI profile wins, a persisted profile bypasses the picker, and
headless execution never opens native UI. An unconfigured native launch keeps
the existing chooser. No protocol, permission or remote-process behavior changes.

The regression with the former picker condition failed for the intended reason.
Tests cover every persisted profile, an explicit CLI override, an unconfigured
native launch and headless execution. The public software CI remains required;
no native pixel or WAN acceptance is implied by configuration tests.

The existing [`ValueSource` API](https://docs.rs/clap/latest/clap/parser/enum.ValueSource.html)
distinguishes a defaulted argument from a command-line choice. The JSON choice
is an application source tracked separately; no new configuration mechanism is
introduced. The API distinction predates the requested research boundary;
this implementation observation is dated 2026-10-07.
Loading