Misc bugfixes - #95
Merged
Merged
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
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.
Summary
A batch of bug fixes and follow-through work across the layer model, watercolor brush, noise sampling, paste, save/export, and canvas resize. Several of the fixes were symptoms of the same underlying asymmetry — masks were modeled as a second-class entity kind with their own parallel document/undo primitives — so the layer work unifies that rather than patching each call site.
Layer entities: one kind-uniform attach/detach model
Masks (and future modifiers) hung off a host through a separate set of document primitives from tree nodes, and consumers had to know which pair to call. Using the wrong one half-detached a mask (unlinked from the host but left in its
filterslist) or reattached it intoroot.children.Documentnow has one primitive pair —link/unlink,reinsert_entity,remove_entity— that routes by the entity's own kind.LayerNodeowns the routing viaChildSlot+attach_child/detach_child(layer.rs); no caller asks what it's holding.move_layerrefuses a filter id outright — a mask has no position in the tree.FilterAddAction/FilterRemoveActionentirely;LayerAddAction/LayerRemoveActionbecomeEntityAddAction/EntityRemoveActionand cover every kind. Adding a new entity kind needs no new undo action. Restore now lands at the original index, so undoing removal of one of several modifiers no longer reorders the stack.DarklyEngine::detach_for_removedispatches on kind, so masks are deletable as ordinary layer-panel rows (single and batch), including in one undo step alongside layers. The "can't delete the last layer" floor counts only tree nodes;merge_downrejects a modifier id instead of indexing the host'schildrenwith a filter index.add_mask_unseededextracts the allocate-texture/ensure-snapshot half ofadd_mask, so duplicate and rich paste reuse it without pushing a spurious undo entry or seeding the mask from the receiving document's active selection.Layer reselection after delete
Deleting a layer left nothing selected (and left an isolated-node id dangling, blanking the canvas).
state/layerTree.ts: a single indexing walk answering liveness, panel order, visibility and parentage, plus the reselection rule — nearest surviving sibling below → above → parent, escalating when the parent died in the same batch. The rule is taken from GIMP (gimp_item_tree_remove_item) and Krita (LayerBox::slotAboutToRemoveRows), which agree on it exactly; citations are in the module header.pruneSelectionAgainstTreebecomesreconcileSelection, running in the one reconciler every tree mutation funnels through — no removal path knows about reselection. It also clears the isolation target when the isolated node dies (matching Krita'sKisImage::aboutToRemoveANode) and expands collapsed ancestors of the reselected row (GIMP's tree view does the same).adoptAppeared, so undoing a delete selects the restored subtree root rather than leaving the selection where it was.app.nodeById/app.activeNodecentralize "resolve a node by id";PropertiesPaneland the text tool stop re-walking the tree.Watercolor pigment buildup
The pickup atlas sampled only
pre_stroke_texture— the dry layer — so the pigment load a dab picked up was frozen for the whole stroke and a mark could never build past its first pass.The pickup pass now samples the live canvas: the stroke scratch composited over the dry layer with
source_over. This is legal because the pass targets the atlas, not the scratch, so there's no read/write alias —Scratchdocuments the two read paths and exposeslive_canvas_bind_group()for the cheap one (the mirror copy is only needed by passes that also write the scratch). Aconstassert pinsATLAS_WIDTH * ATLAS_HEIGHT >= MAX_DABS_PER_PHASE(silent cell aliasing otherwise), and adebug_assertpins the scratch/pre-stroke frame equality the shared UV uniforms depend on.Noise: non-repeating field, pixel-domain scale, 2D per-dab scatter
Three related defects in the noise/image sampling frame:
TILE_SPAN = 16.0was doing double duty as both the field's repeat period and the tile's detail window. It's split intoFIELD_SPAN = 128.0(period — how far the texel budget stretches) andDETAIL_SPAN = 16(whatresolution_for_octavessizes against). The field no longer visibly repeats within a normal view, at identical memory; fine octaves soften instead.vec2(v*64, v*64)— the same scalar on both axes, so every dab landed on thex == ydiagonal, and the 64-unit stride was an exact multiple of the 16-unit period. Replaced byfbm_offset2, a 2D hash bounded to exactly one period.scale_with_brushremoved fromnoiseandimage. Dab space now always samples in oriented dab-pixels, soscaleis a canvas-pixel feature size in both frames. To scale grain with the brush you wirebrush_settings.sizeinto it — and that signal is now published in canvas pixels (the brush diameter), seeded bespoke inseed_sensorsso the CPU slot and the GPU-packed dab field are numerically identical.sizealso declares anatural_range, without which the wire-boundary remap was skipped and a raw0..4value was dumped into a pixel field.Bundled brushes follow:
hair.yamlrewired to the new model, andtwirly_hairbecomessponge(polygon tip, randomized noise rotation/variation).Paste
ImageDataclips, but a normal copy produces a richLayerclip.Clipboard::paste_pixels()returns pixels from either variant.pasteInPlaceis now "Paste into Active Layer" and routes through the same floating→commit_floatingpath as the transform variant, whose R8 branch already writes masks — so pasting into an active mask works, committed or floating.Save / Save As / export
Save and Export Image were separate flows, and Save was disabled outright on Firefox.
SAVE_FORMATStable where each format owns its extension, MIME, picker filter, andproduce()— noswitch (format)at any consumer. The native picker gets a "Save as type" dropdown (.darkly, PNG, JPEG, WebP) and the chosen file type decides document-save vs canvas-export.SaveModaldriving produce + download;saveDocumentresolves only when the whole save is done, socloseGuardcan await it.canSavebecomeshasFilePickerand no longer gates the UI.ExportImageModal.svelte,exportImage.svelte.tsand theexportImageaction are deleted, along with theexportImagehotkey in defaults + all three editor presets. Users bound to Ctrl+Shift+E (or Ctrl+Alt+W under the Photoshop preset) lose that binding — Save As covers it.OffscreenCanvasencode is extracted torgbaToBloband shared by export and the.darklyzip's internalcomposite.png; timelapse export reuses the same picker (acquired inside the click's activation window, before the long encode).Canvas resize freeze (Firefox) + flood-fill commit
canvas.width/heightwrite: Firefox's zero-copy WebGPU present stalls the GPU process when a present straddles a swapchain reconfigure.requestFrame's body is extracted torunFrame, shared with the newrenderNow(which cancels any pending rAF so_framePendingcan't be left set).needs_morenow surfacescompositor.needs_present(). ALost/Outdatedacquire reconfigures and returns without presenting; without this the loop stopped and the reconfigured surface never got a real frame.doc.canvas_rect()while the lazy save snapshotted the layer extent. After an upward canvas resize the canvas rect reaches past the layer — adebug_assertfailure, or an uninitialized-scratch read in release. It now commits the layer extent.Use-after-free on tab close
Closing a tab freed the WASM handle but left the reference live, so an already-queued rAF called
renderon it (Attempt to use a moved value).DarklyInstance.dispose()stops the tool session and stream sources, frees the handle, then nullsenginelast so the render loop's existing guard short-circuits.Text tool
Text objects were created by an
$effectinTextProperties.svelte, so creation depended on the properties panel being mounted. The tool now creates the object itself on pointer-up viacreateTextFromPending(with a reentrancy guard), and theplacementhandoff is gone —PropertiesPanelloses its pending-placement special case. Selecting the text tool fireswarmVectorRenderer, compiling Vello's pipelines (a >1s one-time cost) during the gap before the first click instead of stalling the frame that would show the new box.Hotkey label rendering
Menus, the command palette and the menu bar formatted the raw
hotkeys.<id>config value, which may hold several bindings joined with|— rendering garbage for any multi-binding action.hotkeyLabel(actionId)is now the single entry point for displaying an action's shortcut, andeffectiveHotkeys/effectiveHotkeymove to the config store next to it.Misc
0–1000for large gains. Editor-only — the engine never enforced slider bounds; a value already outside the declared range also widens automatically so loaded gains aren't clamped on first touch.Tests
New:
modifier_removal.rs,paste_mask.rs,flood_fill_resize.rs,noise_field_nonrepeating.rs,dab_variation_scatter.rs, and frontendlayerTree.test.ts,reselection.test.ts,delete_layer.test.ts,save_flow.test.ts,resize_atomic.test.ts,close_use_after_free.test.ts,hotkey_label.test.ts,text_tool_create.test.ts.Extended:
canvas_resize.rs(dropped-present reschedule),watercolor.rs(buildup across flushes, buildup on transparent canvas, wet-neighbour bleed),document/mod.rsunit tests (filter detach/reattach/remove/move),wgsl.rsandsettable_source.rsfor the sampling-frame changes.