From 96f93ff42e1f1bc87dbf80fe693556ed77710c72 Mon Sep 17 00:00:00 2001 From: Thomas Barlow Date: Tue, 9 Jan 2024 17:40:14 +0100 Subject: [PATCH] Fix: False positives/negatives in curves_selection.cc#contains The last element of each 2048-element section was being skipped because the size of an `IndexRange` was being calculated like `range.last() - range.start()`, but it should have been like `range.one_after_last() - range.start()`. For example, the `IndexRange` starting at `0` with a size of `3` contains the values `0`, `1` and `2`, so `.start() == 0`, `.last() == 2` and `.one_after_last() == 3`. False positives could occur because the entirety of the `values` array was always being checked, even when only the first few elements were initialized. An end iterator matching the end of the initialized elements is now used instead of the end of the array itself. The `value` argument was ignored by some code paths, which instead always checked for `true`. This didn't cause any issues currently, because all uses of #contains are searching for `true`, but this may not be the case in the future. Pull Request: https://projects.blender.org/blender/blender/pulls/116834 --- .../blender/editors/curves/intern/curves_selection.cc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/source/blender/editors/curves/intern/curves_selection.cc b/source/blender/editors/curves/intern/curves_selection.cc index 6324741f0d1..e1c70f50ed6 100644 --- a/source/blender/editors/curves/intern/curves_selection.cc +++ b/source/blender/editors/curves/intern/curves_selection.cc @@ -173,7 +173,7 @@ static bool contains(const VArray &varray, for (const int64_t segment_i : IndexRange(sliced_mask.segments_num())) { const IndexMaskSegment segment = sliced_mask.segment(segment_i); for (const int i : segment) { - if (span[i]) { + if (span[i] == value) { return true; } } @@ -191,13 +191,15 @@ static bool contains(const VArray &varray, return init; } constexpr int64_t MaxChunkSize = 512; - for (int64_t start = range.start(); start < range.last(); start += MaxChunkSize) { - const int64_t end = std::min(start + MaxChunkSize, range.last()); + const int64_t slice_end = range.one_after_last(); + for (int64_t start = range.start(); start < slice_end; start += MaxChunkSize) { + const int64_t end = std::min(start + MaxChunkSize, slice_end); const int64_t size = end - start; const IndexMask sliced_mask = indices_to_check.slice(start, size); std::array values; + auto values_end = values.begin() + size; varray.materialize_compressed(sliced_mask, values); - if (std::find(values.begin(), values.end(), true) != values.end()) { + if (std::find(values.begin(), values_end, value) != values_end) { return true; } }