From 723c9ed2f03981d4eb69bf0e6ed7da6ce9b35a32 Mon Sep 17 00:00:00 2001 From: Kenneth Moreland Date: Fri, 2 Feb 2024 13:20:53 -0500 Subject: [PATCH] Support `Fill` for `ArrayHandleStride` Previously, if you called `Fill` on an `ArrayHandleStride`, you would get an exception that said the feature was not supported. It turns out that filling values is very useful in situations where, for example, you need to initialize an array when processing an unknown type (and thus dealing with extracted components). This implementation of `Fill` first attempts to call `Fill` on the contained array. This only works if the stride is set to 1. If this does not work, then the code leverages the precompiled `ArrayCopy`. It does this by first creating a new `ArrayHandleStride` containing the fill value and a modulo of 1 so that value is constantly repeated. It then reconstructs an `ArrayHandleStride` for itself with a modified size and offset to match the start and end indices. Referencing the `ArrayCopy` was tricky because it kept creating circular dependencies among `ArrayHandleStride`, `ArrayExtractComponent`, and `UnknownArrayHandle`. These dependencies were broken by having `ArrayHandleStride` directly use the internal `ArrayCopyUnknown` function and to use a forward declaration of `UnknownArrayHandle` rather than including its header. --- docs/changelog/fill-stride-arrays.md | 22 +++++++ vtkm/cont/ArrayHandle.h | 7 +- vtkm/cont/ArrayHandleStride.cxx | 2 + vtkm/cont/ArrayHandleStride.h | 65 ++++++++++++++++--- vtkm/cont/internal/ArrayCopyUnknown.h | 8 ++- vtkm/cont/testing/UnitTestArrayCopy.cxx | 17 +++++ .../testing/UnitTestArrayExtractComponent.cxx | 16 +++++ 7 files changed, 124 insertions(+), 13 deletions(-) create mode 100644 docs/changelog/fill-stride-arrays.md diff --git a/docs/changelog/fill-stride-arrays.md b/docs/changelog/fill-stride-arrays.md new file mode 100644 index 000000000..7fa55ce61 --- /dev/null +++ b/docs/changelog/fill-stride-arrays.md @@ -0,0 +1,22 @@ +# Support `Fill` for `ArrayHandleStride` + +Previously, if you called `Fill` on an `ArrayHandleStride`, you would get +an exception that said the feature was not supported. It turns out that +filling values is very useful in situations where, for example, you need to +initialize an array when processing an unknown type (and thus dealing with +extracted components). + +This implementation of `Fill` first attempts to call `Fill` on the +contained array. This only works if the stride is set to 1. If this does +not work, then the code leverages the precompiled `ArrayCopy`. It does this +by first creating a new `ArrayHandleStride` containing the fill value and a +modulo of 1 so that value is constantly repeated. It then reconstructs an +`ArrayHandleStride` for itself with a modified size and offset to match the +start and end indices. + +Referencing the `ArrayCopy` was tricky because it kept creating circular +dependencies among `ArrayHandleStride`, `ArrayExtractComponent`, and +`UnknownArrayHandle`. These dependencies were broken by having +`ArrayHandleStride` directly use the internal `ArrayCopyUnknown` function +and to use a forward declaration of `UnknownArrayHandle` rather than +including its header. diff --git a/vtkm/cont/ArrayHandle.h b/vtkm/cont/ArrayHandle.h index bb64e156e..34f6d9a51 100644 --- a/vtkm/cont/ArrayHandle.h +++ b/vtkm/cont/ArrayHandle.h @@ -544,9 +544,10 @@ public: /// @brief Fills the array with a given value. /// - /// After calling this method, every entry in the array from `startIndex` to `endIndex`. - /// of the array is set to `fillValue`. If `startIndex` or `endIndex` is not specified, - /// then the fill happens from the begining or end, respectively. + /// After calling this method, every entry in the array from `startIndex` (inclusive) + /// to `endIndex` (exclusive) of the array is set to `fillValue`. If `startIndex` or + /// `endIndex` is not specified, then the fill happens from the begining or end, + /// respectively. /// VTKM_CONT void Fill(const ValueType& fillValue, vtkm::Id startIndex, diff --git a/vtkm/cont/ArrayHandleStride.cxx b/vtkm/cont/ArrayHandleStride.cxx index e6a84e597..7ce8ec8cd 100644 --- a/vtkm/cont/ArrayHandleStride.cxx +++ b/vtkm/cont/ArrayHandleStride.cxx @@ -11,6 +11,8 @@ #define vtk_m_cont_ArrayHandleStride_cxx #include +#include + namespace vtkm { namespace cont diff --git a/vtkm/cont/ArrayHandleStride.h b/vtkm/cont/ArrayHandleStride.h index 2707313a0..11cea2f04 100644 --- a/vtkm/cont/ArrayHandleStride.h +++ b/vtkm/cont/ArrayHandleStride.h @@ -12,6 +12,7 @@ #include #include +#include #include @@ -257,14 +258,11 @@ public: } } - VTKM_CONT static void Fill(const std::vector&, - const T&, - vtkm::Id, - vtkm::Id, - vtkm::cont::Token&) - { - throw vtkm::cont::ErrorBadType("Fill not supported for ArrayHandleStride."); - } + VTKM_CONT static void Fill(const std::vector& buffers, + const T& fillValue, + vtkm::Id startIndex, + vtkm::Id endIndex, + vtkm::cont::Token& token); VTKM_CONT static ReadPortalType CreateReadPortal( const std::vector& buffers, @@ -398,6 +396,57 @@ vtkm::cont::ArrayHandleStride make_ArrayHandleStride( } } // namespace vtkm::cont +namespace vtkm +{ +namespace cont +{ +namespace internal +{ + +template +VTKM_CONT inline void Storage::Fill( + const std::vector& buffers, + const T& fillValue, + vtkm::Id startIndex, + vtkm::Id endIndex, + vtkm::cont::Token& token) +{ + const StrideInfo& info = GetInfo(buffers); + vtkm::cont::ArrayHandleBasic basicArray = GetBasicArray(buffers); + if ((info.Stride == 1) && (info.Modulo == 0) && (info.Divisor <= 1)) + { + // Standard stride in array allows directly calling fill on the basic array. + basicArray.Fill(fillValue, startIndex + info.Offset, endIndex + info.Offset, token); + } + else + { + // The fill does not necessarily cover a contiguous region. We have to have a loop + // to set it. But we are not allowed to write device code here. Instead, create + // a stride array containing the fill value with a modulo of 1 so that this fill + // value repeates. Then feed this into a precompiled array copy that supports + // stride arrays. + const vtkm::Id numFill = endIndex - startIndex; + auto fillValueArray = vtkm::cont::make_ArrayHandle({ fillValue }); + vtkm::cont::ArrayHandleStride constantArray(fillValueArray, numFill, 1, 0, 1, 1); + vtkm::cont::ArrayHandleStride outputView(GetBasicArray(buffers), + numFill, + info.Stride, + info.ArrayIndex(startIndex), + info.Modulo, + info.Divisor); + // To prevent circular dependencies, this header file does not actually include + // UnknownArrayHandle.h. Thus, it is possible to get a compile error on the following + // line for using a declared but not defined `UnknownArrayHandle`. In the unlikely + // event this occurs, simply include `vtkm/cont/UnknownArrayHandle.h` somewhere up the + // include chain. + vtkm::cont::internal::ArrayCopyUnknown(constantArray, outputView); + } +} + +} +} +} // namespace vtkm::cont::internal + //============================================================================= // Specializations of serialization related classes /// @cond SERIALIZATION diff --git a/vtkm/cont/internal/ArrayCopyUnknown.h b/vtkm/cont/internal/ArrayCopyUnknown.h index c72af7cc0..38a57cf22 100644 --- a/vtkm/cont/internal/ArrayCopyUnknown.h +++ b/vtkm/cont/internal/ArrayCopyUnknown.h @@ -10,14 +10,18 @@ #ifndef vtk_m_cont_internal_ArrayCopyUnknown_h #define vtk_m_cont_internal_ArrayCopyUnknown_h -#include - #include namespace vtkm { namespace cont { + +// Rather than include UnknownArrayHandle.h, we just forward declare the class so +// we can declare our functions and prevent any circular header dependencies from +// core classes. +class UnknownArrayHandle; + namespace internal { diff --git a/vtkm/cont/testing/UnitTestArrayCopy.cxx b/vtkm/cont/testing/UnitTestArrayCopy.cxx index 2ed5e08e6..24c36ff48 100644 --- a/vtkm/cont/testing/UnitTestArrayCopy.cxx +++ b/vtkm/cont/testing/UnitTestArrayCopy.cxx @@ -292,6 +292,23 @@ void TryCopy() TestValues(input, output); } + { + std::cout << "constant -> extracted component" << std::endl; + using ComponentType = typename VTraits::BaseComponentType; + vtkm::cont::ArrayHandle output; + output.Allocate(ARRAY_SIZE); + ValueType invalue = TestValue(7, ValueType{}); + for (vtkm::IdComponent component = 0; component < VTraits::NUM_COMPONENTS; ++component) + { + vtkm::cont::ArrayHandleConstant input( + VTraits::GetComponent(invalue, component), ARRAY_SIZE); + auto extractedComponent = + vtkm::cont::ArrayExtractComponent(output, component, vtkm::CopyFlag::Off); + vtkm::cont::ArrayCopy(input, extractedComponent); + } + TestValues(vtkm::cont::make_ArrayHandleConstant(invalue, ARRAY_SIZE), output); + } + // Test the copy methods in UnknownArrayHandle. Although this would be appropriate in // UnitTestUnknownArrayHandle, it is easier to test copies here. { diff --git a/vtkm/cont/testing/UnitTestArrayExtractComponent.cxx b/vtkm/cont/testing/UnitTestArrayExtractComponent.cxx index 6e052276d..907a34140 100644 --- a/vtkm/cont/testing/UnitTestArrayExtractComponent.cxx +++ b/vtkm/cont/testing/UnitTestArrayExtractComponent.cxx @@ -155,6 +155,22 @@ void CheckOutputArray( GetVecFlatIndex(outValue, numComponents - componentId - 1))); } } + + // Check to make sure you can fill extracted components with a constant value. + for (vtkm::IdComponent componentId = 0; componentId < numComponents; ++componentId) + { + auto componentArray = + vtkm::cont::ArrayExtractComponent(outputArray, componentId, vtkm::CopyFlag::Off); + componentArray.Fill(TestValue(componentId, ComponentType{})); + } + for (vtkm::IdComponent componentId = 0; componentId < numComponents; ++componentId) + { + auto componentArray = + vtkm::cont::ArrayExtractComponent(outputArray, componentId, vtkm::CopyFlag::Off); + auto constantArray = vtkm::cont::make_ArrayHandleConstant( + TestValue(componentId, ComponentType{}), originalArray.GetNumberOfValues()); + VTKM_TEST_ASSERT(test_equal_ArrayHandles(componentArray, constantArray)); + } } void DoTest()