Repository navigation
fts-focus: upstream plus the FTS crash and rendering fixes - #3
Merged
Merged
Conversation
Signed-off-by: Nico Burns <nico@nicoburns.com>
* Enable browser CI for pull requests Signed-off-by: Nico Burns <nico@nicoburns.com> * Use dioxus cli 0.7.7 on windows/linux Signed-off-by: Nico Burns <nico@nicoburns.com> * Use whole dir for artifacts on windows/linux Signed-off-by: Nico Burns <nico@nicoburns.com> * Add nice job names * Upload single file artifacts Signed-off-by: Nico Burns <nico@nicoburns.com> * Fix macos x86_64 dmg location Signed-off-by: Nico Burns <nico@nicoburns.com> --------- Signed-off-by: Nico Burns <nico@nicoburns.com>
Signed-off-by: Nico Burns <nico@nicoburns.com>
…s#416) * perf(browser): move urlbar fuzzy matching to nucleo worker Replace the synchronous nucleo-matcher rescore with a tokio worker that drives the higher-level `nucleo` crate, so each keystroke no longer rescans the entire history on the UI thread. The toolbar pushes queries into the worker via a channel and reads results from a Signal; history entries are pushed in via a use_effect so the matcher restarts when history changes. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * Fix merge conflict --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Nico Burns <nico@nicoburns.com>
(so they don't all show up in the history with the same URL)
Signed-off-by: Nico Burns <nico@nicoburns.com>
* Implement `user-select:none`. This implementation only prevents starting a selection in the node, and does not prevent selecting pseudo-elements with `user-select: auto`. Signed-off-by: Nico Burns <nico@nicoburns.com> * Don't use text cursor for text with `user-select:none` --------- Signed-off-by: Nico Burns <nico@nicoburns.com>
Signed-off-by: Nico Burns <nico@nicoburns.com>
* Add Browser UI build for Android Signed-off-by: Nico Burns <nico@nicoburns.com> * Run Publish Browser on PR Signed-off-by: Nico Burns <nico@nicoburns.com> * Only use Signed Builds environment for main branch builds Signed-off-by: Nico Burns <nico@nicoburns.com> * Only write private key file on main branch Signed-off-by: Nico Burns <nico@nicoburns.com> * Only delete prviate key file on main branch Signed-off-by: Nico Burns <nico@nicoburns.com> * Pass --android and fix artifact path * Run Android build on x86 host (arm runner doesn't include the Android SDK) Signed-off-by: Nico Burns <nico@nicoburns.com> --------- Signed-off-by: Nico Burns <nico@nicoburns.com>
* Rename apk Signed-off-by: Nico Burns <nico@nicoburns.com> * Remove android from name Signed-off-by: Nico Burns <nico@nicoburns.com> --------- Signed-off-by: Nico Burns <nico@nicoburns.com>
…'t exist even though mousebutton is down Signed-off-by: Nico Burns <nico@nicoburns.com>
…latforms Signed-off-by: Nico Burns <nico@nicoburns.com>
Signed-off-by: Nico Burns <nico@nicoburns.com>
Signed-off-by: Nico Burns <nico@nicoburns.com>
Replaces the CustomPaintSource API with a new Custom Widget API based based on the Widget trait. See custom_widget and wgpu_texture examples for details. Signed-off-by: Nico Burns <nico@nicoburns.com>
…ocuments) Signed-off-by: Nico Burns <nico@nicoburns.com>
Signed-off-by: Nico Burns <nico@nicoburns.com>
* Add WASM support Signed-off-by: Nico Burns <nico@nicoburns.com> * wasm_7guis: fix Update/Delete buttons appearing disabled when selection exists Blitz does not invalidate :disabled pseudo-class styling when the disabled attribute changes reactively. Drive the disabled visual state via a .btn-off class instead, matching the class-toggle pattern already used for .list-item.selected. The disabled attribute is kept for semantics; hover suppression is updated to exclude .btn-off buttons. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Upgrade to latest AnyRender `main` --------- Signed-off-by: Nico Burns <nico@nicoburns.com> Co-authored-by: Nico Burns <nico@nicoburns.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Nico Burns <nico@nicoburns.com>
Signed-off-by: Nico Burns <nico@nicoburns.com>
DioxusLabs#429) Fix examples CI build. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…#431) (DioxusLabs#432) Alternative implementation of Nico's hypothesis on DioxusLabs#431. Instead of stripping the inline `canvas.style.width/height` that winit-web writes, drop the `.with_surface_size(...)` call on wasm entirely so the styles never get set in the first place. Host CSS controls the canvas box. Three pieces in one branch: - Examples (seven_guis, todomvc, wasm_hello) no longer call `.with_surface_size(...)` on wasm; the seven_guis and todomvc `index.html` files get `canvas { width: 100%; height: 100% }` so the canvas fills the viewport once winit isn't pinning it. - `View::init` falls back to reading the canvas's CSS layout box (`offset_width × offset_height × devicePixelRatio`) when winit-web's initial `surface_size()` is 0×0 — without this, wgpu fails with a swapchain-size-0 error before the first ResizeObserver fires. - Same wgpu `surface.configure` debounce as DioxusLabs#431, with a coalesce guard so a continuous browser-window drag schedules one timer instead of one per `SurfaceResized`. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
* blitz-dom: honour @font-face descriptors in FontInfoOverride Propagate font-family / font-weight / font-style from the @font-face descriptors all the way through the NetProvider handler to Collection::register_fonts, instead of dropping them and forcing parley to read the family name from the TTF's name table. * blitz-dom: unit tests for stylo_to_fontique_style mapping Cover the four interesting branches of the stylo -> fontique font-style translator added in the prior commit: - StyloFontStyle::Italic -> FontStyle::Italic - Oblique(0deg, 0deg) -> FontStyle::Normal (CSS `normal` round-trip) - Oblique(14deg, 14deg) -> FontStyle::Oblique(Some(14.0)) - Oblique(10deg, 20deg) -> FontStyle::Oblique(Some(10.0)) The Oblique(0, 0) -> Normal mapping is the subtle case stylo's parser emits for bare CSS `normal`; pinning it stops a future refactor from silently regressing upright font matching. * blitz-dom: regression-pin @font-face descriptor honouring Drive load_resource directly with a Resource::Font carrying FontFaceOverrides { family_name: Some(\"AliasedFamily\"), .. } and assert that fontique's Collection ends up with a family registered under the CSS-declared alias rather than the TTF's internal `name` table entry. We bypass HTML parsing here because blitz-html (which owns the parser that drives fetch_font_face) is a downstream crate and would create a circular dependency. The descriptor parsing path is already covered by the stylo_to_fontique_style unit tests in net.rs; this test pins the load-side of the pipeline so a future refactor of load_resource can't silently drop the override again. --------- Co-authored-by: Henrique Anzoategui <hanzoategui@kigo.io>
The HTML parser sets node_to_autofocus, but toolkits that build an element then set attributes (dioxus-native-dom) apply `autofocus` as a normal attribute mutation that never hit that path. set_attribute now focuses the node when autofocus becomes truthy (after flush_is_focussable so the node is actually focussable).
draw_inline_backgrounds painted a background only over the glyph's ascent→descent. Used as a text-selection highlight that leaves a white gap (the line-height leading) between lines. Now a TRANSLUCENT background (alpha < 1 — a selection) fills the full line box so adjacent selected lines meet with no gap, like a browser selection; an OPAQUE background (e.g. a reverse-video block caret) keeps the tight glyph box so the cursor still hugs the character.
… tree The stacking context is built during layout and holds node ids. Remove a positioned child — a popup closing, a modal dismissed — and hit-test before the next layout, and the id is dangling: `Node::with` unwraps a `None` and takes the window down. The panic is hard to place. It arrives on a pointer move, far from whatever removed the node, and names whichever component happened to be on screen — found by mounting an EQ editor whose band popup follows the pointer, where it read as a bug in the EQ. Both the positive- and negative-z loops now skip a node that is gone. There is nothing to hit at a node that does not exist.
…d ones The first pass guarded the two hoisted-children loops and the next pointer move panicked one loop further down, in `paint_children`, for the same reason: these lists are filled during layout and hold node ids, and anything removed between a layout and a hit-test leaves a dangling one. `paint_children` and the inline layout's brush ids get the same treatment. A node that is gone is not one the pointer can be over, so it is skipped. Fixing these one crash at a time does not converge — every `with()` on this path is the same latent panic — so this takes all of them in `hit_inner`.
mark_style_attr_updated set dirty_descendants on the element itself, which tells the style traversal something beneath me is dirty and makes it walk that element's whole subtree. Every other mutation path in the mutator marks ANCESTORS instead, so the traversal reaches the changed node without descending past it. A style attribute changes that element's own computed style. Stylo already propagates to children when the change is one that inherits, so forcing the walk is redundant where it is cheap and ruinous where it is not. Measured on a timeline UI that scrolls by writing one transform onto a wrapper — the standard way to move content without re-rendering it — with about 1,700 nodes inside the wrapper: style: 55ms -> 0.1ms resolve: 65ms -> 3.7ms The frames that changed nothing already cost 0.3ms, so the 55ms was entirely this. Any app that scrolls, pans or animates a container by writing an inline style pays it in proportion to what the container holds.
`draw_svg` passed `ObjectFit::Contain` as a literal, with the line that reads the computed value sitting commented out above it. `draw_image`, ten lines below, already reads it. The initial value of `object-fit` is `fill`, not `contain`, so this was not a conservative default — it letterboxed every SVG whose box was not its own aspect ratio, which is every SVG sized by CSS rather than by its own width and height attributes. A page that sized an `<svg>` to a box and expected it to fill the box got it centred inside it instead, with no way to ask for anything else.
`StyleThreading`'s doc comment said the default was `Parallel` and its `#[derive(Default)]` said `Sequential`. The code won, so every document has been resolving style on one thread — on a tree of a few thousand nodes that is most of a frame. Measured on an arrangement of 8377 nodes at 5120x1440, one changed transform: style and layout 10.65ms a frame sequential, 4.72ms parallel. The default was not arbitrary. Two documents driving Stylo's global pool at once panic with `already mutably borrowed` (DioxusLabs#430), and making callers opt out was the cheapest way to stop that. But it made the slow path the default for every document, including the overwhelming majority that never have a second one. So a document CLAIMS the pool instead of assuming it. One gets the parallel traversal; anyone arriving while it is held traverses sequentially for that frame. The crash cannot happen, the fast path is what you get without asking, and `Sequential` stays for a caller who wants the pool left alone.
`draw_svg` cast the content box through `u32` before handing it to the object-fit arithmetic, so an element 100.7 pixels wide scaled its drawing to 100. Every SVG on a page was squeezed by whatever fraction its own box happened to carry — a sub-pixel error, but a different one per element, which reads as a general softness against a renderer that does not do it. `draw_image` below it needs the integers, because a raster image is sampled on a pixel grid. A vector tree is not.
`flush_styles_to_layout` rebuilt every node's Taffy style and rescanned
its background and mask image layers on every frame, for the whole tree,
whether or not anything had changed. On an arrangement of ten thousand
nodes with one changed transform that was 2.5ms — the largest single
phase in the frame, and all of it re-deriving answers that were already
right.
The gate for it was sitting there commented out, with the damage read
into `_damage` and thrown away:
// if damage.intersects(RestyleDamage::RELAYOUT | CONSTRUCT_BOX) {
node.style = stylo_taffy::to_taffy_style(style);
`propagate_damage_flags` has already run by then, and what it leaves on a
node is that node's damage unioned with everything beneath it, so an
empty one is a reliable answer. With the conversion and the image scans
gated on it, the phase goes to 7us.
The WALK still visits every node, and deliberately. Skipping whole clean
subtrees is the obvious next step and it is wrong: the ordering work
lower down — `paint_children`, the hoisting of positioned children into
a stacking context — depends on children whose damage the parent's own
does not fully describe, and a version that returned early left a
composed window laying out plates at last frame's width. The walk is
cheap; what it was doing at each node was not.
It is 6% of a frame on a large tree and it runs whether or not anything moved, which makes it look like an easy win. It is not: `compute_root_layout` rewrites unrounded layouts for anything its cache missed, and `final_layout` is only ever written by the rounding pass, so gating it on damage left the window laying out at zero size — every surface in the comparison suite went blank. Making it incremental means teaching Taffy to report which nodes it actually recomputed. Noted here so the next person does not spend the afternoon I did.
`draw_custom_widget` carried a `TODO: eliminate clone`. It was not a
tidiness note: the clone is a deep copy of every path a widget drew,
taken once per frame, and on a DAW arrangement that is the whole
picture — tens of thousands of curve segments.
The map is built at the top of `paint_scene` and dropped at the bottom
of it, and no node is painted twice, so there is nothing to preserve.
The map goes behind a `RefCell`, the walk carries the KEY rather than a
borrow, and the scene is removed at the point it is appended.
Measured on a 40-track session at 5120x1440, one custom widget holding
the arrangement, comparing the same gestures before and after:
pan 9.08ms -> 3.91ms (paint 6.08 -> 2.14)
scroll 10.88 -> 3.42 (paint 7.78 -> 2.03)
zoom y 17.76 -> 5.08 (paint 14.05 -> 3.41)
zoom x 6.76 -> 4.32 (paint 4.21 -> 2.41)
…ts events carry Two things a widget could not do. `handle_event` returned nothing, so a hover or a click inside a widget updated state that nothing ever drew — the document has no way to know a widget's contents changed, and there is nobody else to ask. It now returns whether a repaint is needed, and the dispatcher requests one. The default is `false`, which is what a widget ignoring the event means. And `BlitzPointerEvent` is `pub` with `pub` fields of `keyboard_types` types that were imported rather than re-exported, so a crate implementing `Widget` could not name them — it would have to guess which `keyboard_types` in the graph is the right one, and there is more than one.
…g handle_event A widget's contents are its own, so nothing else in the document can know that a hover or a click inside one changed the picture. Without asking, that state is updated and never drawn. Asked as a separate defaulted `needs_redraw()` rather than by making `handle_event` return a bool, deliberately. The signature version is tidier and it is also a breaking change to a public trait: a widget written for a blitz that does not ask stops compiling, and in a workspace where two crates pin different revs of this repo there is no version of the file that satisfies both. Additive, there is nothing to coordinate — a widget compiles against either, and works against either.
`Paint: ...ms` on the render path was scaffolding: it costs more than the frames it measures, it only reaches whoever is watching the terminal, and it fires on a threshold rather than every frame, so it says nothing about the distribution. The measurement itself is worth keeping — encode and present want telling apart, because a window that draws too much and a window that draws little and waits on the compositor are the same frame time and want opposite fixes. So store them the way `LAST_FRAME_MICROS` is already stored, and let the application draw them beside it. Also drops an import `blitz-paint` stopped using when the custom-widget scene moved into the frame. Claude-Session: https://claude.ai/code/session_01Uw7D4C1dc4SChCx3DTrHYD
The document remembers four node ids across events — hover, focus, active and mousedown — and nothing renewed them when the node they named was removed. A subtree that rebuilds mid-interaction (a custom widget being dragged is the ordinary case) left them pointing at a slab slot that was gone, and the next event to read one panicked with `invalid key` on a pointer move, far from the mutation responsible. Cleared at `drop_node_ignoring_parent`, the single point where a node actually leaves the slab, rather than guarded at each read: a guard per read site is one more site to remember next time, and fixing these one crash at a time does not converge. Claude-Session: https://claude.ai/code/session_01Jc13tRSX2gTUJJ5Sso1CfN
`log-phase-times` was in `default`, so every consumer taking blitz-dom with
default features got `Resolve(1): 3.8ms (style: …, layout: …)` printed from
library code, once per frame, forever. In an app that redraws at 40 Hz that
is 40 lines a second burying whatever the application was trying to say.
It is a profiling tool and it stays available as one:
blitz-dom = { features = ["log-phase-times"] }
Claude-Session: https://claude.ai/code/session_01Jc13tRSX2gTUJJ5Sso1CfN
A button or input added and removed within one mutation batch stayed queued in form_nodes; flush then called reset_form_owner on its dead id and the slab panicked with "invalid key" — about half of all launches of the Signal app, as the first renders replace whole subtrees. process_removed_subtree now drops the node from the form, style and title queues, and reset_form_owner ignores an id that no longer exists.
The computed colour was serialised in its own colour space. Under a Tailwind v4 theme that is oklch(), which usvg cannot parse, so every currentColor icon coloured by a class drew black. Convert to legacy sRGB before writing it into the SVG.
8de83e1 skipped the Taffy style conversion for a node with no damage. But the Taffy style holds raw pointers into the Stylo computed values (every calc() length), and a restyle can swap in new computed values without damage when they compare equal — freeing the old ones. The next layout then resolved a calc through freed memory: SIGSEGV in CalcNode::resolve_internal when a surface swapped a batch of Tailwind calc(var(--spacing) * n) classes. Record the address of the computed values each Taffy style came from and re-convert whenever it changes, damage or not.
…overlays) Not implemented yet. Fixed nodes leave Taffy's view of their parent, are laid out as their own roots against the viewport after the main pass, painted from the root stacking context and hit-tested first. File-level plan, deviations, risks and a test list.
…t's own removal remove_and_drop_pe (layout::construct::flush_pseudo_elements, whenever a restyle drops a node's ::before/::after) frees the slab slot directly, bypassing both places that already guard against a dangling reference — process_removed_subtree (mutator.rs, ordinary node removal) and drop_node_ignoring_parent's own forget_node. A hovered, active, focused or selected pseudo-element left hover_node_id et al. pointing at a freed slot, and the next ancestor walk off one of them (node_layout_ancestors, via hover or active) indexed it and panicked with 'invalid key' — far from any ::before/::after in sight, which is why it read as a bug in whichever component was on screen. forget_node is now pub(crate) and shared: extended to also clear a text-selection endpoint naming the node (the same class of gap mutator.rs's copy already covers for ordinary removal), and remove_and_drop_pe calls it for every node it frees, same as drop_node_ignoring_parent already does. Seen consistently after a large re-render — switching profiles in Signal's guitar rig frees a whole patch list at once — on maybe half of launches; a repro that reliably showed the same panic at different call sites (traversal.rs:451, :267) depending on which of the ~50% pre-existing form/style fixes it slipped past.
Hoisted (z-indexed) children are painted from the stacking-context root, outside every clip layer in between, so a z-indexed child of an overflow:scroll/hidden pane was drawn over whatever lay outside the pane once scrolled. Rooting them at the clipping element paints them inside its clip and scroll offset.
An event handler that removes the node under the pointer on pointerdown
(a popup's backdrop closing it) left that id as the active/hover node;
the next pointer event walked its ancestors and panicked on the slab
lookup ("invalid key"), taking the app down.
A node updated in the same batch of mutations that removed its parent
walked into the freed slot ("invalid key", node.rs) and took the app
down.
node_chain (the pointer-move hover chain) panicked with "invalid key" on a live node whose parent link still named a node freed in an earlier batch of mutations — after the rig's amp row re-rendered under the pointer. AncestorTraverser yields only parents that exist, and node_chain looks them up with get(). The dangling link itself is still to be found (it predates this: 8 of these panics in the 2026-09-21 log).
Records the animation time of each resolve so an embedder can bring layout up to date outside the frame loop (an input event arriving after mutations) without moving animations off the window's clock.
absolute_position (pointer-move coordinates), the cursor-icon walk and the inline-root walk followed layout_parent through Node::with (an unwrap): a link left pointing at an anonymous box a rebuild freed panicked the app on the next pointer move — seen after a delay-timing dropdown change, even with layout resolved before events. They now look the parent up and stop when it is gone.
…ick behaviour, not before — a <summary> is focussable and its toggle never ran (upstream's details tests); plus cargo fmt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The FTS Blitz branch (
fts-focus), which the signal desktop app renders with.It's upstream Blitz plus 29 FTS fixes. Among them:
currentColoris serialised as legacy sRGB, so an oklch theme no longer turns icons black.CalcNode::resolve_internal) when many Tailwindcalc()classes swap at once.It merges into
maincleanly:mainhas one commit it lacks. Merge this before the signal PR, which builds against it.