Skip to content

Render on the CPU with winit + softbuffer (drop the OpenGL dependency)#3

Open
domenkozar wants to merge 2 commits into
dsociative:mainfrom
domenkozar:feat/cpu-render
Open

Render on the CPU with winit + softbuffer (drop the OpenGL dependency)#3
domenkozar wants to merge 2 commits into
dsociative:mainfrom
domenkozar:feat/cpu-render

Conversation

@domenkozar

Copy link
Copy Markdown

Implements proposal #3 from #1 (optional pure-CPU rendering).

Replaces eframe + glow (OpenGL) with winit + softbuffer + egui_software_backend, so the crate rasterizes the dialog on the CPU and pulls no GPU/GL stack. This removes libGL from the runtime dependencies and sidesteps the "wgpu needs Vulkan, which may be absent" issue that motivated the glow choice, at no practical cost for a small modal dialog redrawn only on input.

  • One process-wide winit EventLoop reused across dialogs via run_app_on_demand, so multiple GETPIN/CONFIRM on a single gpg-agent connection work. The old per-dialog eframe::run_native fails the second time with "EventLoop can't be recreated".
  • The Assuan protocol layer (main loop, percent_decode/percent_encode_password, respond, PinentryState, show_dialog(...) -> DialogResult) is unchanged.
  • egui / egui-winit / egui_kittest bumped to the 0.34 line (egui_software_backend 0.0.3 pins egui 0.34).
  • No eframe / glow / glutin / wgpu anywhere in the tree; the egui_kittest tests pass headlessly.

Stacking: this is stacked on #2 (zeroizing input), so the diff below includes that change until #2 merges. The commit specific to this PR is 47f526b ("Render on the CPU ...").

This reverses the documented glow decision, so it is very much your call. Happy to hold it, land it as an optional CPU backend behind a feature rather than a replacement, or adjust however you prefer. Verified end to end on Wayland (GETPIN + CONFIRM).

Feed keystrokes directly into a zeroize::Zeroizing<String> and draw a
masked view, rather than an egui TextEdit whose retained widget state and
undo history keep the plaintext around after the dialog closes. Move the
value into the SecretString on submit (no clone; the buffer is zeroized on
drop). Tests drive the field with synthetic input events since the masked
view is no longer an accessible widget.
Replace the eframe/glow (OpenGL) backend with a pure-CPU pipeline: winit
for windowing, egui_software_backend to rasterize egui frames on the CPU,
and softbuffer to present them. No GPU, OpenGL, or Vulkan is required, so
the dialog runs on machines without working GL drivers.

- Bump egui, egui-winit, and egui_kittest to the 0.34 line (pinned by
  egui_software_backend 0.0.3, which pins egui 0.34 / winit 0.30).
- Drive one process-wide winit EventLoop with run_app_on_demand so a
  single gpg-agent connection can show several GETPIN/CONFIRM dialogs.
- Keep the Assuan protocol layer, the zeroizing masked passphrase buffer,
  and the show_dialog(state, want_pin) signature unchanged.
- Port the egui_kittest UI tests to the 0.34 API.

@dsociative dsociative left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Architecture accepted — reversing my documented glow decision is fine by me: dropping libGL plus ~150 packages from the tree, and fixing the "EventLoop can't be recreated" failure on repeated GETPINs (verified: three dialogs in one session) seals it. Binary size is a wash (5.5 MB vs 5.6 MB) and rendering on niri is indistinguishable from glow. One regression to fix before merge:

Clipboard paste is gone. With egui-winit at default-features = false, features = ["wayland"] there is no clipboard backend, so Ctrl+V never produces egui::Event::Paste and the paste arm in pin_dialog_ui is dead code. Confirmed empirically: wl-copy + Ctrl+V pastes on main, does nothing on this build (inside and outside a sandbox, so it's the build, not my environment). Pasting from a password manager is a real pinentry workflow — could you add "clipboard" to the egui-winit features? That only brings back smithay-clipboard/arboard, both already in the tree on main. Leaving "links" off is right for a pinentry — please keep it off.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants