diff --git a/ink/strokes/in_progress_stroke_test.cc b/ink/strokes/in_progress_stroke_test.cc index cf89fd20..58c3b052 100644 --- a/ink/strokes/in_progress_stroke_test.cc +++ b/ink/strokes/in_progress_stroke_test.cc @@ -263,6 +263,23 @@ TEST(InProgressStrokeTest, SetNoiseSeedAndBaseAnimationPhase) { EXPECT_EQ(stroke.GetInputs().GetBaseAnimationPhase(), 0.75f); } +TEST(InProgressStrokeTest, NoiseSeedAndBaseAnimationPhaseAfterUpdateShape) { + InProgressStroke stroke; + stroke.Start(CreateRectangularTestBrush(), /*noise_seed=*/12345, + /*base_animation_phase=*/0.75f); + + absl::StatusOr real_inputs = StrokeInputBatch::Create({ + {.position = {1, 2}, .elapsed_time = Duration32::Seconds(0.0)}, + {.position = {3, 2}, .elapsed_time = Duration32::Seconds(0.1)}, + }); + ASSERT_THAT(real_inputs, IsOk()); + EXPECT_THAT(stroke.EnqueueInputs(*real_inputs, {}), IsOk()); + EXPECT_THAT(stroke.UpdateShape(Duration32::Seconds(0.15)), IsOk()); + + EXPECT_EQ(stroke.GetInputs().GetNoiseSeed(), 12345); + EXPECT_EQ(stroke.GetInputs().GetBaseAnimationPhase(), 0.75f); +} + TEST(InProgressStrokeTest, EnqueueInputsWithoutStart) { InProgressStroke stroke; EXPECT_THAT( diff --git a/ink/strokes/input/stroke_input_batch.cc b/ink/strokes/input/stroke_input_batch.cc index 958793c0..97281e14 100644 --- a/ink/strokes/input/stroke_input_batch.cc +++ b/ink/strokes/input/stroke_input_batch.cc @@ -65,6 +65,12 @@ StrokeInputBatch::ConstIterator StrokeInputBatch::ConstIterator::operator++( } void StrokeInputBatch::Clear() { + ClearInputs(); + noise_seed_ = 0; + base_animation_phase_ = 0.0f; +} + +void StrokeInputBatch::ClearInputs() { if (data_.IsShared()) { data_.Reset(); } else if (data_.HasValue()) { @@ -74,8 +80,6 @@ void StrokeInputBatch::Clear() { size_ = 0; tool_type_ = StrokeInput::ToolType::kUnknown; stroke_unit_length_ = StrokeInput::kNoStrokeUnitLength; - noise_seed_ = 0; - base_animation_phase_ = 0.0f; has_pressure_ = false; has_tilt_ = false; has_orientation_ = false; @@ -206,7 +210,7 @@ absl::Status StrokeInputBatch::Set(int i, const StrokeInput& input) { ABSL_RETURN_IF_ERROR(ValidateSingleInput(input)); if (Size() == 1) { - Clear(); + ClearInputs(); if (!data_.HasValue()) data_.Emplace(); SetInlineFormatMetadata(input); AppendInputToFloatVector(input, data_.MutableValue()); @@ -324,7 +328,11 @@ absl::Status StrokeInputBatch::Append(const StrokeInputBatch& inputs) { if (inputs.IsEmpty()) return absl::OkStatus(); if (IsEmpty()) { + uint32_t old_noise_seed = noise_seed_; + float old_animation_phase = base_animation_phase_; *this = inputs; + noise_seed_ = old_noise_seed; + base_animation_phase_ = old_animation_phase; return absl::OkStatus(); } @@ -379,7 +387,7 @@ void StrokeInputBatch::Erase(int start, int count) { count = std::min(count, Size() - start); if (count == 0) return; if (start == 0 && count == Size()) { - Clear(); + ClearInputs(); return; } diff --git a/ink/strokes/input/stroke_input_batch.h b/ink/strokes/input/stroke_input_batch.h index 1090deab..4fbbd120 100644 --- a/ink/strokes/input/stroke_input_batch.h +++ b/ink/strokes/input/stroke_input_batch.h @@ -88,6 +88,11 @@ class StrokeInputBatch { ConstIterator begin() const; ConstIterator end() const; + // Erases all inputs from the batch, and resets both the noise seed and base + // animation phase to their default values of zero. + // + // This is functionally equivalent to `*this = StrokeInputBatch()`, except + // that it will reuse existing allocations when possible. void Clear(); int Size() const; @@ -116,7 +121,8 @@ class StrokeInputBatch { // // In the special case that this will overwrite the only held `StrokeInput`, // it is valid for the format of `input` to be different from the currently - // held value. + // held value. Regardless, in all cases, the noise seed and base animation + // phase will not be modified. // // Returns an error and does not modify the batch if validation fails. absl::Status Set(int i, const StrokeInput& input); @@ -142,9 +148,9 @@ class StrokeInputBatch { // Returns an error and does not modify the batch if validation fails. absl::Status Append(const StrokeInput& input); - // Validates and appends a sequence of `inputs`. This batch's per-stroke seed - // value is left unchanged, even when appending another batch with a different - // seed value. + // Validates and appends a sequence of `inputs`. This batch's noise seed and + // base animation phase are left unchanged, even when appending another batch + // with a different noise seed and/or base animation phase. // // Returns an error and does not modify the batch if validation fails. absl::Status Append(absl::Span inputs); @@ -160,6 +166,10 @@ class StrokeInputBatch { // If `start` + `count` is greater than `Size()`, then all elements from // `start` until the end of the input batch are erased. CHECK-fails if `start` // is not less than or equal to `Size()`. + // + // This will not change the noise seed or base animation phase associated with + // this input batch, even if all inputs are erased. If you wish to reset + // those values as well, consider using `Clear()`. void Erase(int start, int count = std::numeric_limits::max()); // Returns the current input tool type or `StrokeInput::ToolType::kUnknown` @@ -272,6 +282,12 @@ class StrokeInputBatch { // keeping the stroke total elapsed time the same. void TransformPreservingDuration(const AffineTransform& transform); + // Erases all inputs from the batch, and clears the inline member variables + // that store the "format" of the inputs (i.e. tool type and whether pressure, + // tilt, and orientation are present), but does *not* reset either the noise + // seed nor the base animation phase. + void ClearInputs(); + // Updates the inline member variables that store the "format" of the inputs // (i.e. tool type and whether pressure, tilt, and orientation are present). // This function should only be called when the batch is empty. diff --git a/ink/strokes/input/stroke_input_batch_test.cc b/ink/strokes/input/stroke_input_batch_test.cc index 465454da..33d6ce7a 100644 --- a/ink/strokes/input/stroke_input_batch_test.cc +++ b/ink/strokes/input/stroke_input_batch_test.cc @@ -230,15 +230,34 @@ TEST(StrokeInputBatchTest, AppendEmptyToNonEmpty) { EXPECT_THAT(batch, StrokeInputBatchIsArray({input})); } +TEST(StrokeInputBatchTest, AppendNonEmptyToEmpty) { + StrokeInputBatch batch1; + batch1.SetNoiseSeed(12345u); + batch1.SetBaseAnimationPhase(0.125f); + + StrokeInputBatch batch2; + StrokeInput input = MakeValidTestInput(); + ASSERT_THAT(batch2.Append(input), IsOk()); + EXPECT_THAT(batch2, StrokeInputBatchIsArray({input})); + + EXPECT_THAT(batch1.Append(batch2), IsOk()); + EXPECT_THAT(batch1, StrokeInputBatchIsArray({input})); + EXPECT_EQ(batch1.GetNoiseSeed(), 12345u); + EXPECT_EQ(batch1.GetBaseAnimationPhase(), 0.125f); +} + TEST(StrokeInputBatchTest, SetReplacingOnlyExistingValue) { StrokeInputBatch batch; - std::vector input_vector = MakeValidTestInputSequence(); ASSERT_THAT(batch.Append(input_vector[0]), IsOk()); + batch.SetNoiseSeed(12345u); + batch.SetBaseAnimationPhase(0.125f); StrokeInput replacement = input_vector[1]; EXPECT_THAT(batch.Set(0, replacement), IsOk()); EXPECT_THAT(batch, StrokeInputBatchIsArray({replacement})); + EXPECT_EQ(batch.GetNoiseSeed(), 12345u); + EXPECT_EQ(batch.GetBaseAnimationPhase(), 0.125f); } TEST(StrokeInputBatchTest, Set) { @@ -351,7 +370,8 @@ TEST(StrokeInputBatchTest, Clear) { EXPECT_EQ(batch->GetBaseAnimationPhase(), 0.75f); batch->Clear(); - // Batch should now be empty and the tool type should be unknown. + // Batch should now be empty, the tool type should be unknown, and noise seed + // and base animation phase should be reset to their default values. EXPECT_TRUE(batch->IsEmpty()); EXPECT_EQ(batch->Size(), 0); ASSERT_FALSE(batch->HasPressure()); @@ -1351,11 +1371,16 @@ TEST(StrokeInputBatchTest, EraseWithStartEqualToSize) { TEST(StrokeInputBatchTest, EraseAll) { absl::StatusOr batch = StrokeInputBatch::Create( - MakeValidTestInputSequence(StrokeInput::ToolType::kStylus)); + MakeValidTestInputSequence(StrokeInput::ToolType::kStylus), + /*noise_seed=*/12345, /*base_animation_phase=*/0.75f); ASSERT_THAT(batch, IsOk()); batch->Erase(0, batch->Size()); EXPECT_TRUE(batch->IsEmpty()); EXPECT_EQ(batch->GetToolType(), StrokeInput::ToolType::kUnknown); + // Unlike `Clear()`, `Erase()` does not reset the noise seed or base animation + // phase, even if all inputs get erased. + EXPECT_EQ(batch->GetNoiseSeed(), 12345u); + EXPECT_EQ(batch->GetBaseAnimationPhase(), 0.75f); } TEST(StrokeInputBatchTest, EraseWithNoPressure) {