Skip to content

Engine sync to latest Servo, plus the corner and Meta-key fixes - #5

Merged
Goddv merged 6 commits into
mainfrom
goddv/rust-egui-audit-52ee8d
Aug 22, 2026
Merged

Engine sync to latest Servo, plus the corner and Meta-key fixes#5
Goddv merged 6 commits into
mainfrom
goddv/rust-egui-audit-52ee8d

Conversation

@Goddv

@Goddv Goddv commented Aug 22, 2026

Copy link
Copy Markdown
Owner

Four things: the corner bug from #4, a sync onto the freshly-synced engine, one
input fix that came with it, and then the features that sync actually unlocked.

1. Corners — closes the real half of #4

The glow blend was shared across platforms and is already on main
(7f1859b): chrome_fill_at combined the lit band with theme::mix, which
returns an opaque colour, from two colours already premultiplied down by the
tint — so it produced an opaque colour whose RGB had been scaled by an alpha it
then discarded. Pure epaint arithmetic, no platform in it, which is why "turn
the glow off" helped on both macOS and Windows.

The corner technique was not shared and was applied as though it were.
Cutting a corner out of the framebuffer and painting the chrome back over the
hole matches perfectly — but an erased pixel is only a hole where the window
composites with alpha and something sits behind it. On macOS that is the
NSVisualEffectView; elsewhere the window is opaque and destination-out takes
the colour with it, leaving black. The erase was never gated, so it had been
running on Windows and Linux all along.

pub enum Corners {
    AlreadyRounded, // an internal page rounds itself
    Cut,            // translucent window: cut, repaint at the chrome's tint
    Masked,         // opaque window: hide the square corner under opaque chrome
}

Chosen from whether the backdrop actually installed, not from cfg(target_os).

2. Engine — latest Servo, and the patch is no longer optional

The fork is synced to upstream bd220a152bc0.

The downloads patch is still needed. Upstream at bd220a15 has no embedder
download API under any name; servo/servo#40210 is still open.

It still applies unchanged. Cherry-picked with no conflicts, and the
regenerated diff is byte-identical to the committed one — none of the context
moved. patches/ is untouched.

goddv-patches            929546bf7b1a   (was 36176b7d2c06)
zervo-downloads-f7cd7d8  36176b7d2c06   preserved, pinned by current main
zervo-downloads-a57e1ff  0d5a12189e50   preserved, pinned by v0.4.1

Both older commits were pushed to their own branches and confirmed reachable
before the branch moved, so no release can lose its engine.

The branch is now called goddv-patches rather than zervo-downloads:
it is the place engine patches live, and downloads is only the first one. The
pin is by revision, so nothing depended on the name.

The patch is committed rather than appended. main had stopped building
without it: the 21 August engine bump renamed MouseButton's variants,
src/main.rs follows the new names, and the newest servo on crates.io is
still 0.5.0 with the old ones. A plain cargo build failed with three E0599s
while every workflow stayed green — they all appended the patch, and the one
job that runs on a PR never builds the binary. So .cargo/config.toml carries
it, the four append steps are gone, and the revision lives in one file instead
of four.

⚠️ The part worth reviewing hardest. Moving the pin silently dropped
71 crates from the graph, because upstream has since put webgl and
webcrypto behind cargo features that default-features = false excludes. In
the published 0.5.0 both came for free. Lost were servo-webgl and the entire
crypto stack — aes, rsa, ecdsa, ed25519-dalek, ml-kem, sha3. User
impact would have been window.crypto undefined (so crypto.randomUUID() and
getRandomValues() throwing across much of the web) and
canvas.getContext("webgl") returning null, with nothing naming the browser as
the cause. Both features are now named explicitly; a re-diff of the lockfile
shows 7 crates still absent, all upstream churn Zervo cannot reach — jemalloc
moved behind use-jemalloc, which servo does not re-export.

GStreamer is unchanged and still on by default on macOS and Linux. Windows has
never had a media input at all — a gap, not a regression.

3. Command reports as Meta

winit calls that key Super; the spec and every other browser call it Meta,
so a page testing e.key === "Meta" never saw Command. src/keyboard.rs is a
copy of servoshell's keyutils.rs and predates servo#47330. Diffing the two
tables, it is the only substantive difference.

4. What the bump unlocked

The point of syncing was to be able to wire things, so here they are.

A preferences block

Nine preferences Servo ships off by default. The one that was actually
breaking pages is dom_intersection_observer_enabled: without it
loading="lazy" images never load at all, so any lazy-loading site showed
blank rectangles forever. Alongside it the async clipboard, adopted
stylesheets, container queries, multi-column layout, variable fonts, the visual
viewport, the Permissions API, and WebGL 2.

WebGL 1 is untouched — a canvas asking for webgl still gets it. Verified at
runtime rather than assumed:

PROBE webgl2=YES webgl1=YES IntersectionObserver=YES asyncClipboard=YES
      adoptedStyleSheets=YES containerQueries=YES columns=YES
      visualViewport=YES permissions=YES Notification=YES crypto_subtle=YES

shell_background_color_rgba is set from the resolved palette, so the flash
between navigations is the theme's own background instead of white.

Five delegates that were empty defaults

  • notify_status_text_changed — the link target on hover, bottom-left,
    clipped to the content card.
  • request_unloadbeforeunload, through the existing controls queue.
  • request_permission — a prompt naming the host that asked, replacing a
    silent deny.
  • evaluate_javascript — plus one honest use: ⌘⇧L fills the saved login
    for the page. docs/PARITY.md claimed there was "no way to write into a
    page's fields", and the Settings copy repeated it to users. Neither was true.
    https only, exact host match, and the values cross as JSON literals rather
    than spliced into source, so a password containing a quote is data and not
    syntax. Only offering to save a login is genuinely blocked — there is no
    embedder hook for a submitted form.
  • show_notification — notifications are shown in the window, over the page
    that raised them, and kept behind a bell in the address bar so one you
    missed by six seconds is still there. Toasts grow out of the bell rather than
    appearing beside it: a notification with no visible cause reads as the window
    doing something, not the page. They are made of the same Surface::Menu glass
    as every other floating panel, so a theme that restyles menus restyles these.

Notifications needed a second engine patch — not the obvious one. Servo has
dispatched EmbedderMsg::ShowNotification to the delegate all along, but it
only reaches that call after fetching the notification's image, icon and badge:
show is invoked from the fetch-completion handler, so a notification carrying
none of those queues no requests, nothing ever completes, and it is never
shown at all
. Constructed, no error event, silence. Since a plain
title-and-body notification is what most pages raise, the delegate was
unreachable for the common case.

Found by testing by hand — permission granted, and nothing whatsoever appeared.
The fix is that waiting for an empty set of fetches is over immediately.
goddv-patches now carries two patches; the pin moves to 73db5dc4f44c.

There is no system-notification integration: that wants a signed bundle and
UNUserNotificationCenter on macOS. Icons, badges and images are dropped —
each would need uploading as a texture. Both stated in PARITY.md rather than
quietly omitted.

The bell's codepoint was read out of the vendored font rather than guessed. The
font's post table is version 3.0 and carries no glyph names, so the codepoint
came from the Phosphor stylesheet — after confirming that stylesheet agrees with
all sixty-two existing constants in src/phosphor.rs.

Three more bugs on this side, found by review

The engine patch made notifications arrive; an adversarial review of the new
code (four independent lenses, every finding put to separate agents to refute —
11 raised, 9 survived) found what would have greeted them.

  • Clicks fell through to the page. The toast Area painted through
    ui.painter() and never grew its min_rect, so egui stored it zero-sized,
    layer_id_at never resolved the pointer to that layer, and
    is_pointer_over_egui stayed false — which is what the event loop consults
    to decide a click belongs to the page. A toast showed a pointing-hand cursor,
    refused to be dismissed, and passed the click through to the page underneath.
    DismissNotification and ClearNotifications were unreachable.
  • The morph never ran. animate_bool_with_time(fresh_id, true, _) seeds
    the entry with the target and returns it, so grow was 1.0 on the very first
    frame — every toast snapped to its resting place fully drawn, and egui never
    requested the frames an animation needs. My code comment claimed the
    opposite. Driven by the toast's own age now.
  • The bell overflowed the pill during a load. One 24pt trailing slot, taken
    by the spinner, leaving the bell 21pt outside the glass — visible as the bell
    jumping out of the pill and back on every navigation.

Plus one clock reading per frame instead of two straddling the draw pass, and a
test that read ids out of the model and compared them against the same model,
so a positional id passed it. Confirmed by mutation: under id: items.len()
the old test stays green and the new one fails.

Documentation that had drifted

PARITY.md and TODO.md listed devtools, notifications and protocol handlers
as blocked on the engine; all three are delegate methods that exist. Meanwhile
geolocation, camera and microphone really are engine gaps — Servo ships no
Geolocation and no getUserMedia IDL, so those prompts can never appear
however well the permission plumbing works. The lists now say which is which.

Verification

On macOS against 929546bf7b1a: plain cargo check clean where it previously
failed, clippy clean under -D warnings with and without
--features engine-downloads, fmt clean. The patch was round-tripped onto a
pristine bd220a15 — 18 files, 375 insertions, no fuzz.

Tests are now 77: the ten new ones cover the notification model, including
tag replacement restarting the linger clock, a notification lingering out
without being discarded (which is what the bell opens onto), and the wake
deadline being empty once nothing is counting down — the case that would
otherwise spin the idle scheduler forever, which is the same class of bug this
branch already fixed once in the repaint path.

The feature block and the absence of panics were checked by running the binary,
not by reading it.

Notifications are now verified end to end on macOS against the patched engine:
permission prompt, three toasts rendering top-right newest-first, and the bell
in the address pill with its count badge — screenshotted, not inferred. The
grow-out-of-the-bell morph is fixed by construction and reasoned through, but
has not been watched frame by frame.

Not verified on Windows or Linux. No toolchain here and Servo does not
cross-compile. The Windows corner path is reasoned from the blend equation, not
seen. A tagged build is the first real exercise of both.

Goddv added 3 commits August 22, 2026 15:55
…he OS

Reported on Windows (#4): the content card's corners come out wrong there too,
and the same workaround helps — turn the glow off.

Two separate things were going on, and only one of them was the same bug.

**The glow blend was shared, and is already fixed.** `chrome_fill_at` combined
the lit band with `theme::mix`, which returns an opaque colour, so a translucent
glow came back with its RGB scaled by an alpha it then discarded. That is pure
epaint arithmetic with no platform in it, which is why turning the glow off
helped on both. It now blends premultiplied.

**The corner technique was not shared, and was being applied as though it
were.** Cutting a corner out of the framebuffer and painting the chrome back
over the hole is only meaningful where the window composites with alpha and
something sits behind it — on macOS, the `NSVisualEffectView`. Everywhere else
the window is opaque, `with_transparent(true)` being macOS-only, and the
destination-out pass takes the colour with it and leaves black. The erase was
never gated, so it had been running on Windows and Linux all along; the
previous commit made that worse by taking it from two corners to four.

So the choice is now named rather than assumed. `Corners::{AlreadyRounded, Cut,
Masked}`: an internal page rounds itself; a translucent window cuts and repaints
at the chrome's own tint; an opaque one cannot cut, so it hides the page's
square corner under opaque chrome, which is what hiding means. `main.rs` decides
from whether the backdrop actually went in, not from `cfg(target_os)` — if the
effect view ever fails to install, an erased corner would be a hole onto
nothing.

The masked path now carries the glow too. Dropping it was a workaround for the
blend bug above, and with that fixed the mask can match the lit chrome beside
it instead of standing out against it.

While the signature was being touched: the four Appearance flags become
`CardFrame`. `border` sat between two `f32`s and next to two `Option`s, and
transposing any of them changed the render silently. That also retires the last
`too_many_arguments` suppression in the tree.

Not verified on Windows — there is no toolchain for it here and Servo does not
cross-compile. The macOS path is unchanged and still measured; the Windows path
is reasoned from the blend equation and wants a look before this is called done.
The fork is synced to upstream `bd220a152bc0` (22 August 2026), so this rebases
the downloads patch onto it and moves every pin.

**The patch is still needed.** Upstream at `bd220a15` has no embedder download
API of any kind — no `UnsupportedResponse`, no `notify_response_chunk`, nothing
under another name. servo/servo#40210 is still open and still assigned
elsewhere, which the patch's own commit message already said.

**It still applies unchanged.** Cherry-picked onto `bd220a15` with no
conflicts, and the regenerated diff is byte-identical to the committed one —
none of the context around those 18 files moved. So `patches/` is untouched
here; only the revision changed.

    zervo-downloads          929546bf7b1a   (was 36176b7d2c06)
    zervo-downloads-f7cd7d8  36176b7d2c06   preserved, pinned by current main
    zervo-downloads-a57e1ff  0d5a12189e50   preserved, pinned by v0.4.1

Both older commits were pushed to their own branches and confirmed reachable on
the remote *before* `zervo-downloads` moved, so no release can lose its engine.

**The patch is committed rather than appended.** `.cargo/config.toml` used to
keep it commented out "so a fresh clone builds against the published crate with
no setup", and each workflow appended the same block. That stopped being true on
21 August: the engine bump renamed `MouseButton`'s variants and `src/main.rs`
follows the new names, while the newest `servo` on crates.io is still 0.5.0
carrying the old ones. A plain `cargo build` had been failing with three E0599s
while every workflow stayed green — the workflows all appended the patch, and
the one job that runs on a pull request never builds the binary.

So the block is uncommented and the four append steps are gone, along with the
`SERVO_FORK`/`SERVO_REV` variables that only fed them. The revision now lives in
one file instead of four, which is one fewer thing to drift. `Cargo.lock` is
committed with the git source, as it must be for that to be reproducible.

GStreamer is unchanged and still on by default where it was: macOS and Linux
both default `media` to true and install it themselves. Windows has never had a
`media` input at all — that is a gap, not a regression, and wants its own change.

Verified on macOS against `929546bf7b1a`: plain `cargo check` clean where it
previously failed, clippy clean under `-D warnings` with and without
`--features engine-downloads`, 67 tests passing, fmt clean. The regenerated
patch was also round-tripped onto a pristine `bd220a15` checkout — 18 files,
375 insertions, applies with no fuzz.
winit calls the Command key on macOS — and the Windows key elsewhere — `Super`.
The UI Events spec calls that key `Meta`, and that is what browsers put in
`KeyboardEvent.key`. Zervo passed `Super` straight through, so a page testing
`e.key === "Meta"` never saw Command at all.

`src/keyboard.rs` is a copy of servoshell's `keyutils.rs` and predates the fix
upstream made in servo#47330. Diffing the two tables against the engine this
branch now builds on, that mapping is the only substantive difference between
them; the other is `to_owned()` where servoshell writes `to_string()`.
@Goddv Goddv changed the title Corners: pick the rounding technique from what the window can do Engine sync to latest Servo, plus the corner and Meta-key fixes Aug 22, 2026
Goddv added 3 commits August 22, 2026 17:34
Nine preferences Servo ships off by default and that Zervo has every reason
to have on. IntersectionObserver is the one that was actually breaking
pages: without it `loading="lazy"` images never load at all, so any site
that lazy-loads showed blank rectangles forever. Alongside it the async
clipboard, adopted stylesheets, container queries, multi-column layout,
variable fonts, the visual viewport, the Permissions API, and WebGL 2 —
WebGL 1 is untouched and still answers anything that asks for it by name.

`shell_background_color_rgba` comes from the resolved palette, so the flash
between navigations is the theme's own background rather than white.

Then the delegates that were sitting there as empty defaults:

- `notify_status_text_changed` — the link target, bottom-left, clipped to
  the content card.
- `request_unload` — beforeunload, through the existing controls queue.
- `request_permission` — a prompt, named for the host that asked.
- `evaluate_javascript` — and one honest use of it: ⌘⇧L fills the saved
  login for the page. PARITY.md claimed there was "no way to write into a
  page's fields" and the Settings copy repeated it; neither was true.
  https only, exact host match, and the values cross as JSON literals
  rather than spliced into source.
- `show_notification` — a notification is shown in the window, over the
  page that raised it, and kept behind a bell in the address bar so one
  you missed by six seconds is still there. Toasts grow out of the bell
  rather than appearing beside it: a notification with no visible cause
  reads as the window doing something, not the page. They are made of the
  same `Surface::Menu` glass as every other floating panel, so a theme
  that changes what a menu looks like changes these too.

No system-notification integration — that wants a signed bundle and
`UNUserNotificationCenter` — and the icons, badges and images the spec
allows are dropped. Both said plainly in PARITY.md rather than implied.

The bell's codepoint was read out of the vendored font: `post` is version
3.0 so it carries no glyph names, and the Phosphor stylesheet that agrees
with all sixty-two existing constants is the one it came from.

Ten tests on the notification model — tag replacement restarting the
clock, lingering out hiding without discarding, and the deadline being
empty once nothing is counting down, which is what would otherwise spin
the idle scheduler forever.

The fork's branch is now `goddv-patches`, named for the place engine
patches live rather than the one patch on it. `show_notification` needed
none: Servo has dispatched it to the embedder all along.
Testing by hand found the feature did nothing at all: permission was granted
and no notification ever appeared. Four separate defects, one of them in the
engine.

**The engine.** `Notification`'s show steps wait for the image, icon and badge
a notification may carry, and Servo calls `show` from the fetch completion
handler once the last request lands. A notification carrying none of those
queues no requests, so nothing completes and `show` is never reached: it is
constructed, fires no `error` event, and is never handed to the embedder. Since
a plain title-and-body notification is what most pages raise, the delegate was
unreachable for the common case. Waiting for an empty set of fetches is over
immediately. `goddv-patches` carries it as
`0002-script-show-notification-without-resources.patch`.

So the earlier claim that notifications needed no engine patch was wrong — the
dispatch is there, but nothing ever reached it.

**Clicks fell through to the page.** The toast `Area` painted through
`ui.painter()` and never grew its own `min_rect`, so egui stored it zero-sized,
`layer_id_at` never resolved the pointer to that layer, and
`is_pointer_over_egui` stayed false — which is exactly what the event loop
consults to decide a click belongs to the page. A toast showed a pointing-hand
cursor, refused to be dismissed, and passed the click through to the page
underneath; `DismissNotification` and `ClearNotifications` were unreachable.
`advance_cursor_after_rect` claims the space.

**The morph never ran.** `animate_bool_with_time(fresh_id, true, _)` seeds the
entry with the target and hands it straight back, so `grow` was 1.0 on the
first frame and every frame after — every toast snapped to its resting place
fully drawn, and egui never requested the frames an animation would need. The
ids here are new by construction, so that is all that could ever have happened.
Driven by the toast's own age now, with the event loop asking for frames while
one is still growing.

**The bell overflowed the pill during a load.** The address pill reserved one
24pt trailing slot; the spinner took it and the bell hung 21pt outside the
glass. Since the bell outlives any one page load, that showed as the bell
jumping out of the pill and back on every navigation.

Also: one clock reading per frame. `view` and `next_deadline` each took their
own, straddling the draw pass and the engine's paint, so a toast falling due
between the two was painted and then never woken for — it would sit over the
page until unrelated input.

And a test that did not test what it was named for: it read ids out of the
model and compared them against the same model, so a positional `id` passed it.
Confirmed by mutation — under `id: self.items.len()` the old test stayed green
while the new one fails, which is the point of it.
Beside the security badge, where a page's own status already lives, rather
than at the far end of the pill.

It also stops being a special case. At the right-hand end it was painted after
the address field had already taken its width, so it had to share the spinner's
reserved slot and hung 21pt outside the glass whenever a page was loading —
and since the bell outlives any one page load, that read as the bell jumping
out of the pill and back on every navigation. On the left it is an ordinary
laid-out widget: the field's available width accounts for it, and the slot
arithmetic goes away with it.

The toasts grow out of wherever it is, so the morph follows for free.
@Goddv
Goddv merged commit c58cacb into main Aug 22, 2026
4 checks passed
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.

1 participant