From 72389ed5a8de31bd446bb1479580b1d7b576fc9a Mon Sep 17 00:00:00 2001 From: lucasmerlin Date: Tue, 26 May 2026 16:20:07 +0200 Subject: [PATCH] egui_inspection: PNG-encode screenshots on the wire; collapse protocol feature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - FrameScreenshot now carries PNG bytes instead of raw RGBA (PROTOCOL_VERSION 1→2); add a shared `encode_png` helper behind a new `png` feature so the live plugin and the kittest harness encode frames identically. - Make the protocol module unconditional: drop the `protocol` feature flag and the optional serde/serde_bytes/rmp-serde deps it gated. - plugin.rs: re-stamp screenshot-bearing frames with the current step (so inspectors waiting for step > prev don't reject them) and pump a tail-side repaint while awaiting the GPU readback. --- crates/egui_inspection/Cargo.toml | 20 +++++++------- crates/egui_inspection/src/lib.rs | 8 ++++-- crates/egui_inspection/src/plugin.rs | 36 ++++++++++++++++++++++---- crates/egui_inspection/src/png.rs | 21 +++++++++++++++ crates/egui_inspection/src/protocol.rs | 12 +++++---- 5 files changed, 75 insertions(+), 22 deletions(-) create mode 100644 crates/egui_inspection/src/png.rs diff --git a/crates/egui_inspection/Cargo.toml b/crates/egui_inspection/Cargo.toml index dad208dd6..5ebf17bac 100644 --- a/crates/egui_inspection/Cargo.toml +++ b/crates/egui_inspection/Cargo.toml @@ -21,27 +21,27 @@ all-features = true rustdoc-args = ["--generate-link-to-definition"] [features] -default = ["protocol"] - -## Wire-protocol types (`HarnessMessage`, `InspectorCommand`, …) plus length-prefixed -## MessagePack framing helpers. No `egui` dependency beyond `egui::accesskit`. -protocol = ["dep:rmp-serde", "dep:serde", "dep:serde_bytes", "egui/serde"] +default = [] ## Cross-platform local-socket name helpers ([`transport::socket_name`], ## [`transport::generate_socket_target`]) built on `interprocess`: unix domain sockets on ## unix, named pipes on Windows. Shared by both ends of the connection. transport = ["dep:interprocess", "dep:tempfile"] +## [`encode_png`] — shared screenshot PNG encoder so every peer (live plugin + kittest +## harness) produces identically-encoded frames. +png = ["dep:image", "image/png"] + ## `InspectionPlugin` — an `egui::Plugin` impl that streams frames + accesskit tree to ## an inspector over a local socket and applies received commands. Auto-attaches when ## the [`INSPECTION_SOCKET_ENV_VAR`] env var is set. -plugin = ["protocol", "transport", "dep:image"] +plugin = ["transport", "png"] [dependencies] -egui.workspace = true -serde = { workspace = true, optional = true } -serde_bytes = { version = "0.11.17", optional = true } -rmp-serde = { workspace = true, optional = true } +egui = { workspace = true, features = ["serde"] } +serde.workspace = true +serde_bytes = "0.11.17" +rmp-serde.workspace = true image = { workspace = true, optional = true } interprocess = { version = "2.4", optional = true } tempfile = { workspace = true, optional = true } diff --git a/crates/egui_inspection/src/lib.rs b/crates/egui_inspection/src/lib.rs index be27df02e..6dc62e268 100644 --- a/crates/egui_inspection/src/lib.rs +++ b/crates/egui_inspection/src/lib.rs @@ -3,10 +3,8 @@ //! ## Feature flags #![cfg_attr(feature = "document-features", doc = document_features::document_features!())] -#[cfg(feature = "protocol")] pub mod protocol; -#[cfg(feature = "protocol")] pub use protocol::{ Capabilities, Frame, FrameScreenshot, HarnessMessage, InspectorCommand, MAX_MESSAGE_BYTES, PROTOCOL_VERSION, PeerHello, PeerKind, SourceView, read_message, write_message, @@ -24,6 +22,12 @@ pub const INSPECTION_SOCKET_ENV_VAR: &str = "EGUI_INSPECTION_SOCKET"; #[cfg(feature = "transport")] pub mod transport; +#[cfg(feature = "png")] +mod png; + +#[cfg(feature = "png")] +pub use png::encode_png; + #[cfg(feature = "plugin")] mod plugin; diff --git a/crates/egui_inspection/src/plugin.rs b/crates/egui_inspection/src/plugin.rs index 09a2e9529..fc100d4a0 100644 --- a/crates/egui_inspection/src/plugin.rs +++ b/crates/egui_inspection/src/plugin.rs @@ -211,11 +211,26 @@ impl egui::Plugin for InspectionPlugin { if let Some(mut frame) = self.pending_frame.take() { let [w, h] = [image.size[0] as u32, image.size[1] as u32]; let rgba: Vec = image.pixels.iter().flat_map(|c| c.to_array()).collect(); - frame.screenshot = Some(FrameScreenshot { - width: w, - height: h, - rgba, - }); + match crate::encode_png(w, h, &rgba) { + Ok(png) => { + frame.screenshot = Some(FrameScreenshot { + width: w, + height: h, + png, + }); + } + Err(err) => { + eprintln!("[INSP] PNG encode failed: {err}"); + } + } + // Re-stamp the frame with the *current* step. The stashed `step` was + // captured when we dispatched the screenshot command; in the meantime + // intervening frames (without screenshot) may have been emitted with + // higher step numbers. Inspectors that wait for `step > prev_step` would + // otherwise reject the screenshot-bearing frame because its step has + // regressed. + self.step = self.step.saturating_add(1); + frame.step = self.step; self.send(HarnessMessage::Frame(Box::new(frame))); } break; @@ -296,6 +311,17 @@ impl egui::Plugin for InspectionPlugin { if !want_screenshot { // No screenshot needed — emit immediately. self.send(HarnessMessage::Frame(Box::new(frame))); + // If we're still waiting on a screenshot from a previous dispatch, keep + // pumping repaints from the end of the frame too. `input_hook` already + // does this at frame start, but on reactive apps the GPU readback can + // take several frames to fulfill — without a tail-side repaint the + // integration may go idle between `input_hook` ticks once the captured + // frame finishes presenting. + if self.awaiting_screenshot { + if let Some(ctx) = self.shared_ctx.get() { + ctx.request_repaint(); + } + } return; } diff --git a/crates/egui_inspection/src/png.rs b/crates/egui_inspection/src/png.rs new file mode 100644 index 000000000..c046fca23 --- /dev/null +++ b/crates/egui_inspection/src/png.rs @@ -0,0 +1,21 @@ +//! Shared screenshot PNG encoder. +//! +//! Both peers — the live [`crate::InspectionPlugin`] and `egui_kittest`'s harness inspector +//! — encode their frames here so they produce identically-encoded +//! [`crate::protocol::FrameScreenshot`]s. + +/// Encode tightly-packed RGBA8 pixels (`width * height * 4` bytes) as PNG using `image`'s +/// default settings (`CompressionType::Default` + `FilterType::Adaptive`). +/// +/// PNG keeps high-resolution captures off the hot path of socket throughput — a 1550×2114 +/// RGBA8 buffer is ~13 MiB raw but typically <1 MiB encoded. +/// +/// # Errors +/// When the encoder fails (e.g. the buffer length doesn't match `width * height * 4`). +pub fn encode_png(width: u32, height: u32, rgba: &[u8]) -> Result, image::ImageError> { + use image::ImageEncoder as _; + let mut out = std::io::Cursor::new(Vec::new()); + image::codecs::png::PngEncoder::new(&mut out) + .write_image(rgba, width, height, image::ExtendedColorType::Rgba8)?; + Ok(out.into_inner()) +} diff --git a/crates/egui_inspection/src/protocol.rs b/crates/egui_inspection/src/protocol.rs index c5468371c..c6629a761 100644 --- a/crates/egui_inspection/src/protocol.rs +++ b/crates/egui_inspection/src/protocol.rs @@ -20,7 +20,7 @@ use egui::accesskit; /// non-additive change is made to [`HarnessMessage`] / [`InspectorCommand`] / their /// payload structs. The inspector should refuse peers with a higher major version than /// it understands. -pub const PROTOCOL_VERSION: u32 = 1; +pub const PROTOCOL_VERSION: u32 = 2; /// What kind of egui peer the inspector is talking to. Determines which controls the /// inspector UI should render (Step / Pause buttons make no sense against a live app). @@ -132,11 +132,13 @@ pub struct FrameScreenshot { pub width: u32, /// Image height in physical pixels. pub height: u32, - /// Tightly packed RGBA8 pixels (length = `width * height * 4`). `serde_bytes` encodes - /// this as a msgpack `bin` blob (one type tag + raw bytes) instead of the default - /// `Vec` path of one type tag *per byte*, which would roughly double on-wire size. + /// PNG-encoded image bytes. PNG compression keeps high-resolution captures off the + /// hot path of unix-socket throughput — a 1550×2114 RGBA8 buffer is ~13 MiB raw but + /// typically <1 MiB as PNG. `serde_bytes` encodes this as a msgpack `bin` blob (one + /// type tag + raw bytes) instead of the default `Vec` path of one type tag *per + /// byte*, which would roughly double on-wire size. #[serde(with = "serde_bytes")] - pub rgba: Vec, + pub png: Vec, } /// A single update from the egui peer: accesskit tree + optional screenshot.