From 3327069029479fea550dd0581b6916a2d7f5b86f Mon Sep 17 00:00:00 2001 From: Ink Open Source Date: Sun, 28 Jun 2026 19:13:05 -0700 Subject: [PATCH] Fix eraser handling in Ink Stylus Showcase App When multiple erases happen in quick succession, their background geometric computations can sometimes overlap, causing them to accidentally overwrite one another. This CL mitigates this by queuing the eraser strokes per target node, and processing them sequentially. In addition, the stroke segmenting is deferred, and performed once for all erases are handled. PiperOrigin-RevId: 939563424 --- ink/strokes/BUILD.bazel | 1 + ink/strokes/internal/jni/BUILD.bazel | 2 ++ ink/strokes/internal/jni/stroke_jni.cc | 17 ++++++++-- ink/strokes/internal/jni/stroke_native.cc | 33 +++++++++++++++---- ink/strokes/internal/jni/stroke_native.h | 13 +++++--- ink/strokes/internal/stroke_segmentation.cc | 1 + .../internal/stroke_segmentation_test.cc | 10 ++++++ ink/strokes/stroke.cc | 31 ++++++++++------- ink/strokes/stroke.h | 22 +++++++++---- ink/strokes/stroke_test.cc | 32 +++++++++++------- 10 files changed, 121 insertions(+), 41 deletions(-) diff --git a/ink/strokes/BUILD.bazel b/ink/strokes/BUILD.bazel index f4d7e02f..5fba8150 100644 --- a/ink/strokes/BUILD.bazel +++ b/ink/strokes/BUILD.bazel @@ -33,6 +33,7 @@ cc_library( "//ink/geometry:partitioned_mesh", "//ink/strokes/input:stroke_input_batch", "//ink/strokes/internal:stroke_input_modeler", + "//ink/strokes/internal:stroke_segmentation", "//ink/strokes/internal:stroke_shape_builder", "//ink/strokes/internal:stroke_subtraction", "//ink/strokes/internal:stroke_vertex", diff --git a/ink/strokes/internal/jni/BUILD.bazel b/ink/strokes/internal/jni/BUILD.bazel index 34188636..24349966 100644 --- a/ink/strokes/internal/jni/BUILD.bazel +++ b/ink/strokes/internal/jni/BUILD.bazel @@ -63,6 +63,7 @@ cc_library( "//ink/strokes/input:stroke_input_batch", "@abseil-cpp//absl/log:absl_check", "@abseil-cpp//absl/status", + "@abseil-cpp//absl/status:statusor", ] + select({ "@platforms//os:android": [], "//conditions:default": [ @@ -79,6 +80,7 @@ cc_library( ":stroke_input_jni_helper", ":stroke_native", "//ink/jni/internal:jni_defines", + "//ink/jni/internal:status_jni_helper", ] + select({ "@platforms//os:android": [], "//conditions:default": [ diff --git a/ink/strokes/internal/jni/stroke_jni.cc b/ink/strokes/internal/jni/stroke_jni.cc index 45f4a21b..a46251c2 100644 --- a/ink/strokes/internal/jni/stroke_jni.cc +++ b/ink/strokes/internal/jni/stroke_jni.cc @@ -15,10 +15,13 @@ #include #include "ink/jni/internal/jni_defines.h" +#include "ink/jni/internal/status_jni_helper.h" #include "ink/strokes/internal/jni/stroke_native.h" extern "C" { +using ::ink::jni::ThrowExceptionFromStatusCallback; + JNI_METHOD(strokes, StrokeNative, jlong, createWithBrushAndInputs) (JNIEnv* env, jobject object, jlong brush_native_pointer, jlong inputs_native_pointer) { @@ -60,17 +63,27 @@ JNI_METHOD(strokes, StrokeNative, void, free) StrokeNative_free(native_pointer_to_stroke); } -JNI_METHOD(strokes, MultipleStrokesNative, jlong, createWithPartialErase) +JNI_METHOD(strokes, StrokeNative, jlong, createWithPartialErase) (JNIEnv* env, jobject object, jlong target_stroke_ptr, jlong eraser_shape_ptr, jfloat eraser_a, jfloat eraser_b, jfloat eraser_c, jfloat eraser_d, jfloat eraser_e, jfloat eraser_f, jfloat stroke_a, jfloat stroke_b, jfloat stroke_c, jfloat stroke_d, jfloat stroke_e, jfloat stroke_f) { - return MultipleStrokesNative_createWithPartialErase( + return StrokeNative_createWithPartialErase( target_stroke_ptr, eraser_shape_ptr, eraser_a, eraser_b, eraser_c, eraser_d, eraser_e, eraser_f, stroke_a, stroke_b, stroke_c, stroke_d, stroke_e, stroke_f); } +JNI_METHOD(strokes, MultipleStrokesNative, jlong, createWithSegmentSpatially) +(JNIEnv* env, jobject object, jlong target_stroke_ptr, jfloat transform_a, + jfloat transform_b, jfloat transform_c, jfloat transform_d, jfloat transform_e, + jfloat transform_f, jfloat tolerance) { + return MultipleStrokesNative_createWithSegmentSpatially( + env, target_stroke_ptr, transform_a, transform_b, transform_c, + transform_d, transform_e, transform_f, tolerance, + &ThrowExceptionFromStatusCallback); +} + JNI_METHOD(strokes, MultipleStrokesNative, jint, getStrokeCount) (JNIEnv* env, jobject object, jlong native_pointer) { return MultipleStrokesNative_getStrokeCount(native_pointer); diff --git a/ink/strokes/internal/jni/stroke_native.cc b/ink/strokes/internal/jni/stroke_native.cc index b9dd335c..cbfe4be5 100644 --- a/ink/strokes/internal/jni/stroke_native.cc +++ b/ink/strokes/internal/jni/stroke_native.cc @@ -21,6 +21,7 @@ #include #include "absl/log/absl_check.h" +#include "absl/status/statusor.h" #include "ink/brush/internal/jni/brush_native_helper.h" #include "ink/geometry/affine_transform.h" #include "ink/geometry/internal/jni/partitioned_mesh_native_helper.h" @@ -89,7 +90,7 @@ void StrokeNative_free(int64_t native_pointer) { DeleteNativeStroke(native_pointer); } -int64_t MultipleStrokesNative_createWithPartialErase( +int64_t StrokeNative_createWithPartialErase( int64_t target_stroke_ptr, int64_t eraser_shape_ptr, float eraser_a, float eraser_b, float eraser_c, float eraser_d, float eraser_e, float eraser_f, float stroke_a, float stroke_b, float stroke_c, @@ -99,15 +100,35 @@ int64_t MultipleStrokesNative_createWithPartialErase( AffineTransform stroke_transform(stroke_a, stroke_b, stroke_c, stroke_d, stroke_e, stroke_f); - std::vector fragments = + return NewNativeStroke( CastToStroke(target_stroke_ptr) .PartialErase(CastToPartitionedMesh(eraser_shape_ptr), - eraser_transform, stroke_transform); + eraser_transform, stroke_transform)); +} + +int64_t MultipleStrokesNative_createWithSegmentSpatially( + void* jni_env_pass_through, int64_t target_stroke_ptr, float transform_a, + float transform_b, float transform_c, float transform_d, float transform_e, + float transform_f, float tolerance, + void (*throw_from_status_callback)(void* jni_env, int status_code, + const char* status_str)) { + AffineTransform transform(transform_a, transform_b, transform_c, transform_d, + transform_e, transform_f); + + absl::StatusOr> fragments = + CastToStroke(target_stroke_ptr).SegmentSpatially(transform, tolerance); + + if (!fragments.ok()) { + throw_from_status_callback(jni_env_pass_through, + static_cast(fragments.status().code()), + fragments.status().ToString().c_str()); + return 0; + } MultipleStrokes result; - result.reserve(fragments.size()); - for (size_t i = 0; i < fragments.size(); ++i) { - result.push_back(std::make_unique(std::move(fragments[i]))); + result.reserve(fragments->size()); + for (size_t i = 0; i < fragments->size(); ++i) { + result.push_back(std::make_unique(std::move((*fragments)[i]))); } return NewNativeMultipleStrokes(std::move(result)); } diff --git a/ink/strokes/internal/jni/stroke_native.h b/ink/strokes/internal/jni/stroke_native.h index 1597140c..01e2d128 100644 --- a/ink/strokes/internal/jni/stroke_native.h +++ b/ink/strokes/internal/jni/stroke_native.h @@ -44,15 +44,20 @@ int64_t StrokeNative_newShallowCopyOfShape(int64_t native_pointer_to_stroke); void StrokeNative_free(int64_t native_pointer_to_stroke); -// Returns a pointer to hold the result of a partial erase before hand-off -// of the individual strokes. Uses `unique_ptr` for the handoff to minimize -// copying. -int64_t MultipleStrokesNative_createWithPartialErase( +// Returns a pointer to hold the result of a partial erase. +int64_t StrokeNative_createWithPartialErase( int64_t target_stroke_ptr, int64_t eraser_shape_ptr, float eraser_a, float eraser_b, float eraser_c, float eraser_d, float eraser_e, float eraser_f, float stroke_a, float stroke_b, float stroke_c, float stroke_d, float stroke_e, float stroke_f); +int64_t MultipleStrokesNative_createWithSegmentSpatially( + void* jni_env_pass_through, int64_t target_stroke_ptr, float transform_a, + float transform_b, float transform_c, float transform_d, float transform_e, + float transform_f, float tolerance, + void (*throw_from_status_callback)(void* jni_env, int status_code, + const char* status_str)); + int32_t MultipleStrokesNative_getStrokeCount(int64_t native_pointer); // Releases the pointer to i-th stroke in partial erase result, to be wrapped diff --git a/ink/strokes/internal/stroke_segmentation.cc b/ink/strokes/internal/stroke_segmentation.cc index 82ee1ba6..f9730b6f 100644 --- a/ink/strokes/internal/stroke_segmentation.cc +++ b/ink/strokes/internal/stroke_segmentation.cc @@ -272,6 +272,7 @@ absl::StatusOr> SegmentSpatially( // own component, then merge vertices that share a triangle, then merge // vertices that are within `tolerance` of each other. std::vector vertices = GetAllVertices(shape, transform); + if (vertices.empty()) return std::vector{}; ConnectedComponents components(vertices.size()); ConnectByTriangles(shape, components); ConnectBySpatialProximity(vertices, tolerance, components); diff --git a/ink/strokes/internal/stroke_segmentation_test.cc b/ink/strokes/internal/stroke_segmentation_test.cc index ed1e63f9..34d17545 100644 --- a/ink/strokes/internal/stroke_segmentation_test.cc +++ b/ink/strokes/internal/stroke_segmentation_test.cc @@ -32,8 +32,18 @@ namespace { using ::absl_testing::IsOk; using ::testing::ElementsAre; +using ::testing::IsEmpty; using ::testing::SizeIs; +TEST(StrokeSegmentationTest, SegmentSpatiallyEmptyMesh) { + PartitionedMesh empty_mesh; + absl::StatusOr> components = + SegmentSpatially(empty_mesh, AffineTransform::Identity(), + /*tolerance=*/1.0f); + ASSERT_THAT(components, IsOk()); + EXPECT_THAT(*components, IsEmpty()); +} + TEST(StrokeSegmentationTest, SegmentSpatially) { // C F // |\ |\ diff --git a/ink/strokes/stroke.cc b/ink/strokes/stroke.cc index 52283125..18c22260 100644 --- a/ink/strokes/stroke.cc +++ b/ink/strokes/stroke.cc @@ -34,6 +34,7 @@ #include "ink/geometry/partitioned_mesh.h" #include "ink/strokes/input/stroke_input_batch.h" #include "ink/strokes/internal/stroke_input_modeler.h" +#include "ink/strokes/internal/stroke_segmentation.h" #include "ink/strokes/internal/stroke_shape_builder.h" #include "ink/strokes/internal/stroke_subtraction.h" #include "ink/strokes/internal/stroke_vertex.h" @@ -208,22 +209,28 @@ void Stroke::RegenerateShape() { ABSL_DCHECK_EQ(shape_.RenderGroupCount(), brush_.CoatCount()); } -std::vector Stroke::PartialErase( - const PartitionedMesh& eraser_shape, - const AffineTransform& eraser_transform, - const AffineTransform& stroke_transform) const { +Stroke Stroke::PartialErase(const PartitionedMesh& eraser_shape, + const AffineTransform& eraser_transform, + const AffineTransform& stroke_transform) const { absl::StatusOr remaining_mesh = strokes_internal::Subtract( shape_, stroke_transform, eraser_shape, eraser_transform); - if (!remaining_mesh.ok()) return {*this}; + if (!remaining_mesh.ok()) return *this; - // If the entire stroke is erased, return an empty list. - if (absl::c_none_of(remaining_mesh->Meshes(), [](const Mesh& mesh) { - return mesh.TriangleCount() > 0; - })) { - return {}; - } + return Stroke(brush_, inputs_, *remaining_mesh); +} - return {Stroke(brush_, inputs_, *remaining_mesh)}; +absl::StatusOr> Stroke::SegmentSpatially( + const AffineTransform& transform, float tolerance) const { + absl::StatusOr> partitioned_meshes = + strokes_internal::SegmentSpatially(shape_, transform, tolerance); + if (!partitioned_meshes.ok()) return partitioned_meshes.status(); + + std::vector results; + results.reserve(partitioned_meshes->size()); + for (auto& mesh : *partitioned_meshes) { + results.push_back(Stroke(brush_, inputs_, std::move(mesh))); + } + return results; } } // namespace ink diff --git a/ink/strokes/stroke.h b/ink/strokes/stroke.h index 82b2753b..e78f1c8d 100644 --- a/ink/strokes/stroke.h +++ b/ink/strokes/stroke.h @@ -15,6 +15,8 @@ #ifndef INK_STROKES_STROKE_H_ #define INK_STROKES_STROKE_H_ +#include + #include "absl/status/status.h" #include "ink/brush/brush.h" #include "ink/brush/brush_family.h" @@ -115,15 +117,23 @@ class Stroke { // // The coordinate transformations are expected to be non-degenerate; otherwise // the stroke is returned as is. + Stroke PartialErase(const PartitionedMesh& eraser_shape, + const AffineTransform& eraser_transform, + const AffineTransform& stroke_transform) const; + + // Segments the stroke spatially based on connected components of the mesh. + // + // Connection is based on the geometry of the mesh embedded in R^2. Two + // points in `shape` are connected if their transformed positions are within + // `tolerance` distance of each other. // - // Each resulting fragment retains the original brush and inputs, but has a - // newly computed shape representing the portion remaining after erasure. The + // The returned vector contains a `Stroke` for each connected component, + // constructed with the original brush and inputs, but with a new shape + // representing the portion of the stroke belonging to that component. The // order of the fragments in the returned vector is arbitrary and carries no // guarantee. - std::vector PartialErase( - const PartitionedMesh& eraser_shape, - const AffineTransform& eraser_transform, - const AffineTransform& stroke_transform) const; + absl::StatusOr> SegmentSpatially( + const AffineTransform& transform, float tolerance) const; private: // Regenerates the PartitionedMesh. diff --git a/ink/strokes/stroke_test.cc b/ink/strokes/stroke_test.cc index 07334386..4fed190d 100644 --- a/ink/strokes/stroke_test.cc +++ b/ink/strokes/stroke_test.cc @@ -612,12 +612,10 @@ TEST(StrokeTest, PartialEraseWithEmptyEraserShapeReturnsStroke) { PartitionedMesh empty_eraser_shape; AffineTransform identity = AffineTransform::Identity(); - std::vector result = - stroke.PartialErase(empty_eraser_shape, identity, identity); + Stroke result = stroke.PartialErase(empty_eraser_shape, identity, identity); - ASSERT_THAT(result, SizeIs(1)); - EXPECT_THAT(result[0].GetBrush(), BrushEq(stroke.GetBrush())); - EXPECT_THAT(result[0].GetInputs(), StrokeInputBatchEq(stroke.GetInputs())); + EXPECT_THAT(result.GetBrush(), BrushEq(stroke.GetBrush())); + EXPECT_THAT(result.GetInputs(), StrokeInputBatchEq(stroke.GetInputs())); } TEST(StrokeTest, PartialEraseWithDegenerateTransformReturnsStroke) { @@ -626,13 +624,11 @@ TEST(StrokeTest, PartialEraseWithDegenerateTransformReturnsStroke) { AffineTransform identity = AffineTransform::Identity(); AffineTransform degenerate = AffineTransform::Scale(1, 0); - std::vector result = - stroke.PartialErase(eraser_shape, identity, degenerate); + Stroke result = stroke.PartialErase(eraser_shape, identity, degenerate); - ASSERT_THAT(result, SizeIs(1)); - EXPECT_THAT(result[0].GetBrush(), BrushEq(stroke.GetBrush())); - EXPECT_THAT(result[0].GetInputs(), StrokeInputBatchEq(stroke.GetInputs())); - EXPECT_THAT(result[0].GetShape(), PartitionedMeshDeepEq(stroke.GetShape())); + EXPECT_THAT(result.GetBrush(), BrushEq(stroke.GetBrush())); + EXPECT_THAT(result.GetInputs(), StrokeInputBatchEq(stroke.GetInputs())); + EXPECT_THAT(result.GetShape(), PartitionedMeshDeepEq(stroke.GetShape())); } TEST(StrokeTest, ParticleBrushWithOneDimensionZero) { @@ -659,5 +655,19 @@ TEST(StrokeTest, ParticleBrushWithOneDimensionZero) { EXPECT_THAT(stroke.GetShape().Meshes(), SizeIs(1)); } +TEST(StrokeTest, SegmentSpatiallyReturnsCorrectSegments) { + Stroke stroke(CreateBrush(), CreateFilledInputs()); + ASSERT_FALSE(stroke.GetShape().Meshes().empty()); + + absl::StatusOr> segments = + stroke.SegmentSpatially(AffineTransform::Identity(), /*tolerance=*/1.0f); + ASSERT_THAT(segments, IsOk()); + EXPECT_THAT(*segments, Not(IsEmpty())); + for (const auto& segment : *segments) { + EXPECT_THAT(segment.GetBrush(), BrushEq(stroke.GetBrush())); + EXPECT_THAT(segment.GetInputs(), StrokeInputBatchEq(stroke.GetInputs())); + } +} + } // namespace } // namespace ink