fix(renderer): smooth zoomed drawing and erasing without the memory cost#39
Open
markm39 wants to merge 3 commits into
Open
fix(renderer): smooth zoomed drawing and erasing without the memory cost#39markm39 wants to merge 3 commits into
markm39 wants to merge 3 commits into
Conversation
- keep the active-stroke surface at engine dimensions and anchor it to the visible viewport when zoomed, instead of allocating logical-size times scale pixels (up to 25x canvas memory at 5x zoom, allocated mid-gesture) - re-render finished strokes from vector data when zoomed instead of nearest-neighbor downsampling the magnified preview onto the 1x stroke surface - invalidate only the erased region's stroke tiles after a pixel-erase gesture instead of clearing the whole tile cache - refresh cachedHasVisiblePoints when committing a pixel-erase drag so fully-erased strokes stop responding to selection - cache the pixel-eraser cutout path instead of rebuilding it from every pending circle on each frame - remove dead recordPixelEraseCircleAdded/appendPixelEraseCircleToDelta and unreachable centerline preview branches
Simplify pass over the zoom performance changes: - replace the pixel-eraser background cutout with a kDifference clip of the stroke layer, so every background type shows through without re-deriving background compositing policy (fourth copy removed) - invalidate cachedStrokeSnapshot_ at the mutation site in applyPixelEraserAt instead of special-casing the eraser drag in render() - name the highlighter/marker rule once (isCenterlineStrokedToolType) and share one drawCenterlineStrokePath helper between stroke rendering and the active preview - merge finishStroke's duplicated render-from-geometry branches - hoist kMinimumRenderScale/kMaximumRenderScale and the identity-scale threshold into DrawingTypes.h, shared by the engine and ActiveStrokeRenderer - promote the stroke visibility check to Stroke::hasVisiblePoints and use it from erase, deserialize, and drop the dead DrawingSelection copy - derive erase bounds from the pending eraser path, precompute per-circle bounds once, and use an O(1) stroke outset at pen-up - reset pending pixel-erase state through one helper; make applyPixelEraserAt void and use applyPendingPixelEraseToStrokes' return - wrap active-surface transforms in SkAutoCanvasRestore; drop the constant lastHalfWidth_ member and orphaned drawCalligraphyPathTail
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
1 participant
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.
Closes #36.
What this fixes
Issue #36 reported laggy drawing when zoomed in, especially with highlighter and pixel eraser, plus highlighter strokes showing visible spline segments ("tabs").
Highlighter/marker now render as a single constant-width stroked centerline instead of per-segment multiply-blended quads. This removes the visible segment tabs at high zoom and the per-segment draw cost. The rule is named once (
isCenterlineStrokedToolType) and shared by live preview, stroke commit, and tile rendering, so preview and final go through the same pipeline.Pixel eraser no longer updates stroke metadata, tiles, or the cached snapshot on every input sample. During the drag it only applies a cheap
kClearsweep to the stroke surface (1x path) or clips the pending erase circles out of the stroke layer (zoomed path, so the real background shows through on every paper type). All bookkeeping — per-stroke circle assignment, the undo delta, visibility-cache refresh, and tile invalidation bounded to the erased rect — happens once at pen-up.Pen/crayon/calligraphy at zoom keep the incremental O(1)-per-frame active-stroke surface, but the surface is now anchored to the visible viewport at engine dimensions (constant ~14 MB) instead of allocating
canvas x zoompixels. The previous approach allocated up to 25x canvas memory (~330 MB on iPad at 5x zoom) mid-gesture, with a jetsam risk and a first-frame allocation hitch; on allocation failure live ink silently disappeared. If a consumer setsrenderScalewithout viewport ratios, the renderer degrades to the identity mapping rather than anchoring to the wrong region. Finished strokes drawn while zoomed are re-rendered from vector data at 1x instead of nearest-neighbor downsampling the magnified preview.Cleanups folded in
Stroke::hasVisiblePoints()is now the single visibility check (erase commit, deserialize; removed a dead third copy in DrawingSelection)DrawingTypes.h, shared by the engine andActiveStrokeRenderercachedStrokeSnapshot_invalidated at the mutation site instead of a tool-protocol special case inrender()recordPixelEraseCircleAdded,appendPixelEraseCircleToDelta,drawCalligraphyPathTail, unreachable centerline branches, constantlastHalfWidth_, the constant-trueuseIncrementalActiveSurfaceparameterVerification
clang++ -fsyntax-only -std=c++17against the vendored Skia headersnpm run typecheck,jest(57/57), andnpm run test:native:smoke(compiles, links, and runs the serialization smoke binary) all pass/simplify(4-angle review, 17 findings applied) and/security(no findings) both run🤖 Generated with Claude Code