fix(renderer): smooth zoomed drawing and erasing without the memory cost#39
Merged
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 (laggy zoomed drawing/erasing, highlighter segment seams).
isCenterlineStrokedToolType). Trade-off: self-overlapping highlighter strokes no longer darken at self-intersections.Stroke::hasVisiblePoints()is the single visibility check; zoom-range constants hoisted toDrawingTypes.h; dead code removed (recordPixelEraseCircleAdded,appendPixelEraseCircleToDelta,drawCalligraphyPathTail,lastHalfWidth_, the constantuseIncrementalActiveSurfaceflag).Verification: typecheck, jest (57/57), native serialization smoke test, library build, and
clang++ -fsyntax-onlyon all 17 C++ translation units pass; independent code review and security audit found no issues. On-device verification was not performed (no local iOS dev signing set up); merged with owner's approval on that basis.