From 563f294ed3a121d1a8a1038ae5f621c7b496c4a2 Mon Sep 17 00:00:00 2001 From: markm39 Date: Sun, 24 May 2026 14:07:35 -0500 Subject: [PATCH 1/3] fix(renderer): smooth zoomed highlighter and eraser performance --- cpp/ActiveStrokeRenderer.cpp | 125 ++++++++++++++++-- cpp/ActiveStrokeRenderer.h | 11 +- cpp/PathRenderer.cpp | 27 +--- cpp/SkiaDrawingEngine.cpp | 19 +-- cpp/SkiaDrawingEngine.h | 4 + cpp/SkiaDrawingEngineEraser.cpp | 195 +++++++++++++---------------- cpp/SkiaDrawingEngineRendering.cpp | 99 ++++++++++++++- 7 files changed, 329 insertions(+), 151 deletions(-) diff --git a/cpp/ActiveStrokeRenderer.cpp b/cpp/ActiveStrokeRenderer.cpp index 045726e..b258d14 100644 --- a/cpp/ActiveStrokeRenderer.cpp +++ b/cpp/ActiveStrokeRenderer.cpp @@ -1,11 +1,51 @@ #include "ActiveStrokeRenderer.h" #include "PathRenderer.h" +#include +#include #include +#include namespace nativedrawing { +namespace { + +void drawCenterlinePreview( + SkCanvas* canvas, + const std::vector& points, + const SkPaint& basePaint +) { + if (!canvas || points.size() < 2) { + return; + } + + SkPath path; + path.moveTo(points[0].x, points[0].y); + for (size_t i = 1; i < points.size(); ++i) { + path.lineTo(points[i].x, points[i].y); + } + + float width = 0.0f; + for (const auto& point : points) { + width += point.calculatedWidth; + } + width /= static_cast(points.size()); + + SkPaint paint = basePaint; + paint.setStyle(SkPaint::kStroke_Style); + paint.setStrokeWidth(width); + paint.setStrokeCap(SkPaint::kRound_Cap); + paint.setStrokeJoin(SkPaint::kRound_Join); + paint.setAntiAlias(true); + paint.setBlendMode(SkBlendMode::kSrcOver); + canvas->drawPath(path, paint); +} + +} // namespace + ActiveStrokeRenderer::ActiveStrokeRenderer(int width, int height, PathRenderer* pathRenderer) : pathRenderer_(pathRenderer) + , logicalWidth_(width) + , logicalHeight_(height) , lastRenderedInputIndex_(0) , hasLastEdge_(false) , lastHalfWidth_(-1.0f) { @@ -17,10 +57,7 @@ ActiveStrokeRenderer::ActiveStrokeRenderer(int width, int height, PathRenderer* } } -void ActiveStrokeRenderer::reset() { - if (activeStrokeSurface_) { - activeStrokeSurface_->getCanvas()->clear(SK_ColorTRANSPARENT); - } +void ActiveStrokeRenderer::resetIncrementalState() { cachedActiveSnapshot_ = nullptr; lastRenderedInputIndex_ = 0; overlapBuffer_.clear(); @@ -30,14 +67,42 @@ void ActiveStrokeRenderer::reset() { lastHalfWidth_ = -1.0f; // Will use default baseWidth/2 } +void ActiveStrokeRenderer::reset() { + if (activeStrokeSurface_) { + activeStrokeSurface_->getCanvas()->clear(SK_ColorTRANSPARENT); + } + resetIncrementalState(); +} + +void ActiveStrokeRenderer::ensureSurfaceScale(float surfaceScale) { + const float nextScale = std::max(1.0f, std::min(5.0f, surfaceScale)); + if (activeStrokeSurface_ && std::fabs(nextScale - surfaceScale_) < 0.01f) { + return; + } + + surfaceScale_ = nextScale; + const int scaledWidth = std::max(1, static_cast(std::ceil(logicalWidth_ * surfaceScale_))); + const int scaledHeight = std::max(1, static_cast(std::ceil(logicalHeight_ * surfaceScale_))); + SkImageInfo info = SkImageInfo::MakeN32Premul(scaledWidth, scaledHeight); + activeStrokeSurface_ = SkSurfaces::Raster(info); + if (activeStrokeSurface_) { + activeStrokeSurface_->getCanvas()->clear(SK_ColorTRANSPARENT); + } + resetIncrementalState(); +} + void ActiveStrokeRenderer::renderIncremental( SkCanvas* canvas, const std::vector& points, const SkPaint& paint, - const std::string& toolType + const std::string& toolType, + float surfaceScale ) { + ensureSurfaceScale(surfaceScale); if (!activeStrokeSurface_ || points.size() < 2) return; + const bool isTranslucentCenterlineTool = toolType == "highlighter" || toolType == "marker"; + // Calligraphy: full redraw each frame for clean rendering // The incremental approach causes overlap artifacts on thin strokes if (toolType == "calligraphy") { @@ -70,7 +135,13 @@ void ActiveStrokeRenderer::renderIncremental( bool isFirstSegment = !hasLastEdge_; IncrementalResult result; - if (toolType == "crayon") { + surfaceCanvas->save(); + surfaceCanvas->scale(surfaceScale_, surfaceScale_); + if (isTranslucentCenterlineTool) { + drawCenterlinePreview(surfaceCanvas, segment, paint); + result.lastLeftEdge = SkPoint::Make(segment.back().x, segment.back().y); + result.lastRightEdge = result.lastLeftEdge; + } else if (toolType == "crayon") { result = pathRenderer_->drawCrayonPathIncremental( surfaceCanvas, segment, paint, lastLeftEdge_, lastRightEdge_, isFirstSegment); @@ -94,6 +165,7 @@ void ActiveStrokeRenderer::renderIncremental( pathRenderer_->drawVariableWidthStartCap(surfaceCanvas, segment, paint); } } + surfaceCanvas->restore(); lastLeftEdge_ = result.lastLeftEdge; lastRightEdge_ = result.lastRightEdge; @@ -112,9 +184,7 @@ void ActiveStrokeRenderer::renderIncremental( } // Draw cached portion to output canvas - if (cachedActiveSnapshot_) { - canvas->drawImage(cachedActiveSnapshot_, 0, 0); - } + drawSnapshot(canvas); // Draw "tail" - recent points not yet finalized if (points.size() > lastRenderedInputIndex_) { @@ -129,7 +199,9 @@ void ActiveStrokeRenderer::renderIncremental( } if (tail.size() >= 2) { - if (toolType == "crayon") { + if (isTranslucentCenterlineTool) { + drawCenterlinePreview(canvas, tail, paint); + } else if (toolType == "crayon") { // drawCrayonPathTail already draws end cap pathRenderer_->drawCrayonPathTail(canvas, tail, paint, lastLeftEdge_, lastRightEdge_, hasLastEdge_); @@ -168,27 +240,58 @@ void ActiveStrokeRenderer::renderFinalTail( SkCanvas* surfaceCanvas = activeStrokeSurface_->getCanvas(); - if (toolType == "crayon") { + if (toolType == "highlighter" || toolType == "marker") { + surfaceCanvas->save(); + surfaceCanvas->scale(surfaceScale_, surfaceScale_); + drawCenterlinePreview(surfaceCanvas, finalTail, paint); + surfaceCanvas->restore(); + } else if (toolType == "crayon") { + surfaceCanvas->save(); + surfaceCanvas->scale(surfaceScale_, surfaceScale_); pathRenderer_->drawCrayonPathIncremental( surfaceCanvas, finalTail, paint, lastLeftEdge_, lastRightEdge_, !hasLastEdge_); // Only draw end cap - start cap was already drawn during incremental rendering pathRenderer_->drawCrayonEndCap(surfaceCanvas, points, paint); + surfaceCanvas->restore(); } else if (toolType == "calligraphy") { + surfaceCanvas->save(); + surfaceCanvas->scale(surfaceScale_, surfaceScale_); pathRenderer_->drawCalligraphyPathIncremental( surfaceCanvas, finalTail, paint, lastLeftEdge_, lastRightEdge_, !hasLastEdge_, lastHalfWidth_); // Calligraphy has tapered ends, no caps needed + surfaceCanvas->restore(); } else { + surfaceCanvas->save(); + surfaceCanvas->scale(surfaceScale_, surfaceScale_); pathRenderer_->drawVariableWidthPathIncremental( surfaceCanvas, finalTail, paint, lastLeftEdge_, lastRightEdge_, !hasLastEdge_); // Only draw end cap - start cap was already drawn during incremental rendering pathRenderer_->drawVariableWidthEndCap(surfaceCanvas, points, paint); + surfaceCanvas->restore(); } cachedActiveSnapshot_ = activeStrokeSurface_->makeImageSnapshot(); } +void ActiveStrokeRenderer::drawSnapshot(SkCanvas* canvas) const { + if (!canvas || !cachedActiveSnapshot_) { + return; + } + + if (std::fabs(surfaceScale_ - 1.0f) < 0.01f) { + canvas->drawImage(cachedActiveSnapshot_, 0, 0); + return; + } + + const SkRect dst = SkRect::MakeWH( + static_cast(logicalWidth_), + static_cast(logicalHeight_) + ); + canvas->drawImageRect(cachedActiveSnapshot_, dst, SkSamplingOptions()); +} + } // namespace nativedrawing diff --git a/cpp/ActiveStrokeRenderer.h b/cpp/ActiveStrokeRenderer.h index 70c3473..dfd153a 100644 --- a/cpp/ActiveStrokeRenderer.h +++ b/cpp/ActiveStrokeRenderer.h @@ -41,7 +41,8 @@ class ActiveStrokeRenderer { SkCanvas* canvas, const std::vector& points, const SkPaint& paint, - const std::string& toolType + const std::string& toolType, + float surfaceScale = 1.0f ); /** @@ -59,6 +60,8 @@ class ActiveStrokeRenderer { */ sk_sp getSnapshot() const { return cachedActiveSnapshot_; } + void drawSnapshot(SkCanvas* canvas) const; + /** * Get the last rendered input index */ @@ -66,6 +69,9 @@ class ActiveStrokeRenderer { private: PathRenderer* pathRenderer_; + int logicalWidth_; + int logicalHeight_; + float surfaceScale_ = 1.0f; // Active stroke surface for incremental rendering sk_sp activeStrokeSurface_; @@ -78,6 +84,9 @@ class ActiveStrokeRenderer { bool hasLastEdge_; float lastHalfWidth_; // For calligraphy width continuity + void ensureSurfaceScale(float surfaceScale); + void resetIncrementalState(); + static constexpr size_t OVERLAP = 2; // Spline overlap for Catmull-Rom }; diff --git a/cpp/PathRenderer.cpp b/cpp/PathRenderer.cpp index 2cdcd62..1080b75 100644 --- a/cpp/PathRenderer.cpp +++ b/cpp/PathRenderer.cpp @@ -90,24 +90,10 @@ void PathRenderer::drawVariableWidthPath( fillPaint.setAlpha(static_cast(baseAlpha * pressureAlphaMod)); } - // For multiply blend (highlighter), draw individual segments so overlaps darken properly - // For other blend modes, draw as single path for performance - bool isMultiplyBlend = (fillPaint.asBlendMode() == SkBlendMode::kMultiply); - - if (isMultiplyBlend && leftEdge.size() > 1) { - // Draw as individual quad segments - each segment composites separately - // This makes self-overlapping strokes darken at intersections - for (size_t i = 0; i < leftEdge.size() - 1; i++) { - SkPath segmentPath; - segmentPath.moveTo(leftEdge[i]); - segmentPath.lineTo(leftEdge[i + 1]); - segmentPath.lineTo(rightEdge[i + 1]); - segmentPath.lineTo(rightEdge[i]); - segmentPath.close(); - canvas->drawPath(segmentPath, fillPaint); - } - } else if (!leftEdge.empty() && !rightEdge.empty()) { - // Build single filled path with rounded caps for non-multiply blends + if (!leftEdge.empty() && !rightEdge.empty()) { + // Build a single filled path. Drawing highlighter/marker as separate + // multiply-blended segments creates visible tabs at high zoom and costs + // one draw call per segment. strokePath.moveTo(leftEdge[0]); // Draw along left edge @@ -361,8 +347,6 @@ IncrementalResult PathRenderer::drawVariableWidthPathIncremental( IncrementalResult result = {}; if (points.empty() || points.size() < 2 || !canvas) return result; - SkColor baseColor = basePaint.getColor(); - // Generate smoothed points using helper std::vector smoothedPoints; interpolateSplinePoints( @@ -394,8 +378,7 @@ IncrementalResult PathRenderer::drawVariableWidthPathIncremental( } path.close(); - SkPaint fillPaint; - fillPaint.setColor(baseColor); + SkPaint fillPaint = basePaint; fillPaint.setStyle(SkPaint::kFill_Style); fillPaint.setAntiAlias(true); canvas->drawPath(path, fillPaint); diff --git a/cpp/SkiaDrawingEngine.cpp b/cpp/SkiaDrawingEngine.cpp index a6d2ecc..8bc4975 100644 --- a/cpp/SkiaDrawingEngine.cpp +++ b/cpp/SkiaDrawingEngine.cpp @@ -144,6 +144,8 @@ void SkiaDrawingEngine::touchBegan( currentPoints_.clear(); currentPath_.reset(); predictedPointCount_ = 0; // Reset prediction tracking for new stroke + pendingPixelEraseEntries_.clear(); + pendingPixelEraserCircles_.clear(); clearActiveShapePreview(); // Reset incremental active stroke state @@ -335,17 +337,17 @@ void SkiaDrawingEngine::touchEnded(long timestamp) { if (currentTool_ == "eraser" && eraserMode_ == "object") { eraseObjects(); } else if (currentTool_ == "eraser" && eraserMode_ == "pixel") { - // Commit a single PixelErase delta covering every (stroke, circle) - // pair that was added during this drag. pendingPixelEraseEntries_ - // was populated incrementally by recordPixelEraseCircleAdded - // during applyPixelEraserAt calls. Reset for the next drag. + applyPendingPixelEraseToStrokes(); if (!pendingPixelEraseEntries_.empty()) { StrokeDelta delta; delta.kind = StrokeDelta::Kind::PixelErase; delta.pixelEraseEntries = std::move(pendingPixelEraseEntries_); commitDelta(std::move(delta)); + cachedStrokeSnapshot_ = nullptr; + invalidateAllStrokeTiles(); } pendingPixelEraseEntries_.clear(); + pendingPixelEraserCircles_.clear(); // DON'T set needsStrokeRedraw_ - kClear visual is already correct // Full redraw only needed on undo/redo/deserialize @@ -616,7 +618,9 @@ void SkiaDrawingEngine::finishStroke(long endTimestamp) { if (strokeSurface_ && currentTool_ != "eraser") { SkCanvas* strokeCanvas = strokeSurface_->getCanvas(); - if (isRecognizedShapeToolType(stroke.toolType)) { + if (isRecognizedShapeToolType(stroke.toolType) + || stroke.toolType == "highlighter" + || stroke.toolType == "marker") { SkPaint strokePaint = stroke.paint; if (!stroke.isEraser) { uint8_t baseAlpha = stroke.paint.getAlpha(); @@ -630,10 +634,7 @@ void SkiaDrawingEngine::finishStroke(long endTimestamp) { } // Composite active stroke onto persistent stroke surface - sk_sp activeImage = activeStrokeRenderer_->getSnapshot(); - if (activeImage) { - strokeCanvas->drawImage(activeImage, 0, 0); - } + activeStrokeRenderer_->drawSnapshot(strokeCanvas); } // Update cached snapshot diff --git a/cpp/SkiaDrawingEngine.h b/cpp/SkiaDrawingEngine.h index 6911b35..aee6e56 100644 --- a/cpp/SkiaDrawingEngine.h +++ b/cpp/SkiaDrawingEngine.h @@ -144,7 +144,9 @@ class SkiaDrawingEngine { // emit one PixelErase delta covering the whole drag. Reset at the // start of each eraser touch. std::vector pendingPixelEraseEntries_; + std::vector pendingPixelEraserCircles_; void recordPixelEraseCircleAdded(size_t strokeIndex, const EraserCircle& circle); + bool applyPendingPixelEraseToStrokes(); int width_; int height_; @@ -308,9 +310,11 @@ class SkiaDrawingEngine { bool updateActiveShapePreviewForPoint(float x, float y); void redrawStrokes(); void markStrokeCachesDirty(); + void invalidateAllStrokeTiles(); void invalidateStrokeTilesForRect(const SkRect& bounds); void renderScaleAwareStrokes(SkCanvas* canvas); void renderActiveContent(SkCanvas* canvas, bool useIncrementalActiveSurface); + void renderActivePixelEraserCutout(SkCanvas* canvas); sk_sp renderStrokeTile(const StrokeTileKey& key, int tileWidth, int tileHeight, float scale); void pruneStrokeTileCache(); void redrawEraserMask(); // Dual-surface: only redraws eraser circles to mask diff --git a/cpp/SkiaDrawingEngineEraser.cpp b/cpp/SkiaDrawingEngineEraser.cpp index 609eb2e..5686d44 100644 --- a/cpp/SkiaDrawingEngineEraser.cpp +++ b/cpp/SkiaDrawingEngineEraser.cpp @@ -1,6 +1,7 @@ #include "SkiaDrawingEngine.h" #include "EraserRenderer.h" +#include "ShapeRecognition.h" #include "StrokeSplitter.h" #include @@ -107,10 +108,6 @@ void SkiaDrawingEngine::bakeEraserCircles() { } bool SkiaDrawingEngine::applyPixelEraserAt(float eraserX, float eraserY, float radius) { - // Pixel-perfect eraser: store eraser circles WITH strokes for render-time clipping - // No splitting - curves stay intact, eraser data moves with stroke - bool anyModified = false; - // Build list of circles to add (may include interpolated circles for full coverage) std::vector circlesToAdd; circlesToAdd.push_back({eraserX, eraserY, radius}); @@ -135,112 +132,36 @@ bool SkiaDrawingEngine::applyPixelEraserAt(float eraserX, float eraserY, float r } } - for (size_t strokeIndex = 0; strokeIndex < strokes_.size(); ++strokeIndex) { - auto& stroke = strokes_[strokeIndex]; - if (stroke.isEraser) continue; - if (stroke.points.size() < 2) continue; - - // Check if eraser path intersects stroke bounds (expand for full path) - SkRect bounds = stroke.path.getBounds(); - bounds.outset(radius, radius); - - bool affectsStroke = false; - for (const auto& circle : circlesToAdd) { - if (bounds.contains(circle.x, circle.y)) { - affectsStroke = true; - break; - } - } - if (!affectsStroke) continue; - - // Store all eraser circles with this stroke for full coverage. - // Each circle is also recorded into pendingPixelEraseEntries_ so - // touchEnded can emit a single PixelErase delta covering the - // whole drag. Without this the per-stroke erasedBy mutations - // would have no record in history -- pixel erase wouldn't undo. - for (const auto& circle : circlesToAdd) { - stroke.erasedBy.push_back(circle); - recordPixelEraseCircleAdded(strokeIndex, circle); - } - - // OPTIMIZATION: Only update visibility if stroke was previously visible - // Skip strokes already marked invisible - they stay invisible - if (stroke.cachedHasVisiblePoints) { - // Quick check: do new circles potentially cover remaining visible points? - // Only do full check every 50 circles to avoid O(n^2) during heavy erasing - size_t circleCount = stroke.erasedBy.size(); - if (circleCount % 50 == 0 || circlesToAdd.size() > 10) { - bool hasVisible = false; - for (const auto& pt : stroke.points) { - bool pointVisible = true; - for (const auto& circle : stroke.erasedBy) { - float dx = pt.x - circle.x; - float dy = pt.y - circle.y; - float totalRadius = circle.radius + pt.calculatedWidth / 2.0f; - if (dx * dx + dy * dy <= totalRadius * totalRadius) { - pointVisible = false; - break; - } - } - if (pointVisible) { - hasVisible = true; - break; - } - } - stroke.cachedHasVisiblePoints = hasVisible; - } - } + pendingPixelEraserCircles_.insert( + pendingPixelEraserCircles_.end(), + circlesToAdd.begin(), + circlesToAdd.end() + ); - anyModified = true; - } + // Apply kClear directly to strokeSurface_ for instant feedback. Stroke + // metadata is updated once at touch end so the drag loop stays cheap. + if (strokeSurface_) { + SkCanvas* canvas = strokeSurface_->getCanvas(); + SkPaint clearPaint; + clearPaint.setBlendMode(SkBlendMode::kClear); + clearPaint.setAntiAlias(true); - if (anyModified) { - SkRect erasedBounds = SkRect::MakeLTRB( - eraserX - radius, - eraserY - radius, - eraserX + radius, - eraserY + radius - ); if (hasLastEraserPoint_) { - erasedBounds.join(SkRect::MakeLTRB( - lastEraserX_ - lastEraserRadius_, - lastEraserY_ - lastEraserRadius_, - lastEraserX_ + lastEraserRadius_, - lastEraserY_ + lastEraserRadius_ - )); + SkPath eraserPath; + eraserPath.moveTo(lastEraserX_, lastEraserY_); + eraserPath.lineTo(eraserX, eraserY); + + clearPaint.setStyle(SkPaint::kStroke_Style); + clearPaint.setStrokeWidth(radius * 2.0f); + clearPaint.setStrokeCap(SkPaint::kRound_Cap); + clearPaint.setStrokeJoin(SkPaint::kRound_Join); + canvas->drawPath(eraserPath, clearPaint); + } else { + canvas->drawCircle(eraserX, eraserY, radius, clearPaint); } - erasedBounds.outset(radius * 2.0f, radius * 2.0f); - invalidateStrokeTilesForRect(erasedBounds); - - // OPTIMIZATION: Apply kClear directly to stroke surface for instant feedback - // This avoids full redraw during active erasing - much faster - if (strokeSurface_) { - SkCanvas* canvas = strokeSurface_->getCanvas(); - SkPaint clearPaint; - clearPaint.setBlendMode(SkBlendMode::kClear); - clearPaint.setAntiAlias(true); - - // Draw stroked path from last position to current for full coverage - // Individual circles leave gaps when moving quickly - if (hasLastEraserPoint_) { - // Draw a stroked line from last position to current - SkPath eraserPath; - eraserPath.moveTo(lastEraserX_, lastEraserY_); - eraserPath.lineTo(eraserX, eraserY); - - clearPaint.setStyle(SkPaint::kStroke_Style); - clearPaint.setStrokeWidth(radius * 2.0f); // Diameter - clearPaint.setStrokeCap(SkPaint::kRound_Cap); - clearPaint.setStrokeJoin(SkPaint::kRound_Join); - canvas->drawPath(eraserPath, clearPaint); - } else { - // First point - just draw circle - canvas->drawCircle(eraserX, eraserY, radius, clearPaint); - } - cachedStrokeSnapshot_ = strokeSurface_->makeImageSnapshot(); - } - // DON'T set needsStrokeRedraw_ = true - defer full redraw to touchEnded + // Keep the live surface hot during the drag. Snapshotting here forces + // full-surface work per input sample; touchEnded refreshes caches. } // Track position for next call @@ -249,6 +170,70 @@ bool SkiaDrawingEngine::applyPixelEraserAt(float eraserX, float eraserY, float r lastEraserRadius_ = radius; hasLastEraserPoint_ = true; + return true; +} + +bool SkiaDrawingEngine::applyPendingPixelEraseToStrokes() { + if (pendingPixelEraserCircles_.empty()) { + return false; + } + + SkRect eraseBounds = SkRect::MakeEmpty(); + for (const auto& circle : pendingPixelEraserCircles_) { + eraseBounds.join(SkRect::MakeLTRB( + circle.x - circle.radius, + circle.y - circle.radius, + circle.x + circle.radius, + circle.y + circle.radius + )); + } + + bool anyModified = false; + + for (size_t strokeIndex = 0; strokeIndex < strokes_.size(); ++strokeIndex) { + auto& stroke = strokes_[strokeIndex]; + if (stroke.isEraser || stroke.points.size() < 2) { + continue; + } + + SkRect bounds = stroke.path.getBounds(); + const float strokeOutset = std::max(8.0f, averageCalculatedWidth(stroke.points)); + bounds.outset(strokeOutset, strokeOutset); + if (!bounds.intersects(eraseBounds)) { + continue; + } + + std::vector circlesForStroke; + circlesForStroke.reserve(pendingPixelEraserCircles_.size()); + for (const auto& circle : pendingPixelEraserCircles_) { + SkRect circleBounds = SkRect::MakeLTRB( + circle.x - circle.radius, + circle.y - circle.radius, + circle.x + circle.radius, + circle.y + circle.radius + ); + if (bounds.intersects(circleBounds)) { + circlesForStroke.push_back(circle); + } + } + if (circlesForStroke.empty()) { + continue; + } + + stroke.erasedBy.insert( + stroke.erasedBy.end(), + circlesForStroke.begin(), + circlesForStroke.end() + ); + + StrokeDelta::PixelEraseEntry entry; + entry.strokeIndex = strokeIndex; + entry.addedCircles = std::move(circlesForStroke); + pendingPixelEraseEntries_.push_back(std::move(entry)); + + anyModified = true; + } + return anyModified; } diff --git a/cpp/SkiaDrawingEngineRendering.cpp b/cpp/SkiaDrawingEngineRendering.cpp index 45a2ede..15efc2a 100644 --- a/cpp/SkiaDrawingEngineRendering.cpp +++ b/cpp/SkiaDrawingEngineRendering.cpp @@ -87,6 +87,15 @@ void SkiaDrawingEngine::markStrokeCachesDirty() { } } +void SkiaDrawingEngine::invalidateAllStrokeTiles() { + strokeTileEpoch_++; + if (strokeTileEpoch_ == 0) { + strokeTileEpoch_ = 1; + } + strokeTileCache_.clear(); + strokeTileCacheBytes_ = 0; +} + void SkiaDrawingEngine::invalidateStrokeTilesForRect(const SkRect& bounds) { if (strokeTileCache_.empty() || bounds.isEmpty()) { return; @@ -172,6 +181,25 @@ void SkiaDrawingEngine::renderStrokeGeometry(SkCanvas* canvas, const Stroke& str return; } + if (stroke.toolType == "highlighter" || stroke.toolType == "marker") { + SkPath centerPath = stroke.path; + if (centerPath.isEmpty()) { + smoothPath(stroke.points, centerPath); + } + if (centerPath.isEmpty()) { + return; + } + + SkPaint strokePaint = paint; + strokePaint.setStyle(SkPaint::kStroke_Style); + strokePaint.setStrokeWidth(averageCalculatedWidth(stroke.points)); + strokePaint.setStrokeCap(SkPaint::kRound_Cap); + strokePaint.setStrokeJoin(SkPaint::kRound_Join); + strokePaint.setAntiAlias(true); + canvas->drawPath(centerPath, strokePaint); + return; + } + if (stroke.toolType == "crayon") { pathRenderer_->drawCrayonPath(canvas, stroke.points, paint, false); } else if (stroke.toolType == "calligraphy") { @@ -439,8 +467,22 @@ void SkiaDrawingEngine::renderActiveContent(SkCanvas* canvas, bool useIncrementa renderStrokeGeometry(canvas, previewStroke, previewPaint); } else if (currentPoints_.size() >= 2 && currentTool_ != "select" && currentTool_ != "eraser") { - if (useIncrementalActiveSurface) { - activeStrokeRenderer_->renderIncremental(canvas, currentPoints_, currentPaint_, currentTool_); + if (currentTool_ == "highlighter" || currentTool_ == "marker") { + Stroke activeStroke; + activeStroke.points = currentPoints_; + activeStroke.paint = currentPaint_; + activeStroke.toolType = currentTool_; + SkPaint previewPaint = currentPaint_; + previewPaint.setBlendMode(SkBlendMode::kSrcOver); + renderStrokeGeometry(canvas, activeStroke, previewPaint); + } else if (useIncrementalActiveSurface) { + activeStrokeRenderer_->renderIncremental( + canvas, + currentPoints_, + currentPaint_, + currentTool_, + renderScale_ + ); } else { Stroke activeStroke; activeStroke.points = currentPoints_; @@ -451,6 +493,49 @@ void SkiaDrawingEngine::renderActiveContent(SkCanvas* canvas, bool useIncrementa } } +void SkiaDrawingEngine::renderActivePixelEraserCutout(SkCanvas* canvas) { + if (!canvas + || currentTool_ != "eraser" + || eraserMode_ != "pixel" + || pendingPixelEraserCircles_.empty()) { + return; + } + + SkPath eraserPath; + for (const auto& circle : pendingPixelEraserCircles_) { + eraserPath.addCircle(circle.x, circle.y, circle.radius); + } + if (eraserPath.isEmpty()) { + return; + } + + if (backgroundType_ == "pdf" && !pdfBackgroundImage_) { + SkPaint clearPaint; + clearPaint.setBlendMode(SkBlendMode::kClear); + clearPaint.setAntiAlias(true); + canvas->drawPath(eraserPath, clearPaint); + return; + } + + canvas->save(); + canvas->clipPath(eraserPath, SkClipOp::kIntersect, true); + if (backgroundType_ == "pdf" || backgroundType_ == "plain") { + canvas->drawColor(SK_ColorWHITE); + } + + if (backgroundType_ != "pdf" || pdfBackgroundImage_) { + backgroundRenderer_->drawBackground( + canvas, + backgroundType_, + width_, + height_, + pdfBackgroundImage_, + backgroundOriginY_ + ); + } + canvas->restore(); +} + void SkiaDrawingEngine::redrawEraserMask() { if (!needsEraserMaskRedraw_) return; @@ -512,7 +597,8 @@ void SkiaDrawingEngine::render(SkCanvas* canvas) { canvas->save(); canvas->scale(renderScale_, renderScale_); - renderActiveContent(canvas, false); + renderActiveContent(canvas, true); + renderActivePixelEraserCutout(canvas); if (showEraserCursor_ && eraserCursorRadius_ > 0) { SkPaint cursorPaint; @@ -651,8 +737,15 @@ void SkiaDrawingEngine::render(SkCanvas* canvas) { canvas->restore(); } } + } else if (currentTool_ == "eraser" + && eraserMode_ == "pixel" + && !currentPoints_.empty() + && strokeSurface_) { + strokeSurface_->draw(canvas, 0, 0); } else if (cachedStrokeSnapshot_) { canvas->drawImage(cachedStrokeSnapshot_, 0, 0); + } else if (strokeSurface_) { + strokeSurface_->draw(canvas, 0, 0); } } } From 4c83e3e40387dfd93bceb4ee418d6aa58af3e316 Mon Sep 17 00:00:00 2001 From: markm39 Date: Wed, 1 Jul 2026 22:49:34 -0500 Subject: [PATCH 2/3] fix(renderer): anchor zoomed stroke preview to viewport - 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 --- cpp/ActiveStrokeRenderer.cpp | 160 +++++++++++++---------------- cpp/ActiveStrokeRenderer.h | 39 +++++-- cpp/DrawingHistory.cpp | 18 ---- cpp/DrawingHistory.h | 6 -- cpp/SkiaDrawingEngine.cpp | 17 +-- cpp/SkiaDrawingEngine.h | 15 +-- cpp/SkiaDrawingEngineEraser.cpp | 34 ++++++ cpp/SkiaDrawingEngineRendering.cpp | 31 ++---- 8 files changed, 166 insertions(+), 154 deletions(-) diff --git a/cpp/ActiveStrokeRenderer.cpp b/cpp/ActiveStrokeRenderer.cpp index b258d14..b00eaff 100644 --- a/cpp/ActiveStrokeRenderer.cpp +++ b/cpp/ActiveStrokeRenderer.cpp @@ -3,45 +3,9 @@ #include #include #include -#include namespace nativedrawing { -namespace { - -void drawCenterlinePreview( - SkCanvas* canvas, - const std::vector& points, - const SkPaint& basePaint -) { - if (!canvas || points.size() < 2) { - return; - } - - SkPath path; - path.moveTo(points[0].x, points[0].y); - for (size_t i = 1; i < points.size(); ++i) { - path.lineTo(points[i].x, points[i].y); - } - - float width = 0.0f; - for (const auto& point : points) { - width += point.calculatedWidth; - } - width /= static_cast(points.size()); - - SkPaint paint = basePaint; - paint.setStyle(SkPaint::kStroke_Style); - paint.setStrokeWidth(width); - paint.setStrokeCap(SkPaint::kRound_Cap); - paint.setStrokeJoin(SkPaint::kRound_Join); - paint.setAntiAlias(true); - paint.setBlendMode(SkBlendMode::kSrcOver); - canvas->drawPath(path, paint); -} - -} // namespace - ActiveStrokeRenderer::ActiveStrokeRenderer(int width, int height, PathRenderer* pathRenderer) : pathRenderer_(pathRenderer) , logicalWidth_(width) @@ -74,35 +38,77 @@ void ActiveStrokeRenderer::reset() { resetIncrementalState(); } -void ActiveStrokeRenderer::ensureSurfaceScale(float surfaceScale) { - const float nextScale = std::max(1.0f, std::min(5.0f, surfaceScale)); - if (activeStrokeSurface_ && std::fabs(nextScale - surfaceScale_) < 0.01f) { +void ActiveStrokeRenderer::ensureViewportAlignment(const ActiveStrokeViewport& viewport) { + const float requestedScale = std::max(1.0f, std::min(5.0f, viewport.scale)); + + // The surface is fixed at logical dimensions, so the covered region at + // `scale` magnification is logical/scale. Cap the scale at the highest + // magnification whose viewport still fits; inconsistent viewport data + // (e.g. a consumer that sets renderScale without viewport ratios) + // degrades to the identity mapping instead of an anchored surface that + // misses where the user is drawing. + const float viewWidth = viewport.width > 0.0f + ? viewport.width + : static_cast(logicalWidth_); + const float viewHeight = viewport.height > 0.0f + ? viewport.height + : static_cast(logicalHeight_); + const float fitScale = std::min( + static_cast(logicalWidth_) / std::max(1.0f, viewWidth), + static_cast(logicalHeight_) / std::max(1.0f, viewHeight) + ); + const float nextScale = std::max(1.0f, std::min(requestedScale, fitScale)); + + float nextOriginX = 0.0f; + float nextOriginY = 0.0f; + if (nextScale > 1.01f) { + const float coveredWidth = static_cast(logicalWidth_) / nextScale; + const float coveredHeight = static_cast(logicalHeight_) / nextScale; + nextOriginX = std::max(0.0f, std::min( + viewport.left, static_cast(logicalWidth_) - coveredWidth)); + nextOriginY = std::max(0.0f, std::min( + viewport.top, static_cast(logicalHeight_) - coveredHeight)); + } + + if (activeStrokeSurface_ + && std::fabs(nextScale - viewportScale_) < 0.01f + && std::fabs(nextOriginX - viewportOriginX_) < 0.5f + && std::fabs(nextOriginY - viewportOriginY_) < 0.5f) { return; } - surfaceScale_ = nextScale; - const int scaledWidth = std::max(1, static_cast(std::ceil(logicalWidth_ * surfaceScale_))); - const int scaledHeight = std::max(1, static_cast(std::ceil(logicalHeight_ * surfaceScale_))); - SkImageInfo info = SkImageInfo::MakeN32Premul(scaledWidth, scaledHeight); - activeStrokeSurface_ = SkSurfaces::Raster(info); + viewportScale_ = nextScale; + viewportOriginX_ = nextOriginX; + viewportOriginY_ = nextOriginY; + + if (!activeStrokeSurface_) { + SkImageInfo info = SkImageInfo::MakeN32Premul(logicalWidth_, logicalHeight_); + activeStrokeSurface_ = SkSurfaces::Raster(info); + } if (activeStrokeSurface_) { activeStrokeSurface_->getCanvas()->clear(SK_ColorTRANSPARENT); } + // Anchoring changed mid-stroke: drop incremental progress so the next + // renderIncremental call re-renders every point into the fresh surface. resetIncrementalState(); } +void ActiveStrokeRenderer::applySurfaceTransform(SkCanvas* surfaceCanvas) const { + surfaceCanvas->save(); + surfaceCanvas->scale(viewportScale_, viewportScale_); + surfaceCanvas->translate(-viewportOriginX_, -viewportOriginY_); +} + void ActiveStrokeRenderer::renderIncremental( SkCanvas* canvas, const std::vector& points, const SkPaint& paint, const std::string& toolType, - float surfaceScale + const ActiveStrokeViewport& viewport ) { - ensureSurfaceScale(surfaceScale); + ensureViewportAlignment(viewport); if (!activeStrokeSurface_ || points.size() < 2) return; - const bool isTranslucentCenterlineTool = toolType == "highlighter" || toolType == "marker"; - // Calligraphy: full redraw each frame for clean rendering // The incremental approach causes overlap artifacts on thin strokes if (toolType == "calligraphy") { @@ -135,13 +141,8 @@ void ActiveStrokeRenderer::renderIncremental( bool isFirstSegment = !hasLastEdge_; IncrementalResult result; - surfaceCanvas->save(); - surfaceCanvas->scale(surfaceScale_, surfaceScale_); - if (isTranslucentCenterlineTool) { - drawCenterlinePreview(surfaceCanvas, segment, paint); - result.lastLeftEdge = SkPoint::Make(segment.back().x, segment.back().y); - result.lastRightEdge = result.lastLeftEdge; - } else if (toolType == "crayon") { + applySurfaceTransform(surfaceCanvas); + if (toolType == "crayon") { result = pathRenderer_->drawCrayonPathIncremental( surfaceCanvas, segment, paint, lastLeftEdge_, lastRightEdge_, isFirstSegment); @@ -149,13 +150,6 @@ void ActiveStrokeRenderer::renderIncremental( if (isFirstSegment) { pathRenderer_->drawCrayonStartCap(surfaceCanvas, segment, paint); } - } else if (toolType == "calligraphy") { - result = pathRenderer_->drawCalligraphyPathIncremental( - surfaceCanvas, segment, paint, - lastLeftEdge_, lastRightEdge_, isFirstSegment, - lastHalfWidth_); - lastHalfWidth_ = result.lastHalfWidth; - // Calligraphy has tapered ends, no caps needed } else { result = pathRenderer_->drawVariableWidthPathIncremental( surfaceCanvas, segment, paint, @@ -199,16 +193,10 @@ void ActiveStrokeRenderer::renderIncremental( } if (tail.size() >= 2) { - if (isTranslucentCenterlineTool) { - drawCenterlinePreview(canvas, tail, paint); - } else if (toolType == "crayon") { + if (toolType == "crayon") { // drawCrayonPathTail already draws end cap pathRenderer_->drawCrayonPathTail(canvas, tail, paint, lastLeftEdge_, lastRightEdge_, hasLastEdge_); - } else if (toolType == "calligraphy") { - // Calligraphy has tapered ends, no caps needed - pathRenderer_->drawCalligraphyPathTail(canvas, tail, paint, - lastLeftEdge_, lastRightEdge_, hasLastEdge_, lastHalfWidth_); } else { pathRenderer_->drawVariableWidthPathTail(canvas, tail, paint, lastLeftEdge_, lastRightEdge_, hasLastEdge_); @@ -240,39 +228,27 @@ void ActiveStrokeRenderer::renderFinalTail( SkCanvas* surfaceCanvas = activeStrokeSurface_->getCanvas(); - if (toolType == "highlighter" || toolType == "marker") { - surfaceCanvas->save(); - surfaceCanvas->scale(surfaceScale_, surfaceScale_); - drawCenterlinePreview(surfaceCanvas, finalTail, paint); - surfaceCanvas->restore(); - } else if (toolType == "crayon") { - surfaceCanvas->save(); - surfaceCanvas->scale(surfaceScale_, surfaceScale_); + applySurfaceTransform(surfaceCanvas); + if (toolType == "crayon") { pathRenderer_->drawCrayonPathIncremental( surfaceCanvas, finalTail, paint, lastLeftEdge_, lastRightEdge_, !hasLastEdge_); // Only draw end cap - start cap was already drawn during incremental rendering pathRenderer_->drawCrayonEndCap(surfaceCanvas, points, paint); - surfaceCanvas->restore(); } else if (toolType == "calligraphy") { - surfaceCanvas->save(); - surfaceCanvas->scale(surfaceScale_, surfaceScale_); pathRenderer_->drawCalligraphyPathIncremental( surfaceCanvas, finalTail, paint, lastLeftEdge_, lastRightEdge_, !hasLastEdge_, lastHalfWidth_); // Calligraphy has tapered ends, no caps needed - surfaceCanvas->restore(); } else { - surfaceCanvas->save(); - surfaceCanvas->scale(surfaceScale_, surfaceScale_); pathRenderer_->drawVariableWidthPathIncremental( surfaceCanvas, finalTail, paint, lastLeftEdge_, lastRightEdge_, !hasLastEdge_); // Only draw end cap - start cap was already drawn during incremental rendering pathRenderer_->drawVariableWidthEndCap(surfaceCanvas, points, paint); - surfaceCanvas->restore(); } + surfaceCanvas->restore(); cachedActiveSnapshot_ = activeStrokeSurface_->makeImageSnapshot(); } @@ -282,16 +258,22 @@ void ActiveStrokeRenderer::drawSnapshot(SkCanvas* canvas) const { return; } - if (std::fabs(surfaceScale_ - 1.0f) < 0.01f) { + if (viewportScale_ <= 1.01f) { canvas->drawImage(cachedActiveSnapshot_, 0, 0); return; } - const SkRect dst = SkRect::MakeWH( - static_cast(logicalWidth_), - static_cast(logicalHeight_) + const SkRect dst = SkRect::MakeXYWH( + viewportOriginX_, + viewportOriginY_, + static_cast(logicalWidth_) / viewportScale_, + static_cast(logicalHeight_) / viewportScale_ + ); + canvas->drawImageRect( + cachedActiveSnapshot_, + dst, + SkSamplingOptions(SkFilterMode::kLinear) ); - canvas->drawImageRect(cachedActiveSnapshot_, dst, SkSamplingOptions()); } } // namespace nativedrawing diff --git a/cpp/ActiveStrokeRenderer.h b/cpp/ActiveStrokeRenderer.h index dfd153a..d8dbff5 100644 --- a/cpp/ActiveStrokeRenderer.h +++ b/cpp/ActiveStrokeRenderer.h @@ -13,12 +13,29 @@ namespace nativedrawing { class PathRenderer; +/** + * Viewport the active stroke is being drawn in, in logical (document) + * coordinates. When scale > 1 the preview surface is anchored to this + * region so the magnified stroke stays crisp without growing the surface. + */ +struct ActiveStrokeViewport { + float scale = 1.0f; + float left = 0.0f; + float top = 0.0f; + float width = 0.0f; // <= 0 means the full canvas is visible + float height = 0.0f; // <= 0 means the full canvas is visible +}; + /** * ActiveStrokeRenderer - Handles incremental O(1) rendering of active strokes * * Extracted from SkiaDrawingEngine for maintainability. * Implements surface caching and incremental rendering to maintain 60-120fps * during stroke input, regardless of stroke complexity. + * + * The surface is always allocated at the engine's logical dimensions. When + * the viewport is zoomed, the surface covers only the visible region at + * magnification instead of the whole canvas, keeping memory constant. */ class ActiveStrokeRenderer { public: @@ -32,17 +49,18 @@ class ActiveStrokeRenderer { /** * Render the active stroke incrementally to the output canvas * - * @param canvas Output canvas to draw to + * @param canvas Output canvas to draw to (already scaled to the viewport) * @param points Current stroke points * @param paint Paint to use for rendering * @param toolType Current tool type (pen, crayon, etc.) + * @param viewport Visible region and zoom the stroke is drawn in */ void renderIncremental( SkCanvas* canvas, const std::vector& points, const SkPaint& paint, const std::string& toolType, - float surfaceScale = 1.0f + const ActiveStrokeViewport& viewport ); /** @@ -56,12 +74,16 @@ class ActiveStrokeRenderer { ); /** - * Get the cached active stroke image for compositing + * Draw the cached active stroke onto a canvas in logical coordinates. */ - sk_sp getSnapshot() const { return cachedActiveSnapshot_; } - void drawSnapshot(SkCanvas* canvas) const; + /** + * Effective magnification the preview surface is currently anchored at. + * 1.0 means the surface maps 1:1 onto the full canvas. + */ + float viewportScale() const { return viewportScale_; } + /** * Get the last rendered input index */ @@ -71,7 +93,9 @@ class ActiveStrokeRenderer { PathRenderer* pathRenderer_; int logicalWidth_; int logicalHeight_; - float surfaceScale_ = 1.0f; + float viewportScale_ = 1.0f; + float viewportOriginX_ = 0.0f; + float viewportOriginY_ = 0.0f; // Active stroke surface for incremental rendering sk_sp activeStrokeSurface_; @@ -84,7 +108,8 @@ class ActiveStrokeRenderer { bool hasLastEdge_; float lastHalfWidth_; // For calligraphy width continuity - void ensureSurfaceScale(float surfaceScale); + void ensureViewportAlignment(const ActiveStrokeViewport& viewport); + void applySurfaceTransform(SkCanvas* surfaceCanvas) const; void resetIncrementalState(); static constexpr size_t OVERLAP = 2; // Spline overlap for Catmull-Rom diff --git a/cpp/DrawingHistory.cpp b/cpp/DrawingHistory.cpp index 90e6434..038c559 100644 --- a/cpp/DrawingHistory.cpp +++ b/cpp/DrawingHistory.cpp @@ -18,24 +18,6 @@ void commitStrokeDelta( } } -void appendPixelEraseCircleToDelta( - std::vector& pendingPixelEraseEntries, - size_t strokeIndex, - const EraserCircle& circle -) { - for (auto& entry : pendingPixelEraseEntries) { - if (entry.strokeIndex == strokeIndex) { - entry.addedCircles.push_back(circle); - return; - } - } - - StrokeDelta::PixelEraseEntry entry; - entry.strokeIndex = strokeIndex; - entry.addedCircles.push_back(circle); - pendingPixelEraseEntries.push_back(std::move(entry)); -} - void applyStrokeDelta( const StrokeDelta& delta, std::vector& strokes, diff --git a/cpp/DrawingHistory.h b/cpp/DrawingHistory.h index b9ede97..78d2a14 100644 --- a/cpp/DrawingHistory.h +++ b/cpp/DrawingHistory.h @@ -14,12 +14,6 @@ void commitStrokeDelta( size_t maxHistoryEntries ); -void appendPixelEraseCircleToDelta( - std::vector& pendingPixelEraseEntries, - size_t strokeIndex, - const EraserCircle& circle -); - void applyStrokeDelta( const StrokeDelta& delta, std::vector& strokes, diff --git a/cpp/SkiaDrawingEngine.cpp b/cpp/SkiaDrawingEngine.cpp index 8bc4975..98da3d6 100644 --- a/cpp/SkiaDrawingEngine.cpp +++ b/cpp/SkiaDrawingEngine.cpp @@ -118,10 +118,6 @@ void SkiaDrawingEngine::commitDelta(StrokeDelta&& delta) { commitStrokeDelta(undoStack_, redoStack_, std::move(delta), MAX_HISTORY_ENTRIES); } -void SkiaDrawingEngine::recordPixelEraseCircleAdded(size_t strokeIndex, const EraserCircle& circle) { - appendPixelEraseCircleToDelta(pendingPixelEraseEntries_, strokeIndex, circle); -} - void SkiaDrawingEngine::applyDelta(const StrokeDelta& delta) { applyStrokeDelta(delta, strokes_, eraserCircles_, *pathRenderer_); } @@ -146,6 +142,7 @@ void SkiaDrawingEngine::touchBegan( predictedPointCount_ = 0; // Reset prediction tracking for new stroke pendingPixelEraseEntries_.clear(); pendingPixelEraserCircles_.clear(); + pendingPixelEraserPath_.reset(); clearActiveShapePreview(); // Reset incremental active stroke state @@ -344,10 +341,10 @@ void SkiaDrawingEngine::touchEnded(long timestamp) { delta.pixelEraseEntries = std::move(pendingPixelEraseEntries_); commitDelta(std::move(delta)); cachedStrokeSnapshot_ = nullptr; - invalidateAllStrokeTiles(); } pendingPixelEraseEntries_.clear(); pendingPixelEraserCircles_.clear(); + pendingPixelEraserPath_.reset(); // DON'T set needsStrokeRedraw_ - kClear visual is already correct // Full redraw only needed on undo/redo/deserialize @@ -627,7 +624,7 @@ void SkiaDrawingEngine::finishStroke(long endTimestamp) { strokePaint.setAlpha(static_cast(baseAlpha * stroke.originalAlphaMod)); } renderStrokeGeometry(strokeCanvas, stroke, strokePaint); - } else { + } else if (activeStrokeRenderer_->viewportScale() <= 1.01f) { // Render any remaining tail points to complete the stroke if (currentPoints_.size() > activeStrokeRenderer_->getLastRenderedIndex()) { activeStrokeRenderer_->renderFinalTail(currentPoints_, currentPaint_, currentTool_); @@ -635,6 +632,14 @@ void SkiaDrawingEngine::finishStroke(long endTimestamp) { // Composite active stroke onto persistent stroke surface activeStrokeRenderer_->drawSnapshot(strokeCanvas); + } else { + // Zoomed: the active surface covers only the magnified viewport, + // so re-render the finished stroke from vector data at 1x rather + // than downsampling the preview pixels. + SkPaint strokePaint = stroke.paint; + strokePaint.setAlpha( + static_cast(stroke.paint.getAlpha() * stroke.originalAlphaMod)); + renderStrokeGeometry(strokeCanvas, stroke, strokePaint); } // Update cached snapshot diff --git a/cpp/SkiaDrawingEngine.h b/cpp/SkiaDrawingEngine.h index aee6e56..3f79903 100644 --- a/cpp/SkiaDrawingEngine.h +++ b/cpp/SkiaDrawingEngine.h @@ -138,14 +138,16 @@ class SkiaDrawingEngine { void revertDelta(const StrokeDelta& delta); // backward (used by undo) // Pixel-eraser accumulator. During an eraser drag (touchBegan eraser - // -> touchMoved... -> touchEnded), each applyPixelEraserAt call - // appends circles to multiple strokes' erasedBy vectors. We - // accumulate the (strokeIndex, circles) pairs here so touchEnded can - // emit one PixelErase delta covering the whole drag. Reset at the - // start of each eraser touch. + // -> touchMoved... -> touchEnded), applyPixelEraserAt only collects + // circles (and mirrors them into pendingPixelEraserPath_ for the + // zoomed cutout overlay). touchEnded calls + // applyPendingPixelEraseToStrokes, which assigns the circles to the + // strokes they intersect, fills pendingPixelEraseEntries_, and emits + // one PixelErase delta covering the whole drag. Reset at the start of + // each eraser touch. std::vector pendingPixelEraseEntries_; std::vector pendingPixelEraserCircles_; - void recordPixelEraseCircleAdded(size_t strokeIndex, const EraserCircle& circle); + SkPath pendingPixelEraserPath_; bool applyPendingPixelEraseToStrokes(); int width_; @@ -310,7 +312,6 @@ class SkiaDrawingEngine { bool updateActiveShapePreviewForPoint(float x, float y); void redrawStrokes(); void markStrokeCachesDirty(); - void invalidateAllStrokeTiles(); void invalidateStrokeTilesForRect(const SkRect& bounds); void renderScaleAwareStrokes(SkCanvas* canvas); void renderActiveContent(SkCanvas* canvas, bool useIncrementalActiveSurface); diff --git a/cpp/SkiaDrawingEngineEraser.cpp b/cpp/SkiaDrawingEngineEraser.cpp index 5686d44..dea6b49 100644 --- a/cpp/SkiaDrawingEngineEraser.cpp +++ b/cpp/SkiaDrawingEngineEraser.cpp @@ -137,6 +137,11 @@ bool SkiaDrawingEngine::applyPixelEraserAt(float eraserX, float eraserY, float r circlesToAdd.begin(), circlesToAdd.end() ); + // Mirror the circles into a cached path so the zoomed cutout overlay + // doesn't rebuild an ever-growing path on every frame of the drag. + for (const auto& circle : circlesToAdd) { + pendingPixelEraserPath_.addCircle(circle.x, circle.y, circle.radius); + } // Apply kClear directly to strokeSurface_ for instant feedback. Stroke // metadata is updated once at touch end so the drag loop stays cheap. @@ -226,6 +231,29 @@ bool SkiaDrawingEngine::applyPendingPixelEraseToStrokes() { circlesForStroke.end() ); + // Refresh the visibility cache once per gesture so fully-erased + // strokes stop responding to selection. + if (stroke.cachedHasVisiblePoints) { + bool hasVisible = false; + for (const auto& pt : stroke.points) { + bool pointVisible = true; + for (const auto& circle : stroke.erasedBy) { + float dx = pt.x - circle.x; + float dy = pt.y - circle.y; + float totalRadius = circle.radius + pt.calculatedWidth / 2.0f; + if (dx * dx + dy * dy <= totalRadius * totalRadius) { + pointVisible = false; + break; + } + } + if (pointVisible) { + hasVisible = true; + break; + } + } + stroke.cachedHasVisiblePoints = hasVisible; + } + StrokeDelta::PixelEraseEntry entry; entry.strokeIndex = strokeIndex; entry.addedCircles = std::move(circlesForStroke); @@ -234,6 +262,12 @@ bool SkiaDrawingEngine::applyPendingPixelEraseToStrokes() { anyModified = true; } + if (anyModified) { + SkRect invalidateBounds = eraseBounds; + invalidateBounds.outset(8.0f, 8.0f); + invalidateStrokeTilesForRect(invalidateBounds); + } + return anyModified; } diff --git a/cpp/SkiaDrawingEngineRendering.cpp b/cpp/SkiaDrawingEngineRendering.cpp index 15efc2a..64c3f14 100644 --- a/cpp/SkiaDrawingEngineRendering.cpp +++ b/cpp/SkiaDrawingEngineRendering.cpp @@ -87,15 +87,6 @@ void SkiaDrawingEngine::markStrokeCachesDirty() { } } -void SkiaDrawingEngine::invalidateAllStrokeTiles() { - strokeTileEpoch_++; - if (strokeTileEpoch_ == 0) { - strokeTileEpoch_ = 1; - } - strokeTileCache_.clear(); - strokeTileCacheBytes_ = 0; -} - void SkiaDrawingEngine::invalidateStrokeTilesForRect(const SkRect& bounds) { if (strokeTileCache_.empty() || bounds.isEmpty()) { return; @@ -476,12 +467,18 @@ void SkiaDrawingEngine::renderActiveContent(SkCanvas* canvas, bool useIncrementa previewPaint.setBlendMode(SkBlendMode::kSrcOver); renderStrokeGeometry(canvas, activeStroke, previewPaint); } else if (useIncrementalActiveSurface) { + ActiveStrokeViewport viewport; + viewport.scale = renderScale_; + viewport.left = visibleLeft_; + viewport.top = visibleTop_; + viewport.width = visibleWidth_; + viewport.height = visibleHeight_; activeStrokeRenderer_->renderIncremental( canvas, currentPoints_, currentPaint_, currentTool_, - renderScale_ + viewport ); } else { Stroke activeStroke; @@ -497,15 +494,7 @@ void SkiaDrawingEngine::renderActivePixelEraserCutout(SkCanvas* canvas) { if (!canvas || currentTool_ != "eraser" || eraserMode_ != "pixel" - || pendingPixelEraserCircles_.empty()) { - return; - } - - SkPath eraserPath; - for (const auto& circle : pendingPixelEraserCircles_) { - eraserPath.addCircle(circle.x, circle.y, circle.radius); - } - if (eraserPath.isEmpty()) { + || pendingPixelEraserPath_.isEmpty()) { return; } @@ -513,12 +502,12 @@ void SkiaDrawingEngine::renderActivePixelEraserCutout(SkCanvas* canvas) { SkPaint clearPaint; clearPaint.setBlendMode(SkBlendMode::kClear); clearPaint.setAntiAlias(true); - canvas->drawPath(eraserPath, clearPaint); + canvas->drawPath(pendingPixelEraserPath_, clearPaint); return; } canvas->save(); - canvas->clipPath(eraserPath, SkClipOp::kIntersect, true); + canvas->clipPath(pendingPixelEraserPath_, SkClipOp::kIntersect, true); if (backgroundType_ == "pdf" || backgroundType_ == "plain") { canvas->drawColor(SK_ColorWHITE); } From 01b7a31cbe6aaf19ea97726408e96bab592e7ff3 Mon Sep 17 00:00:00 2001 From: markm39 Date: Wed, 1 Jul 2026 23:08:07 -0500 Subject: [PATCH 3/3] refactor(renderer): consolidate zoom rendering and eraser helpers 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 --- cpp/ActiveStrokeRenderer.cpp | 102 +++++++++++----------- cpp/ActiveStrokeRenderer.h | 3 +- cpp/DrawingSelection.cpp | 23 ----- cpp/DrawingSerialization.cpp | 20 +---- cpp/DrawingTypes.cpp | 23 +++++ cpp/DrawingTypes.h | 12 +++ cpp/ShapeRecognition.cpp | 4 + cpp/ShapeRecognition.h | 4 + cpp/SkiaDrawingEngine.cpp | 41 ++++----- cpp/SkiaDrawingEngine.h | 14 ++- cpp/SkiaDrawingEngineEraser.cpp | 60 ++++++------- cpp/SkiaDrawingEngineRendering.cpp | 132 ++++++++++++----------------- 12 files changed, 204 insertions(+), 234 deletions(-) diff --git a/cpp/ActiveStrokeRenderer.cpp b/cpp/ActiveStrokeRenderer.cpp index b00eaff..806c094 100644 --- a/cpp/ActiveStrokeRenderer.cpp +++ b/cpp/ActiveStrokeRenderer.cpp @@ -11,8 +11,7 @@ ActiveStrokeRenderer::ActiveStrokeRenderer(int width, int height, PathRenderer* , logicalWidth_(width) , logicalHeight_(height) , lastRenderedInputIndex_(0) - , hasLastEdge_(false) - , lastHalfWidth_(-1.0f) { + , hasLastEdge_(false) { SkImageInfo info = SkImageInfo::MakeN32Premul(width, height); activeStrokeSurface_ = SkSurfaces::Raster(info); @@ -28,7 +27,6 @@ void ActiveStrokeRenderer::resetIncrementalState() { hasLastEdge_ = false; lastLeftEdge_ = SkPoint::Make(0, 0); lastRightEdge_ = SkPoint::Make(0, 0); - lastHalfWidth_ = -1.0f; // Will use default baseWidth/2 } void ActiveStrokeRenderer::reset() { @@ -39,7 +37,8 @@ void ActiveStrokeRenderer::reset() { } void ActiveStrokeRenderer::ensureViewportAlignment(const ActiveStrokeViewport& viewport) { - const float requestedScale = std::max(1.0f, std::min(5.0f, viewport.scale)); + const float requestedScale = + std::clamp(viewport.scale, kMinimumRenderScale, kMaximumRenderScale); // The surface is fixed at logical dimensions, so the covered region at // `scale` magnification is logical/scale. Cap the scale at the highest @@ -57,17 +56,17 @@ void ActiveStrokeRenderer::ensureViewportAlignment(const ActiveStrokeViewport& v static_cast(logicalWidth_) / std::max(1.0f, viewWidth), static_cast(logicalHeight_) / std::max(1.0f, viewHeight) ); - const float nextScale = std::max(1.0f, std::min(requestedScale, fitScale)); + const float nextScale = std::max(kMinimumRenderScale, std::min(requestedScale, fitScale)); float nextOriginX = 0.0f; float nextOriginY = 0.0f; - if (nextScale > 1.01f) { + if (nextScale > kIdentityRenderScaleThreshold) { const float coveredWidth = static_cast(logicalWidth_) / nextScale; const float coveredHeight = static_cast(logicalHeight_) / nextScale; - nextOriginX = std::max(0.0f, std::min( - viewport.left, static_cast(logicalWidth_) - coveredWidth)); - nextOriginY = std::max(0.0f, std::min( - viewport.top, static_cast(logicalHeight_) - coveredHeight)); + nextOriginX = std::clamp( + viewport.left, 0.0f, static_cast(logicalWidth_) - coveredWidth); + nextOriginY = std::clamp( + viewport.top, 0.0f, static_cast(logicalHeight_) - coveredHeight); } if (activeStrokeSurface_ @@ -94,7 +93,6 @@ void ActiveStrokeRenderer::ensureViewportAlignment(const ActiveStrokeViewport& v } void ActiveStrokeRenderer::applySurfaceTransform(SkCanvas* surfaceCanvas) const { - surfaceCanvas->save(); surfaceCanvas->scale(viewportScale_, viewportScale_); surfaceCanvas->translate(-viewportOriginX_, -viewportOriginY_); } @@ -141,25 +139,27 @@ void ActiveStrokeRenderer::renderIncremental( bool isFirstSegment = !hasLastEdge_; IncrementalResult result; - applySurfaceTransform(surfaceCanvas); - if (toolType == "crayon") { - result = pathRenderer_->drawCrayonPathIncremental( - surfaceCanvas, segment, paint, - lastLeftEdge_, lastRightEdge_, isFirstSegment); - // Draw start cap on first segment - if (isFirstSegment) { - pathRenderer_->drawCrayonStartCap(surfaceCanvas, segment, paint); - } - } else { - result = pathRenderer_->drawVariableWidthPathIncremental( - surfaceCanvas, segment, paint, - lastLeftEdge_, lastRightEdge_, isFirstSegment); - // Draw start cap on first segment - if (isFirstSegment) { - pathRenderer_->drawVariableWidthStartCap(surfaceCanvas, segment, paint); + { + SkAutoCanvasRestore acr(surfaceCanvas, true); + applySurfaceTransform(surfaceCanvas); + if (toolType == "crayon") { + result = pathRenderer_->drawCrayonPathIncremental( + surfaceCanvas, segment, paint, + lastLeftEdge_, lastRightEdge_, isFirstSegment); + // Draw start cap on first segment + if (isFirstSegment) { + pathRenderer_->drawCrayonStartCap(surfaceCanvas, segment, paint); + } + } else { + result = pathRenderer_->drawVariableWidthPathIncremental( + surfaceCanvas, segment, paint, + lastLeftEdge_, lastRightEdge_, isFirstSegment); + // Draw start cap on first segment + if (isFirstSegment) { + pathRenderer_->drawVariableWidthStartCap(surfaceCanvas, segment, paint); + } } } - surfaceCanvas->restore(); lastLeftEdge_ = result.lastLeftEdge; lastRightEdge_ = result.lastRightEdge; @@ -228,27 +228,31 @@ void ActiveStrokeRenderer::renderFinalTail( SkCanvas* surfaceCanvas = activeStrokeSurface_->getCanvas(); - applySurfaceTransform(surfaceCanvas); - if (toolType == "crayon") { - pathRenderer_->drawCrayonPathIncremental( - surfaceCanvas, finalTail, paint, - lastLeftEdge_, lastRightEdge_, !hasLastEdge_); - // Only draw end cap - start cap was already drawn during incremental rendering - pathRenderer_->drawCrayonEndCap(surfaceCanvas, points, paint); - } else if (toolType == "calligraphy") { - pathRenderer_->drawCalligraphyPathIncremental( - surfaceCanvas, finalTail, paint, - lastLeftEdge_, lastRightEdge_, !hasLastEdge_, - lastHalfWidth_); - // Calligraphy has tapered ends, no caps needed - } else { - pathRenderer_->drawVariableWidthPathIncremental( - surfaceCanvas, finalTail, paint, - lastLeftEdge_, lastRightEdge_, !hasLastEdge_); - // Only draw end cap - start cap was already drawn during incremental rendering - pathRenderer_->drawVariableWidthEndCap(surfaceCanvas, points, paint); + { + SkAutoCanvasRestore acr(surfaceCanvas, true); + applySurfaceTransform(surfaceCanvas); + if (toolType == "crayon") { + pathRenderer_->drawCrayonPathIncremental( + surfaceCanvas, finalTail, paint, + lastLeftEdge_, lastRightEdge_, !hasLastEdge_); + // Only draw end cap - start cap was already drawn during incremental rendering + pathRenderer_->drawCrayonEndCap(surfaceCanvas, points, paint); + } else if (toolType == "calligraphy") { + // Calligraphy renders the whole stroke here (its live preview is a + // direct full redraw), so there is no carried half-width; -1 selects + // the default baseWidth/2. Tapered ends, no caps needed. + pathRenderer_->drawCalligraphyPathIncremental( + surfaceCanvas, finalTail, paint, + lastLeftEdge_, lastRightEdge_, !hasLastEdge_, + -1.0f); + } else { + pathRenderer_->drawVariableWidthPathIncremental( + surfaceCanvas, finalTail, paint, + lastLeftEdge_, lastRightEdge_, !hasLastEdge_); + // Only draw end cap - start cap was already drawn during incremental rendering + pathRenderer_->drawVariableWidthEndCap(surfaceCanvas, points, paint); + } } - surfaceCanvas->restore(); cachedActiveSnapshot_ = activeStrokeSurface_->makeImageSnapshot(); } @@ -258,7 +262,7 @@ void ActiveStrokeRenderer::drawSnapshot(SkCanvas* canvas) const { return; } - if (viewportScale_ <= 1.01f) { + if (viewportScale_ <= kIdentityRenderScaleThreshold) { canvas->drawImage(cachedActiveSnapshot_, 0, 0); return; } diff --git a/cpp/ActiveStrokeRenderer.h b/cpp/ActiveStrokeRenderer.h index d8dbff5..1ac665f 100644 --- a/cpp/ActiveStrokeRenderer.h +++ b/cpp/ActiveStrokeRenderer.h @@ -106,9 +106,10 @@ class ActiveStrokeRenderer { std::vector overlapBuffer_; SkPoint lastLeftEdge_, lastRightEdge_; bool hasLastEdge_; - float lastHalfWidth_; // For calligraphy width continuity void ensureViewportAlignment(const ActiveStrokeViewport& viewport); + // Maps logical coordinates onto the viewport-anchored surface. Callers + // wrap the call in SkAutoCanvasRestore. void applySurfaceTransform(SkCanvas* surfaceCanvas) const; void resetIncrementalState(); diff --git a/cpp/DrawingSelection.cpp b/cpp/DrawingSelection.cpp index 7e3f20c..dab3350 100644 --- a/cpp/DrawingSelection.cpp +++ b/cpp/DrawingSelection.cpp @@ -28,29 +28,6 @@ static bool isPointInErasedRegion(float x, float y, const Stroke& stroke) { return false; } -// Helper: Check if a stroke has any visible (non-erased) points -static bool hasVisiblePoints(const Stroke& stroke) { - if (stroke.erasedBy.empty()) return true; // No erasure = fully visible - - // Check if any stroke point is outside all eraser circles - for (const auto& pt : stroke.points) { - bool pointVisible = true; - for (const auto& circle : stroke.erasedBy) { - float dx = pt.x - circle.x; - float dy = pt.y - circle.y; - // Use stroke width to check if the rendered point would be visible - float strokeRadius = pt.calculatedWidth / 2.0f; - float totalRadius = circle.radius + strokeRadius; - if (dx*dx + dy*dy <= totalRadius * totalRadius) { - pointVisible = false; - break; - } - } - if (pointVisible) return true; // Found at least one visible point - } - return false; // All points are erased -} - bool DrawingSelection::selectStrokeAtMatching( float x, float y, std::vector& strokes, diff --git a/cpp/DrawingSerialization.cpp b/cpp/DrawingSerialization.cpp index 17ad081..e7b48d5 100644 --- a/cpp/DrawingSerialization.cpp +++ b/cpp/DrawingSerialization.cpp @@ -313,25 +313,7 @@ bool DrawingSerialization::deserialize( } // Compute cached visibility for selection optimization - if (!stroke.erasedBy.empty()) { - stroke.cachedHasVisiblePoints = false; - for (const auto& pt : stroke.points) { - bool pointVisible = true; - for (const auto& circle : stroke.erasedBy) { - float dx = pt.x - circle.x; - float dy = pt.y - circle.y; - float totalRadius = circle.radius + pt.calculatedWidth / 2.0f; - if (dx * dx + dy * dy <= totalRadius * totalRadius) { - pointVisible = false; - break; - } - } - if (pointVisible) { - stroke.cachedHasVisiblePoints = true; - break; - } - } - } + stroke.cachedHasVisiblePoints = stroke.hasVisiblePoints(); } // Version < 4: erasedBy stays empty (no per-stroke eraser data) diff --git a/cpp/DrawingTypes.cpp b/cpp/DrawingTypes.cpp index ace5920..3b5a5ac 100644 --- a/cpp/DrawingTypes.cpp +++ b/cpp/DrawingTypes.cpp @@ -6,6 +6,29 @@ namespace nativedrawing { +bool Stroke::hasVisiblePoints() const { + if (erasedBy.empty()) { + return true; + } + + for (const auto& pt : points) { + bool pointVisible = true; + for (const auto& circle : erasedBy) { + float dx = pt.x - circle.x; + float dy = pt.y - circle.y; + float totalRadius = circle.radius + pt.calculatedWidth / 2.0f; + if (dx * dx + dy * dy <= totalRadius * totalRadius) { + pointVisible = false; + break; + } + } + if (pointVisible) { + return true; + } + } + return false; +} + void Stroke::ensureEraserCacheValid() const { if (erasedBy.size() == cachedEraserCount) { return; diff --git a/cpp/DrawingTypes.h b/cpp/DrawingTypes.h index 5b3639d..3fc9fcf 100644 --- a/cpp/DrawingTypes.h +++ b/cpp/DrawingTypes.h @@ -54,8 +54,20 @@ struct Stroke { // Ensure cachedEraserPath is up-to-date with erasedBy circles // Builds stroked path matching EraserRenderer::drawEraserCirclesAsStrokes void ensureEraserCacheValid() const; + + // True if any point survives all erasedBy circles (accounting for the + // rendered stroke radius). Source of truth for cachedHasVisiblePoints. + bool hasVisiblePoints() const; }; +// Zoom range the engine renders at. setRenderViewport clamps and buckets +// into this range; ActiveStrokeRenderer anchors its preview surface with +// the same bounds so the two never diverge. +constexpr float kMinimumRenderScale = 1.0f; +constexpr float kMaximumRenderScale = 5.0f; +// Scales at or below this render 1:1; above it the scale-aware paths kick in. +constexpr float kIdentityRenderScaleThreshold = 1.01f; + // Delta-based history. Each entry describes ONE operation that was // applied to strokes_, sized in proportion to the operation -- a single // stroke add is ~1-3 KB, a pixel erase pass over K strokes is ~K * 12 diff --git a/cpp/ShapeRecognition.cpp b/cpp/ShapeRecognition.cpp index aa89bc2..a5f11e3 100644 --- a/cpp/ShapeRecognition.cpp +++ b/cpp/ShapeRecognition.cpp @@ -17,6 +17,10 @@ bool isRecognizedShapeToolType(const std::string& toolType) { || toolType == "shape-polygon"; } +bool isCenterlineStrokedToolType(const std::string& toolType) { + return toolType == "highlighter" || toolType == "marker"; +} + bool buildRecognizedShapePath( const std::string& toolType, const std::vector& points, diff --git a/cpp/ShapeRecognition.h b/cpp/ShapeRecognition.h index 8d72776..fa029b4 100644 --- a/cpp/ShapeRecognition.h +++ b/cpp/ShapeRecognition.h @@ -15,6 +15,10 @@ struct ShapeCandidate { bool isRecognizedShapeToolType(const std::string& toolType); +// Tools rendered as a single constant-width stroked centerline path +// (translucent tools where per-segment compositing would show seams). +bool isCenterlineStrokedToolType(const std::string& toolType); + bool buildRecognizedShapePath( const std::string& toolType, const std::vector& points, diff --git a/cpp/SkiaDrawingEngine.cpp b/cpp/SkiaDrawingEngine.cpp index 98da3d6..2f8374f 100644 --- a/cpp/SkiaDrawingEngine.cpp +++ b/cpp/SkiaDrawingEngine.cpp @@ -140,9 +140,7 @@ void SkiaDrawingEngine::touchBegan( currentPoints_.clear(); currentPath_.reset(); predictedPointCount_ = 0; // Reset prediction tracking for new stroke - pendingPixelEraseEntries_.clear(); - pendingPixelEraserCircles_.clear(); - pendingPixelEraserPath_.reset(); + resetPendingPixelErase(); clearActiveShapePreview(); // Reset incremental active stroke state @@ -334,17 +332,13 @@ void SkiaDrawingEngine::touchEnded(long timestamp) { if (currentTool_ == "eraser" && eraserMode_ == "object") { eraseObjects(); } else if (currentTool_ == "eraser" && eraserMode_ == "pixel") { - applyPendingPixelEraseToStrokes(); - if (!pendingPixelEraseEntries_.empty()) { + if (applyPendingPixelEraseToStrokes()) { StrokeDelta delta; delta.kind = StrokeDelta::Kind::PixelErase; delta.pixelEraseEntries = std::move(pendingPixelEraseEntries_); commitDelta(std::move(delta)); - cachedStrokeSnapshot_ = nullptr; } - pendingPixelEraseEntries_.clear(); - pendingPixelEraserCircles_.clear(); - pendingPixelEraserPath_.reset(); + resetPendingPixelErase(); // DON'T set needsStrokeRedraw_ - kClear visual is already correct // Full redraw only needed on undo/redo/deserialize @@ -615,16 +609,21 @@ void SkiaDrawingEngine::finishStroke(long endTimestamp) { if (strokeSurface_ && currentTool_ != "eraser") { SkCanvas* strokeCanvas = strokeSurface_->getCanvas(); - if (isRecognizedShapeToolType(stroke.toolType) - || stroke.toolType == "highlighter" - || stroke.toolType == "marker") { + // Recognized shapes and centerline tools always re-render from vector + // data. Other tools do too when zoomed: the active surface covers only + // the magnified viewport, so re-rendering at 1x beats downsampling the + // preview pixels. At 1x the accumulated preview surface is composited + // directly, which is O(1) in stroke length. + const bool renderFromGeometry = isRecognizedShapeToolType(stroke.toolType) + || isCenterlineStrokedToolType(stroke.toolType) + || activeStrokeRenderer_->viewportScale() > kIdentityRenderScaleThreshold; + + if (renderFromGeometry) { SkPaint strokePaint = stroke.paint; - if (!stroke.isEraser) { - uint8_t baseAlpha = stroke.paint.getAlpha(); - strokePaint.setAlpha(static_cast(baseAlpha * stroke.originalAlphaMod)); - } + strokePaint.setAlpha( + static_cast(stroke.paint.getAlpha() * stroke.originalAlphaMod)); renderStrokeGeometry(strokeCanvas, stroke, strokePaint); - } else if (activeStrokeRenderer_->viewportScale() <= 1.01f) { + } else { // Render any remaining tail points to complete the stroke if (currentPoints_.size() > activeStrokeRenderer_->getLastRenderedIndex()) { activeStrokeRenderer_->renderFinalTail(currentPoints_, currentPaint_, currentTool_); @@ -632,14 +631,6 @@ void SkiaDrawingEngine::finishStroke(long endTimestamp) { // Composite active stroke onto persistent stroke surface activeStrokeRenderer_->drawSnapshot(strokeCanvas); - } else { - // Zoomed: the active surface covers only the magnified viewport, - // so re-render the finished stroke from vector data at 1x rather - // than downsampling the preview pixels. - SkPaint strokePaint = stroke.paint; - strokePaint.setAlpha( - static_cast(stroke.paint.getAlpha() * stroke.originalAlphaMod)); - renderStrokeGeometry(strokeCanvas, stroke, strokePaint); } // Update cached snapshot diff --git a/cpp/SkiaDrawingEngine.h b/cpp/SkiaDrawingEngine.h index 3f79903..9cf083c 100644 --- a/cpp/SkiaDrawingEngine.h +++ b/cpp/SkiaDrawingEngine.h @@ -314,8 +314,15 @@ class SkiaDrawingEngine { void markStrokeCachesDirty(); void invalidateStrokeTilesForRect(const SkRect& bounds); void renderScaleAwareStrokes(SkCanvas* canvas); - void renderActiveContent(SkCanvas* canvas, bool useIncrementalActiveSurface); - void renderActivePixelEraserCutout(SkCanvas* canvas); + void renderActiveContent(SkCanvas* canvas); + // Draw points as a single constant-width stroked centerline (highlighter/ + // marker). Uses cachedPath when non-empty, else smooths from points. + void drawCenterlineStrokePath( + SkCanvas* canvas, + const std::vector& points, + const SkPath& cachedPath, + const SkPaint& paint + ); sk_sp renderStrokeTile(const StrokeTileKey& key, int tileWidth, int tileHeight, float scale); void pruneStrokeTileCache(); void redrawEraserMask(); // Dual-surface: only redraws eraser circles to mask @@ -329,7 +336,8 @@ class SkiaDrawingEngine { // PencilKit-style pixel eraser: immediately splits strokes at eraser point // Returns true if any strokes were modified - bool applyPixelEraserAt(float x, float y, float radius); + void applyPixelEraserAt(float x, float y, float radius); + void resetPendingPixelErase(); mutable std::recursive_mutex stateMutex_; }; diff --git a/cpp/SkiaDrawingEngineEraser.cpp b/cpp/SkiaDrawingEngineEraser.cpp index dea6b49..18ce9fa 100644 --- a/cpp/SkiaDrawingEngineEraser.cpp +++ b/cpp/SkiaDrawingEngineEraser.cpp @@ -1,7 +1,6 @@ #include "SkiaDrawingEngine.h" #include "EraserRenderer.h" -#include "ShapeRecognition.h" #include "StrokeSplitter.h" #include @@ -107,7 +106,13 @@ void SkiaDrawingEngine::bakeEraserCircles() { printf("[C++] bakeEraserCircles: Baked %zu circles total\n", bakedCircleCount_); } -bool SkiaDrawingEngine::applyPixelEraserAt(float eraserX, float eraserY, float radius) { +void SkiaDrawingEngine::resetPendingPixelErase() { + pendingPixelEraseEntries_.clear(); + pendingPixelEraserCircles_.clear(); + pendingPixelEraserPath_.reset(); +} + +void SkiaDrawingEngine::applyPixelEraserAt(float eraserX, float eraserY, float radius) { // Build list of circles to add (may include interpolated circles for full coverage) std::vector circlesToAdd; circlesToAdd.push_back({eraserX, eraserY, radius}); @@ -166,7 +171,9 @@ bool SkiaDrawingEngine::applyPixelEraserAt(float eraserX, float eraserY, float r } // Keep the live surface hot during the drag. Snapshotting here forces - // full-surface work per input sample; touchEnded refreshes caches. + // full-surface work per input sample; the snapshot is invalidated so + // render() falls back to drawing the surface directly. + cachedStrokeSnapshot_ = nullptr; } // Track position for next call @@ -174,8 +181,6 @@ bool SkiaDrawingEngine::applyPixelEraserAt(float eraserX, float eraserY, float r lastEraserY_ = eraserY; lastEraserRadius_ = radius; hasLastEraserPoint_ = true; - - return true; } bool SkiaDrawingEngine::applyPendingPixelEraseToStrokes() { @@ -183,9 +188,14 @@ bool SkiaDrawingEngine::applyPendingPixelEraseToStrokes() { return false; } - SkRect eraseBounds = SkRect::MakeEmpty(); + // The pending path is exactly the pending circles, so its bounds are the + // union of their bounds. + const SkRect eraseBounds = pendingPixelEraserPath_.getBounds(); + + std::vector circleBounds; + circleBounds.reserve(pendingPixelEraserCircles_.size()); for (const auto& circle : pendingPixelEraserCircles_) { - eraseBounds.join(SkRect::MakeLTRB( + circleBounds.push_back(SkRect::MakeLTRB( circle.x - circle.radius, circle.y - circle.radius, circle.x + circle.radius, @@ -202,23 +212,18 @@ bool SkiaDrawingEngine::applyPendingPixelEraseToStrokes() { } SkRect bounds = stroke.path.getBounds(); - const float strokeOutset = std::max(8.0f, averageCalculatedWidth(stroke.points)); + // Conservative O(1) outset covering the rendered stroke width; + // per-circle bounds below already include the eraser radius. + const float strokeOutset = std::max(8.0f, stroke.paint.getStrokeWidth() * 2.0f); bounds.outset(strokeOutset, strokeOutset); if (!bounds.intersects(eraseBounds)) { continue; } std::vector circlesForStroke; - circlesForStroke.reserve(pendingPixelEraserCircles_.size()); - for (const auto& circle : pendingPixelEraserCircles_) { - SkRect circleBounds = SkRect::MakeLTRB( - circle.x - circle.radius, - circle.y - circle.radius, - circle.x + circle.radius, - circle.y + circle.radius - ); - if (bounds.intersects(circleBounds)) { - circlesForStroke.push_back(circle); + for (size_t i = 0; i < pendingPixelEraserCircles_.size(); ++i) { + if (bounds.intersects(circleBounds[i])) { + circlesForStroke.push_back(pendingPixelEraserCircles_[i]); } } if (circlesForStroke.empty()) { @@ -234,24 +239,7 @@ bool SkiaDrawingEngine::applyPendingPixelEraseToStrokes() { // Refresh the visibility cache once per gesture so fully-erased // strokes stop responding to selection. if (stroke.cachedHasVisiblePoints) { - bool hasVisible = false; - for (const auto& pt : stroke.points) { - bool pointVisible = true; - for (const auto& circle : stroke.erasedBy) { - float dx = pt.x - circle.x; - float dy = pt.y - circle.y; - float totalRadius = circle.radius + pt.calculatedWidth / 2.0f; - if (dx * dx + dy * dy <= totalRadius * totalRadius) { - pointVisible = false; - break; - } - } - if (pointVisible) { - hasVisible = true; - break; - } - } - stroke.cachedHasVisiblePoints = hasVisible; + stroke.cachedHasVisiblePoints = stroke.hasVisiblePoints(); } StrokeDelta::PixelEraseEntry entry; diff --git a/cpp/SkiaDrawingEngineRendering.cpp b/cpp/SkiaDrawingEngineRendering.cpp index 64c3f14..be9a240 100644 --- a/cpp/SkiaDrawingEngineRendering.cpp +++ b/cpp/SkiaDrawingEngineRendering.cpp @@ -21,8 +21,6 @@ namespace { constexpr int kStrokeTileSize = 512; constexpr size_t kMaxStrokeTileCacheBytes = 96 * 1024 * 1024; -constexpr float kMinimumRenderScale = 1.0f; -constexpr float kMaximumRenderScale = 5.0f; SkColor swapRedBlueChannels(SkColor color) { return SkColorSetARGB( @@ -172,22 +170,8 @@ void SkiaDrawingEngine::renderStrokeGeometry(SkCanvas* canvas, const Stroke& str return; } - if (stroke.toolType == "highlighter" || stroke.toolType == "marker") { - SkPath centerPath = stroke.path; - if (centerPath.isEmpty()) { - smoothPath(stroke.points, centerPath); - } - if (centerPath.isEmpty()) { - return; - } - - SkPaint strokePaint = paint; - strokePaint.setStyle(SkPaint::kStroke_Style); - strokePaint.setStrokeWidth(averageCalculatedWidth(stroke.points)); - strokePaint.setStrokeCap(SkPaint::kRound_Cap); - strokePaint.setStrokeJoin(SkPaint::kRound_Join); - strokePaint.setAntiAlias(true); - canvas->drawPath(centerPath, strokePaint); + if (isCenterlineStrokedToolType(stroke.toolType)) { + drawCenterlineStrokePath(canvas, stroke.points, stroke.path, paint); return; } @@ -442,7 +426,29 @@ void SkiaDrawingEngine::renderScaleAwareStrokes(SkCanvas* canvas) { } } -void SkiaDrawingEngine::renderActiveContent(SkCanvas* canvas, bool useIncrementalActiveSurface) { +void SkiaDrawingEngine::drawCenterlineStrokePath( + SkCanvas* canvas, + const std::vector& points, + const SkPath& cachedPath, + const SkPaint& paint +) { + SkPath centerPath = cachedPath; + if (centerPath.isEmpty()) { + smoothPath(points, centerPath); + } + if (centerPath.isEmpty()) { + return; + } + + SkPaint strokePaint = paint; + strokePaint.setStyle(SkPaint::kStroke_Style); + strokePaint.setStrokeWidth(averageCalculatedWidth(points)); + strokePaint.setStrokeCap(SkPaint::kRound_Cap); + strokePaint.setStrokeJoin(SkPaint::kRound_Join); + canvas->drawPath(centerPath, strokePaint); +} + +void SkiaDrawingEngine::renderActiveContent(SkCanvas* canvas) { if (hasActiveShapePreview_ && !activeShapePreviewPoints_.empty()) { Stroke previewStroke; previewStroke.points = activeShapePreviewPoints_; @@ -451,22 +457,24 @@ void SkiaDrawingEngine::renderActiveContent(SkCanvas* canvas, bool useIncrementa previewStroke.toolType = activeShapePreviewToolType_; SkPaint previewPaint = currentPaint_; - if (currentTool_ != "highlighter" && currentTool_ != "marker") { + if (!isCenterlineStrokedToolType(currentTool_)) { const float pressureAlphaMod = 0.85f + (averagePressure(currentPoints_) * 0.15f); previewPaint.setAlpha(static_cast(previewPaint.getAlpha() * pressureAlphaMod)); } renderStrokeGeometry(canvas, previewStroke, previewPaint); } else if (currentPoints_.size() >= 2 && currentTool_ != "select" && currentTool_ != "eraser") { - if (currentTool_ == "highlighter" || currentTool_ == "marker") { - Stroke activeStroke; - activeStroke.points = currentPoints_; - activeStroke.paint = currentPaint_; - activeStroke.toolType = currentTool_; + if (isCenterlineStrokedToolType(currentTool_)) { + // Full redraw each frame. Multiply blend is neutralized for the + // preview: the stroke is drawn over the composited canvas here, + // whereas the finished stroke multiplies only against other + // strokes on the transparent stroke layer. SkPaint previewPaint = currentPaint_; - previewPaint.setBlendMode(SkBlendMode::kSrcOver); - renderStrokeGeometry(canvas, activeStroke, previewPaint); - } else if (useIncrementalActiveSurface) { + if (previewPaint.asBlendMode() == SkBlendMode::kMultiply) { + previewPaint.setBlendMode(SkBlendMode::kSrcOver); + } + drawCenterlineStrokePath(canvas, currentPoints_, SkPath(), previewPaint); + } else { ActiveStrokeViewport viewport; viewport.scale = renderScale_; viewport.left = visibleLeft_; @@ -480,51 +488,10 @@ void SkiaDrawingEngine::renderActiveContent(SkCanvas* canvas, bool useIncrementa currentTool_, viewport ); - } else { - Stroke activeStroke; - activeStroke.points = currentPoints_; - activeStroke.paint = currentPaint_; - activeStroke.toolType = currentTool_; - renderStrokeGeometry(canvas, activeStroke, currentPaint_); } } } -void SkiaDrawingEngine::renderActivePixelEraserCutout(SkCanvas* canvas) { - if (!canvas - || currentTool_ != "eraser" - || eraserMode_ != "pixel" - || pendingPixelEraserPath_.isEmpty()) { - return; - } - - if (backgroundType_ == "pdf" && !pdfBackgroundImage_) { - SkPaint clearPaint; - clearPaint.setBlendMode(SkBlendMode::kClear); - clearPaint.setAntiAlias(true); - canvas->drawPath(pendingPixelEraserPath_, clearPaint); - return; - } - - canvas->save(); - canvas->clipPath(pendingPixelEraserPath_, SkClipOp::kIntersect, true); - if (backgroundType_ == "pdf" || backgroundType_ == "plain") { - canvas->drawColor(SK_ColorWHITE); - } - - if (backgroundType_ != "pdf" || pdfBackgroundImage_) { - backgroundRenderer_->drawBackground( - canvas, - backgroundType_, - width_, - height_, - pdfBackgroundImage_, - backgroundOriginY_ - ); - } - canvas->restore(); -} - void SkiaDrawingEngine::redrawEraserMask() { if (!needsEraserMaskRedraw_) return; @@ -555,7 +522,7 @@ void SkiaDrawingEngine::redrawEraserMask() { void SkiaDrawingEngine::render(SkCanvas* canvas) { std::lock_guard lock(stateMutex_); - const bool useScaleAwarePath = renderScale_ > 1.01f; + const bool useScaleAwarePath = renderScale_ > kIdentityRenderScaleThreshold; if (useScaleAwarePath) { if (backgroundType_ == "pdf") { @@ -582,12 +549,26 @@ void SkiaDrawingEngine::render(SkCanvas* canvas) { } canvas->restore(); + // During a pixel-erase drag the stroke tiles are stale (metadata is + // committed at pen-up), so clip the erased circles out of the stroke + // layer; the background drawn above shows through, whatever its type. + const bool livePixelErase = currentTool_ == "eraser" + && eraserMode_ == "pixel" + && !pendingPixelEraserPath_.isEmpty(); + if (livePixelErase) { + SkPath deviceEraserPath = pendingPixelEraserPath_; + deviceEraserPath.transform(SkMatrix::Scale(renderScale_, renderScale_)); + canvas->save(); + canvas->clipPath(deviceEraserPath, SkClipOp::kDifference, true); + } renderScaleAwareStrokes(canvas); + if (livePixelErase) { + canvas->restore(); + } canvas->save(); canvas->scale(renderScale_, renderScale_); - renderActiveContent(canvas, true); - renderActivePixelEraserCutout(canvas); + renderActiveContent(canvas); if (showEraserCursor_ && eraserCursorRadius_ > 0) { SkPaint cursorPaint; @@ -726,11 +707,6 @@ void SkiaDrawingEngine::render(SkCanvas* canvas) { canvas->restore(); } } - } else if (currentTool_ == "eraser" - && eraserMode_ == "pixel" - && !currentPoints_.empty() - && strokeSurface_) { - strokeSurface_->draw(canvas, 0, 0); } else if (cachedStrokeSnapshot_) { canvas->drawImage(cachedStrokeSnapshot_, 0, 0); } else if (strokeSurface_) { @@ -740,7 +716,7 @@ void SkiaDrawingEngine::render(SkCanvas* canvas) { } // 4. Draw active stroke incrementally (O(1) per frame instead of O(n)) - renderActiveContent(canvas, true); + renderActiveContent(canvas); // Draw eraser cursor for pixel eraser if (showEraserCursor_ && eraserCursorRadius_ > 0) {