diff --git a/docs/changelog/output-vec-like-fix.md b/docs/changelog/output-vec-like-fix.md new file mode 100644 index 000000000..6b32ccd43 --- /dev/null +++ b/docs/changelog/output-vec-like-fix.md @@ -0,0 +1,36 @@ +# Support using arrays with dynamic Vec-likes as output arrays + +When you use an `ArrayHandle` as an output array in a worklet (for example, +as a `FieldOut`), the fetch operation does not read values from the array +during the `Load`. Instead, it just constructs a new object. This makes +sense as an output array is expected to have garbage in it anyway. + +This is a problem for some special arrays that contain `Vec`-like objects +that are sized dynamically. For example, if you use an +`ArrayHandleGroupVecVariable`, each entry is a dynamically sized `Vec`. The +array is referenced by creating a special version of `Vec` that holds a +reference to the array portal and an index. Components are retrieved and +set by accessing the memory in the array portal. This allows us to have a +dynamically sized `Vec` in the execution environment without having to +allocate within the worklet. + +The problem comes when we want to use one of these arrays with `Vec`-like +objects for an output. The typical fetch fails because you cannot construct +one of these `Vec`-like objects without an array portal to bind it to. In +these cases, we need the fetch to create the `Vec`-like object by reading +it from the array. Even though the data will be garbage, you get the +necessary buffer into the array (and nothing more). + +Previously, the problem was fixed by creating partial specializations of +the `Fetch` for these `ArrayHandle`s. This worked OK as long as you were +using the array directly. However, the approach failed if the `ArrayHandle` +was wrapped in another `ArrayHandle` (for example, if an `ArrayHandleView` +was applied to an `ArrayHandleGroupVecVariable`). + +To get around this problem and simplify things, the basic `Fetch` for +direct output arrays is changed to handle all cases where the values in the +`ArrayHandle` cannot be directly constructed. A compile-time check of the +array's value type is checked with `std::is_default_constructible`. If it +can be constructed, then the array is not accessed. If it cannot be +constructed, then it grabs a value out of the array. + diff --git a/vtkm/VecFromPortal.h b/vtkm/VecFromPortal.h index 0a0381ca5..9906c0e61 100644 --- a/vtkm/VecFromPortal.h +++ b/vtkm/VecFromPortal.h @@ -31,14 +31,6 @@ class VecFromPortal public: using ComponentType = typename std::remove_const::type; - VTKM_SUPPRESS_EXEC_WARNINGS - VTKM_EXEC_CONT - VecFromPortal() - : NumComponents(0) - , Offset(0) - { - } - VTKM_SUPPRESS_EXEC_WARNINGS VTKM_EXEC_CONT VecFromPortal(const PortalType& portal, vtkm::IdComponent numComponents = 0, vtkm::Id offset = 0) diff --git a/vtkm/cont/ArrayHandleGroupVecVariable.h b/vtkm/cont/ArrayHandleGroupVecVariable.h index ed0401855..1c28ab194 100644 --- a/vtkm/cont/ArrayHandleGroupVecVariable.h +++ b/vtkm/cont/ArrayHandleGroupVecVariable.h @@ -314,10 +314,6 @@ make_ArrayHandleGroupVecVariable(const ComponentsArrayHandleType& componentsArra } } // namespace vtkm::cont -//============================================================================= -// Specializations of worklet arguments using ArrayHandleGropuVecVariable -#include - //============================================================================= // Specializations of serialization related classes /// @cond SERIALIZATION diff --git a/vtkm/cont/ArrayHandleRecombineVec.h b/vtkm/cont/ArrayHandleRecombineVec.h index 59d7e5755..14e971ee8 100644 --- a/vtkm/cont/ArrayHandleRecombineVec.h +++ b/vtkm/cont/ArrayHandleRecombineVec.h @@ -78,7 +78,14 @@ public: VTKM_EXEC_CONT RecombineVec& operator=(const RecombineVec& src) { - this->DoCopy(src); + if ((&this->Portals[0] != &src.Portals[0]) || (this->Index != src.Index)) + { + this->DoCopy(src); + } + else + { + // Copying to myself. Do not need to do anything. + } return *this; } @@ -648,8 +655,4 @@ struct ArrayExtractComponentImpl } } // namespace vtkm::cont -//============================================================================= -// Specializations of worklet arguments using ArrayHandleGropuVecVariable -#include - #endif //vtk_m_cont_ArrayHandleRecombineVec_h diff --git a/vtkm/cont/testing/UnitTestArrayHandleRecombineVec.cxx b/vtkm/cont/testing/UnitTestArrayHandleRecombineVec.cxx index 7bedc750b..d488948c0 100644 --- a/vtkm/cont/testing/UnitTestArrayHandleRecombineVec.cxx +++ b/vtkm/cont/testing/UnitTestArrayHandleRecombineVec.cxx @@ -10,6 +10,7 @@ #include +#include #include #include @@ -80,8 +81,12 @@ struct TestRecombineVecAsOutput vtkm::cont::Invoker invoke; invoke(PassThrough{}, baseArray, recombinedArray); - VTKM_TEST_ASSERT(test_equal_ArrayHandles(baseArray, outputArray)); + + // Try outputing to a recombine vec inside of another fancy ArrayHandle. + auto reverseOutput = vtkm::cont::make_ArrayHandleReverse(recombinedArray); + invoke(PassThrough{}, baseArray, reverseOutput); + VTKM_TEST_ASSERT(test_equal_ArrayHandles(baseArray, reverseOutput)); } }; diff --git a/vtkm/exec/arg/CMakeLists.txt b/vtkm/exec/arg/CMakeLists.txt index 92e774ead..036cbd97a 100644 --- a/vtkm/exec/arg/CMakeLists.txt +++ b/vtkm/exec/arg/CMakeLists.txt @@ -19,8 +19,6 @@ set(headers FetchTagArrayDirectIn.h FetchTagArrayDirectInOut.h FetchTagArrayDirectOut.h - FetchTagArrayDirectOutArrayHandleGroupVecVariable.h - FetchTagArrayDirectOutArrayHandleRecombineVec.h FetchTagArrayNeighborhoodIn.h FetchTagArrayTopologyMapIn.h FetchTagExecObject.h diff --git a/vtkm/exec/arg/FetchTagArrayDirectOut.h b/vtkm/exec/arg/FetchTagArrayDirectOut.h index 1ca3d997e..6cfd02d4a 100644 --- a/vtkm/exec/arg/FetchTagArrayDirectOut.h +++ b/vtkm/exec/arg/FetchTagArrayDirectOut.h @@ -13,6 +13,8 @@ #include #include +#include + namespace vtkm { namespace exec @@ -35,14 +37,15 @@ struct Fetch { + using ValueType = typename ExecObjectType::ValueType; + VTKM_SUPPRESS_EXEC_WARNINGS template - VTKM_EXEC auto Load(const ThreadIndicesType&, const ExecObjectType&) const -> - typename ExecObjectType::ValueType + VTKM_EXEC ValueType Load(const ThreadIndicesType& indices, + const ExecObjectType& arrayPortal) const { - // Load is a no-op for this fetch. - using ValueType = typename ExecObjectType::ValueType; - return ValueType(); + return this->DoLoad( + indices, arrayPortal, typename std::is_default_constructible::type{}); } VTKM_SUPPRESS_EXEC_WARNINGS @@ -51,9 +54,32 @@ struct Fetch(value)); } + +private: + VTKM_SUPPRESS_EXEC_WARNINGS + template + VTKM_EXEC ValueType DoLoad(const ThreadIndicesType&, const ExecObjectType&, std::true_type) const + { + // Load is a no-op for this fetch. + return ValueType(); + } + + VTKM_SUPPRESS_EXEC_WARNINGS + template + VTKM_EXEC ValueType DoLoad(const ThreadIndicesType& indices, + const ExecObjectType& arrayPortal, + std::false_type) const + { + // Cannot create a ValueType object, so pull one out of the array portal. This may seem + // weird because an output array often has garbage in it. However, this case can happen + // with special arrays with Vec-like values that reference back to the array memory. + // For example, with ArrayHandleRecombineVec, the values are actual objects that point + // back to the array for on demand reading and writing. You need the buffer established + // by the array even if there is garbage in that array. + return arrayPortal.Get(indices.GetOutputIndex()); + } }; } } diff --git a/vtkm/exec/arg/FetchTagArrayDirectOutArrayHandleGroupVecVariable.h b/vtkm/exec/arg/FetchTagArrayDirectOutArrayHandleGroupVecVariable.h deleted file mode 100644 index 72e953672..000000000 --- a/vtkm/exec/arg/FetchTagArrayDirectOutArrayHandleGroupVecVariable.h +++ /dev/null @@ -1,69 +0,0 @@ -//============================================================================ -// Copyright (c) Kitware, Inc. -// All rights reserved. -// See LICENSE.txt for details. -// -// This software is distributed WITHOUT ANY WARRANTY; without even -// the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR -// PURPOSE. See the above copyright notice for more information. -//============================================================================ -#ifndef vtk_m_exec_arg_FetchTagArrayDirectOutArrayHandleGroupVecVariable_h -#define vtk_m_exec_arg_FetchTagArrayDirectOutArrayHandleGroupVecVariable_h - -#include - -// We need to override the fetch for output fields using -// ArrayPortalGroupVecVariable because this portal does not behave like most -// ArrayPortals. Usually you ignore the Load and implement the Store. But if -// you ignore the Load, the VecFromPortal gets no portal to set values into. -// Instead, you need to implement the Load to point to the array portal. You -// can also ignore the Store because the data is already set in the array at -// that point. - -// This file is included from ArrayHandleGroupVecVariable.h - -namespace vtkm -{ -namespace exec -{ -namespace arg -{ - -// We need to override the fetch for output fields using -// ArrayPortalGroupVecVariable because this portal does not behave like most -// ArrayPortals. Usually you ignore the Load and implement the Store. But if -// you ignore the Load, the VecFromPortal gets no portal to set values into. -// Instead, you need to implement the Load to point to the array portal. You -// can also ignore the Store because the data is already set in the array at -// that point. -template -struct Fetch> -{ - using ExecObjectType = - vtkm::internal::ArrayPortalGroupVecVariable; - using ValueType = typename ExecObjectType::ValueType; - - VTKM_SUPPRESS_EXEC_WARNINGS - template - VTKM_EXEC ValueType Load(const ThreadIndicesType& indices, - const ExecObjectType& arrayPortal) const - { - return arrayPortal.Get(indices.GetOutputIndex()); - } - - VTKM_SUPPRESS_EXEC_WARNINGS - template - VTKM_EXEC void Store(const ThreadIndicesType&, const ExecObjectType&, const ValueType&) const - { - // We can actually ignore this because the VecFromPortal will already have - // set new values in the array. - } -}; - -} -} -} // namespace vtkm::exec::arg - -#endif //vtk_m_exec_arg_FetchTagArrayDirectOutArrayHandleGroupVecVariable_h diff --git a/vtkm/exec/arg/FetchTagArrayDirectOutArrayHandleRecombineVec.h b/vtkm/exec/arg/FetchTagArrayDirectOutArrayHandleRecombineVec.h deleted file mode 100644 index 1ad42b8e0..000000000 --- a/vtkm/exec/arg/FetchTagArrayDirectOutArrayHandleRecombineVec.h +++ /dev/null @@ -1,46 +0,0 @@ -//============================================================================ -// Copyright (c) Kitware, Inc. -// All rights reserved. -// See LICENSE.txt for details. -// -// This software is distributed WITHOUT ANY WARRANTY; without even -// the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR -// PURPOSE. See the above copyright notice for more information. -//============================================================================ -#ifndef vtk_m_exec_arg_FetchTagArrayDirectOutArrayHandleRecombineVec_h -#define vtk_m_exec_arg_FetchTagArrayDirectOutArrayHandleRecombineVec_h - -#include -#include - -// The `Fetch` for direct array out breaks for `ArrayHandleRecombineVec` because the `Load` -// method attempts to create a `vtkm::internal::RecombineVec` with a default constructor, -// which does not exist. Instead, have the direct out `Fetch` behave like the direct in/out -// `Fetch`, which loads the initial value from the array. The actual load will not load the -// data but rather set up the portals in the returned object, which is necessary for the -// later `Store` to work anyway. - -// This file is included from ArrayHandleRecombineVec.h - -namespace vtkm -{ -namespace exec -{ -namespace arg -{ - -template -struct Fetch> - : Fetch> -{ -}; - -} -} -} // namespace vtkm::exec::arg - -#endif //vtk_m_exec_arg_FetchTagArrayDirectOutArrayHandleRecombineVec_h