Migrate PdfCraft to egui 0.37 with reconciled platform and editor behavior

Preserve current main, pin the reviewed forks, and cover hidden input,
IME lifecycle, bidi rendering, persisted settings, and editor geometry.

Repository gates pass; native/browser acceptance and paired app performance
remain explicitly open for human review.
This commit is contained in:
skylartaylor
2026-10-25 02:45:21 +01:00
342 changed files with 28709 additions and 2959 deletions
+94 -30
View File
@@ -84,7 +84,24 @@ help and feedback: https://discord.gg/artcraft"
);
fn main() -> ExitCode {
let args: Vec<String> = std::env::args().skip(1).collect();
// `std::env::args` panics on an argument that is not UTF-8 ("called `Result::unwrap()` on an
// `Err` value"), which a Latin-1 file name is, and it panics while collecting them: even
// `--help` ended that way. Such a name cannot be passed on either, since the paths here are
// `str`, so it is a usage error (photocraft#1108).
let mut args: Vec<String> = Vec::new();
for arg in std::env::args_os().skip(1) {
match arg.into_string() {
Ok(a) => args.push(a),
Err(bad) => {
let _ = writeln!(
std::io::stderr().lock(),
"pdfcraft-cli: `{}` is not valid UTF-8, so it cannot be used as an argument",
bad.to_string_lossy()
);
return ExitCode::FAILURE;
}
}
}
// `<command> --help` too; only right after the command, so a later `-h` (a value) still reaches it.
let command = match args.get(1).map(String::as_str) {
Some("--help" | "-h") => Some("--help"),
@@ -169,7 +186,7 @@ fn version() -> Result<(), CliError> {
/// found so scripts and CI can gate on it, and when the audit itself cannot run.
fn audit(args: &[String]) -> Result<(), CliError> {
validate_options("audit", args, AUDIT_OPTIONS)?;
let path = *positional(args).first().ok_or("audit: missing file")?;
let path = *positional(args, AUDIT_OPTIONS).first().ok_or("audit: missing file")?;
let bytes = read(path)?;
let password = flag(args, "--password");
let doc = pdfcraft_cos::Document::open_with_password(bytes.clone(), password).map_err(|e| format!("{path}: {e}"))?;
@@ -208,6 +225,28 @@ struct OptionSpec {
takes_value: bool,
}
/// The options whose value is a file or folder. Only these refuse a value that is another
/// option: a file named `--password` is never what was meant, while a password can be anything
/// (`--password --page` is a password of `--page`, which tests/text.rs pins), and so can a title.
/// The way to name a file that does start with `-` is `./--name`, as `--root` already offered.
const PATH_VALUE_OPTIONS: [&str; 6] = ["--out", "--out-dir", "--json", "--root", "--control", "--script"];
/// Whether a token can be the value of a path option. Another option cannot: `--out --password`
/// used to write the PDF to a file called `--password` and exit 0 (photocraft#6944).
fn can_be_path_value(name: &str, value: Option<&str>) -> bool {
!PATH_VALUE_OPTIONS.contains(&name) || value.is_some_and(|v| !v.is_empty() && !v.starts_with("--"))
}
/// The complaint about a path option whose value is missing or is another option.
fn needs_value(command: &str, name: &str, value: Option<&str>) -> String {
match value {
Some(v) if !v.is_empty() => {
format!("{command}: {name} needs a value and `{v}` is another option; write `./{v}` for a file of that name")
}
_ => format!("{command}: {name} needs a value"),
}
}
/// Refuse unknown options and malformed confinement flags before any file is read.
fn validate_options(command: &str, args: &[String], specs: &[OptionSpec]) -> Result<(), CliError> {
if specs.iter().any(|spec| spec.name == "--root") {
@@ -230,9 +269,19 @@ fn validate_options(command: &str, args: &[String], specs: &[OptionSpec]) -> Res
let Some(spec) = specs.iter().find(|spec| spec.name == arg) else {
return Err(format!("{command}: unknown option --{name}").into());
};
// Other missing or malformed values are left to the command, which reports them in its own
// terms (`text` and `extract` explain what a page value must be).
i += if spec.takes_value { 2 } else { 1 };
// A file's name is not another option, which also closes the gap the walk left for
// those: an unknown option in that value position was skipped along with it. Other
// values, missing or malformed, are left to the command, which reports them in its
// own terms (`text` and `extract` explain what a page value must be).
i += if spec.takes_value {
let value = args.get(i + 1).map(String::as_str);
if !can_be_path_value(spec.name, value) {
return Err(needs_value(command, spec.name, value).into());
}
2
} else {
1
};
} else {
i += 1;
}
@@ -278,19 +327,24 @@ const UI_OPTIONS: &[OptionSpec] = &[OptionSpec { name: "--control", takes_value:
#[cfg(feature = "mcp")]
const MCP_OPTIONS: &[OptionSpec] = &[OptionSpec { name: "--root", takes_value: true }, OptionSpec { name: "--compact", takes_value: false }];
fn positional(args: &[String]) -> Vec<&str> {
/// The arguments that are not options or their values. Which tokens are values is the spec's
/// answer: skipping the one after every option made `check-one --edit file.pdf` report "missing
/// file", the file having been read as `--edit`'s value.
fn positional<'a>(args: &'a [String], specs: &[OptionSpec]) -> Vec<&'a str> {
let mut out = Vec::new();
let mut skip = false;
for a in args {
if skip {
skip = false;
continue;
let mut i = 0;
while let Some(a) = args.get(i) {
match specs.iter().find(|spec| spec.name == a.as_str()) {
Some(spec) if spec.takes_value => i += 2,
// An option that takes no value, and an unknown one, which `validate_options` has
// already refused by the time this runs.
Some(_) => i += 1,
None if a.starts_with("--") => i += 1,
None => {
out.push(a.as_str());
i += 1;
}
}
if a.starts_with("--") {
skip = true;
continue;
}
out.push(a.as_str());
}
out
}
@@ -301,7 +355,7 @@ fn read(path: &str) -> Result<Arc<Vec<u8>>, String> {
fn info(args: &[String]) -> Result<(), CliError> {
validate_options("info", args, INFO_OPTIONS)?;
let path = *positional(args).first().ok_or("info: missing file")?;
let path = *positional(args, INFO_OPTIONS).first().ok_or("info: missing file")?;
let bytes = read(path)?;
let password = flag(args, "--password");
// The renderer takes only the user password of R2–R4 files; `Session::open` takes either.
@@ -354,7 +408,7 @@ fn info(args: &[String]) -> Result<(), CliError> {
fn text(args: &[String]) -> Result<(), CliError> {
validate_options("text", args, TEXT_OPTIONS)?;
let path = *positional(args).first().ok_or("text: missing file")?;
let path = *positional(args, TEXT_OPTIONS).first().ok_or("text: missing file")?;
let mut selected_page = None;
// Validate every supplied value before reading, retaining the first valid selection.
let mut options = args.iter();
@@ -482,7 +536,13 @@ fn edit(args: &[String]) -> Result<(), CliError> {
continue;
}
}
let value = args.get(i + 1).map(String::as_str).ok_or_else(|| format!("edit: {arg} needs a value"))?;
// `edit` parses its own options (they run in order), so `--out` is checked here in the
// same way: it wrote a PDF to a file called `--full`.
let value = args.get(i + 1).map(String::as_str);
if !can_be_path_value(arg, value) {
return Err(needs_value("edit", arg, value).into());
}
let value = value.ok_or_else(|| format!("edit: {arg} needs a value"))?;
match arg {
"--out" => out = Some(value),
"--password" => password = Some(value),
@@ -569,7 +629,7 @@ fn file_stem(path: &str) -> String {
fn combine(args: &[String]) -> Result<(), CliError> {
validate_options("combine", args, COMBINE_OPTIONS)?;
let out = flag(args, "--out").ok_or("combine: missing --out")?;
let inputs = positional(args);
let inputs = positional(args, COMBINE_OPTIONS);
if inputs.len() < 2 {
return Err("combine: give at least two input files".into());
}
@@ -580,7 +640,7 @@ fn combine(args: &[String]) -> Result<(), CliError> {
fn extract(args: &[String]) -> Result<(), CliError> {
validate_options("extract", args, EXTRACT_OPTIONS)?;
let path = *positional(args).first().ok_or("extract: missing file")?;
let path = *positional(args, EXTRACT_OPTIONS).first().ok_or("extract: missing file")?;
let out = flag(args, "--out").ok_or("extract: missing --out")?;
let pages = page_list(flag(args, "--pages").ok_or("extract: missing --pages")?)?;
let mut session = pdfcraft_engine::Session::new();
@@ -592,7 +652,7 @@ fn extract(args: &[String]) -> Result<(), CliError> {
fn split(args: &[String]) -> Result<(), CliError> {
validate_options("split", args, SPLIT_OPTIONS)?;
use pdfcraft_engine::SplitBy;
let path = *positional(args).first().ok_or("split: missing file")?;
let path = *positional(args, SPLIT_OPTIONS).first().ok_or("split: missing file")?;
let by = match (flag(args, "--every"), flag(args, "--before")) {
(Some(n), None) => {
// A count the splitter cannot hold is refused here, before the file is opened,
@@ -637,14 +697,14 @@ fn parse_dpi(value: &str) -> Result<f32, String> {
fn render(args: &[String]) -> Result<(), CliError> {
validate_options("render", args, RENDER_OPTIONS)?;
let path = *positional(args).first().ok_or("render: missing file")?;
let path = *positional(args, RENDER_OPTIONS).first().ok_or("render: missing file")?;
let page: usize =
flag(args, "--page").unwrap_or("1").parse().ok().filter(|page| *page > 0).ok_or("bad --page: expected a positive page number")?;
let page_index = page - 1;
let dpi = parse_dpi(flag(args, "--dpi").unwrap_or("96"))?;
let out = flag(args, "--out").ok_or("render: missing --out (.png, .jpg, .tif or .pam)")?;
// The file is what its name says (#248): a `.png` used to get a netpbm PAM stream.
let format = match Path::new(out).extension().and_then(|e| e.to_str()).map(str::to_ascii_lowercase).as_deref() {
let format = match Path::new(out).extension().and_then(std::ffi::OsStr::to_str).map(str::to_ascii_lowercase).as_deref() {
Some("png") => Some(ImageFormat::Png),
Some("jpg" | "jpeg") => Some(ImageFormat::Jpeg { quality: 90 }),
Some("tif" | "tiff") => Some(ImageFormat::Tiff),
@@ -688,6 +748,10 @@ fn render(args: &[String]) -> Result<(), CliError> {
pdfcraft_render::RenderWarning::ContentTruncated => {
let _ = writeln!(std::io::stderr().lock(), "warning: page {page}: content past the page's safety budget was skipped");
}
pdfcraft_render::RenderWarning::ColourFallback => {
let _ =
writeln!(std::io::stderr().lock(), "warning: page {page}: a colour that could not be computed was painted in a fallback colour");
}
}
}
Ok(())
@@ -696,7 +760,7 @@ fn render(args: &[String]) -> Result<(), CliError> {
/// Child-process body for `check`: prints one JSON line.
fn check_one(args: &[String]) -> Result<(), CliError> {
validate_options("check-one", args, CHECK_ONE_OPTIONS)?;
let path = *positional(args).first().ok_or("check-one: missing file")?;
let path = *positional(args, CHECK_ONE_OPTIONS).first().ok_or("check-one: missing file")?;
let dpi = parse_dpi(flag(args, "--dpi").unwrap_or("36"))?;
let start = Instant::now();
let bytes = read(path)?;
@@ -779,7 +843,7 @@ fn collect(paths: &[&str]) -> Vec<PathBuf> {
if let Ok(rd) = std::fs::read_dir(&p) {
stack.extend(rd.flatten().map(|e| e.path()));
}
} else if p.extension().and_then(|e| e.to_str()).is_some_and(|e| e.eq_ignore_ascii_case("pdf")) {
} else if p.extension().and_then(std::ffi::OsStr::to_str).is_some_and(|e| e.eq_ignore_ascii_case("pdf")) {
out.push(p);
}
}
@@ -802,7 +866,7 @@ fn check(args: &[String]) -> Result<(), CliError> {
);
let dpi = flag(args, "--dpi").unwrap_or("36").to_string();
parse_dpi(&dpi)?;
let files = collect(&positional(args));
let files = collect(&positional(args, CHECK_OPTIONS));
if files.is_empty() {
return Err("check: no PDF files found".into());
}
@@ -985,9 +1049,9 @@ fn run(args: &[String]) -> Result<(), CliError> {
}
return Ok(());
}
let tool = *positional(args).first().ok_or("run: missing tool name (see `pdfcraft-cli tools`)")?;
let tool = *positional(args, RUN_OPTIONS).first().ok_or("run: missing tool name (see `pdfcraft-cli tools`)")?;
let mut obj = serde_json::Map::new();
for kv in positional(args).iter().skip(1) {
for kv in positional(args, RUN_OPTIONS).iter().skip(1) {
let (k, v) = kv.split_once('=').ok_or(format!("run: expected key=value, got {kv:?}"))?;
let value = serde_json::from_str(v).unwrap_or_else(|_| serde_json::Value::String(v.to_string()));
obj.insert(k.to_string(), value);
@@ -1053,7 +1117,7 @@ fn ui(args: &[String]) -> Result<(), CliError> {
// Refused rather than truncated, which would quietly reach some other port.
let port = u16::try_from(port).map_err(|_| format!("{file}: {port} is not a port number"))?;
let token = info["token"].as_str().ok_or(format!("{file}: no token"))?;
let pos = positional(args);
let pos = positional(args, UI_OPTIONS);
let method = *pos.first().ok_or("ui: missing method (state, inspect, click, drag, type, key, command, commands, set, open, screenshot)")?;
let method = if method.starts_with("ui.") { method.to_string() } else { format!("ui.{method}") };
let mut params = serde_json::Map::new();
+86
View File
@@ -27,3 +27,89 @@ fn non_edit_commands_reject_unknown_options_before_io() {
assert!(stderr.contains("unknown option") && stderr.contains(unknown), "{args:?}: {stderr}");
}
}
/// photocraft#6944: an option took the next token whatever it was, so a line with a value missing
/// in the middle of it did something else entirely — `extract in.pdf --pages 1 --out --password`
/// wrote the PDF to a file called `--password` and exited 0. The unknown option hidden in a value
/// position passed the check above too, since the walk skipped it along with the option.
///
/// Only the options whose value is a file or folder refuse one: a password or a title can be any
/// string, `--page` included, which `text`'s own tests pin and this does not change.
#[test]
fn a_file_option_refuses_a_value_that_is_another_option() {
let dir = std::env::temp_dir().join(format!("pdfcraft-cli-values-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).expect("temp dir");
for (option, args) in [
("--out", vec!["extract", "missing.pdf", "--pages", "1", "--out", "--password"]),
("--out", vec!["combine", "a.pdf", "b.pdf", "--out", "--nonsense"]),
("--out", vec!["render", "missing.pdf", "--out", "--dpi", "96"]),
("--out-dir", vec!["split", "missing.pdf", "--every", "2", "--out-dir", "--password"]),
("--json", vec!["check", "missing.pdf", "--json", "--dpi"]),
("--control", vec!["ui", "--control", "--out", "state"]),
// `edit` parses its own options, in the order given, and had the same gap.
("--out", vec!["edit", "missing.pdf", "--out", "--full"]),
] {
let out = Command::new(BIN).current_dir(&dir).args(&args).output().expect("run cli");
let stderr = String::from_utf8_lossy(&out.stderr);
assert!(!out.status.success(), "{args:?} unexpectedly succeeded");
assert!(stderr.contains(&format!("{option} needs a value")), "{args:?}: {stderr}");
assert!(stderr.contains("another option"), "{args:?}: {stderr}");
}
assert_eq!(std::fs::read_dir(&dir).expect("read dir").count(), 0, "nothing was written");
let _ = std::fs::remove_dir_all(&dir);
}
/// And a file whose name really does start with `-` is still reachable, the way the shell reaches
/// one and `--root` already documented: `./--name`. The error is then about the input file, which
/// is as far as these lines get here.
#[test]
fn a_file_whose_name_starts_with_dashes_is_still_reachable() {
// The error is about the file the command could not open, so the test asks which file it
// names -- not how the OS words "not found", which differs on Windows (#3849 review).
for (names, args) in [
("missing.pdf", vec!["extract", "missing.pdf", "--pages", "1", "--out", "./--dash.pdf"]),
("missing.pdf", vec!["edit", "missing.pdf", "--out", "./--dash.pdf"]),
("./--dash.pdf", vec!["info", "./--dash.pdf"]),
] {
let out = Command::new(BIN).args(&args).output().expect("run cli");
let stderr = String::from_utf8_lossy(&out.stderr);
assert!(!out.status.success(), "{args:?}: no such file either way");
assert!(stderr.contains(names), "{args:?}: the error is about {names}: {stderr}");
// Not a usage error: the `./--dash.pdf` reached the command as a file name.
assert!(!stderr.contains("needs a value") && !stderr.contains("unknown option"), "{args:?}: {stderr}");
}
}
/// The token after an option that takes no value is not its value: `check-one --edit file.pdf`
/// reported "check-one: missing file", the file having been read as `--edit`'s operand.
#[test]
fn a_flag_that_takes_no_value_does_not_swallow_the_file() {
for args in [vec!["check-one", "--edit", "missing.pdf"], vec!["check-one", "missing.pdf", "--edit"]] {
let out = Command::new(BIN).args(&args).output().expect("run cli");
let stderr = String::from_utf8_lossy(&out.stderr);
assert!(!out.status.success(), "{args:?}");
// Which file the error is about, not how the OS says it is missing (#3849 review).
assert!(stderr.contains("missing.pdf"), "{args:?}: the error is about the file: {stderr}");
assert!(!stderr.contains("missing file"), "the file was found, and then not found: {stderr}");
assert!(!stderr.contains("needs a value"), "{args:?}: {stderr}");
}
}
/// photocraft#1108: an argument that is not UTF-8 panicked in `std::env::args` ("called
/// `Result::unwrap()` on an `Err` value"), while collecting the arguments — so it panicked even
/// for `--help`. Such a name cannot be used here, where paths are `str`, so it is a usage error.
#[cfg(unix)]
#[test]
fn an_argument_that_is_not_utf8_is_an_error_not_a_panic() {
use std::ffi::OsStr;
use std::os::unix::ffi::OsStrExt;
for args in [vec![OsStr::new("info"), OsStr::from_bytes(b"/tmp/caf\xe9.pdf")], vec![OsStr::new("--help"), OsStr::from_bytes(b"caf\xe9")]] {
let out = Command::new(BIN).args(&args).output().expect("run cli");
let stderr = String::from_utf8_lossy(&out.stderr);
assert_eq!(out.status.code(), Some(1), "an error, not a panic (101): {stderr}");
assert!(stderr.contains("not valid UTF-8"), "{stderr}");
assert!(!stderr.contains("panicked"), "{stderr}");
}
}
+1 -1
View File
@@ -12,7 +12,7 @@ workspace = true
[dependencies]
pdfcraft-ui-egui = { workspace = true }
eframe = { workspace = true, features = ["default_fonts", "glow", "persistence", "accesskit"] }
egui = { workspace = true }
egui = { workspace = true, features = ["default_fonts", "monochrome_emoji_fonts"] }
[target.'cfg(target_arch = "wasm32")'.dependencies]
wasm-bindgen-futures = "0.4"
+2 -2
View File
@@ -18,8 +18,8 @@ pdfcraft-ui-egui = { workspace = true }
pdfcraft-engine = { workspace = true }
# wgpu draws; OpenGL (glow) is the fallback when wgpu can't start on an old or unusual GPU or
# driver (#461, #435, #392), and can be chosen with PDFCRAFT_RENDERER=gl.
eframe = { workspace = true, features = ["default_fonts", "wgpu", "glow", "accesskit", "wayland", "x11", "persistence", "links"] }
egui = { workspace = true }
eframe = { workspace = true, features = ["default_fonts", "wgpu", "glow", "accesskit", "wayland", "x11", "persistence", "links", "clipboard"] }
egui = { workspace = true, features = ["default_fonts", "monochrome_emoji_fonts"] }
serde_json = { workspace = true }
# Records from every crate go to standard error and the log file (src/logging.rs).
log = { workspace = true }
+72 -41
View File
@@ -357,6 +357,9 @@ fn native_options(integrated: bool, app_frame: bool, renderer: eframe::Renderer)
let persistence_path = settings_dir().map(|d| d.join("app.ron"));
// Set explicitly, so the renderer never depends on which one eframe defaults to.
let mut native = eframe::NativeOptions { viewport, persistence_path, renderer, ..Default::default() };
// PdfCraft installs its own last-resort system face (`PDFCRAFT_SYSTEM_FONTS`); eframe's
// provider would read every installed face, including proprietary ones, into every UI pass.
native.system_font_fallback = false;
if renderer == eframe::Renderer::Wgpu {
configure_gpu(&mut native);
}
@@ -795,6 +798,41 @@ fn pick_adapter(adapters: &[(u32, u32, eframe::wgpu::DeviceType)], displays: &[D
#[cfg(test)]
mod tests {
/// The lockfile package block for `name`, or a panic naming the missing package.
fn locked_package(root: &std::path::Path, name: &str) -> String {
let lock = std::fs::read_to_string(root.join("Cargo.lock")).expect("Cargo.lock");
lock.split("[[package]]")
.find(|p| p.contains(&format!("\nname = \"{name}\"\n")))
.map(str::to_owned)
.unwrap_or_else(|| panic!("{name} is not in Cargo.lock"))
}
/// Interim graph check for the window stack while the reviewed fork revision is not yet
/// published. The old `vendor/winit` / `vendor/egui-wgpu` path patches were removed from the
/// published manifest; local integration supplies the reviewed worktrees through an external
/// Cargo config patch. A git source must name the public storytold fork; a path source is the
/// local integration graph. Behaviour (surface fitting, error scopes, scroll phases) is tested
/// directly elsewhere; the retained vendor copies keep provenance until the fork revision is
/// pinned. No public revision is invented here.
fn assert_window_stack_graph(root: &std::path::Path, name: &str, version: &str) -> String {
let package = locked_package(root, name);
assert!(package.contains(&format!("\nversion = \"{version}\"\n")), "{name} must resolve to {version}: {package}");
if let Some(source) = package.lines().find_map(|line| line.strip_prefix("source = ")) {
assert!(source.contains("github.com/storytold/"), "{name} must come from the reviewed storytold fork once published, not {source}");
assert!(
source.split(|c: char| !c.is_ascii_hexdigit()).any(|part| part.len() == 40),
"{name} must pin the fork's 40-character revision, not {source}"
);
}
// A path patch has no `source =` in Cargo.lock. The local integration config is the only
// allowed path source; a machine-local path must never be in the published manifest,
// which the root-manifest assertion below checks. A path entry carries no remote identity
// to verify, so this test does not claim the resolved code is the published fork.
let manifest = std::fs::read_to_string(root.join("Cargo.toml")).expect("root Cargo.toml");
assert!(!manifest.contains(&format!("{name} = {{ path = \"vendor/")), "vendor/{name} must not be a published path patch");
package
}
use pdfcraft_ui_egui::control::Endpoint;
fn endpoint() -> std::io::Result<Endpoint> {
@@ -1064,27 +1102,16 @@ mod tests {
}
#[test]
fn egui_wgpu_carries_the_surface_size_fit() {
// Issue #577: egui-wgpu 0.36.2 configures a window's surface at the window's size, and
fn vendor_provenance_egui_wgpu_surface_fit_identity() {
// Issue #577: egui-wgpu configured a window's surface at the window's size, and
// `Surface::configure` panics when that's beyond the device's `max_texture_dimension_2d`
// (emilk/egui#8361). vendor/egui-wgpu fits it within the limit. A dependency bump that
// resolves egui-wgpu from crates.io again, or a re-vendored copy without the patch, would
// bring the crash back: re-apply the patch, or drop the copy once an egui release has a
// fix (vendor/README.md).
// (emilk/egui#8361). The reviewed egui 0.37 worktree carries E2 (`surface_fit`); the
// behavioural test above exercises it. The old 0.36 vendor copy is retained for
// provenance only and is no longer a published path patch, so reading it would not prove
// what the app runs.
let root = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../..");
let lock = std::fs::read_to_string(root.join("Cargo.lock")).unwrap();
let egui_wgpu = lock.split("[[package]]").find(|p| p.contains("\nname = \"egui-wgpu\"\n")).expect("egui-wgpu is in Cargo.lock");
assert!(!egui_wgpu.contains("\nsource = "), "egui-wgpu must resolve to vendor/egui-wgpu, not:{egui_wgpu}");
// `surface_fit` is tested above; these keep it in the paths that size a surface.
let painter = std::fs::read_to_string(root.join("vendor/egui-wgpu/src/winit.rs")).unwrap().replace("\r\n", "\n");
for patch in [
"let (width, height, render_scale) = surface_fit(window_width, window_height, max_side);",
"let (fit_width, fit_height, _) = surface_fit(width, height, self.max_surface_side());",
"pixels_per_point: pixels_per_point * surface_state.render_scale,",
"old_state.window_width,\n old_state.window_height,",
] {
assert!(painter.contains(patch), "vendor/egui-wgpu lost its surface size patch: {patch}");
}
assert_window_stack_graph(&root, "egui-wgpu", "0.37.0");
assert!(root.join("vendor/egui-wgpu/src/winit.rs").exists(), "the vendor copy and its provenance stay until the fork revision is pinned");
}
#[test]
@@ -1141,23 +1168,17 @@ mod tests {
}
#[test]
fn egui_wgpu_returns_a_surface_it_cannot_configure_as_an_error() {
// Issue #519: `catch_errors` is tested above; this keeps it around the first configure of
// every new window's surface, and its error reaching eframe as `WgpuError` (eframe then
// returns `Error::Wgpu`, which `main` retries with OpenGL). Without it a failed configure
// panics before the app is created, and the OpenGL retry never runs.
fn vendor_provenance_egui_wgpu_catch_errors_identity() {
// Issue #519: E3 (`catch_errors`) is tested above against a real device. The reviewed 0.37
// worktree carries it around the first configure of every new window's surface, and its
// error reaches eframe as `WgpuError` (eframe then returns `Error::Wgpu`, which `main`
// retries with OpenGL). Without it a failed configure panics before the app is created.
// `catch_errors` is exercised against a real device in the test above and below; the
// graph check makes sure the app runs the reviewed 0.37 worktree (the retained vendor
// copy is not read as proof of behaviour).
let root = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../..");
let lock = std::fs::read_to_string(root.join("Cargo.lock")).unwrap();
let egui_wgpu = lock.split("[[package]]").find(|p| p.contains("\nname = \"egui-wgpu\"\n")).expect("egui-wgpu is in Cargo.lock");
assert!(!egui_wgpu.contains("\nsource = "), "egui-wgpu must resolve to vendor/egui-wgpu, not:{egui_wgpu}");
let painter = std::fs::read_to_string(root.join("vendor/egui-wgpu/src/winit.rs")).unwrap().replace("\r\n", "\n");
let add_surface = painter.split("async fn add_surface(").nth(1).and_then(|s| s.split("\n fn ").next()).expect("add_surface");
for patch in [
"let installed = catch_errors(&device, || {\n self.install_surface(surface, viewport_id, size.width, size.height, false);\n })",
"return Err(crate::WgpuError::ConfigureSurface(error));",
] {
assert!(add_surface.contains(patch), "vendor/egui-wgpu lost its surface configure patch: {patch}");
}
assert_window_stack_graph(&root, "egui-wgpu", "0.37.0");
assert!(root.join("vendor/egui-wgpu/src/winit.rs").exists(), "the vendor copy and its provenance stay until the fork revision is pinned");
}
#[test]
@@ -1199,13 +1220,23 @@ mod tests {
// dependency bump that resolves winit from crates.io again, or a re-vendored copy without
// the patch, would silently bring the bug back: re-apply the patch, or drop the copy once a
// winit 0.30 release has the fix (vendor/README.md).
// W1 (and W2–W13) were audited against the reviewed winit worktree: the vendor rows in
// `vendor/README.md` match the fork's PATCHES.md entries W1–W13. The vendor source is no
// longer a published path patch, so this checks the actual graph, not the old copy.
let root = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../..");
let lock = std::fs::read_to_string(root.join("Cargo.lock")).unwrap();
let winit = lock.split("[[package]]").find(|p| p.contains("\nname = \"winit\"\n")).expect("winit is in Cargo.lock");
assert!(!winit.contains("\nsource = "), "winit must resolve to vendor/winit, not:{winit}");
let dpi_changed = std::fs::read_to_string(root.join("vendor/winit/src/platform_impl/windows/event_loop.rs")).unwrap();
let patch = "if !WIN10_BUILD_VERSION.is_some_and(|build| build < 22000) {\n new_outer_rect = suggested_rect;";
assert!(dpi_changed.contains(patch), "vendor/winit lost its WM_DPICHANGED patch");
assert_window_stack_graph(&root, "winit", "0.30.13");
let event_loop = std::fs::read_to_string(root.join("vendor/winit/src/platform_impl/windows/event_loop.rs")).unwrap();
// The audited W1 hunk itself, not merely the file's presence: Windows 11 applies the
// rect Windows suggests, while Windows 10 keeps the old adjustment.
for marker in [
"PdfCraft patch: from Windows 11 (build 22000) on, apply the rect Windows suggests",
"if !WIN10_BUILD_VERSION.is_some_and(|build| build < 22000) {",
"new_outer_rect = suggested_rect;",
] {
assert!(event_loop.contains(marker), "vendor/winit lost the W1 monitor-scale patch: {marker}");
}
let provenance = std::fs::read_to_string(root.join("vendor/winit/.cargo_vcs_info.json")).unwrap();
assert!(provenance.contains("e9809ef54b18499bb4f2cac945719ecc2a61061b"), "vendor/winit keeps its upstream base provenance");
}
const NVIDIA: (u32, u32) = (0x10de, 0x2684);
+1
View File
@@ -7,6 +7,7 @@ mod event {
pub use winit::event::TouchPhase;
}
mod vendor_graph;
mod winit_macos_emoji_picker;
mod winit_macos_ime_commit;
mod winit_macos_ime_discard;
+78
View File
@@ -0,0 +1,78 @@
//! Dependency-graph verification for the egui 0.37 window stack.
//!
//! The twelve `winit_*` modules next to this one read the retained `vendor/winit` /
//! `vendor/egui-wgpu` copies. They are provenance for W1–W13 and E2+E3, not proof of what the app
//! runs: the root manifest pins both crates to immutable storytold git revisions, and the retained
//! copies are not path patches. These tests check the resolved graph and the retained evidence
//! instead of treating the copies as the runtime source. The winit fork's own W17 tests own patch
//! behaviour.
fn root() -> std::path::PathBuf {
std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../..")
}
/// Every `[[package]]` block in `Cargo.lock` whose `name` is exactly `name`.
fn locked_packages(name: &str) -> Vec<String> {
let lock = std::fs::read_to_string(root().join("Cargo.lock")).expect("Cargo.lock");
let needle = format!("name = \"{name}\"");
lock.split("[[package]]").filter(|package| package.lines().any(|line| line.trim() == needle.as_str())).map(str::to_owned).collect()
}
/// The one resolved package called `name`. A duplicate name is a lockfile problem, not something
/// to resolve by taking the first match.
fn locked(name: &str) -> String {
let mut packages = locked_packages(name);
assert_eq!(packages.len(), 1, "Cargo.lock must contain exactly one `{name}` package, found {}", packages.len());
packages.remove(0)
}
/// The resolved package must be the exact canonical storytold git source at the reviewed
/// 40-character revision, including the `Cargo.lock` URL and fragment. A path source (no `source`
/// line) is refused: it would make the graph depend on a developer's checkout. A published path
/// patch is refused as well.
fn assert_resolved(name: &str, version: &str, url: &str, rev: &str) {
let package = locked(name);
let expected_version = format!("version = \"{version}\"");
assert!(package.lines().any(|line| line.trim() == expected_version.as_str()), "{name} must resolve to {version}: {package}");
let sources: Vec<&str> = package.lines().filter_map(|line| line.trim().strip_prefix("source = ")).collect();
assert_eq!(sources.len(), 1, "{name} must have exactly one `source` in Cargo.lock; a local path source is not accepted: {package}");
let expected_source = format!("\"git+{url}?rev={rev}#{rev}\"");
assert_eq!(sources[0], expected_source.as_str(), "{name} must resolve to the exact canonical git URL, revision and fragment, not {}", sources[0]);
let manifest = std::fs::read_to_string(root().join("Cargo.toml")).expect("root Cargo.toml");
assert!(!manifest.contains(&format!("{name} = {{ path = \"vendor/")), "vendor/{name} cannot stay a published path patch");
}
#[test]
fn vendor_provenance_window_stack_graph_is_resolved() {
assert_resolved("winit", "0.30.13", "https://github.com/storytold/winit.git", "a517b3846ee8457d84ee4975110bfda6797c44dc");
assert_resolved("egui-wgpu", "0.37.0", "https://github.com/storytold/egui.git", "6d518fc799681571963842f95b55a6b68431b15a");
// Retained provenance: the audited copies and their recorded W1–W13 / E2+E3 rows.
let readme = std::fs::read_to_string(root().join("vendor/README.md")).expect("vendor/README.md");
for marker in ["W1–W13", "surface_fit", "catch_errors", "e9809ef54b18499bb4f2cac945719ecc2a61061b"] {
assert!(readme.contains(marker), "vendor/README.md lost window-stack provenance: {marker}");
}
let vcs = std::fs::read_to_string(root().join("vendor/winit/.cargo_vcs_info.json")).expect("vendor/winit provenance");
assert!(vcs.contains("e9809ef54b18499bb4f2cac945719ecc2a61061b"), "vendor/winit keeps its upstream base");
for path in ["vendor/winit/src/platform_impl", "vendor/egui-wgpu/src/winit.rs"] {
assert!(root().join(path).exists(), "retain {path} as provenance for the pinned fork");
}
}
/// Renderer, security and PostScript vendors are unrelated to the UI port and must stay path patches.
#[test]
fn renderer_and_security_vendors_remain() {
let manifest = std::fs::read_to_string(root().join("Cargo.toml")).expect("root Cargo.toml");
for vendor in [
"vendor/hayro",
"vendor/hayro-interpret",
"vendor/hayro-syntax",
"vendor/hayro-cmap",
"vendor/hayro-jbig2",
"vendor/hayro-postscript",
"vendor/lopdf",
"vendor/vello_common",
] {
assert!(manifest.contains(&format!("path = \"{vendor}\"")), "{vendor} must stay a path patch");
}
}
@@ -1,7 +1,9 @@
//! vendor/winit patch (8) (vendor/README.md).
//! Provenance-only: reads the retained `vendor/` copy, not the resolved dependency, so it is
//! not runtime coverage. `vendor_graph.rs` checks the resolved graph identity.
//! Retained vendor provenance for winit patch (8) (vendor/README.md); `vendor_graph.rs` checks the resolved graph.
#[test]
fn winit_commits_macos_text_inserted_outside_a_key_press() {
fn vendor_provenance_winit_macos_text_inserted_outside_a_key_press() {
// The macOS emoji picker inserts its text with `insertText` outside any key press and without
// a composition. winit 0.30.13 reports `insertText` only as part of a key press or a
// composition, so an emoji picked into a PdfCraft text field was dropped. vendor/winit commits
@@ -1,7 +1,9 @@
//! vendor/winit patch (6) (vendor/README.md).
//! Provenance-only: reads the retained `vendor/` copy, not the resolved dependency, so it is
//! not runtime coverage. `vendor_graph.rs` checks the resolved graph identity.
//! Retained vendor provenance for winit patch (6) (vendor/README.md); `vendor_graph.rs` checks the resolved graph.
#[test]
fn winit_commits_macos_compositions_that_end_without_marked_text() {
fn vendor_provenance_winit_macos_compositions_that_end_without_marked_text() {
// winit 0.30.13 treats `insertText` as an IME commit only while marked text is left. Input
// methods that end a composition with `unmarkText` first lost the committed text, and Apple
// Korean, which clears its marked text before inserting the key typed after a syllable (`한`
@@ -1,7 +1,9 @@
//! vendor/winit patch (7) (vendor/README.md).
//! Provenance-only: reads the retained `vendor/` copy, not the resolved dependency, so it is
//! not runtime coverage. `vendor_graph.rs` checks the resolved graph identity.
//! Retained vendor provenance for winit patch (7) (vendor/README.md); `vendor_graph.rs` checks the resolved graph.
#[test]
fn winit_discards_the_macos_composition_when_ime_is_disallowed() {
fn vendor_provenance_winit_discards_the_macos_composition_when_ime_is_disallowed() {
// egui disallows IME when a text field loses focus. winit 0.30.13 then forgets the marked text
// but doesn't tell the input method, which keeps the composition and continues it once IME is
// allowed again: the abandoned syllable comes back in the next field. vendor/winit calls
@@ -1,7 +1,9 @@
//! vendor/winit patch (10) (vendor/README.md).
//! Provenance-only: reads the retained `vendor/` copy, not the resolved dependency, so it is
//! not runtime coverage. `vendor_graph.rs` checks the resolved graph identity.
//! Retained vendor provenance for winit patch (10) (vendor/README.md); `vendor_graph.rs` checks the resolved graph.
#[test]
fn winit_retries_the_first_apple_korean_key_once() {
fn vendor_provenance_winit_retries_the_first_apple_korean_key_once() {
// On the first key in a freshly started app, Apple Korean can answer with `insertText` and the
// raw compatibility jamo instead of starting a composition, so typing g k s in PdfCraft gave
// ㅎㅏㄴ instead of 한. vendor/winit interprets that one key again before sending it to the
@@ -1,7 +1,9 @@
//! vendor/winit patch (9) (vendor/README.md).
//! Provenance-only: reads the retained `vendor/` copy, not the resolved dependency, so it is
//! not runtime coverage. `vendor_graph.rs` checks the resolved graph identity.
//! Retained vendor provenance for winit patch (9) (vendor/README.md); `vendor_graph.rs` checks the resolved graph.
#[test]
fn winit_sends_a_key_committed_through_macos_ime_only_once() {
fn vendor_provenance_winit_sends_a_key_committed_through_macos_ime_only_once() {
// Apple Korean answers Space with no syllable in progress with `setMarkedText(" ")`,
// `insertText(" ")` and `setMarkedText("")`. The last call resets winit 0.30.13's IME state, so
// the key went to the app both as `Ime::Commit(" ")` and as `KeyboardInput` with text " ", and
@@ -1,7 +1,9 @@
//! vendor/winit patch (12) (vendor/README.md).
//! Provenance-only: reads the retained `vendor/` copy, not the resolved dependency, so it is
//! not runtime coverage. `vendor_graph.rs` checks the resolved graph identity.
//! Retained vendor provenance for winit patch (12) (vendor/README.md); `vendor_graph.rs` checks the resolved graph.
#[test]
fn winit_lets_macos_title_bar_drags_reach_the_app() {
fn vendor_provenance_winit_lets_macos_title_bar_drags_reach_the_app() {
// On macOS the tab strip is the title bar: a transparent title bar over a full-size content
// view. NSView answers YES to `mouseDownCanMoveWindow` for a view that isn't opaque, so AppKit
// moved the whole window for a drag that began on a document tab, and tabs could not be
+13 -5
View File
@@ -1,3 +1,5 @@
//! Provenance-only: reads the retained `vendor/` copy, not the resolved dependency, so it is
//! not runtime coverage. `vendor_graph.rs` checks the resolved graph identity.
#![cfg(target_os = "linux")]
use egui::{Event, Modifiers, MouseWheelUnit, RawInput, TouchPhase, vec2};
@@ -27,7 +29,13 @@ fn wheel_trace(source: AxisSource, discrete: bool) -> Vec<(f32, egui::Vec2)> {
let (unit, dy) = if discrete { (MouseWheelUnit::Line, 1.0) } else { (MouseWheelUnit::Point, 24.0) };
let input = RawInput {
time: Some(frame as f64 / 60.0),
events: vec![Event::MouseWheel { unit, delta: vec2(0.0, dy), phase: egui_phase(phase), modifiers }],
events: vec![Event::MouseWheel {
unit,
delta: vec2(0.0, dy),
phase: egui_phase(phase),
modifiers,
source: egui::MouseWheelSource::Unknown,
}],
..Default::default()
};
ctx.run_ui(input, |ui| seen.push(ui.input(|i| (i.zoom_delta(), i.smooth_scroll_delta)))).textures_delta.clear();
@@ -36,7 +44,7 @@ fn wheel_trace(source: AxisSource, discrete: bool) -> Vec<(f32, egui::Vec2)> {
}
#[test]
fn releasing_ctrl_after_pixel_wheel_zoom_returns_to_scrolling() {
fn vendor_provenance_releasing_ctrl_after_pixel_wheel_zoom_returns_to_scrolling() {
for source in [AxisSource::Wheel, AxisSource::WheelTilt, AxisSource::Continuous] {
let seen = wheel_trace(source, false);
assert!(seen[1].0 > 1.0, "Ctrl must zoom: {source:?}");
@@ -46,7 +54,7 @@ fn releasing_ctrl_after_pixel_wheel_zoom_returns_to_scrolling() {
}
#[test]
fn discrete_wheels_still_follow_current_modifiers() {
fn vendor_provenance_discrete_wheels_still_follow_current_modifiers() {
let seen = wheel_trace(AxisSource::Wheel, true);
assert!(seen[1].0 > 1.0);
assert_eq!(seen[2].0, 1.0);
@@ -54,7 +62,7 @@ fn discrete_wheels_still_follow_current_modifiers() {
}
#[test]
fn finger_gestures_keep_their_modifier_until_the_gesture_ends() {
fn vendor_provenance_finger_gestures_keep_their_modifier_until_the_gesture_ends() {
// A finger scroll's momentum may arrive after Ctrl is released.
let seen = wheel_trace(AxisSource::Finger, false);
assert!(seen[1].0 > 1.0);
@@ -65,7 +73,7 @@ fn finger_gestures_keep_their_modifier_until_the_gesture_ends() {
}
#[test]
fn unknown_sources_keep_the_existing_phase_policy() {
fn vendor_provenance_unknown_sources_keep_the_existing_phase_policy() {
assert_eq!(scroll::phase(event::TouchPhase::Ended, None, false, false), event::TouchPhase::Started);
assert_eq!(scroll::phase(event::TouchPhase::Started, None, false, false), event::TouchPhase::Moved);
assert_eq!(scroll::phase(event::TouchPhase::Ended, None, true, false), event::TouchPhase::Moved);
@@ -1,7 +1,9 @@
//! vendor/winit patch (2) (vendor/README.md).
//! Provenance-only: reads the retained `vendor/` copy, not the resolved dependency, so it is
//! not runtime coverage. `vendor_graph.rs` checks the resolved graph identity.
//! Retained vendor provenance for winit patch (2) (vendor/README.md); `vendor_graph.rs` checks the resolved graph.
#[test]
fn winit_keeps_the_wayland_text_input_until_the_seat_goes() {
fn vendor_provenance_winit_keeps_the_wayland_text_input_until_the_seat_goes() {
// winit 0.30.13 destroys a Wayland seat's text input whenever the seat loses any capability (a
// touch screen or tablet unplugged, a remote-desktop session ending) and recreates it only when
// a capability is added, so IME stops working in a running PdfCraft until it is restarted.
@@ -1,7 +1,9 @@
//! vendor/winit patch (4) (vendor/README.md).
//! Provenance-only: reads the retained `vendor/` copy, not the resolved dependency, so it is
//! not runtime coverage. `vendor_graph.rs` checks the resolved graph identity.
//! Retained vendor provenance for winit patch (4) (vendor/README.md); `vendor_graph.rs` checks the resolved graph.
#[test]
fn winit_maximizes_a_hidden_windows_window_only_when_it_is_shown() {
fn vendor_provenance_winit_maximizes_a_hidden_windows_window_only_when_it_is_shown() {
// eframe creates PdfCraft's window hidden, restores the saved maximized state and shows the
// window once its first frame is drawn. winit 0.30.13 applies the maximized state with
// `ShowWindow(SW_MAXIMIZE)`, which shows a hidden window, so a window closed maximized came back
@@ -1,7 +1,9 @@
//! vendor/winit patch (3) (vendor/README.md).
//! Provenance-only: reads the retained `vendor/` copy, not the resolved dependency, so it is
//! not runtime coverage. `vendor_graph.rs` checks the resolved graph identity.
//! Retained vendor provenance for winit patch (3) (vendor/README.md); `vendor_graph.rs` checks the resolved graph.
#[test]
fn winit_reads_the_windows_ime_cursor_only_when_it_is_reported() {
fn vendor_provenance_winit_reads_the_windows_ime_cursor_only_when_it_is_reported() {
// winit 0.30.13 reads `GCS_CURSORPOS` for every composition without a target clause, whether
// or not `WM_IME_COMPOSITION` says the IME set it. The Microsoft Korean IME never sets it, so
// the composition's cursor read as 0, before the syllable being composed. vendor/winit puts it
+3 -1
View File
@@ -1,5 +1,7 @@
//! Provenance-only: reads the retained `vendor/` copy, not the resolved dependency, so it is
//! not runtime coverage. `vendor_graph.rs` checks the resolved graph identity.
#[test]
fn winit_carries_the_x11_server_dpi_fallback() {
fn vendor_provenance_winit_carries_the_x11_server_dpi_fallback() {
// The behavior tests live beside the policy in winit (util::randr::tests).
// This guards PdfCraft's vendored copy, which workspace tests do not test itself.
let root = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../..");
@@ -1,7 +1,9 @@
//! vendor/winit patch (5) (vendor/README.md).
//! Provenance-only: reads the retained `vendor/` copy, not the resolved dependency, so it is
//! not runtime coverage. `vendor_graph.rs` checks the resolved graph identity.
//! Retained vendor provenance for winit patch (5) (vendor/README.md); `vendor_graph.rs` checks the resolved graph.
#[test]
fn winit_focuses_the_x11_input_context_it_replaces() {
fn vendor_provenance_winit_focuses_the_x11_input_context_it_replaces() {
// egui allows IME when a text field gains focus and disallows it when the field loses focus.
// On X11, winit 0.30.13 replaces the window's input context each time and leaves focusing the
// new one to the next focus event, which never comes while PdfCraft's window keeps the focus: