From 58fc4d8267168d3d77a5e6810e00a84014191254 Mon Sep 17 00:00:00 2001 From: Kenneth Moreland Date: Thu, 23 Mar 2023 07:07:44 -0600 Subject: [PATCH 01/22] Fix potential deadlock in distributed contour tree The `HierarchicalAugmenter` used in the distributed contour tree filter attempts to save memory by releasing buffers used to send and receive data after the DIY enqueues and dequeues are posted. This works as long as the DIY serialization process copies the data in the arrays. However, changes are coming where the data is sent directly from the buffers. These changes cause a deadlock because the enqueue places a read lock until the data is sent while the release tried to get a write lock. The solution is simply to "forget" the array rather than explicitly delete it. In this case, the array will automatically be deleted once everyone is done with it. --- .../HierarchicalAugmenter.h | 8 +++++-- .../HierarchicalAugmenterFunctor.h | 5 +++- .../HierarchicalAugmenterInOutData.h | 23 +------------------ 3 files changed, 11 insertions(+), 25 deletions(-) diff --git a/vtkm/filter/scalar_topology/worklet/contourtree_distributed/HierarchicalAugmenter.h b/vtkm/filter/scalar_topology/worklet/contourtree_distributed/HierarchicalAugmenter.h index fa020eb89..a9ad55f7f 100644 --- a/vtkm/filter/scalar_topology/worklet/contourtree_distributed/HierarchicalAugmenter.h +++ b/vtkm/filter/scalar_topology/worklet/contourtree_distributed/HierarchicalAugmenter.h @@ -467,8 +467,12 @@ void HierarchicalAugmenter::RetrieveInAttachmentPoints() template void HierarchicalAugmenter::ReleaseSwapArrays() { // ReleaseSwapArrays() - this->OutData.ReleaseResources(); - this->InData.ReleaseResources(); + // Rather than explicitly delete the arrays, we are going to "forget" them and + // just release our reference count on them. If no one else is using them, the + // memory will actually be deleted. But if an array is being used, it will + // continue to be managed until it is not. + this->OutData = decltype(this->OutData){}; + this->InData = decltype(this->InData){}; } // ReleaseSwapArrays() diff --git a/vtkm/filter/scalar_topology/worklet/contourtree_distributed/HierarchicalAugmenterFunctor.h b/vtkm/filter/scalar_topology/worklet/contourtree_distributed/HierarchicalAugmenterFunctor.h index c200a0fec..bd2dcdcba 100644 --- a/vtkm/filter/scalar_topology/worklet/contourtree_distributed/HierarchicalAugmenterFunctor.h +++ b/vtkm/filter/scalar_topology/worklet/contourtree_distributed/HierarchicalAugmenterFunctor.h @@ -127,7 +127,10 @@ public: blockData->HierarchicalAugmenter.PrepareOutAttachmentPoints(round); // TODO/FIXME: Correct function? Correct round? rp.enqueue(target, blockData->HierarchicalAugmenter.OutData); - // TODO/FIXME: Is it save to already release HierarchicalAugmenter.OutData (and InData) here. Don't we free the arrays before the other block had the chance to complete its rp.dequeue? + // Note: HierarchicalAugmenter.ReleaseSwapArrays() does not necessarily delete the + // arrays. Rather, it releases the reference to them. If, for example, the data + // for HierarchicalAugmenter.OutData is still in flight, the data will continue to + // exist until it is sent. blockData->HierarchicalAugmenter.ReleaseSwapArrays(); } } diff --git a/vtkm/filter/scalar_topology/worklet/contourtree_distributed/hierarchical_augmenter/HierarchicalAugmenterInOutData.h b/vtkm/filter/scalar_topology/worklet/contourtree_distributed/hierarchical_augmenter/HierarchicalAugmenterInOutData.h index 7a980f692..a6b3b9959 100644 --- a/vtkm/filter/scalar_topology/worklet/contourtree_distributed/hierarchical_augmenter/HierarchicalAugmenterInOutData.h +++ b/vtkm/filter/scalar_topology/worklet/contourtree_distributed/hierarchical_augmenter/HierarchicalAugmenterInOutData.h @@ -101,34 +101,13 @@ public: } /// Destructor - ~HierarchicalAugmenterInOutData(); - - /// Clear all arrays - void ReleaseResources(); + ~HierarchicalAugmenterInOutData() = default; /// Print contents fo this objects std::string DebugPrint(std::string message, const char* fileName, long lineNum); }; // class HierarchicalAugmenterInOutData -template -HierarchicalAugmenterInOutData::~HierarchicalAugmenterInOutData() -{ - this->ReleaseResources(); -} - -// routine to release memory used for out arrays -template -void HierarchicalAugmenterInOutData::ReleaseResources() -{ // ReleaseResources() - this->GlobalRegularIds.ReleaseResources(); - this->DataValues.ReleaseResources(); - this->SupernodeIds.ReleaseResources(); - this->Superparents.ReleaseResources(); - this->SuperparentRounds.ReleaseResources(); - this->WhichRounds.ReleaseResources(); -} // ReleaseResources() - template std::string HierarchicalAugmenterInOutData::DebugPrint(std::string message, const char* fileName, From 1538fc02fbc5b9cbe41bc29bcf3b151a3a3dfb2b Mon Sep 17 00:00:00 2001 From: Louis Gombert Date: Fri, 28 Apr 2023 16:32:11 +0200 Subject: [PATCH 02/22] Implement Flying Edges for all structured CellSets In addition to using uniform coordinates, the ContourFlyingEdges filter can now process any type of coordinate system, making the filter use Flying Edges in more cases --- docs/changelog/flying-edges-structured.md | 5 ++ .../clean_grid/testing/UnitTestCleanGrid.cxx | 10 +-- vtkm/filter/contour/Contour.cxx | 7 +- vtkm/filter/contour/ContourFlyingEdges.cxx | 22 +---- .../contour/testing/UnitTestContourFilter.cxx | 68 ++++++++++++--- .../contour/worklet/ContourFlyingEdges.h | 6 +- .../contour/worklet/contour/FlyingEdges.h | 17 ++-- .../worklet/contour/FlyingEdgesPass4.h | 36 +++----- .../worklet/contour/FlyingEdgesPass4X.h | 80 +++++++++++------- .../contour/FlyingEdgesPass4XWithNormals.h | 82 ++++++++++++------- .../worklet/contour/FlyingEdgesPass4Y.h | 43 +++++----- 11 files changed, 212 insertions(+), 164 deletions(-) create mode 100644 docs/changelog/flying-edges-structured.md diff --git a/docs/changelog/flying-edges-structured.md b/docs/changelog/flying-edges-structured.md new file mode 100644 index 000000000..a961afb70 --- /dev/null +++ b/docs/changelog/flying-edges-structured.md @@ -0,0 +1,5 @@ +# Implement Flying Edges for structured cellsets with rectilinear and curvilinear coordinates + +When Flying Edges was introduced to compute contours of a 3D structured cellset, it could only process uniform coordinates. This limitation is now lifted : an alternative interpolation function can be used in the fourth pass of the algorithm in order to support rectilinear and curvilinear coordinate systems. + +Accordingly, the `Contour` filter now calls `ContourFlyingEdges` instead of `ContourMarchingCells` for these newly supported cases. diff --git a/vtkm/filter/clean_grid/testing/UnitTestCleanGrid.cxx b/vtkm/filter/clean_grid/testing/UnitTestCleanGrid.cxx index 740e6d9dc..c5eb5cc3f 100644 --- a/vtkm/filter/clean_grid/testing/UnitTestCleanGrid.cxx +++ b/vtkm/filter/clean_grid/testing/UnitTestCleanGrid.cxx @@ -13,7 +13,7 @@ #include #include #include -#include +#include namespace { @@ -72,13 +72,7 @@ void TestPointMerging() vtkm::cont::testing::MakeTestDataSet makeDataSet; vtkm::cont::DataSet baseData = makeDataSet.Make3DUniformDataSet3(vtkm::Id3(4, 4, 4)); - //Convert the baseData implicit points to explicit points, since the contour - //filter for uniform data always does point merging - vtkm::cont::ArrayHandle newcoords; - vtkm::cont::ArrayCopy(baseData.GetCoordinateSystem().GetData(), newcoords); - baseData.AddPointField(baseData.GetCoordinateSystemName(), newcoords); - - vtkm::filter::contour::Contour marchingCubes; + vtkm::filter::contour::ContourMarchingCells marchingCubes; marchingCubes.SetIsoValue(0.05); marchingCubes.SetMergeDuplicatePoints(false); marchingCubes.SetActiveField("pointvar"); diff --git a/vtkm/filter/contour/Contour.cxx b/vtkm/filter/contour/Contour.cxx index ebbd896c0..466a3657d 100644 --- a/vtkm/filter/contour/Contour.cxx +++ b/vtkm/filter/contour/Contour.cxx @@ -34,11 +34,8 @@ vtkm::cont::DataSet Contour::DoExecute(const vtkm::cont::DataSet& inDataSet) auto inCoords = inDataSet.GetCoordinateSystem(this->GetActiveCoordinateSystemIndex()).GetData(); std::unique_ptr implementation; - // For now, Flying Edges is only used for 3D Structured CellSets, - // using Uniform coordinates. - if (inCellSet.template IsType>() && - inCoords.template IsType< - vtkm::cont::ArrayHandle>()) + // Flying Edges is only used for 3D Structured CellSets + if (inCellSet.template IsType>()) { VTKM_LOG_S(vtkm::cont::LogLevel::Info, "Using flying edges"); implementation.reset(new vtkm::filter::contour::ContourFlyingEdges); diff --git a/vtkm/filter/contour/ContourFlyingEdges.cxx b/vtkm/filter/contour/ContourFlyingEdges.cxx index f457a0be8..457565e55 100644 --- a/vtkm/filter/contour/ContourFlyingEdges.cxx +++ b/vtkm/filter/contour/ContourFlyingEdges.cxx @@ -44,13 +44,10 @@ vtkm::cont::DataSet ContourFlyingEdges::DoExecute(const vtkm::cont::DataSet& inD const vtkm::cont::CoordinateSystem& inCoords = inDataSet.GetCoordinateSystem(this->GetActiveCoordinateSystemIndex()); - if (!inCellSet.template IsType>() || - !inCoords.GetData() - .template IsType< - vtkm::cont::ArrayHandle>()) + if (!inCellSet.template IsType>()) { throw vtkm::cont::ErrorFilterExecution("This filter is only available for 3-Dimensional " - "Structured Cell Sets using uniform point coordinates."); + "Structured Cell Sets"); } // Get the CellSet's known dynamic type @@ -74,22 +71,11 @@ vtkm::cont::DataSet ContourFlyingEdges::DoExecute(const vtkm::cont::DataSet& inD if (this->GenerateNormals && !this->GetComputeFastNormals()) { - outputCells = worklet.Run( - ivalues, - inputCells, - inCoords.GetData().AsArrayHandle(), - concrete, - vertices, - normals); + outputCells = worklet.Run(ivalues, inputCells, inCoords, concrete, vertices, normals); } else { - outputCells = worklet.Run( - ivalues, - inputCells, - inCoords.GetData().AsArrayHandle(), - concrete, - vertices); + outputCells = worklet.Run(ivalues, inputCells, inCoords, concrete, vertices); } }; diff --git a/vtkm/filter/contour/testing/UnitTestContourFilter.cxx b/vtkm/filter/contour/testing/UnitTestContourFilter.cxx index d1e159ce5..86aaa3aeb 100644 --- a/vtkm/filter/contour/testing/UnitTestContourFilter.cxx +++ b/vtkm/filter/contour/testing/UnitTestContourFilter.cxx @@ -176,24 +176,12 @@ public: void TestUnsupportedFlyingEdges() const { vtkm::cont::testing::MakeTestDataSet maker; - - vtkm::cont::DataSet rectilinearDataset = maker.Make3DRectilinearDataSet0(); vtkm::cont::DataSet explicitDataSet = maker.Make3DExplicitDataSet0(); vtkm::filter::contour::ContourFlyingEdges filter; filter.SetIsoValue(2.0); filter.SetActiveField("pointvar"); - try - { - filter.Execute(rectilinearDataset); - VTKM_TEST_FAIL("Flying Edges filter should not run on datasets with rectilinear coordinates"); - } - catch (vtkm::cont::ErrorFilterExecution&) - { - std::cout << "Execution successfully aborted" << std::endl; - } - try { filter.Execute(explicitDataSet); @@ -205,6 +193,58 @@ public: } } + template + void TestNonUniformStructured() const + { + auto pathname = + vtkm::cont::testing::Testing::DataPath("rectilinear/simple_rectilinear1_ascii.vtk"); + vtkm::io::VTKDataSetReader reader(pathname); + vtkm::cont::DataSet rectilinearDataset = reader.ReadDataSet(); + + // Single-cell contour + ContourFilterType filter; + filter.SetActiveField("var"); + filter.SetIsoValue(2.0); + vtkm::cont::DataSet outputSingleCell = filter.Execute(rectilinearDataset); + auto coordinates = outputSingleCell.GetCoordinateSystem() + .GetData() + .AsArrayHandle>(); + + VTKM_TEST_ASSERT(outputSingleCell.GetNumberOfPoints() == 3, + "Wrong number of points in rectilinear contour"); + VTKM_TEST_ASSERT(outputSingleCell.GetNumberOfCells() == 1, + "Wrong number of cells in rectilinear contour"); + VTKM_TEST_ASSERT(outputSingleCell.GetCellSet().GetCellShape(0) == vtkm::CELL_SHAPE_TRIANGLE, + "Wrong contour cell shape"); + + auto expectedCoordinates = + vtkm::cont::make_ArrayHandle({ vtkm::Vec3f{ 10.0f, -10.0f, 9.66341f }, + vtkm::Vec3f{ 9.30578f, -10.0f, 10.0f }, + vtkm::Vec3f{ 10.0f, -9.78842f, 10.0f } }); + VTKM_TEST_ASSERT(test_equal_ArrayHandles(coordinates, expectedCoordinates), + "Wrong contour coordinates"); + + // Generating normals triggers a different worklet for Flying Edges pass 4, + // But it should not change anything on the contour itself. + filter.SetGenerateNormals(true); + vtkm::cont::DataSet outputNormals = filter.Execute(rectilinearDataset); + coordinates = outputSingleCell.GetCoordinateSystem() + .GetData() + .AsArrayHandle>(); + VTKM_TEST_ASSERT(test_equal_ArrayHandles(coordinates, expectedCoordinates), + "Wrong contour coordinates"); + + // Full contour + filter.SetIsoValue(3.0); + filter.SetGenerateNormals(false); + vtkm::cont::DataSet output = filter.Execute(rectilinearDataset); + + VTKM_TEST_ASSERT(output.GetNumberOfPoints() == 93, + "Wrong number of points in rectilinear contour"); + VTKM_TEST_ASSERT(output.GetNumberOfCells() == 144, + "Wrong number of cells in rectilinear contour"); + } + void operator()() const { this->TestContourUniformGrid(72); @@ -220,6 +260,10 @@ public: this->TestContourWedges(); this->TestContourWedges(); + this->TestNonUniformStructured(); + this->TestNonUniformStructured(); + this->TestNonUniformStructured(); + this->TestUnsupportedFlyingEdges(); } diff --git a/vtkm/filter/contour/worklet/ContourFlyingEdges.h b/vtkm/filter/contour/worklet/ContourFlyingEdges.h index 85dabcf78..1a77c014a 100644 --- a/vtkm/filter/contour/worklet/ContourFlyingEdges.h +++ b/vtkm/filter/contour/worklet/ContourFlyingEdges.h @@ -67,13 +67,14 @@ public: // Filter called without normals generation template vtkm::cont::CellSetSingleType<> Run( const std::vector& isovalues, const vtkm::cont::CellSetStructured<3>& cells, - const vtkm::cont::ArrayHandleUniformPointCoordinates& coordinateSystem, + const CoordsType& coordinateSystem, const vtkm::cont::ArrayHandle& input, vtkm::cont::ArrayHandle, StorageTagVertices>& vertices) { @@ -87,6 +88,7 @@ public: // Filter called with normals generation template Run( const std::vector& isovalues, const vtkm::cont::CellSetStructured<3>& cells, - const vtkm::cont::ArrayHandleUniformPointCoordinates& coordinateSystem, + const CoordsType& coordinateSystem, const vtkm::cont::ArrayHandle& input, vtkm::cont::ArrayHandle, StorageTagVertices>& vertices, vtkm::cont::ArrayHandle, StorageTagNormals>& normals) diff --git a/vtkm/filter/contour/worklet/contour/FlyingEdges.h b/vtkm/filter/contour/worklet/contour/FlyingEdges.h index b9f605124..e5bd379c8 100644 --- a/vtkm/filter/contour/worklet/contour/FlyingEdges.h +++ b/vtkm/filter/contour/worklet/contour/FlyingEdges.h @@ -41,6 +41,7 @@ vtkm::Id extend_by(vtkm::cont::ArrayHandle& handle, vtkm::Id size) //---------------------------------------------------------------------------- template vtkm::cont::CellSetSingleType<> execute( const vtkm::cont::CellSetStructured<3>& cells, - const vtkm::cont::ArrayHandleUniformPointCoordinates& coordinateSystem, + const CoordsType coordinateSystem, const std::vector& isovalues, const vtkm::cont::ArrayHandle& inputField, vtkm::cont::ArrayHandle, StorageTagVertices>& points, @@ -56,18 +57,10 @@ vtkm::cont::CellSetSingleType<> execute( vtkm::worklet::contour::CommonState& sharedState) { vtkm::cont::Invoker invoke; - - vtkm::Vec3f origin, spacing; - { //extract out the origin and spacing as these are needed for Pass4 to properly - //interpolate the new points - auto portal = coordinateSystem.ReadPortal(); - origin = portal.GetOrigin(); - spacing = portal.GetSpacing(); - } auto pdims = cells.GetPointDimensions(); vtkm::cont::ArrayHandle edgeCases; - edgeCases.Allocate(coordinateSystem.GetNumberOfValues()); + edgeCases.Allocate(coordinateSystem.GetData().GetNumberOfValues()); vtkm::cont::CellSetStructured<2> metaDataMesh2D; vtkm::cont::ArrayHandle metaDataLinearSums; //per point of metaDataMesh @@ -158,8 +151,7 @@ vtkm::cont::CellSetSingleType<> execute( { VTKM_LOG_SCOPE(vtkm::cont::LogLevel::Perf, "FlyingEdges Pass4"); - launchComputePass4 pass4( - pdims, origin, spacing, multiContourCellOffset, multiContourPointOffset); + auto pass4 = launchComputePass4(pdims, multiContourCellOffset, multiContourPointOffset); detail::extend_by(points, newPointSize); if (sharedState.GenerateNormals) @@ -171,6 +163,7 @@ vtkm::cont::CellSetSingleType<> execute( pass4, newPointSize, isoval, + coordinateSystem, inputField, edgeCases, metaDataMesh2D, diff --git a/vtkm/filter/contour/worklet/contour/FlyingEdgesPass4.h b/vtkm/filter/contour/worklet/contour/FlyingEdgesPass4.h index 9905e7bdb..fe66dac49 100644 --- a/vtkm/filter/contour/worklet/contour/FlyingEdgesPass4.h +++ b/vtkm/filter/contour/worklet/contour/FlyingEdgesPass4.h @@ -28,20 +28,14 @@ namespace flying_edges struct launchComputePass4 { vtkm::Id3 PointDims; - vtkm::Vec3f Origin; - vtkm::Vec3f Spacing; vtkm::Id CellWriteOffset; vtkm::Id PointWriteOffset; launchComputePass4(const vtkm::Id3& pdims, - const vtkm::Vec3f& origin, - const vtkm::Vec3f& spacing, vtkm::Id multiContourCellOffset, vtkm::Id multiContourPointOffset) : PointDims(pdims) - , Origin(origin) - , Spacing(spacing) , CellWriteOffset(multiContourCellOffset) , PointWriteOffset(multiContourPointOffset) { @@ -49,6 +43,7 @@ struct launchComputePass4 template & inputField, vtkm::cont::ArrayHandle edgeCases, vtkm::cont::CellSetStructured<2>& metaDataMesh2D, @@ -71,12 +67,8 @@ struct launchComputePass4 vtkm::cont::Invoker invoke(device); if (sharedState.GenerateNormals) { - ComputePass4XWithNormals worklet4(isoval, - this->PointDims, - this->Origin, - this->Spacing, - this->CellWriteOffset, - this->PointWriteOffset); + ComputePass4XWithNormals worklet4( + isoval, this->PointDims, this->CellWriteOffset, this->PointWriteOffset); invoke(worklet4, metaDataMesh2D, metaDataSums, @@ -84,6 +76,7 @@ struct launchComputePass4 metaDataMax, metaDataNumTris, edgeCases, + coordinateSystem, inputField, triangle_topology, sharedState.InterpolationEdgeIds, @@ -94,12 +87,8 @@ struct launchComputePass4 } else { - ComputePass4X worklet4(isoval, - this->PointDims, - this->Origin, - this->Spacing, - this->CellWriteOffset, - this->PointWriteOffset); + ComputePass4X worklet4( + isoval, this->PointDims, this->CellWriteOffset, this->PointWriteOffset); invoke(worklet4, metaDataMesh2D, metaDataSums, @@ -107,6 +96,7 @@ struct launchComputePass4 metaDataMax, metaDataNumTris, edgeCases, + coordinateSystem, inputField, triangle_topology, sharedState.InterpolationEdgeIds, @@ -120,6 +110,7 @@ struct launchComputePass4 template & inputField, vtkm::cont::ArrayHandle edgeCases, vtkm::cont::CellSetStructured<2>& metaDataMesh2D, @@ -157,11 +149,8 @@ struct launchComputePass4 sharedState.CellIdMap); //This needs to be done on array handle view ( start = this->PointWriteOffset, len = newPointSize) - ComputePass5Y worklet5(this->PointDims, - this->Origin, - this->Spacing, - this->PointWriteOffset, - sharedState.GenerateNormals); + ComputePass5Y worklet5(this->PointDims, this->PointWriteOffset, sharedState.GenerateNormals); + invoke(worklet5, vtkm::cont::make_ArrayHandleView( sharedState.InterpolationEdgeIds, this->PointWriteOffset, newPointSize), @@ -169,6 +158,7 @@ struct launchComputePass4 sharedState.InterpolationWeights, this->PointWriteOffset, newPointSize), vtkm::cont::make_ArrayHandleView(points, this->PointWriteOffset, newPointSize), inputField, + coordinateSystem, normals); return true; diff --git a/vtkm/filter/contour/worklet/contour/FlyingEdgesPass4X.h b/vtkm/filter/contour/worklet/contour/FlyingEdgesPass4X.h index 99cf6550a..f6cddc9b8 100644 --- a/vtkm/filter/contour/worklet/contour/FlyingEdgesPass4X.h +++ b/vtkm/filter/contour/worklet/contour/FlyingEdgesPass4X.h @@ -32,9 +32,6 @@ struct ComputePass4X : public vtkm::worklet::WorkletVisitCellsWithPoints { vtkm::Id3 PointDims; - vtkm::Vec3f Origin; - vtkm::Vec3f Spacing; - T IsoValue; vtkm::Id CellWriteOffset; @@ -43,13 +40,9 @@ struct ComputePass4X : public vtkm::worklet::WorkletVisitCellsWithPoints ComputePass4X() {} ComputePass4X(T value, const vtkm::Id3& pdims, - const vtkm::Vec3f& origin, - const vtkm::Vec3f& spacing, vtkm::Id multiContourCellOffset, vtkm::Id multiContourPointOffset) : PointDims(pdims) - , Origin(origin) - , Spacing(spacing) , IsoValue(value) , CellWriteOffset(multiContourCellOffset) , PointWriteOffset(multiContourPointOffset) @@ -62,6 +55,7 @@ struct ComputePass4X : public vtkm::worklet::WorkletVisitCellsWithPoints FieldInPoint axis_maxs, WholeArrayIn cell_tri_count, WholeArrayIn edgeData, + WholeArrayIn coords, WholeArrayIn data, WholeArrayOut connectivity, WholeArrayOut edgeIds, @@ -69,13 +63,14 @@ struct ComputePass4X : public vtkm::worklet::WorkletVisitCellsWithPoints WholeArrayOut inputCellIds, WholeArrayOut points); using ExecutionSignature = - void(ThreadIndices, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, WorkIndex); + void(ThreadIndices, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _13, WorkIndex); template VTKM_EXEC inline void Generate(const vtkm::Vec& boundaryStatus, const vtkm::Id3& ijk, const WholeDataField& field, const WholeIEdgeField& interpolatedEdgeIds, const WholeWeightField& weights, + const WholeCoordsField coords, const WholePointField& points, const vtkm::Id4& startPos, const vtkm::Id3& incs, @@ -188,7 +187,7 @@ struct ComputePass4X : public vtkm::worklet::WorkletVisitCellsWithPoints interpolatedEdgeIds.Set(writeIndex, pos); weights.Set(writeIndex, static_cast(t)); - auto coord = this->InterpolateCoordinate(t, ijk, ijk + vtkm::Id3{ 1, 0, 0 }); + auto coord = this->InterpolateCoordinate(coords, t, ijk, ijk + vtkm::Id3{ 1, 0, 0 }); points.Set(writeIndex, coord); } if (edgeUses[4]) @@ -201,7 +200,7 @@ struct ComputePass4X : public vtkm::worklet::WorkletVisitCellsWithPoints interpolatedEdgeIds.Set(writeIndex, pos); weights.Set(writeIndex, static_cast(t)); - auto coord = this->InterpolateCoordinate(t, ijk, ijk + vtkm::Id3{ 0, 1, 0 }); + auto coord = this->InterpolateCoordinate(coords, t, ijk, ijk + vtkm::Id3{ 0, 1, 0 }); points.Set(writeIndex, coord); } if (edgeUses[8]) @@ -214,7 +213,7 @@ struct ComputePass4X : public vtkm::worklet::WorkletVisitCellsWithPoints interpolatedEdgeIds.Set(writeIndex, pos); weights.Set(writeIndex, static_cast(t)); - auto coord = this->InterpolateCoordinate(t, ijk, ijk + vtkm::Id3{ 0, 0, 1 }); + auto coord = this->InterpolateCoordinate(coords, t, ijk, ijk + vtkm::Id3{ 0, 0, 1 }); points.Set(writeIndex, coord); } } @@ -231,30 +230,30 @@ struct ComputePass4X : public vtkm::worklet::WorkletVisitCellsWithPoints const bool onZ = boundaryStatus[AxisToSum::zindex] & FlyingEdges3D::MaxBoundary; if (onX) //+x boundary { - this->InterpolateEdge(ijk, pos[0], incs, 5, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, points); - this->InterpolateEdge(ijk, pos[0], incs, 9, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, points); + this->InterpolateEdge(ijk, pos[0], incs, 5, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, coords, points); + this->InterpolateEdge(ijk, pos[0], incs, 9, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, coords, points); if (onY) //+x +y { - this->InterpolateEdge(ijk, pos[0], incs, 11, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, points); + this->InterpolateEdge(ijk, pos[0], incs, 11, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, coords, points); } if (onZ) //+x +z { - this->InterpolateEdge(ijk, pos[0], incs, 7, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, points); + this->InterpolateEdge(ijk, pos[0], incs, 7, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, coords, points); } } if (onY) //+y boundary { - this->InterpolateEdge(ijk, pos[0], incs, 1, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, points); - this->InterpolateEdge(ijk, pos[0], incs, 10, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, points); + this->InterpolateEdge(ijk, pos[0], incs, 1, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, coords, points); + this->InterpolateEdge(ijk, pos[0], incs, 10, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, coords, points); if (onZ) //+y +z boundary { - this->InterpolateEdge(ijk, pos[0], incs, 3, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, points); + this->InterpolateEdge(ijk, pos[0], incs, 3, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, coords, points); } } if (onZ) //+z boundary { - this->InterpolateEdge(ijk, pos[0], incs, 2, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, points); - this->InterpolateEdge(ijk, pos[0], incs, 6, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, points); + this->InterpolateEdge(ijk, pos[0], incs, 2, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, coords, points); + this->InterpolateEdge(ijk, pos[0], incs, 6, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, coords, points); } // clang-format on } @@ -264,6 +263,7 @@ struct ComputePass4X : public vtkm::worklet::WorkletVisitCellsWithPoints template VTKM_EXEC inline void InterpolateEdge(const vtkm::Id3& ijk, vtkm::Id currentIdx, @@ -274,6 +274,7 @@ struct ComputePass4X : public vtkm::worklet::WorkletVisitCellsWithPoints const WholeField& field, const WholeIEdgeField& interpolatedEdgeIds, const WholeWeightField& weights, + const WholeCoordsField& coords, const WholePointField& points) const { using AxisToSum = SumXAxis; @@ -300,30 +301,49 @@ struct ComputePass4X : public vtkm::worklet::WorkletVisitCellsWithPoints T t = static_cast((this->IsoValue - s0) / (s1 - s0)); weights.Set(writeIndex, static_cast(t)); - auto coord = this->InterpolateCoordinate(t, ijk + offsets1, ijk + offsets2); + auto coord = this->InterpolateCoordinate(coords, t, ijk + offsets1, ijk + offsets2); points.Set(writeIndex, coord); } + // Fast interpolation method for uniform coordinates //---------------------------------------------------------------------------- - inline VTKM_EXEC vtkm::Vec3f InterpolateCoordinate(T t, - const vtkm::Id3& ijk0, - const vtkm::Id3& ijk1) const + inline VTKM_EXEC vtkm::Vec3f InterpolateCoordinate( + vtkm::internal::ArrayPortalUniformPointCoordinates coords, + T t, + const vtkm::Id3& ijk0, + const vtkm::Id3& ijk1) const { return vtkm::Vec3f( - this->Origin[0] + - this->Spacing[0] * + coords.GetOrigin()[0] + + coords.GetSpacing()[0] * (static_cast(ijk0[0]) + static_cast(t) * static_cast(ijk1[0] - ijk0[0])), - this->Origin[1] + - this->Spacing[1] * + coords.GetOrigin()[1] + + coords.GetSpacing()[1] * (static_cast(ijk0[1]) + static_cast(t) * static_cast(ijk1[1] - ijk0[1])), - this->Origin[2] + - this->Spacing[2] * + coords.GetOrigin()[2] + + coords.GetSpacing()[2] * (static_cast(ijk0[2]) + static_cast(t) * static_cast(ijk1[2] - ijk0[2]))); } + + // Interpolation for explicit coordinates + //---------------------------------------------------------------------------- + template + inline VTKM_EXEC vtkm::Vec3f InterpolateCoordinate(CoordsPortal coords, + T t, + const vtkm::Id3& ijk0, + const vtkm::Id3& ijk1) const + { + return (1.0f - static_cast(t)) * + coords.Get(ijk0[0] + this->PointDims[0] * ijk0[1] + + this->PointDims[0] * this->PointDims[1] * ijk0[2]) + + static_cast(t) * + coords.Get(ijk1[0] + this->PointDims[0] * ijk1[1] + + this->PointDims[0] * this->PointDims[1] * ijk1[2]); + } }; } } diff --git a/vtkm/filter/contour/worklet/contour/FlyingEdgesPass4XWithNormals.h b/vtkm/filter/contour/worklet/contour/FlyingEdgesPass4XWithNormals.h index aea23acc7..678052299 100644 --- a/vtkm/filter/contour/worklet/contour/FlyingEdgesPass4XWithNormals.h +++ b/vtkm/filter/contour/worklet/contour/FlyingEdgesPass4XWithNormals.h @@ -31,9 +31,6 @@ struct ComputePass4XWithNormals : public vtkm::worklet::WorkletVisitCellsWithPoi { vtkm::Id3 PointDims; - vtkm::Vec3f Origin; - vtkm::Vec3f Spacing; - T IsoValue; vtkm::Id CellWriteOffset; @@ -42,13 +39,9 @@ struct ComputePass4XWithNormals : public vtkm::worklet::WorkletVisitCellsWithPoi ComputePass4XWithNormals() {} ComputePass4XWithNormals(T value, const vtkm::Id3& pdims, - const vtkm::Vec3f& origin, - const vtkm::Vec3f& spacing, vtkm::Id multiContourCellOffset, vtkm::Id multiContourPointOffset) : PointDims(pdims) - , Origin(origin) - , Spacing(spacing) , IsoValue(value) , CellWriteOffset(multiContourCellOffset) , PointWriteOffset(multiContourPointOffset) @@ -61,6 +54,7 @@ struct ComputePass4XWithNormals : public vtkm::worklet::WorkletVisitCellsWithPoi FieldInPoint axis_maxs, WholeArrayIn cell_tri_count, WholeArrayIn edgeData, + WholeArrayIn coords, WholeArrayIn data, WholeArrayOut connectivity, WholeArrayOut edgeIds, @@ -69,13 +63,14 @@ struct ComputePass4XWithNormals : public vtkm::worklet::WorkletVisitCellsWithPoi WholeArrayOut points, WholeArrayOut normals); using ExecutionSignature = - void(ThreadIndices, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _13, WorkIndex); + void(ThreadIndices, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _13, _14, WorkIndex); template VTKM_EXEC inline void Generate(const vtkm::Vec& boundaryStatus, @@ -169,6 +167,7 @@ struct ComputePass4XWithNormals : public vtkm::worklet::WorkletVisitCellsWithPoi const WholeDataField& field, const WholeIEdgeField& interpolatedEdgeIds, const WholeWeightField& weights, + const WholeCoordsField coords, const WholePointField& points, const WholeNormalField& normals, const vtkm::Id4& startPos, @@ -197,7 +196,7 @@ struct ComputePass4XWithNormals : public vtkm::worklet::WorkletVisitCellsWithPoi weights.Set(writeIndex, static_cast(t)); auto ijk1 = ijk + vtkm::Id3{ 1, 0, 0 }; - auto coord = this->InterpolateCoordinate(t, ijk, ijk1); + auto coord = this->InterpolateCoordinate(coords, t, ijk, ijk1); points.Set(writeIndex, coord); //gradient generation @@ -216,7 +215,7 @@ struct ComputePass4XWithNormals : public vtkm::worklet::WorkletVisitCellsWithPoi weights.Set(writeIndex, static_cast(t)); auto ijk1 = ijk + vtkm::Id3{ 0, 1, 0 }; - auto coord = this->InterpolateCoordinate(t, ijk, ijk1); + auto coord = this->InterpolateCoordinate(coords, t, ijk, ijk1); points.Set(writeIndex, coord); //gradient generation @@ -235,7 +234,7 @@ struct ComputePass4XWithNormals : public vtkm::worklet::WorkletVisitCellsWithPoi weights.Set(writeIndex, static_cast(t)); auto ijk1 = ijk + vtkm::Id3{ 0, 0, 1 }; - auto coord = this->InterpolateCoordinate(t, ijk, ijk1); + auto coord = this->InterpolateCoordinate(coords, t, ijk, ijk1); points.Set(writeIndex, coord); //gradient generation @@ -258,38 +257,38 @@ struct ComputePass4XWithNormals : public vtkm::worklet::WorkletVisitCellsWithPoi if (onX) //+x boundary { this->InterpolateEdge( - fullyInterior, ijk, pos[0], incs, 5, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, points, normals); + fullyInterior, ijk, pos[0], incs, 5, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, coords, points, normals); this->InterpolateEdge( - fullyInterior, ijk, pos[0], incs, 9, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, points, normals); + fullyInterior, ijk, pos[0], incs, 9, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, coords, points, normals); if (onY) //+x +y { this->InterpolateEdge( - fullyInterior, ijk, pos[0], incs, 11, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, points, normals); + fullyInterior, ijk, pos[0], incs, 11, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, coords, points, normals); } if (onZ) //+x +z { this->InterpolateEdge( - fullyInterior, ijk, pos[0], incs, 7, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, points, normals); + fullyInterior, ijk, pos[0], incs, 7, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, coords, points, normals); } } if (onY) //+y boundary { this->InterpolateEdge( - fullyInterior, ijk, pos[0], incs, 1, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, points, normals); + fullyInterior, ijk, pos[0], incs, 1, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, coords, points, normals); this->InterpolateEdge( - fullyInterior, ijk, pos[0], incs, 10, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, points, normals); + fullyInterior, ijk, pos[0], incs, 10, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, coords, points, normals); if (onZ) //+y +z boundary { this->InterpolateEdge( - fullyInterior, ijk, pos[0], incs, 3, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, points, normals); + fullyInterior, ijk, pos[0], incs, 3, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, coords, points, normals); } } if (onZ) //+z boundary { this->InterpolateEdge( - fullyInterior, ijk, pos[0], incs, 2, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, points, normals); + fullyInterior, ijk, pos[0], incs, 2, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, coords, points, normals); this->InterpolateEdge( - fullyInterior, ijk, pos[0], incs, 6, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, points, normals); + fullyInterior, ijk, pos[0], incs, 6, edgeUses, edgeIds, field, interpolatedEdgeIds, weights, coords, points, normals); } // clang-format on } @@ -300,6 +299,7 @@ struct ComputePass4XWithNormals : public vtkm::worklet::WorkletVisitCellsWithPoi typename WholeIEdgeField, typename WholeWeightField, typename WholePointField, + typename WholeCoordsField, typename WholeNormalField> VTKM_EXEC inline void InterpolateEdge(bool fullyInterior, const vtkm::Id3& ijk, @@ -311,6 +311,7 @@ struct ComputePass4XWithNormals : public vtkm::worklet::WorkletVisitCellsWithPoi const WholeField& field, const WholeIEdgeField& interpolatedEdgeIds, const WholeWeightField& weights, + const WholeCoordsField& coords, const WholePointField& points, const WholeNormalField& normals) const { @@ -338,36 +339,55 @@ struct ComputePass4XWithNormals : public vtkm::worklet::WorkletVisitCellsWithPoi T t = static_cast((this->IsoValue - s0) / (s1 - s0)); weights.Set(writeIndex, static_cast(t)); - auto coord = this->InterpolateCoordinate(t, ijk + offsets1, ijk + offsets2); + auto coord = this->InterpolateCoordinate(coords, t, ijk + offsets1, ijk + offsets2); points.Set(writeIndex, coord); - auto g0 = this->ComputeGradient(fullyInterior, ijk + offsets1, incs, iEdge[0], field); auto g1 = this->ComputeGradient(fullyInterior, ijk + offsets2, incs, iEdge[1], field); + auto g0 = this->ComputeGradient(fullyInterior, ijk + offsets1, incs, iEdge[0], field); g1 = g0 + (t * (g1 - g0)); normals.Set(writeIndex, vtkm::Normal(g1)); } + // Fast interpolation method for uniform coordinates //---------------------------------------------------------------------------- - inline VTKM_EXEC vtkm::Vec3f InterpolateCoordinate(T t, - const vtkm::Id3& ijk0, - const vtkm::Id3& ijk1) const + inline VTKM_EXEC vtkm::Vec3f InterpolateCoordinate( + const vtkm::internal::ArrayPortalUniformPointCoordinates& coords, + T t, + const vtkm::Id3& ijk0, + const vtkm::Id3& ijk1) const { return vtkm::Vec3f( - this->Origin[0] + - this->Spacing[0] * + coords.GetOrigin()[0] + + coords.GetSpacing()[0] * (static_cast(ijk0[0]) + static_cast(t) * static_cast(ijk1[0] - ijk0[0])), - this->Origin[1] + - this->Spacing[1] * + coords.GetOrigin()[1] + + coords.GetSpacing()[1] * (static_cast(ijk0[1]) + static_cast(t) * static_cast(ijk1[1] - ijk0[1])), - this->Origin[2] + - this->Spacing[2] * + coords.GetOrigin()[2] + + coords.GetSpacing()[2] * (static_cast(ijk0[2]) + static_cast(t) * static_cast(ijk1[2] - ijk0[2]))); } + // Interpolation for explicit coordinates + //---------------------------------------------------------------------------- + template + inline VTKM_EXEC vtkm::Vec3f InterpolateCoordinate(const CoordsPortal& coords, + T t, + const vtkm::Id3& ijk0, + const vtkm::Id3& ijk1) const + { + return (1.0f - static_cast(t)) * + coords.Get(ijk0[0] + this->PointDims[0] * ijk0[1] + + this->PointDims[0] * this->PointDims[1] * ijk0[2]) + + static_cast(t) * + coords.Get(ijk1[0] + this->PointDims[0] * ijk1[1] + + this->PointDims[0] * this->PointDims[1] * ijk1[2]); + } + //---------------------------------------------------------------------------- template VTKM_EXEC vtkm::Vec3f ComputeGradient(bool fullyInterior, diff --git a/vtkm/filter/contour/worklet/contour/FlyingEdgesPass4Y.h b/vtkm/filter/contour/worklet/contour/FlyingEdgesPass4Y.h index 05e73ba13..08a16f3b4 100644 --- a/vtkm/filter/contour/worklet/contour/FlyingEdgesPass4Y.h +++ b/vtkm/filter/contour/worklet/contour/FlyingEdgesPass4Y.h @@ -276,17 +276,11 @@ struct ComputePass4Y : public vtkm::worklet::WorkletVisitCellsWithPoints template struct ComputePass5Y : public vtkm::worklet::WorkletMapField { - - vtkm::internal::ArrayPortalUniformPointCoordinates Coordinates; + vtkm::Id3 PointDims; vtkm::Id NormalWriteOffset; - ComputePass5Y() {} - ComputePass5Y(const vtkm::Id3& pdims, - const vtkm::Vec3f& origin, - const vtkm::Vec3f& spacing, - vtkm::Id normalWriteOffset, - bool generateNormals) - : Coordinates(pdims, origin, spacing) + ComputePass5Y(const vtkm::Id3& pdims, vtkm::Id normalWriteOffset, bool generateNormals) + : PointDims(pdims) , NormalWriteOffset(normalWriteOffset) { if (!generateNormals) @@ -299,20 +293,25 @@ struct ComputePass5Y : public vtkm::worklet::WorkletMapField FieldIn interpWeight, FieldOut points, WholeArrayIn field, + WholeArrayIn coords, WholeArrayOut normals); - using ExecutionSignature = void(_1, _2, _3, _4, _5, WorkIndex); + using ExecutionSignature = void(_1, _2, _3, _4, _5, _6, WorkIndex); - template + template VTKM_EXEC void operator()(const vtkm::Id2& interpEdgeIds, vtkm::FloatDefault weight, vtkm::Vec& outPoint, const WholeInputField& field, + const WholeCoordsField& coords, WholeNormalField& normals, vtkm::Id oidx) const { { - vtkm::Vec3f point1 = this->Coordinates.Get(interpEdgeIds[0]); - vtkm::Vec3f point2 = this->Coordinates.Get(interpEdgeIds[1]); + vtkm::Vec3f point1 = coords.Get(interpEdgeIds[0]); + vtkm::Vec3f point2 = coords.Get(interpEdgeIds[1]); outPoint = vtkm::Lerp(point1, point2, weight); } @@ -320,15 +319,13 @@ struct ComputePass5Y : public vtkm::worklet::WorkletMapField if (this->NormalWriteOffset >= 0) { vtkm::Vec g0, g1; - const vtkm::Id3& dims = this->Coordinates.GetDimensions(); - vtkm::Id3 ijk{ interpEdgeIds[0] % dims[0], - (interpEdgeIds[0] / dims[0]) % dims[1], - interpEdgeIds[0] / (dims[0] * dims[1]) }; + vtkm::Id3 ijk{ interpEdgeIds[0] % this->PointDims[0], + (interpEdgeIds[0] / this->PointDims[0]) % this->PointDims[1], + interpEdgeIds[0] / (this->PointDims[0] * this->PointDims[1]) }; vtkm::worklet::gradient::StructuredPointGradient gradient; - vtkm::exec::BoundaryState boundary(ijk, dims); - vtkm::exec::FieldNeighborhood - coord_neighborhood(this->Coordinates, boundary); + vtkm::exec::BoundaryState boundary(ijk, this->PointDims); + vtkm::exec::FieldNeighborhood coord_neighborhood(coords, boundary); vtkm::exec::FieldNeighborhood field_neighborhood(field, boundary); @@ -337,9 +334,9 @@ struct ComputePass5Y : public vtkm::worklet::WorkletMapField gradient(boundary, coord_neighborhood, field_neighborhood, g0); //compute the gradient at point 2. This optimization can be optimized - boundary.IJK = vtkm::Id3{ interpEdgeIds[1] % dims[0], - (interpEdgeIds[1] / dims[0]) % dims[1], - interpEdgeIds[1] / (dims[0] * dims[1]) }; + boundary.IJK = vtkm::Id3{ interpEdgeIds[1] % this->PointDims[0], + (interpEdgeIds[1] / this->PointDims[0]) % this->PointDims[1], + interpEdgeIds[1] / (this->PointDims[0] * this->PointDims[1]) }; gradient(boundary, coord_neighborhood, field_neighborhood, g1); vtkm::Vec3f n = vtkm::Lerp(g0, g1, weight); From 61c002072f7f355d50dcf7171622b140861ac8f6 Mon Sep 17 00:00:00 2001 From: Li-Ta Lo Date: Wed, 10 May 2023 10:17:14 -0600 Subject: [PATCH 03/22] cleanup C++ usage, use unique_ptr for PIMPL --- vtkm/rendering/ScalarRenderer.cxx | 31 +++------ vtkm/rendering/ScalarRenderer.h | 8 ++- vtkm/rendering/raytracing/ScalarRenderer.cxx | 68 +++++--------------- vtkm/rendering/raytracing/ScalarRenderer.h | 19 +++--- 4 files changed, 40 insertions(+), 86 deletions(-) diff --git a/vtkm/rendering/ScalarRenderer.cxx b/vtkm/rendering/ScalarRenderer.cxx index 6d013d50d..473fefa89 100644 --- a/vtkm/rendering/ScalarRenderer.cxx +++ b/vtkm/rendering/ScalarRenderer.cxx @@ -13,7 +13,6 @@ #include #include -#include #include #include #include @@ -22,7 +21,6 @@ #include #include - namespace vtkm { namespace rendering @@ -30,30 +28,21 @@ namespace rendering struct ScalarRenderer::InternalsType { - bool ValidDataSet; - vtkm::Int32 Width; - vtkm::Int32 Height; - vtkm::Float32 DefaultValue; + bool ValidDataSet = false; + vtkm::Int32 Width = 1024; + vtkm::Int32 Height = 1024; + vtkm::Float32 DefaultValue = vtkm::Nan32(); vtkm::cont::DataSet DataSet; vtkm::rendering::raytracing::ScalarRenderer Tracer; vtkm::Bounds ShapeBounds; - - VTKM_CONT - InternalsType() - : ValidDataSet(false) - , Width(1024) - , Height(1024) - , DefaultValue(vtkm::Nan32()) - { - } }; ScalarRenderer::ScalarRenderer() - : Internals(new InternalsType) + : Internals(std::make_unique()) { } -ScalarRenderer::~ScalarRenderer() {} +ScalarRenderer::~ScalarRenderer() = default; void ScalarRenderer::SetWidth(const vtkm::Int32 width) { @@ -90,16 +79,16 @@ void ScalarRenderer::SetInput(vtkm::cont::DataSet& dataSet) if (triExtractor.GetNumberOfTriangles() > 0) { - auto triIntersector = std::make_shared(); + auto triIntersector = std::make_unique(); triIntersector->SetData(coords, triExtractor.GetTriangles()); - this->Internals->Tracer.SetShapeIntersector(triIntersector); this->Internals->ShapeBounds = triIntersector->GetShapeBounds(); + this->Internals->Tracer.SetShapeIntersector(std::unique_ptr{ + static_cast(triIntersector.release()) }); } } ScalarRenderer::Result ScalarRenderer::Render(const vtkm::rendering::Camera& camera) { - if (!Internals->ValidDataSet) { throw vtkm::cont::ErrorBadValue("ScalarRenderer: input never set"); @@ -182,7 +171,7 @@ ScalarRenderer::Result ScalarRenderer::Render(const vtkm::rendering::Camera& cam vtkm::cont::DataSet ScalarRenderer::Result::ToDataSet() { - if (Scalars.size() == 0) + if (Scalars.empty()) { throw vtkm::cont::ErrorBadValue("ScalarRenderer: result empty"); } diff --git a/vtkm/rendering/ScalarRenderer.h b/vtkm/rendering/ScalarRenderer.h index b414991e8..a4c3fdb7e 100644 --- a/vtkm/rendering/ScalarRenderer.h +++ b/vtkm/rendering/ScalarRenderer.h @@ -25,6 +25,11 @@ class VTKM_RENDERING_EXPORT ScalarRenderer public: ScalarRenderer(); + // Note: since we are using std::unique_ptr for PIMPL, the compiler does + // not know the real size of std::unique_ptr at this point. Thus, we + // can not have the definition of the destructor here. We need to declare it + // here and have an implementation in .cxx. The implementation could just + // be = default. ~ScalarRenderer(); void SetInput(vtkm::cont::DataSet& dataSet); @@ -47,10 +52,9 @@ public: ScalarRenderer::Result Render(const vtkm::rendering::Camera& camera); - private: struct InternalsType; - std::shared_ptr Internals; + std::unique_ptr Internals; }; } } //namespace vtkm::rendering diff --git a/vtkm/rendering/raytracing/ScalarRenderer.cxx b/vtkm/rendering/raytracing/ScalarRenderer.cxx index 536a628ba..9d43f164c 100644 --- a/vtkm/rendering/raytracing/ScalarRenderer.cxx +++ b/vtkm/rendering/raytracing/ScalarRenderer.cxx @@ -10,15 +10,11 @@ #include #include -#include -#include #include -#include #include #include #include -#include #include namespace vtkm @@ -39,10 +35,10 @@ public: { private: vtkm::Vec3f_32 LightPosition; - vtkm::Vec3f_32 LightAmbient; - vtkm::Vec3f_32 LightDiffuse; - vtkm::Vec3f_32 LightSpecular; - vtkm::Float32 SpecularExponent; + vtkm::Vec3f_32 LightAmbient{ .5f, .5f, .5f }; + vtkm::Vec3f_32 LightDiffuse{ .7f, .7f, .7f }; + vtkm::Vec3f_32 LightSpecular{ .7f, .7f, .7f }; + vtkm::Float32 SpecularExponent = 20.f; vtkm::Vec3f_32 CameraPosition; vtkm::Vec3f_32 LookAt; Precision MissScalar; @@ -58,31 +54,16 @@ public: , LookAt(lookAt) , MissScalar(missScalar) { - //Set up some default lighting parameters for now - LightAmbient[0] = .5f; - LightAmbient[1] = .5f; - LightAmbient[2] = .5f; - LightDiffuse[0] = .7f; - LightDiffuse[1] = .7f; - LightDiffuse[2] = .7f; - LightSpecular[0] = .7f; - LightSpecular[1] = .7f; - LightSpecular[2] = .7f; - SpecularExponent = 20.f; } using ControlSignature = void(FieldIn, FieldIn, FieldIn, FieldOut); using ExecutionSignature = void(_1, _2, _3, _4); - //using ExecutionSignature = void(_1, _2, _3, _4, WorkIndex); - //template VTKM_EXEC void operator()(const vtkm::Id& hitIdx, const vtkm::Vec& normal, const vtkm::Vec& intersection, - Precision& output) const //, - // const vtkm::Id& idx) const + Precision& output) const { - if (hitIdx < 0) { output = MissScalar; @@ -145,7 +126,6 @@ public: }; //class Shade - //template VTKM_CONT void run(Ray& rays, const vtkm::rendering::raytracing::Camera& camera, const Precision missScalar, @@ -173,7 +153,7 @@ private: public: VTKM_CONT - FilterDepth(const Precision missScalar) + explicit FilterDepth(const Precision missScalar) : MissScalar(missScalar) { } @@ -201,7 +181,7 @@ private: public: VTKM_CONT - WriteBuffer(const Precision missScalar) + explicit WriteBuffer(const Precision missScalar) : MissScalar(missScalar) { } @@ -227,9 +207,6 @@ template class WriteDepthBuffer : public vtkm::worklet::WorkletMapField { public: - VTKM_CONT - WriteDepthBuffer() {} - typedef void ControlSignature(FieldIn, FieldOut); typedef void ExecutionSignature(_1, _2); @@ -237,22 +214,15 @@ public: }; //class WriteDepthBuffer } // namespace detail -ScalarRenderer::ScalarRenderer() - : IntersectorValid(false) +void ScalarRenderer::SetShapeIntersector(std::unique_ptr intersector) { -} - -ScalarRenderer::~ScalarRenderer() {} - -void ScalarRenderer::SetShapeIntersector(std::shared_ptr intersector) -{ - Intersector = intersector; + Intersector = std::move(intersector); IntersectorValid = true; } void ScalarRenderer::AddField(const vtkm::cont::Field& scalarField) { - vtkm::cont::ArrayHandle ranges = scalarField.GetRange(); + const auto& ranges = scalarField.GetRange(); if (ranges.GetNumberOfValues() != 1) { throw vtkm::cont::ErrorBadValue("ScalarRenderer(AddField): field must be a scalar"); @@ -316,8 +286,6 @@ void ScalarRenderer::RenderOnDevice(Ray& rays, AddBuffer(rays, missScalar, Fields[f].GetName()); } - - const vtkm::Int32 numChannels = 1; ChannelBuffer buffer(numChannels, rays.NumRays); detail::SurfaceShade surfaceShade; @@ -325,24 +293,20 @@ void ScalarRenderer::RenderOnDevice(Ray& rays, buffer.SetName("shading"); rays.Buffers.push_back(buffer); - - vtkm::worklet::DispatcherMapField>( - detail::FilterDepth(missScalar)) - .Invoke(rays.HitIdx, rays.Distance); + this->Invoke(detail::FilterDepth{ missScalar }, rays.HitIdx, rays.Distance); time = renderTimer.GetElapsedTime(); logger->CloseLogEntry(time); } // RenderOnDevice template -void ScalarRenderer::AddBuffer(Ray& rays, Precision missScalar, const std::string name) +void ScalarRenderer::AddBuffer(Ray& rays, Precision missScalar, const std::string& name) { const vtkm::Int32 numChannels = 1; ChannelBuffer buffer(numChannels, rays.NumRays); - vtkm::worklet::DispatcherMapField>( - detail::WriteBuffer(missScalar)) - .Invoke(rays.HitIdx, rays.Scalar, buffer.Buffer); + this->Invoke( + detail::WriteBuffer{ missScalar }, rays.HitIdx, rays.Scalar, buffer.Buffer); buffer.SetName(name); rays.Buffers.push_back(buffer); @@ -354,9 +318,7 @@ void ScalarRenderer::AddDepthBuffer(Ray& rays) const vtkm::Int32 numChannels = 1; ChannelBuffer buffer(numChannels, rays.NumRays); - vtkm::worklet::DispatcherMapField>( - detail::WriteDepthBuffer()) - .Invoke(rays.Depth, buffer.Buffer); + this->Invoke(detail::WriteDepthBuffer{}, rays.Depth, buffer.Buffer); buffer.SetName("depth"); rays.Buffers.push_back(buffer); diff --git a/vtkm/rendering/raytracing/ScalarRenderer.h b/vtkm/rendering/raytracing/ScalarRenderer.h index 93a32a5a7..60efa87b2 100644 --- a/vtkm/rendering/raytracing/ScalarRenderer.h +++ b/vtkm/rendering/raytracing/ScalarRenderer.h @@ -17,6 +17,7 @@ #include #include + namespace vtkm { namespace rendering @@ -26,10 +27,13 @@ namespace raytracing class VTKM_RENDERING_EXPORT ScalarRenderer { +private: + vtkm::cont::Invoker Invoke; + protected: - std::shared_ptr Intersector; + std::unique_ptr Intersector; std::vector Fields; - bool IntersectorValid; + bool IntersectorValid = false; template void RenderOnDevice(Ray& rays, @@ -37,19 +41,14 @@ protected: vtkm::rendering::raytracing::Camera& cam); template - void AddBuffer(Ray& rays, Precision missScalar, const std::string name); + void AddBuffer(Ray& rays, Precision missScalar, const std::string& name); template void AddDepthBuffer(Ray& rays); public: VTKM_CONT - ScalarRenderer(); - VTKM_CONT - ~ScalarRenderer(); - - VTKM_CONT - void SetShapeIntersector(std::shared_ptr intersector); + void SetShapeIntersector(std::unique_ptr intersector); VTKM_CONT void AddField(const vtkm::cont::Field& scalarField); @@ -68,4 +67,4 @@ public: } } } // namespace vtkm::rendering::raytracing -#endif //vtk_m_rendering_raytracing_RayTracer_h +#endif //vtk_m_rendering_raytracing_ScalarRenderer_h From 50a9efc2f51c00e5772ac17068b67ec32993b54d Mon Sep 17 00:00:00 2001 From: Li-Ta Lo Date: Wed, 10 May 2023 17:28:30 -0600 Subject: [PATCH 04/22] use rvalue ref for parameter --- vtkm/rendering/ScalarRenderer.cxx | 3 +-- vtkm/rendering/raytracing/ScalarRenderer.cxx | 2 +- vtkm/rendering/raytracing/ScalarRenderer.h | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/vtkm/rendering/ScalarRenderer.cxx b/vtkm/rendering/ScalarRenderer.cxx index 473fefa89..7e692c4b7 100644 --- a/vtkm/rendering/ScalarRenderer.cxx +++ b/vtkm/rendering/ScalarRenderer.cxx @@ -82,8 +82,7 @@ void ScalarRenderer::SetInput(vtkm::cont::DataSet& dataSet) auto triIntersector = std::make_unique(); triIntersector->SetData(coords, triExtractor.GetTriangles()); this->Internals->ShapeBounds = triIntersector->GetShapeBounds(); - this->Internals->Tracer.SetShapeIntersector(std::unique_ptr{ - static_cast(triIntersector.release()) }); + this->Internals->Tracer.SetShapeIntersector(std::move(triIntersector)); } } diff --git a/vtkm/rendering/raytracing/ScalarRenderer.cxx b/vtkm/rendering/raytracing/ScalarRenderer.cxx index 9d43f164c..9549c5017 100644 --- a/vtkm/rendering/raytracing/ScalarRenderer.cxx +++ b/vtkm/rendering/raytracing/ScalarRenderer.cxx @@ -214,7 +214,7 @@ public: }; //class WriteDepthBuffer } // namespace detail -void ScalarRenderer::SetShapeIntersector(std::unique_ptr intersector) +void ScalarRenderer::SetShapeIntersector(std::unique_ptr&& intersector) { Intersector = std::move(intersector); IntersectorValid = true; diff --git a/vtkm/rendering/raytracing/ScalarRenderer.h b/vtkm/rendering/raytracing/ScalarRenderer.h index 60efa87b2..7d2e41284 100644 --- a/vtkm/rendering/raytracing/ScalarRenderer.h +++ b/vtkm/rendering/raytracing/ScalarRenderer.h @@ -48,7 +48,7 @@ protected: public: VTKM_CONT - void SetShapeIntersector(std::unique_ptr intersector); + void SetShapeIntersector(std::unique_ptr&& intersector); VTKM_CONT void AddField(const vtkm::cont::Field& scalarField); From 6e75be33ec0bcfea8d045a5a5ea68e162abe0b97 Mon Sep 17 00:00:00 2001 From: Li-Ta Lo Date: Wed, 10 May 2023 17:31:50 -0600 Subject: [PATCH 05/22] unique_ptr know if itself is valid --- vtkm/rendering/raytracing/ScalarRenderer.cxx | 3 +-- vtkm/rendering/raytracing/ScalarRenderer.h | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/vtkm/rendering/raytracing/ScalarRenderer.cxx b/vtkm/rendering/raytracing/ScalarRenderer.cxx index 9549c5017..f5d12d847 100644 --- a/vtkm/rendering/raytracing/ScalarRenderer.cxx +++ b/vtkm/rendering/raytracing/ScalarRenderer.cxx @@ -217,7 +217,6 @@ public: void ScalarRenderer::SetShapeIntersector(std::unique_ptr&& intersector) { Intersector = std::move(intersector); - IntersectorValid = true; } void ScalarRenderer::AddField(const vtkm::cont::Field& scalarField) @@ -265,7 +264,7 @@ void ScalarRenderer::RenderOnDevice(Ray& rays, { throw vtkm::cont::ErrorBadValue("ScalarRenderer: no fields added"); } - if (!IntersectorValid) + if (!Intersector) { throw vtkm::cont::ErrorBadValue("ScalarRenderer: intersector never set"); } diff --git a/vtkm/rendering/raytracing/ScalarRenderer.h b/vtkm/rendering/raytracing/ScalarRenderer.h index 7d2e41284..aacecc6c8 100644 --- a/vtkm/rendering/raytracing/ScalarRenderer.h +++ b/vtkm/rendering/raytracing/ScalarRenderer.h @@ -33,7 +33,6 @@ private: protected: std::unique_ptr Intersector; std::vector Fields; - bool IntersectorValid = false; template void RenderOnDevice(Ray& rays, From 066e6a696930a3ac6cc8a324c0b088b5639bd6c1 Mon Sep 17 00:00:00 2001 From: Li-Ta Lo Date: Wed, 10 May 2023 20:05:34 -0600 Subject: [PATCH 06/22] I think this is the proper way doing PIMPL --- vtkm/rendering/ScalarRenderer.cxx | 2 ++ vtkm/rendering/ScalarRenderer.h | 17 ++++++++++++----- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/vtkm/rendering/ScalarRenderer.cxx b/vtkm/rendering/ScalarRenderer.cxx index 7e692c4b7..e62a8e682 100644 --- a/vtkm/rendering/ScalarRenderer.cxx +++ b/vtkm/rendering/ScalarRenderer.cxx @@ -42,6 +42,8 @@ ScalarRenderer::ScalarRenderer() { } +ScalarRenderer::ScalarRenderer(ScalarRenderer&&) noexcept = default; +ScalarRenderer& ScalarRenderer::operator=(ScalarRenderer&&) noexcept = default; ScalarRenderer::~ScalarRenderer() = default; void ScalarRenderer::SetWidth(const vtkm::Int32 width) diff --git a/vtkm/rendering/ScalarRenderer.h b/vtkm/rendering/ScalarRenderer.h index a4c3fdb7e..54f60e676 100644 --- a/vtkm/rendering/ScalarRenderer.h +++ b/vtkm/rendering/ScalarRenderer.h @@ -25,11 +25,18 @@ class VTKM_RENDERING_EXPORT ScalarRenderer public: ScalarRenderer(); - // Note: since we are using std::unique_ptr for PIMPL, the compiler does - // not know the real size of std::unique_ptr at this point. Thus, we - // can not have the definition of the destructor here. We need to declare it - // here and have an implementation in .cxx. The implementation could just - // be = default. + // Disable copying due to unique_ptr; + ScalarRenderer(const ScalarRenderer&) = delete; + ScalarRenderer& operator=(const ScalarRenderer&) = delete; + + // Note: we are using std::unique_ptr with default deleter for PIMPL, + // the compiler does not know how to delete SomeIncompleteType* at this point. Thus, we + // can not omit the declaration of destructor, move assignment operator and let the + // compiler generate implicit default version (which it can't.) We also can not have + // inline definitions of them here. We need to declare them here and have definitions in + // .cxx where SomeIncompleteType becomes complete. The implementations could just be = default. + ScalarRenderer(ScalarRenderer&&) noexcept; + ScalarRenderer& operator=(ScalarRenderer&&) noexcept; ~ScalarRenderer(); void SetInput(vtkm::cont::DataSet& dataSet); From 83b1cb9ecd2bd88f6a95077b71a6239b293afb22 Mon Sep 17 00:00:00 2001 From: Vicente Adolfo Bolea Sanchez Date: Mon, 8 May 2023 15:33:07 -0400 Subject: [PATCH 07/22] ci,warnings: add configure warning exceptions --- .gitlab/ci/check_warnings.cmake | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/.gitlab/ci/check_warnings.cmake b/.gitlab/ci/check_warnings.cmake index 0eae9f322..646db7081 100644 --- a/.gitlab/ci/check_warnings.cmake +++ b/.gitlab/ci/check_warnings.cmake @@ -10,16 +10,31 @@ ## ##============================================================================= -# Find the path the logs from the last configure +set(configure_warning_exceptions + ".*CMake Warning at CMake/VTKmDetermineVersion.cmake.*" + ) + +# Find the path of the logs from the last configure set(cnf_log_path "${CMAKE_SOURCE_DIR}/build/Testing/Temporary/LastConfigure*.log") file(GLOB cnf_log_files ${cnf_log_path}) +# Check for warnings during the configure phase foreach(file IN LISTS cnf_log_files) file(STRINGS ${file} lines) - string(FIND "${lines}" "Warning" line) - if (NOT ${line} EQUAL "-1") - message(FATAL_ERROR "Configure warnings detected, please check cdash-commit job") - endif() + foreach(line IN LISTS lines) + if ("${line}" MATCHES "Warning|WARNING|warning") + set(exception_matches FALSE) + foreach(exception IN LISTS configure_warning_exceptions) + if (${line} MATCHES "${exception}") + set(exception_matches TRUE) + break() + endif() + endforeach() + if (NOT exception_matches) + message(FATAL_ERROR "Configure warnings detected, please check cdash-commit job: ${line}") + endif() + endif() + endforeach() endforeach() # `compile_num_warnings` contains a single integer symbolizing the number of From 44650c2e4872b1e036d52e28861bac119baaffc2 Mon Sep 17 00:00:00 2001 From: Kenneth Moreland Date: Fri, 12 May 2023 12:00:17 -0600 Subject: [PATCH 08/22] Fix unused variable in contour test --- vtkm/filter/contour/testing/UnitTestContourFilter.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vtkm/filter/contour/testing/UnitTestContourFilter.cxx b/vtkm/filter/contour/testing/UnitTestContourFilter.cxx index 86aaa3aeb..61ccdc97b 100644 --- a/vtkm/filter/contour/testing/UnitTestContourFilter.cxx +++ b/vtkm/filter/contour/testing/UnitTestContourFilter.cxx @@ -228,7 +228,7 @@ public: // But it should not change anything on the contour itself. filter.SetGenerateNormals(true); vtkm::cont::DataSet outputNormals = filter.Execute(rectilinearDataset); - coordinates = outputSingleCell.GetCoordinateSystem() + coordinates = outputNormals.GetCoordinateSystem() .GetData() .AsArrayHandle>(); VTKM_TEST_ASSERT(test_equal_ArrayHandles(coordinates, expectedCoordinates), From 3c1afe53dca2221dfc0c205d93a7b59765b48dee Mon Sep 17 00:00:00 2001 From: Kenneth Moreland Date: Wed, 19 Apr 2023 11:14:05 -0600 Subject: [PATCH 09/22] Fix new instances of ArrayHandleRuntimeVec in UnknownArrayHandle `UnknownArrayHandle` is supposed to treat `ArrayHandleRuntimeVec` the same as `ArrayHandleBasic`. However, the `NewInstance` methods were failing because they need custom handling of the vec size. --- .../unknown-runtime-vec-new-instance.md | 8 ++++ vtkm/cont/UnknownArrayHandle.cxx | 37 +++++++++++++++++++ .../testing/UnitTestUnknownArrayHandle.cxx | 22 +++++++++++ 3 files changed, 67 insertions(+) create mode 100644 docs/changelog/unknown-runtime-vec-new-instance.md diff --git a/docs/changelog/unknown-runtime-vec-new-instance.md b/docs/changelog/unknown-runtime-vec-new-instance.md new file mode 100644 index 000000000..5a13b1567 --- /dev/null +++ b/docs/changelog/unknown-runtime-vec-new-instance.md @@ -0,0 +1,8 @@ +# Fix new instances of ArrayHandleRuntimeVec in UnknownArrayHandle + +`UnknownArrayHandle` is supposed to treat `ArrayHandleRuntimeVec` the same +as `ArrayHandleBasic`. However, the `NewInstance` methods were failing +because they need custom handling of the vec size. Special cases in the +`UnknownArrayHandle::NewInstance*()` methods have been added to fix this +problem. + diff --git a/vtkm/cont/UnknownArrayHandle.cxx b/vtkm/cont/UnknownArrayHandle.cxx index 1c5f1f4ab..21037bc99 100644 --- a/vtkm/cont/UnknownArrayHandle.cxx +++ b/vtkm/cont/UnknownArrayHandle.cxx @@ -177,6 +177,16 @@ VTKM_CONT bool UnknownArrayHandle::IsValid() const VTKM_CONT UnknownArrayHandle UnknownArrayHandle::NewInstance() const { + if (this->IsStorageType()) + { + // Special case for `ArrayHandleRuntimeVec`, which (1) can be used in place of + // a basic array in `UnknownArrayHandle` and (2) needs a special construction to + // capture the correct number of components. Also note that we are allowing this + // special case to be implemented in `NewInstanceBasic` because it has a better + // fallback (throw an exception rather than create a potentially incompatible + // with the wrong number of components). + return this->NewInstanceBasic(); + } UnknownArrayHandle newArray; if (this->Container) { @@ -188,6 +198,25 @@ VTKM_CONT UnknownArrayHandle UnknownArrayHandle::NewInstance() const VTKM_CONT UnknownArrayHandle UnknownArrayHandle::NewInstanceBasic() const { UnknownArrayHandle newArray; + if (this->IsStorageType()) + { + // Special case for `ArrayHandleRuntimeVec`, which (1) can be used in place of + // a basic array in `UnknownArrayHandle` and (2) needs a special construction to + // capture the correct number of components. + auto runtimeVecArrayCreator = [&](auto exampleComponent) { + using ComponentType = decltype(exampleComponent); + if (this->IsBaseComponentType()) + { + newArray = + vtkm::cont::make_ArrayHandleRuntimeVec(this->GetNumberOfComponentsFlat()); + } + }; + vtkm::ListForEach(runtimeVecArrayCreator, vtkm::TypeListBaseC{}); + if (newArray.IsValid()) + { + return newArray; + } + } if (this->Container) { newArray.Container = this->Container->NewInstanceBasic(); @@ -197,6 +226,14 @@ VTKM_CONT UnknownArrayHandle UnknownArrayHandle::NewInstanceBasic() const VTKM_CONT UnknownArrayHandle UnknownArrayHandle::NewInstanceFloatBasic() const { + if (this->IsStorageType()) + { + // Special case for `ArrayHandleRuntimeVec`, which (1) can be used in place of + // a basic array in `UnknownArrayHandle` and (2) needs a special construction to + // capture the correct number of components. + return vtkm::cont::make_ArrayHandleRuntimeVec( + this->GetNumberOfComponentsFlat()); + } UnknownArrayHandle newArray; if (this->Container) { diff --git a/vtkm/cont/testing/UnitTestUnknownArrayHandle.cxx b/vtkm/cont/testing/UnitTestUnknownArrayHandle.cxx index 036487fd5..b8ae60a62 100644 --- a/vtkm/cont/testing/UnitTestUnknownArrayHandle.cxx +++ b/vtkm/cont/testing/UnitTestUnknownArrayHandle.cxx @@ -11,6 +11,7 @@ #include #include +#include #include #include #include @@ -574,6 +575,27 @@ void TryConvertRuntimeVec() VTKM_TEST_ASSERT(unknownWithRuntimeVec.CanConvert()); BasicArrayType outputArray = unknownWithRuntimeVec.AsArrayHandle(); VTKM_TEST_ASSERT(test_equal_ArrayHandles(inputArray, outputArray)); + + std::cout << " Copy ArrayHandleRuntimeVec to a new instance" << std::endl; + vtkm::cont::UnknownArrayHandle unknownCopy = unknownWithRuntimeVec.NewInstance(); + VTKM_TEST_ASSERT(unknownWithRuntimeVec.GetNumberOfComponentsFlat() == + unknownCopy.GetNumberOfComponentsFlat()); + vtkm::cont::ArrayCopy(unknownWithRuntimeVec, unknownCopy); + VTKM_TEST_ASSERT(test_equal_ArrayHandles(inputArray, unknownCopy)); + + std::cout << " Copy ArrayHandleRuntimeVec as basic array" << std::endl; + unknownCopy = unknownWithRuntimeVec.NewInstanceBasic(); + VTKM_TEST_ASSERT(unknownWithRuntimeVec.GetNumberOfComponentsFlat() == + unknownCopy.GetNumberOfComponentsFlat()); + vtkm::cont::ArrayCopy(unknownWithRuntimeVec, unknownCopy); + VTKM_TEST_ASSERT(test_equal_ArrayHandles(inputArray, unknownCopy)); + + std::cout << " Copy ArrayHandleRuntimeVec to float array" << std::endl; + unknownCopy = unknownWithRuntimeVec.NewInstanceFloatBasic(); + VTKM_TEST_ASSERT(unknownWithRuntimeVec.GetNumberOfComponentsFlat() == + unknownCopy.GetNumberOfComponentsFlat()); + vtkm::cont::ArrayCopy(unknownWithRuntimeVec, unknownCopy); + VTKM_TEST_ASSERT(test_equal_ArrayHandles(inputArray, unknownCopy)); } void TryConvertRuntimeVec() From d05025ce8592be1ac2561260b52e2a6d3c6f00d7 Mon Sep 17 00:00:00 2001 From: Li-Ta Lo Date: Mon, 15 May 2023 13:05:06 -0600 Subject: [PATCH 10/22] simplify comment --- vtkm/rendering/ScalarRenderer.cxx | 6 +++--- vtkm/rendering/ScalarRenderer.h | 11 +++-------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/vtkm/rendering/ScalarRenderer.cxx b/vtkm/rendering/ScalarRenderer.cxx index e62a8e682..059f1646a 100644 --- a/vtkm/rendering/ScalarRenderer.cxx +++ b/vtkm/rendering/ScalarRenderer.cxx @@ -46,7 +46,7 @@ ScalarRenderer::ScalarRenderer(ScalarRenderer&&) noexcept = default; ScalarRenderer& ScalarRenderer::operator=(ScalarRenderer&&) noexcept = default; ScalarRenderer::~ScalarRenderer() = default; -void ScalarRenderer::SetWidth(const vtkm::Int32 width) +void ScalarRenderer::SetWidth(vtkm::Int32 width) { if (width < 1) { @@ -55,12 +55,12 @@ void ScalarRenderer::SetWidth(const vtkm::Int32 width) Internals->Width = width; } -void ScalarRenderer::SetDefaultValue(const vtkm::Float32 value) +void ScalarRenderer::SetDefaultValue(vtkm::Float32 value) { Internals->DefaultValue = value; } -void ScalarRenderer::SetHeight(const vtkm::Int32 height) +void ScalarRenderer::SetHeight(vtkm::Int32 height) { if (height < 1) { diff --git a/vtkm/rendering/ScalarRenderer.h b/vtkm/rendering/ScalarRenderer.h index 54f60e676..3cd2ff37b 100644 --- a/vtkm/rendering/ScalarRenderer.h +++ b/vtkm/rendering/ScalarRenderer.h @@ -29,20 +29,15 @@ public: ScalarRenderer(const ScalarRenderer&) = delete; ScalarRenderer& operator=(const ScalarRenderer&) = delete; - // Note: we are using std::unique_ptr with default deleter for PIMPL, - // the compiler does not know how to delete SomeIncompleteType* at this point. Thus, we - // can not omit the declaration of destructor, move assignment operator and let the - // compiler generate implicit default version (which it can't.) We also can not have - // inline definitions of them here. We need to declare them here and have definitions in - // .cxx where SomeIncompleteType becomes complete. The implementations could just be = default. ScalarRenderer(ScalarRenderer&&) noexcept; ScalarRenderer& operator=(ScalarRenderer&&) noexcept; + // Default destructor implemented in source file to support PIMPL idiom. ~ScalarRenderer(); void SetInput(vtkm::cont::DataSet& dataSet); - void SetWidth(const vtkm::Int32 width); - void SetHeight(const vtkm::Int32 height); + void SetWidth(vtkm::Int32 width); + void SetHeight(vtkm::Int32 height); void SetDefaultValue(vtkm::Float32 value); struct VTKM_RENDERING_EXPORT Result From 7c0f40fe18ac7a348dffaa4f47350e512c5fa469 Mon Sep 17 00:00:00 2001 From: Vicente Adolfo Bolea Sanchez Date: Mon, 15 May 2023 19:47:22 -0400 Subject: [PATCH 11/22] ci: setup CUDA arch before sccache --- .gitlab/ci/config/initial_config.cmake | 38 +++++++++++++------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/.gitlab/ci/config/initial_config.cmake b/.gitlab/ci/config/initial_config.cmake index 66be36678..ab82d6835 100644 --- a/.gitlab/ci/config/initial_config.cmake +++ b/.gitlab/ci/config/initial_config.cmake @@ -156,6 +156,25 @@ foreach(option IN LISTS options) endforeach() +# We need to use VTKm_CUDA_Architecture for older CMake versions +if(vtkm_cuda_arch) + if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.18) + if(vtkm_cuda_arch STREQUAL "maxwell") + set(CMAKE_CUDA_ARCHITECTURES "50" CACHE STRING "") + elseif(vtkm_cuda_arch STREQUAL "pascal") + set(CMAKE_CUDA_ARCHITECTURES "60" CACHE STRING "") + elseif(vtkm_cuda_arch STREQUAL "volta") + set(CMAKE_CUDA_ARCHITECTURES "70" CACHE STRING "") + elseif(vtkm_cuda_arch STREQUAL "turing") + set(CMAKE_CUDA_ARCHITECTURES "75" CACHE STRING "") + elseif(vtkm_cuda_arch STREQUAL "ampere") + set(CMAKE_CUDA_ARCHITECTURES "80" CACHE STRING "") + endif() + else() + set(VTKm_CUDA_Architecture "${vtkm_cuda_arch}" CACHE STRING "") + endif() +endif() + # Compile tutorials on all builders. The code is small and basic. And since # it is the tutorial, it should work really well. set(VTKm_ENABLE_TUTORIALS "ON" CACHE STRING "") @@ -181,22 +200,3 @@ endif() if(sanitizers) set(VTKm_USE_SANITIZER "${sanitizers}" CACHE STRING "" FORCE) endif() - -# We need to use VTKm_CUDA_Architecture for older CMake versions -if(vtkm_cuda_arch) - if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.18) - if(vtkm_cuda_arch STREQUAL "maxwell") - set(CMAKE_CUDA_ARCHITECTURES "50" CACHE STRING "") - elseif(vtkm_cuda_arch STREQUAL "pascal") - set(CMAKE_CUDA_ARCHITECTURES "60" CACHE STRING "") - elseif(vtkm_cuda_arch STREQUAL "volta") - set(CMAKE_CUDA_ARCHITECTURES "70" CACHE STRING "") - elseif(vtkm_cuda_arch STREQUAL "turing") - set(CMAKE_CUDA_ARCHITECTURES "75" CACHE STRING "") - elseif(vtkm_cuda_arch STREQUAL "ampere") - set(CMAKE_CUDA_ARCHITECTURES "80" CACHE STRING "") - endif() - else() - set(VTKm_CUDA_Architecture "${vtkm_cuda_arch}" CACHE STRING "") - endif() -endif() From 09c0139b4e40d153b68c69252330367333314c69 Mon Sep 17 00:00:00 2001 From: Kenneth Moreland Date: Thu, 6 Apr 2023 16:55:04 -0600 Subject: [PATCH 12/22] Fix operator for IteratorFromArrayPortal There was an error in `operator-=` for `IteratorFromArrayPortal` that went by unnoticed. The operator is fixed and regression tests for the operators has been added. --- docs/changelog/fix-iterator-operator.md | 5 ++ vtkm/cont/internal/IteratorFromArrayPortal.h | 2 +- .../UnitTestIteratorFromArrayPortal.cxx | 72 +++++++++++++++++++ 3 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 docs/changelog/fix-iterator-operator.md diff --git a/docs/changelog/fix-iterator-operator.md b/docs/changelog/fix-iterator-operator.md new file mode 100644 index 000000000..0a72db7f8 --- /dev/null +++ b/docs/changelog/fix-iterator-operator.md @@ -0,0 +1,5 @@ +# Fixed operator for IteratorFromArrayPortal + +There was an error in `operator-=` for `IteratorFromArrayPortal` that went +by unnoticed. The operator is fixed and regression tests for the operators +has been added. diff --git a/vtkm/cont/internal/IteratorFromArrayPortal.h b/vtkm/cont/internal/IteratorFromArrayPortal.h index 911e0e497..f6085364e 100644 --- a/vtkm/cont/internal/IteratorFromArrayPortal.h +++ b/vtkm/cont/internal/IteratorFromArrayPortal.h @@ -98,7 +98,7 @@ public: VTKM_EXEC_CONT iter& operator-=(difference_type n) { - this->Index += static_cast(n); + this->Index -= static_cast(n); VTKM_ASSERT(this->Index >= 0); return *this; } diff --git a/vtkm/cont/testing/UnitTestIteratorFromArrayPortal.cxx b/vtkm/cont/testing/UnitTestIteratorFromArrayPortal.cxx index 2a8466ace..d2d5e8af5 100644 --- a/vtkm/cont/testing/UnitTestIteratorFromArrayPortal.cxx +++ b/vtkm/cont/testing/UnitTestIteratorFromArrayPortal.cxx @@ -11,6 +11,7 @@ #include #include +#include #include #include @@ -115,6 +116,74 @@ struct TemplatedTests "Did not get correct values when writing to iterator."); } + void TestOperators() + { + struct Functor + { + VTKM_EXEC ValueType operator()(vtkm::Id index) const { return TestValue(index, ValueType{}); } + }; + Functor functor; + + auto array = vtkm::cont::make_ArrayHandleImplicit(functor, ARRAY_SIZE); + auto portal = array.ReadPortal(); + + VTKM_TEST_ASSERT(test_equal(portal.Get(0), functor(0))); + ::CheckPortal(portal); + + // Normally, you would use `ArrayPortalToIterators`, but we want to test this + // class specifically. + using IteratorType = vtkm::cont::internal::IteratorFromArrayPortal; + IteratorType begin{ portal }; + IteratorType end{ portal, ARRAY_SIZE }; + + VTKM_TEST_ASSERT(test_equal(*begin, functor(0))); + VTKM_TEST_ASSERT(test_equal(begin[0], functor(0))); + VTKM_TEST_ASSERT(test_equal(begin[3], functor(3))); + + IteratorType iter = begin; + VTKM_TEST_ASSERT(test_equal(*iter, functor(0))); + VTKM_TEST_ASSERT(test_equal(*(iter++), functor(0))); + VTKM_TEST_ASSERT(test_equal(*iter, functor(1))); + VTKM_TEST_ASSERT(test_equal(*(++iter), functor(2))); + VTKM_TEST_ASSERT(test_equal(*iter, functor(2))); + + VTKM_TEST_ASSERT(test_equal(*(iter--), functor(2))); + VTKM_TEST_ASSERT(test_equal(*iter, functor(1))); + VTKM_TEST_ASSERT(test_equal(*(--iter), functor(0))); + VTKM_TEST_ASSERT(test_equal(*iter, functor(0))); + + VTKM_TEST_ASSERT(test_equal(*(iter += 3), functor(3))); + VTKM_TEST_ASSERT(test_equal(*(iter -= 3), functor(0))); + + VTKM_TEST_ASSERT(end - begin == ARRAY_SIZE); + + VTKM_TEST_ASSERT(test_equal(*(iter + 3), functor(3))); + VTKM_TEST_ASSERT(test_equal(*(3 + iter), functor(3))); + iter += 3; + VTKM_TEST_ASSERT(test_equal(*(iter - 3), functor(0))); + + VTKM_TEST_ASSERT(iter == (begin + 3)); + VTKM_TEST_ASSERT(!(iter != (begin + 3))); + VTKM_TEST_ASSERT(iter != begin); + VTKM_TEST_ASSERT(!(iter == begin)); + + VTKM_TEST_ASSERT(!(iter < begin)); + VTKM_TEST_ASSERT(!(iter < (begin + 3))); + VTKM_TEST_ASSERT((iter < end)); + + VTKM_TEST_ASSERT(!(iter <= begin)); + VTKM_TEST_ASSERT((iter <= (begin + 3))); + VTKM_TEST_ASSERT((iter <= end)); + + VTKM_TEST_ASSERT((iter > begin)); + VTKM_TEST_ASSERT(!(iter > (begin + 3))); + VTKM_TEST_ASSERT(!(iter > end)); + + VTKM_TEST_ASSERT((iter >= begin)); + VTKM_TEST_ASSERT((iter >= (begin + 3))); + VTKM_TEST_ASSERT(!(iter >= end)); + } + void operator()() { ValueType array[ARRAY_SIZE]; @@ -133,6 +202,9 @@ struct TemplatedTests std::cout << " Test write to iterator." << std::endl; TestIteratorWrite(portal); + + std::cout << " Test operators." << std::endl; + TestOperators(); } }; From a8b4e5a6291d995f51e2785ab8e75ed74f22b29a Mon Sep 17 00:00:00 2001 From: Kenneth Moreland Date: Tue, 16 May 2023 10:03:19 -0600 Subject: [PATCH 13/22] Add GetNumberOfActiveFields method to FilterField --- vtkm/filter/FilterField.h | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/vtkm/filter/FilterField.h b/vtkm/filter/FilterField.h index 1e6d7389c..0005fd4ac 100644 --- a/vtkm/filter/FilterField.h +++ b/vtkm/filter/FilterField.h @@ -46,7 +46,7 @@ public: vtkm::cont::Field::Association association = vtkm::cont::Field::Association::Any) { auto index_st = static_cast(index); - ResizeIfNeeded(index_st); + this->ResizeIfNeeded(index_st); this->ActiveFieldNames[index_st] = name; this->ActiveFieldAssociation[index_st] = association; } @@ -79,7 +79,7 @@ public: void SetActiveCoordinateSystem(vtkm::IdComponent index, vtkm::Id coord_idx) { auto index_st = static_cast(index); - ResizeIfNeeded(index_st); + this->ResizeIfNeeded(index_st); this->ActiveCoordinateSystemIndices[index_st] = coord_idx; } @@ -120,6 +120,19 @@ public: } ///@} + /// \brief Return the number of active fields currently set. + /// + /// The general interface to `FilterField` allows a user to set an arbitrary number + /// of active fields (indexed 0 and on). This method returns the number of active + /// fields that are set. Note that the filter implementation is free to ignore + /// any active fields it does not support. Also note that an active field can be + /// set to be either a named field or a coordinate system. + vtkm::IdComponent GetNumberOfActiveFields() const + { + VTKM_ASSERT(this->ActiveFieldNames.size() == this->UseCoordinateSystemAsField.size()); + return static_cast(this->UseCoordinateSystemAsField.size()); + } + protected: VTKM_CONT const vtkm::cont::Field& GetFieldFromDataSet(const vtkm::cont::DataSet& input) const From 5bdd3c7bc2c524a3ddf52de8d27518d0c77c0aea Mon Sep 17 00:00:00 2001 From: Kenneth Moreland Date: Tue, 16 May 2023 12:36:47 -0600 Subject: [PATCH 14/22] Move ArrayHandleRuntimeVec metadata to a separate class Originally, the metadata structure used by the `ArrayHandleRuntimeVec` storage was a nested class of its `Storage`. But the `Storage` is templated on the component type, and the metadata object is the same regardless. In addition to the typical minor issue of having the compiler create several identical classes, this caused problems when pulling arrays from equivalent but technically different C types (for example, `long` is the same as either `int` or `long long`). --- vtkm/cont/ArrayHandleRuntimeVec.h | 10 ++++++---- vtkm/cont/testing/UnitTestUnknownArrayHandle.cxx | 13 +++++++++++-- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/vtkm/cont/ArrayHandleRuntimeVec.h b/vtkm/cont/ArrayHandleRuntimeVec.h index 8b6575796..c7ef442fc 100644 --- a/vtkm/cont/ArrayHandleRuntimeVec.h +++ b/vtkm/cont/ArrayHandleRuntimeVec.h @@ -133,6 +133,11 @@ struct VTKM_ALWAYS_EXPORT StorageTagRuntimeVec namespace internal { +struct RuntimeVecMetaData +{ + vtkm::IdComponent NumberOfComponents; +}; + template class Storage, vtkm::cont::StorageTagRuntimeVec> { @@ -152,10 +157,7 @@ class Storage, vtkm::cont::StorageTagRunti (std::is_same::value), "Used invalid ComponentsPortal type with expected ComponentsStorageTag."); - struct Info - { - vtkm::IdComponent NumberOfComponents; - }; + using Info = RuntimeVecMetaData; VTKM_CONT static std::vector ComponentsBuffers( const std::vector& buffers) diff --git a/vtkm/cont/testing/UnitTestUnknownArrayHandle.cxx b/vtkm/cont/testing/UnitTestUnknownArrayHandle.cxx index b8ae60a62..6b9f8045c 100644 --- a/vtkm/cont/testing/UnitTestUnknownArrayHandle.cxx +++ b/vtkm/cont/testing/UnitTestUnknownArrayHandle.cxx @@ -544,11 +544,10 @@ void TrySetMultiplexerArray() CheckUnknownArray, vtkm::List>(unknownArray, 1); } -template +template ::ComponentType> void TryConvertRuntimeVec() { using BasicArrayType = vtkm::cont::ArrayHandle; - using BasicComponentType = typename vtkm::VecFlat::ComponentType; constexpr vtkm::IdComponent numFlatComponents = vtkm::VecFlat::NUM_COMPONENTS; using RuntimeArrayType = vtkm::cont::ArrayHandleRuntimeVec; @@ -614,6 +613,16 @@ void TryConvertRuntimeVec() std::cout << " Vec of Vecs of Vecs." << std::endl; TryConvertRuntimeVec, 2>>(); + + std::cout << " Compatible but different C types." << std::endl; + if (sizeof(int) == sizeof(long)) + { + TryConvertRuntimeVec, long>(); + } + else // assuming sizeof(long long) == sizeof(long) + { + TryConvertRuntimeVec, long>(); + } } struct DefaultTypeFunctor From b59580bb821a7c279429cff7caf98fc067cf181d Mon Sep 17 00:00:00 2001 From: Kenneth Moreland Date: Tue, 16 May 2023 11:56:24 -0600 Subject: [PATCH 15/22] Allow CompositeVectors filter to build any size vector Using the new `ArrayHandleRuntimeVec` feature, we can construct an array with any vec sized value. --- docs/changelog/composite-vector-filter.md | 2 +- .../field_transform/CompositeVectors.cxx | 159 ++++++++++-------- .../filter/field_transform/CompositeVectors.h | 27 ++- .../testing/UnitTestCompositeVectors.cxx | 45 ++--- 4 files changed, 129 insertions(+), 104 deletions(-) diff --git a/docs/changelog/composite-vector-filter.md b/docs/changelog/composite-vector-filter.md index 5c35d3fe3..bd2157426 100644 --- a/docs/changelog/composite-vector-filter.md +++ b/docs/changelog/composite-vector-filter.md @@ -1,3 +1,3 @@ # New Composite Vector filter -The composite vector filter combines multiple scalar fields into a single vector field. Scalar fields are selected as the active input fields, and the combined vector field is set at the output. The current composite vector filter only supports 2d and 3d scalar field composition. Users may use `vtkm::cont::make_ArrayHandleCompositeVector` to execute a more flexible scalar field composition. +The composite vector filter combines multiple scalar fields into a single vector field. Scalar fields are selected as the active input fields, and the combined vector field is set at the output. diff --git a/vtkm/filter/field_transform/CompositeVectors.cxx b/vtkm/filter/field_transform/CompositeVectors.cxx index 526dac91b..d35c1f238 100644 --- a/vtkm/filter/field_transform/CompositeVectors.cxx +++ b/vtkm/filter/field_transform/CompositeVectors.cxx @@ -7,94 +7,119 @@ // the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR // PURPOSE. See the above copyright notice for more information. //============================================================================ -#include -#include -#include + #include +#include +#include +#include +#include + +#include +#include + +namespace +{ + +// Extracts a component from an UnknownArrayHandle and returns the extracted component +// as an UnknownArrayHandle. Perhaps this functionality should be part of UnknownArrayHandle +// proper, but its use is probably rare. Note that this implementation makes some assumptions +// on its use in the CompositeVectors filter. +VTKM_CONT vtkm::cont::UnknownArrayHandle ExtractComponent( + const vtkm::cont::UnknownArrayHandle& array, + vtkm::IdComponent componentIndex) +{ + vtkm::cont::UnknownArrayHandle extractedComponentArray; + auto resolveType = [&](auto componentExample) { + using ComponentType = decltype(componentExample); + if (array.IsBaseComponentType()) + { + extractedComponentArray = + array.ExtractComponent(componentIndex, vtkm::CopyFlag::Off); + } + }; + vtkm::ListForEach(resolveType, vtkm::TypeListBaseC{}); + return extractedComponentArray; +} + +} // anonymous namespace + namespace vtkm { namespace filter { namespace field_transform { + +VTKM_CONT void CompositeVectors::SetFieldNameList(const std::vector& fieldNameList, + vtkm::cont::Field::Association association) +{ + vtkm::IdComponent index = 0; + for (auto& fieldName : fieldNameList) + { + this->SetActiveField(index, fieldName, association); + ++index; + } +} + +VTKM_CONT vtkm::IdComponent CompositeVectors::GetNumberOfFields() const +{ + return this->GetNumberOfActiveFields(); +} + VTKM_CONT vtkm::cont::DataSet CompositeVectors::DoExecute(const vtkm::cont::DataSet& inDataSet) { + vtkm::IdComponent numComponents = this->GetNumberOfFields(); + if (numComponents < 1) + { + throw vtkm::cont::ErrorBadValue( + "No input fields to combine into a vector for CompositeVectors."); + } + vtkm::cont::UnknownArrayHandle outArray; - if (this->NumberOfFields < 2) - { - throw vtkm::cont::ErrorFilterExecution("FieldNameList is supposed to be larger than 2."); - } - else if (this->NumberOfFields == 2) - { - auto fieldAssociation = this->GetFieldFromDataSet(0, inDataSet).GetAssociation(); - if (fieldAssociation != this->GetFieldFromDataSet(1, inDataSet).GetAssociation()) + + // Allocate output array to the correct type. + vtkm::cont::Field firstField = this->GetFieldFromDataSet(0, inDataSet); + vtkm::Id numValues = firstField.GetNumberOfValues(); + vtkm::cont::Field::Association association = firstField.GetAssociation(); + auto allocateOutput = [&](auto exampleComponent) { + using ComponentType = decltype(exampleComponent); + if (firstField.GetData().IsBaseComponentType()) { - throw vtkm::cont::ErrorFilterExecution("Field 0 and Field 2 have different associations."); + outArray = vtkm::cont::ArrayHandleRuntimeVec{ numComponents }; } - auto resolveType2d = [&](const auto& field0) { - using T = typename std::decay_t::ValueType; - vtkm::cont::ArrayHandle field1; - vtkm::cont::ArrayCopyShallowIfPossible(this->GetFieldFromDataSet(1, inDataSet).GetData(), - field1); - - auto compositedArray = vtkm::cont::make_ArrayHandleCompositeVector(field0, field1); - - using VecType = vtkm::Vec; - using ArrayHandleType = vtkm::cont::ArrayHandle; - ArrayHandleType result; - vtkm::cont::ArrayCopy(compositedArray, result); - outArray = result; - }; - const auto& inField0 = this->GetFieldFromDataSet(0, inDataSet); - inField0.GetData().CastAndCallForTypes( - resolveType2d); - } - else if (this->NumberOfFields == 3) + }; + vtkm::ListForEach(allocateOutput, vtkm::TypeListBaseC{}); + if (!outArray.IsValid() || (outArray.GetNumberOfComponentsFlat() != numComponents)) { - auto fieldAssociation0 = this->GetFieldFromDataSet(0, inDataSet).GetAssociation(); - auto fieldAssociation1 = this->GetFieldFromDataSet(1, inDataSet).GetAssociation(); - auto fieldAssociation2 = this->GetFieldFromDataSet(2, inDataSet).GetAssociation(); + throw vtkm::cont::ErrorBadType("Unable to allocate output array due to unexpected type."); + } + outArray.Allocate(numValues); - if (fieldAssociation0 != fieldAssociation1 || fieldAssociation1 != fieldAssociation2 || - fieldAssociation0 != fieldAssociation2) + // Iterate over all component fields and copy them into the output array. + for (vtkm::IdComponent componentIndex = 0; componentIndex < numComponents; ++componentIndex) + { + vtkm::cont::Field inScalarField = this->GetFieldFromDataSet(componentIndex, inDataSet); + if (inScalarField.GetData().GetNumberOfComponentsFlat() != 1) { - throw vtkm::cont::ErrorFilterExecution( - "Field 0, Field 1 and Field 2 have different associations."); + throw vtkm::cont::ErrorBadValue("All input fields to CompositeVectors must be scalars."); + } + if (inScalarField.GetAssociation() != association) + { + throw vtkm::cont::ErrorBadValue( + "All scalar fields must have the same association (point, cell, etc.)."); + } + if (inScalarField.GetNumberOfValues() != numValues) + { + throw vtkm::cont::ErrorBadValue("Inconsistent number of field values."); } - auto resolveType3d = [&](const auto& field0) { - using T = typename std::decay_t::ValueType; - vtkm::cont::ArrayHandle field1; - vtkm::cont::ArrayCopyShallowIfPossible(this->GetFieldFromDataSet(1, inDataSet).GetData(), - field1); - vtkm::cont::ArrayHandle field2; - vtkm::cont::ArrayCopyShallowIfPossible(this->GetFieldFromDataSet(2, inDataSet).GetData(), - field2); - auto compositedArray = vtkm::cont::make_ArrayHandleCompositeVector(field0, field1, field2); - - using VecType = vtkm::Vec; - using ArrayHandleType = vtkm::cont::ArrayHandle; - ArrayHandleType result; - // ArrayHandleCompositeVector currently does not implement the ability to - // get to values on the control side, so copy to an array that is accessible. - vtkm::cont::ArrayCopy(compositedArray, result); - outArray = result; - }; - - const auto& inField0 = this->GetFieldFromDataSet(0, inDataSet); - inField0.GetData().CastAndCallForTypes( - resolveType3d); - } - else - { - throw vtkm::cont::ErrorFilterExecution( - "Using make_ArrayHandleCompositeVector to composite vectors more than 3."); + ExtractComponent(outArray, componentIndex).DeepCopyFrom(inScalarField.GetData()); } - return this->CreateResultField( - inDataSet, this->GetOutputFieldName(), this->GetActiveFieldAssociation(), outArray); + return this->CreateResultField(inDataSet, this->GetOutputFieldName(), association, outArray); } + } // namespace field_transform } // namespace vtkm::filter } // namespace vtkm diff --git a/vtkm/filter/field_transform/CompositeVectors.h b/vtkm/filter/field_transform/CompositeVectors.h index 7709e8efb..5cc881f06 100644 --- a/vtkm/filter/field_transform/CompositeVectors.h +++ b/vtkm/filter/field_transform/CompositeVectors.h @@ -20,8 +20,15 @@ namespace filter namespace field_transform { -/// \brief The composite vector filter combines multiple scalar fields into a single vector field. -/// Scalar fields are selected as the active input fields, and the combined vector field is set at the output. +/// \brief Combine multiple scalar fields into a single vector field. +/// +/// Scalar fields are selected as the active input fields, and the combined vector +/// field is set at the output. The `SetFieldNameList` method takes a `std::vector` +/// of field names to use as the component fields. Alternately, the superclass' +/// set active field methods can be used to select the fields independently. +/// +/// All of the input fields must be scalar values. The type of the first field +/// determines the type of the output vector field. /// class VTKM_FILTER_FIELD_TRANSFORM_EXPORT CompositeVectors : public vtkm::filter::FilterField { @@ -33,24 +40,12 @@ public: VTKM_CONT void SetFieldNameList( const std::vector& fieldNameList, - vtkm::cont::Field::Association association = vtkm::cont::Field::Association::Any) - { + vtkm::cont::Field::Association association = vtkm::cont::Field::Association::Any); - vtkm::IdComponent index = 0; - for (auto& fieldName : fieldNameList) - { - this->SetActiveField(index, fieldName, association); - ++index; - } - this->NumberOfFields = static_cast(fieldNameList.size()); - } - - VTKM_CONT - vtkm::IdComponent GetNumberOfFields() { return this->NumberOfFields; } + VTKM_CONT vtkm::IdComponent GetNumberOfFields() const; private: vtkm::cont::DataSet DoExecute(const vtkm::cont::DataSet& input) override; - vtkm::IdComponent NumberOfFields; }; } // namespace field_transform } // namespace vtkm::filter diff --git a/vtkm/filter/field_transform/testing/UnitTestCompositeVectors.cxx b/vtkm/filter/field_transform/testing/UnitTestCompositeVectors.cxx index 2082cb946..4950f82b9 100644 --- a/vtkm/filter/field_transform/testing/UnitTestCompositeVectors.cxx +++ b/vtkm/filter/field_transform/testing/UnitTestCompositeVectors.cxx @@ -20,19 +20,19 @@ vtkm::cont::DataSet MakeDataSet(vtkm::IdComponent numArrays) vtkm::IdComponent arrayLen = 100; - for (vtkm::IdComponent i = 0; i < numArrays; ++i) + for (vtkm::IdComponent fieldIndex = 0; fieldIndex < numArrays; ++fieldIndex) { std::vector pointarray; std::vector cellarray; - for (vtkm::IdComponent j = 0; j < arrayLen; ++j) + for (vtkm::Id valueIndex = 0; valueIndex < arrayLen; ++valueIndex) { - pointarray.push_back(static_cast(i * 1.1 + j * 1.1)); - cellarray.push_back(static_cast(i * 2.1 + j * 2.1)); + pointarray.push_back(static_cast(fieldIndex * 1.1 + valueIndex * 1.1)); + cellarray.push_back(static_cast(fieldIndex * 2.1 + valueIndex * 2.1)); } - dataSet.AddPointField("pointArray" + std::to_string(i), pointarray); - dataSet.AddCellField("cellArray" + std::to_string(i), cellarray); + dataSet.AddPointField("pointArray" + std::to_string(fieldIndex), pointarray); + dataSet.AddCellField("cellArray" + std::to_string(fieldIndex), cellarray); } return dataSet; @@ -44,7 +44,7 @@ void CheckResults(vtkm::cont::DataSet inDataSet, const std::string compositedName) { //there are only three fields for this testing , it is ok to use vtkm::IdComponent - vtkm::IdComponent dims = static_cast(FieldNames.size()); + vtkm::IdComponent numComponents = static_cast(FieldNames.size()); auto compositedField = inDataSet.GetField(compositedName); vtkm::IdComponent compositedFieldLen = static_cast(compositedField.GetNumberOfValues()); @@ -55,9 +55,9 @@ void CheckResults(vtkm::cont::DataSet inDataSet, auto compFieldReadPortal = compFieldArrayCopy.ReadPortal(); - for (vtkm::IdComponent i = 0; i < dims; i++) + for (vtkm::IdComponent componentIndex = 0; componentIndex < numComponents; componentIndex++) { - auto field = inDataSet.GetField(FieldNames[i]); + auto field = inDataSet.GetField(FieldNames[componentIndex]); VTKM_TEST_ASSERT(compositedField.GetAssociation() == field.GetAssociation(), "Got bad association value."); @@ -68,11 +68,11 @@ void CheckResults(vtkm::cont::DataSet inDataSet, field.GetData().AsArrayHandle(fieldArrayHandle); auto fieldReadPortal = fieldArrayHandle.ReadPortal(); - for (vtkm::IdComponent j = 0; j < fieldLen; j++) + for (vtkm::Id valueIndex = 0; valueIndex < fieldLen; valueIndex++) { - auto compFieldVec = compFieldReadPortal.Get(j); - auto comFieldValue = compFieldVec[static_cast(i)]; - auto fieldValue = fieldReadPortal.Get(j); + auto compFieldVec = compFieldReadPortal.Get(valueIndex); + auto comFieldValue = compFieldVec[static_cast(componentIndex)]; + auto fieldValue = fieldReadPortal.Get(valueIndex); VTKM_TEST_ASSERT(comFieldValue == fieldValue, "Got bad field value."); } } @@ -80,27 +80,30 @@ void CheckResults(vtkm::cont::DataSet inDataSet, template -void TestCompositeVectors(vtkm::IdComponent dim) +void TestCompositeVectors(vtkm::IdComponent numComponents) { - vtkm::cont::DataSet inDataSet = MakeDataSet(dim); + vtkm::cont::DataSet inDataSet = MakeDataSet(numComponents); vtkm::filter::field_transform::CompositeVectors filter; + // For the first pass (point field), we are going to use the generic `SetActiveField` method + // and let the filter figure out how many fields. std::vector pointFieldNames; - for (vtkm::IdComponent i = 0; i < dim; i++) + for (vtkm::IdComponent componentIndex = 0; componentIndex < numComponents; componentIndex++) { - pointFieldNames.push_back("pointArray" + std::to_string(i)); + pointFieldNames.push_back("pointArray" + std::to_string(componentIndex)); + filter.SetActiveField(componentIndex, "pointArray" + std::to_string(componentIndex)); } - filter.SetFieldNameList(pointFieldNames, vtkm::cont::Field::Association::Points); filter.SetOutputFieldName("CompositedFieldPoint"); vtkm::cont::DataSet outDataSetPointAssoci = filter.Execute(inDataSet); CheckResults( outDataSetPointAssoci, pointFieldNames, filter.GetOutputFieldName()); + // For the second pass (cell field), we will use the `SetFieldNameList` method. std::vector cellFieldNames; - for (vtkm::IdComponent i = 0; i < dim; i++) + for (vtkm::IdComponent componentIndex = 0; componentIndex < numComponents; componentIndex++) { - cellFieldNames.push_back("cellArray" + std::to_string(i)); + cellFieldNames.push_back("cellArray" + std::to_string(componentIndex)); } filter.SetFieldNameList(cellFieldNames, vtkm::cont::Field::Association::Cells); filter.SetOutputFieldName("CompositedFieldCell"); @@ -114,8 +117,10 @@ void CompositeVectors() { TestCompositeVectors(2); TestCompositeVectors(3); + TestCompositeVectors>(5); TestCompositeVectors(2); TestCompositeVectors(3); + TestCompositeVectors>(5); } } // anonymous namespace From ab2792b78a7595833da7f3b12461a9ffee49c099 Mon Sep 17 00:00:00 2001 From: Vicente Adolfo Bolea Sanchez Date: Fri, 21 Apr 2023 19:19:56 -0400 Subject: [PATCH 16/22] perftest: no mark disabled upload steps in MR - Removed the PerformanceTestCleanUp, not needed since we only do a git fetch once. --- CMake/testing/VTKmPerformanceTest.cmake | 50 ++++++++++++------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/CMake/testing/VTKmPerformanceTest.cmake b/CMake/testing/VTKmPerformanceTest.cmake index 88f22ede6..3aed96b5c 100644 --- a/CMake/testing/VTKmPerformanceTest.cmake +++ b/CMake/testing/VTKmPerformanceTest.cmake @@ -76,6 +76,14 @@ function(add_benchmark_test benchmark) set(VTKm_PERF_COMPARE_JSON "${CMAKE_BINARY_DIR}/$ENV{CI_COMMIT_SHA}_${VTKm_PERF_NAME}.json") endif() + # Only upload when we are inside a CI build and in master. We need to check + # if VTKM_BENCH_RECORDS_TOKEN is either defined or non-empty, the reason is + # that in Gitlab CI Variables for protected branches are also defined in MR + # from forks, however, they are empty. + if (DEFINED ENV{VTKM_BENCH_RECORDS_TOKEN} AND ENV{VTKM_BENCH_RECORDS_TOKEN}) + set(enable_upload TRUE) + endif() + set(test_name "PerformanceTest${VTKm_PERF_NAME}") ###TEST INVOKATIONS########################################################## @@ -90,13 +98,6 @@ function(add_benchmark_test benchmark) set_property(TEST PerformanceTestFetch PROPERTY FIXTURES_SETUP "FixturePerformanceTestSetup") endif() - if (NOT TEST PerformanceTestCleanUp) - add_test(NAME "PerformanceTestCleanUp" - COMMAND ${CMAKE_COMMAND} -E rm -rf "${VTKm_PERF_REPO}" - ) - set_property(TEST PerformanceTestCleanUp PROPERTY FIXTURES_CLEANUP "FixturePerformanceTestCleanUp") - endif() - add_test(NAME "${test_name}Run" COMMAND ${CMAKE_COMMAND} "-DVTKm_PERF_BENCH_DEVICE=Any" @@ -111,14 +112,6 @@ function(add_benchmark_test benchmark) -P "${VTKm_SOURCE_DIR}/CMake/testing/VTKmPerformanceTestRun.cmake" ) - add_test(NAME "${test_name}Upload" - COMMAND ${CMAKE_COMMAND} - "-DVTKm_PERF_REPO=${VTKm_PERF_REPO}" - "-DVTKm_PERF_COMPARE_JSON=${VTKm_PERF_COMPARE_JSON}" - "-DVTKm_SOURCE_DIR=${VTKm_SOURCE_DIR}" - -P "${VTKm_SOURCE_DIR}/CMake/testing/VTKmPerformanceTestUpload.cmake" - ) - add_test(NAME "${test_name}Report" COMMAND ${CMAKE_COMMAND} "-DVTKm_BINARY_DIR=${VTKm_BINARY_DIR}" @@ -132,27 +125,34 @@ function(add_benchmark_test benchmark) -P "${VTKm_SOURCE_DIR}/CMake/testing/VTKmPerformanceTestReport.cmake" ) + if (enable_upload) + add_test(NAME "${test_name}Upload" + COMMAND ${CMAKE_COMMAND} + "-DVTKm_PERF_REPO=${VTKm_PERF_REPO}" + "-DVTKm_PERF_COMPARE_JSON=${VTKm_PERF_COMPARE_JSON}" + "-DVTKm_SOURCE_DIR=${VTKm_SOURCE_DIR}" + -P "${VTKm_SOURCE_DIR}/CMake/testing/VTKmPerformanceTestUpload.cmake" + ) + + set_tests_properties("${test_name}Upload" PROPERTIES + DEPENDS ${test_name}Report + FIXTURES_REQUIRED "FixturePerformanceTestCleanUp" + REQUIRED_FILES "${VTKm_PERF_COMPARE_JSON}" + RUN_SERIAL ON) + endif() + ###TEST PROPERTIES########################################################### - set_property(TEST ${test_name}Upload PROPERTY DEPENDS ${test_name}Report) set_property(TEST ${test_name}Report PROPERTY DEPENDS ${test_name}Run) set_property(TEST ${test_name}Report PROPERTY FIXTURES_REQUIRED "FixturePerformanceTestSetup") - set_property(TEST ${test_name}Upload PROPERTY FIXTURES_REQUIRED "FixturePerformanceTestCleanUp") - set_tests_properties("${test_name}Report" "${test_name}Upload" + set_tests_properties("${test_name}Report" PROPERTIES REQUIRED_FILES "${VTKm_PERF_COMPARE_JSON}") set_tests_properties("${test_name}Run" "${test_name}Report" - "${test_name}Upload" "PerformanceTestFetch" - "PerformanceTestCleanUp" PROPERTIES RUN_SERIAL ON) set_tests_properties(${test_name}Run PROPERTIES TIMEOUT 1800) - - # Only upload when we are inside a CI build - if (NOT DEFINED ENV{CI_COMMIT_SHA} OR NOT DEFINED ENV{VTKM_BENCH_RECORDS_TOKEN}) - set_tests_properties(${test_name}Upload PROPERTIES DISABLED TRUE) - endif() endfunction() From bb9e7a0d6f8ea2706f7e2e38c17cfee21467fdbe Mon Sep 17 00:00:00 2001 From: Kenneth Moreland Date: Mon, 17 Apr 2023 11:41:18 -0600 Subject: [PATCH 17/22] Handle any Vec size in VTKDataSetReader The legacy VTK file reader previously only supported a specific set of Vec lengths (i.e., 1, 2, 3, 4, 6, and 9). This is because a basic array handle has to have the vec length compiled in. However, the new `ArrayHandleRuntimeVec` feature is capable of reading in any vec-length and can be leveraged to read in arbitrarily sized vectors in field arrays. --- docs/changelog/read-any-vec-size.md | 7 + vtkm/cont/ArrayHandleRuntimeVec.h | 67 ++++++- vtkm/io/VTKDataSetReaderBase.cxx | 200 ++----------------- vtkm/io/internal/VTKDataSetTypes.h | 52 ++--- vtkm/io/testing/UnitTestVTKDataSetWriter.cxx | 22 +- 5 files changed, 122 insertions(+), 226 deletions(-) create mode 100644 docs/changelog/read-any-vec-size.md diff --git a/docs/changelog/read-any-vec-size.md b/docs/changelog/read-any-vec-size.md new file mode 100644 index 000000000..df6ff8204 --- /dev/null +++ b/docs/changelog/read-any-vec-size.md @@ -0,0 +1,7 @@ +# VTKDataSetReader handles any Vec size. + +The legacy VTK file reader previously only supported a specific set of Vec +lengths (i.e., 1, 2, 3, 4, 6, and 9). This is because a basic array handle +has to have the vec length compiled in. However, the new +`ArrayHandleRuntimeVec` feature is capable of reading in any vec-length and +can be leveraged to read in arbitrarily sized vectors in field arrays. diff --git a/vtkm/cont/ArrayHandleRuntimeVec.h b/vtkm/cont/ArrayHandleRuntimeVec.h index c7ef442fc..eab0c3f18 100644 --- a/vtkm/cont/ArrayHandleRuntimeVec.h +++ b/vtkm/cont/ArrayHandleRuntimeVec.h @@ -372,10 +372,13 @@ public: ///@} }; -/// \c make_ArrayHandleRuntimeVec is convenience function to generate an -/// ArrayHandleRuntimeVec. It takes in an ArrayHandle of values and an -/// array handle of offsets and returns an array handle with consecutive -/// entries grouped in a Vec. +/// `make_ArrayHandleRuntimeVec` is convenience function to generate an +/// `ArrayHandleRuntimeVec`. It takes the number of components stored in +/// each value's `Vec`, which must be specified on the construction of +/// the `ArrayHandleRuntimeVec`. If not specified, the number of components +/// is set to 1. `make_ArrayHandleRuntimeVec` can also optionally take an +/// existing array of components, which will be grouped into `Vec` values +/// based on the specified number of components. /// template VTKM_CONT auto make_ArrayHandleRuntimeVec( @@ -402,6 +405,62 @@ VTKM_CONT auto make_ArrayHandleRuntimeVec( return make_ArrayHandleRuntimeVec(1, componentsArray); } +/// A convenience function for creating an `ArrayHandleRuntimeVec` from a standard C array. +/// +template +VTKM_CONT auto make_ArrayHandleRuntimeVec(vtkm::IdComponent numComponents, + const T* array, + vtkm::Id numberOfValues, + vtkm::CopyFlag copy) +{ + return make_ArrayHandleRuntimeVec(numComponents, + vtkm::cont::make_ArrayHandle(array, numberOfValues, copy)); +} + +/// A convenience function to move a user-allocated array into an `ArrayHandleRuntimeVec`. +/// The provided array pointer will be reset to `nullptr`. +/// If the array was not allocated with the `new[]` operator, then deleter and reallocater +/// functions must be provided. +/// +template +VTKM_CONT auto make_ArrayHandleRuntimeVecMove( + vtkm::IdComponent numComponents, + T*& array, + vtkm::Id numberOfValues, + vtkm::cont::internal::BufferInfo::Deleter deleter = internal::SimpleArrayDeleter, + vtkm::cont::internal::BufferInfo::Reallocater reallocater = internal::SimpleArrayReallocater) +{ + return make_ArrayHandleRuntimeVec( + numComponents, vtkm::cont::make_ArrayHandleMove(array, numberOfValues, deleter, reallocater)); +} + +/// A convenience function for creating an `ArrayHandleRuntimeVec` from an `std::vector`. +/// +template +VTKM_CONT auto make_ArrayHandleRuntimeVec(vtkm::IdComponent numComponents, + const std::vector& array, + vtkm::CopyFlag copy) +{ + return make_ArrayHandleRuntimeVec(numComponents, vtkm::cont::make_ArrayHandle(array, copy)); +} + +/// Move an `std::vector` into an `ArrayHandleRuntimeVec`. +/// +template +VTKM_CONT auto make_ArrayHandleRuntimeVecMove(vtkm::IdComponent numComponents, + std::vector&& array) +{ + return make_ArrayHandleRuntimeVec(numComponents, make_ArrayHandleMove(std::move(array))); +} + +template +VTKM_CONT auto make_ArrayHandleRuntimeVec(vtkm::IdComponent numComponents, + std::vector&& array, + vtkm::CopyFlag vtkmNotUsed(copy)) +{ + return make_ArrayHandleRuntimeVecMove(numComponents, std::move(array)); +} + namespace internal { diff --git a/vtkm/io/VTKDataSetReaderBase.cxx b/vtkm/io/VTKDataSetReaderBase.cxx index 03f28c7bd..82605cadd 100644 --- a/vtkm/io/VTKDataSetReaderBase.cxx +++ b/vtkm/io/VTKDataSetReaderBase.cxx @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -35,162 +36,6 @@ inline void PrintVTKDataFileSummary(const vtkm::io::internal::VTKDataSetFile& df << std::endl; } -// Since Fields and DataSets store data in the default UnknownArrayHandle, convert -// the data to the closest type supported by default. The following will -// need to be updated if UnknownArrayHandle or TypeListCommon changes. -template -struct ClosestCommonType -{ - using Type = T; -}; -template <> -struct ClosestCommonType -{ - using Type = vtkm::Int32; -}; -template <> -struct ClosestCommonType -{ - using Type = vtkm::Int32; -}; -template <> -struct ClosestCommonType -{ - using Type = vtkm::Int32; -}; -template <> -struct ClosestCommonType -{ - using Type = vtkm::Int32; -}; -template <> -struct ClosestCommonType -{ - using Type = vtkm::Int64; -}; -template <> -struct ClosestCommonType -{ - using Type = vtkm::Int64; -}; - -template -struct ClosestFloat -{ - using Type = T; -}; -template <> -struct ClosestFloat -{ - using Type = vtkm::Float32; -}; -template <> -struct ClosestFloat -{ - using Type = vtkm::Float32; -}; -template <> -struct ClosestFloat -{ - using Type = vtkm::Float32; -}; -template <> -struct ClosestFloat -{ - using Type = vtkm::Float32; -}; -template <> -struct ClosestFloat -{ - using Type = vtkm::Float64; -}; -template <> -struct ClosestFloat -{ - using Type = vtkm::Float64; -}; -template <> -struct ClosestFloat -{ - using Type = vtkm::Float64; -}; -template <> -struct ClosestFloat -{ - using Type = vtkm::Float64; -}; - -template -vtkm::cont::UnknownArrayHandle CreateUnknownArrayHandle(const std::vector& vec) -{ - switch (vtkm::VecTraits::NUM_COMPONENTS) - { - case 1: - { - using CommonType = typename ClosestCommonType::Type; - constexpr bool not_same = !std::is_same::value; - if (not_same) - { - VTKM_LOG_S(vtkm::cont::LogLevel::Info, - "Type " << vtkm::io::internal::DataTypeName::Name() - << " is currently unsupported. Converting to " - << vtkm::io::internal::DataTypeName::Name() << "."); - } - - vtkm::cont::ArrayHandle output; - output.Allocate(static_cast(vec.size())); - auto portal = output.WritePortal(); - for (vtkm::Id i = 0; i < output.GetNumberOfValues(); ++i) - { - portal.Set(i, static_cast(vec[static_cast(i)])); - } - - return vtkm::cont::UnknownArrayHandle(output); - } - case 2: - case 3: - case 9: - { - constexpr auto numComps = vtkm::VecTraits::NUM_COMPONENTS; - - using InComponentType = typename vtkm::VecTraits::ComponentType; - using OutComponentType = typename ClosestFloat::Type; - using CommonType = vtkm::Vec; - constexpr bool not_same = !std::is_same::value; - if (not_same) - { - VTKM_LOG_S(vtkm::cont::LogLevel::Info, - "Type " << vtkm::io::internal::DataTypeName::Name() << "[" - << vtkm::VecTraits::GetNumberOfComponents(T()) << "] " - << "is currently unsupported. Converting to " - << vtkm::io::internal::DataTypeName::Name() << "[" - << numComps << "]."); - } - - vtkm::cont::ArrayHandle output; - output.Allocate(static_cast(vec.size())); - auto portal = output.WritePortal(); - for (vtkm::Id i = 0; i < output.GetNumberOfValues(); ++i) - { - CommonType outval = CommonType(); - for (vtkm::IdComponent j = 0; j < numComps; ++j) - { - outval[j] = static_cast( - vtkm::VecTraits::GetComponent(vec[static_cast(i)], j)); - } - portal.Set(i, outval); - } - - return vtkm::cont::UnknownArrayHandle(output); - } - default: - { - VTKM_LOG_S(vtkm::cont::LogLevel::Warn, "Only 1, 2, 3, or 9 components supported. Skipping."); - return vtkm::cont::UnknownArrayHandle(vtkm::cont::ArrayHandle()); - } - } -} - } // anonymous namespace namespace vtkm @@ -676,27 +521,23 @@ void VTKDataSetReaderBase::ReadGlobalOrPedigreeIds(vtkm::cont::Field::Associatio class VTKDataSetReaderBase::SkipArrayVariant { public: - SkipArrayVariant(VTKDataSetReaderBase* reader, std::size_t numElements) + SkipArrayVariant(VTKDataSetReaderBase* reader, + std::size_t numElements, + vtkm::IdComponent numComponents) : Reader(reader) - , NumElements(numElements) + , TotalSize(numElements * static_cast(numComponents)) { } template void operator()(T) const { - this->Reader->SkipArray(this->NumElements, T()); - } - - template - void operator()(vtkm::IdComponent numComponents, T) const - { - this->Reader->SkipArray(this->NumElements * static_cast(numComponents), T()); + this->Reader->SkipArray(this->TotalSize, T()); } protected: VTKDataSetReaderBase* Reader; - std::size_t NumElements; + std::size_t TotalSize; }; class VTKDataSetReaderBase::ReadArrayVariant : public SkipArrayVariant @@ -705,9 +546,11 @@ public: ReadArrayVariant(VTKDataSetReaderBase* reader, vtkm::cont::Field::Association association, std::size_t numElements, + vtkm::IdComponent numComponents, vtkm::cont::UnknownArrayHandle& data) - : SkipArrayVariant(reader, numElements) + : SkipArrayVariant(reader, numElements, numComponents) , Association(association) + , NumComponents(numComponents) , Data(&data) { } @@ -715,12 +558,13 @@ public: template void operator()(T) const { - std::vector buffer(this->NumElements); + std::vector buffer(this->TotalSize); this->Reader->ReadArray(buffer); if ((this->Association != vtkm::cont::Field::Association::Cells) || (this->Reader->GetCellsPermutation().GetNumberOfValues() < 1)) { - *this->Data = CreateUnknownArrayHandle(buffer); + *this->Data = + vtkm::cont::make_ArrayHandleRuntimeVecMove(this->NumComponents, std::move(buffer)); } else { @@ -734,20 +578,14 @@ public: std::size_t inIndex = static_cast(permutation.Get(outIndex)); permutedBuffer[static_cast(outIndex)] = buffer[inIndex]; } - *this->Data = CreateUnknownArrayHandle(permutedBuffer); + *this->Data = + vtkm::cont::make_ArrayHandleRuntimeVecMove(this->NumComponents, std::move(permutedBuffer)); } } - template - void operator()(vtkm::IdComponent numComponents, T) const - { - VTKM_LOG_S(vtkm::cont::LogLevel::Warn, - "Support for " << numComponents << " components not implemented. Skipping."); - SkipArrayVariant::operator()(numComponents, T()); - } - private: vtkm::cont::Field::Association Association; + vtkm::IdComponent NumComponents; vtkm::cont::UnknownArrayHandle* Data; }; @@ -764,8 +602,8 @@ void VTKDataSetReaderBase::DoSkipArrayVariant(std::string dataType, else { vtkm::io::internal::DataType typeId = vtkm::io::internal::DataTypeId(dataType); - vtkm::io::internal::SelectTypeAndCall( - typeId, numComponents, SkipArrayVariant(this, numElements)); + vtkm::io::internal::SelectTypeAndCall(typeId, + SkipArrayVariant(this, numElements, numComponents)); } } @@ -791,7 +629,7 @@ vtkm::cont::UnknownArrayHandle VTKDataSetReaderBase::DoReadArrayVariant( { vtkm::io::internal::DataType typeId = vtkm::io::internal::DataTypeId(dataType); vtkm::io::internal::SelectTypeAndCall( - typeId, numComponents, ReadArrayVariant(this, association, numElements, data)); + typeId, ReadArrayVariant(this, association, numElements, numComponents, data)); } return data; diff --git a/vtkm/io/internal/VTKDataSetTypes.h b/vtkm/io/internal/VTKDataSetTypes.h index 483573a6f..df7ad903c 100644 --- a/vtkm/io/internal/VTKDataSetTypes.h +++ b/vtkm/io/internal/VTKDataSetTypes.h @@ -170,73 +170,45 @@ struct DataTypeName static const char* Name() { return "double"; } }; -template -inline void SelectVecTypeAndCall(T, vtkm::IdComponent numComponents, const Functor& functor) -{ - switch (numComponents) - { - case 1: - functor(T()); - break; - case 2: - functor(vtkm::Vec()); - break; - case 3: - functor(vtkm::Vec()); - break; - case 4: - functor(vtkm::Vec()); - break; - case 9: - functor(vtkm::Vec()); - break; - default: - functor(numComponents, T()); - break; - } -} - template -inline void SelectTypeAndCall(DataType dtype, - vtkm::IdComponent numComponents, - const Functor& functor) +inline void SelectTypeAndCall(DataType dtype, Functor&& functor) { switch (dtype) { case DTYPE_BIT: - SelectVecTypeAndCall(DummyBitType(), numComponents, functor); + functor(DummyBitType()); break; case DTYPE_UNSIGNED_CHAR: - SelectVecTypeAndCall(vtkm::UInt8(), numComponents, functor); + functor(vtkm::UInt8()); break; case DTYPE_CHAR: - SelectVecTypeAndCall(vtkm::Int8(), numComponents, functor); + functor(vtkm::Int8()); break; case DTYPE_UNSIGNED_SHORT: - SelectVecTypeAndCall(vtkm::UInt16(), numComponents, functor); + functor(vtkm::UInt16()); break; case DTYPE_SHORT: - SelectVecTypeAndCall(vtkm::Int16(), numComponents, functor); + functor(vtkm::Int16()); break; case DTYPE_UNSIGNED_INT: - SelectVecTypeAndCall(vtkm::UInt32(), numComponents, functor); + functor(vtkm::UInt32()); break; case DTYPE_INT: - SelectVecTypeAndCall(vtkm::Int32(), numComponents, functor); + functor(vtkm::Int32()); break; case DTYPE_UNSIGNED_LONG: case DTYPE_UNSIGNED_LONG_LONG: - SelectVecTypeAndCall(vtkm::UInt64(), numComponents, functor); + functor(vtkm::UInt64()); break; case DTYPE_LONG: case DTYPE_LONG_LONG: - SelectVecTypeAndCall(vtkm::Int64(), numComponents, functor); + functor(vtkm::Int64()); break; case DTYPE_FLOAT: - SelectVecTypeAndCall(vtkm::Float32(), numComponents, functor); + functor(vtkm::Float32()); break; case DTYPE_DOUBLE: - SelectVecTypeAndCall(vtkm::Float64(), numComponents, functor); + functor(vtkm::Float64()); break; default: assert(false); diff --git a/vtkm/io/testing/UnitTestVTKDataSetWriter.cxx b/vtkm/io/testing/UnitTestVTKDataSetWriter.cxx index 06caecd69..c7498c3cd 100644 --- a/vtkm/io/testing/UnitTestVTKDataSetWriter.cxx +++ b/vtkm/io/testing/UnitTestVTKDataSetWriter.cxx @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -122,7 +123,7 @@ void CheckWrittenReadData(const vtkm::cont::DataSet& originalData, VTKM_TEST_ASSERT(fileData.HasField(originalField.GetName(), originalField.GetAssociation())); vtkm::cont::Field fileField = fileData.GetField(originalField.GetName(), originalField.GetAssociation()); - vtkm::cont::CastAndCall(originalField, CheckSameField{}, fileField); + VTKM_TEST_ASSERT(test_equal_ArrayHandles(originalField.GetData(), fileField.GetData())); } VTKM_TEST_ASSERT(fileData.GetNumberOfCoordinateSystems() > 0); @@ -261,12 +262,31 @@ void TestVTKCompoundWrite() std::remove("chirp.vtk"); } +void TestVTKOddVecSizes() +{ + vtkm::cont::DataSetBuilderUniform dsb; + vtkm::cont::DataSet dataSet = dsb.Create({ 2, 2, 2 }); + + vtkm::cont::ArrayHandle> vec5Array; + vec5Array.Allocate(dataSet.GetNumberOfPoints()); + SetPortal(vec5Array.WritePortal()); + dataSet.AddPointField("vec5", vec5Array); + + vtkm::cont::ArrayHandleSOA> vec13Array; + vec13Array.Allocate(dataSet.GetNumberOfPoints()); + SetPortal(vec13Array.WritePortal()); + dataSet.AddPointField("vec13", vec13Array); + + TestVTKWriteTestData("OddVecSizes", dataSet); +} + void TestVTKWrite() { TestVTKExplicitWrite(); TestVTKUniformWrite(); TestVTKRectilinearWrite(); TestVTKCompoundWrite(); + TestVTKOddVecSizes(); } } //Anonymous namespace From c802adcbebfb795c8e434da5d584adfedfa9f4a9 Mon Sep 17 00:00:00 2001 From: Kenneth Moreland Date: Mon, 22 May 2023 09:26:06 -0600 Subject: [PATCH 18/22] Add support for CastAndCallVariableVecField in FilterField The `FilterField` class provides convenience functions for subclasses to determine the `ArrayHandle` type for scalar and vector fields. However, you needed to know the specific size of vectors. For filters that support an input field of any type, a new form, `CastAndCallVariableVecField` has been added. This calls the underlying functor with an `ArrayHandleRecombineVec` of the appropriate component type. The `CastAndaCallVariableVecField` method also reduces the number of instances created by having a float fallback for any component type that does not satisfy the field types. --- docs/changelog/cast-and-call-variable-vec.md | 12 ++++ vtkm/filter/FilterField.h | 71 ++++++++++++++++++++ vtkm/filter/vector_analysis/DotProduct.cxx | 24 ++----- 3 files changed, 88 insertions(+), 19 deletions(-) create mode 100644 docs/changelog/cast-and-call-variable-vec.md diff --git a/docs/changelog/cast-and-call-variable-vec.md b/docs/changelog/cast-and-call-variable-vec.md new file mode 100644 index 000000000..d089dad77 --- /dev/null +++ b/docs/changelog/cast-and-call-variable-vec.md @@ -0,0 +1,12 @@ +# Added support for `CastAndCallVariableVecField` in `FilterField` + +The `FilterField` class provides convenience functions for subclasses to +determine the `ArrayHandle` type for scalar and vector fields. However, you +needed to know the specific size of vectors. For filters that support an +input field of any type, a new form, `CastAndCallVariableVecField` has been +added. This calls the underlying functor with an `ArrayHandleRecombineVec` +of the appropriate component type. + +The `CastAndaCallVariableVecField` method also reduces the number of +instances created by having a float fallback for any component type that +does not satisfy the field types. diff --git a/vtkm/filter/FilterField.h b/vtkm/filter/FilterField.h index 0005fd4ac..aa89f5943 100644 --- a/vtkm/filter/FilterField.h +++ b/vtkm/filter/FilterField.h @@ -13,6 +13,8 @@ #include +#include + namespace vtkm { namespace filter @@ -160,6 +162,16 @@ protected: } } + ///@{ + /// \brief Convenience method to get the array from a filter's input scalar field. + /// + /// A field filter typically gets its input fields using the internal `GetFieldFromDataSet`. + /// To use this field in a worklet, it eventually needs to be converted to an + /// `ArrayHandle`. If the input field is limited to be a scalar field, then this method + /// provides a convenient way to determine the correct array type. Like other `CastAndCall` + /// methods, it takes as input a `Field` (or `UnknownArrayHandle`) and a function/functor + /// to call with the appropriate `ArrayHandle` type. + /// template VTKM_CONT void CastAndCallScalarField(const vtkm::cont::UnknownArrayHandle& fieldArray, Functor&& functor, @@ -178,6 +190,7 @@ protected: this->CastAndCallScalarField( field.GetData(), std::forward(functor), std::forward(args)...); } + ///@} private: @@ -189,6 +202,18 @@ private: }; protected: + ///@{ + /// \brief Convenience method to get the array from a filter's input vector field. + /// + /// A field filter typically gets its input fields using the internal `GetFieldFromDataSet`. + /// To use this field in a worklet, it eventually needs to be converted to an + /// `ArrayHandle`. If the input field is limited to be a vector field with vectors of a + /// specific size, then this method provides a convenient way to determine the correct array + /// type. Like other `CastAndCall` methods, it takes as input a `Field` (or + /// `UnknownArrayHandle`) and a function/functor to call with the appropriate `ArrayHandle` + /// type. You also have to provide the vector size as the first template argument. + /// For example `CastAndCallVecField<3>(field, functor);`. + /// template VTKM_CONT void CastAndCallVecField(const vtkm::cont::UnknownArrayHandle& fieldArray, Functor&& functor, @@ -208,6 +233,52 @@ protected: this->CastAndCallVecField( field.GetData(), std::forward(functor), std::forward(args)...); } + ///@} + + ///@{ + /// This method is like `CastAndCallVecField` except that it can be used for a + /// field of unknown vector size (or scalars). This method will call the given + /// functor with an `ArrayHandleRecombineVec`. + /// + /// Note that there are limitations with using `ArrayHandleRecombineVec` within a + /// worklet. Because the size of the vectors are not known at compile time, you + /// cannot just create an intermediate `Vec` of the correct size. Typically, you + /// must allocate the output array (for example, with `ArrayHandleRuntimeVec`), and + /// the worklet must iterate over the components and store them in the prealocated + /// output. + /// + template + VTKM_CONT void CastAndCallVariableVecField(const vtkm::cont::UnknownArrayHandle& fieldArray, + Functor&& functor, + Args&&... args) const + { + if (fieldArray.IsBaseComponentType()) + { + functor(fieldArray.ExtractArrayFromComponents(), std::forward(args)...); + } + else if (fieldArray.IsBaseComponentType()) + { + functor(fieldArray.ExtractArrayFromComponents(), std::forward(args)...); + } + else + { + // Field component type is not directly supported. Copy to floating point array. + vtkm::cont::UnknownArrayHandle floatArray = fieldArray.NewInstanceFloatBasic(); + vtkm::cont::ArrayCopy(fieldArray, floatArray); + functor(floatArray.ExtractArrayFromComponents(), + std::forward(args)...); + } + } + + template + VTKM_CONT void CastAndCallVariableVecField(const vtkm::cont::Field& field, + Functor&& functor, + Args&&... args) const + { + this->CastAndCallVariableVecField( + field.GetData(), std::forward(functor), std::forward(args)...); + } + ///@} /// \brief Create the output data set for `DoExecute` /// diff --git a/vtkm/filter/vector_analysis/DotProduct.cxx b/vtkm/filter/vector_analysis/DotProduct.cxx index 14eacd949..c96202871 100644 --- a/vtkm/filter/vector_analysis/DotProduct.cxx +++ b/vtkm/filter/vector_analysis/DotProduct.cxx @@ -78,11 +78,9 @@ VTKM_CONT DotProduct::DotProduct() VTKM_CONT vtkm::cont::DataSet DotProduct::DoExecute(const vtkm::cont::DataSet& inDataSet) { vtkm::cont::Field primaryField = this->GetFieldFromDataSet(0, inDataSet); - vtkm::cont::UnknownArrayHandle primaryArray = primaryField.GetData(); - vtkm::cont::Field secondaryField = this->GetFieldFromDataSet(1, inDataSet); - if (primaryArray.GetNumberOfComponentsFlat() != + if (primaryField.GetData().GetNumberOfComponentsFlat() != secondaryField.GetData().GetNumberOfComponentsFlat()) { throw vtkm::cont::ErrorFilterExecution( @@ -91,22 +89,10 @@ VTKM_CONT vtkm::cont::DataSet DotProduct::DoExecute(const vtkm::cont::DataSet& i vtkm::cont::UnknownArrayHandle outArray; - if (primaryArray.IsBaseComponentType()) - { - outArray = - DoDotProduct(primaryArray.ExtractArrayFromComponents(), secondaryField); - } - else if (primaryArray.IsBaseComponentType()) - { - outArray = - DoDotProduct(primaryArray.ExtractArrayFromComponents(), secondaryField); - } - else - { - primaryArray = primaryField.GetDataAsDefaultFloat(); - outArray = - DoDotProduct(primaryArray.ExtractArrayFromComponents(), secondaryField); - } + auto resolveArray = [&](const auto& concretePrimaryField) { + outArray = DoDotProduct(concretePrimaryField, secondaryField); + }; + this->CastAndCallVariableVecField(primaryField, resolveArray); return this->CreateResultField(inDataSet, this->GetOutputFieldName(), From fc9085367388d29250735999883636dc93e5c83f Mon Sep 17 00:00:00 2001 From: Vicente Adolfo Bolea Sanchez Date: Wed, 24 May 2023 19:51:51 -0400 Subject: [PATCH 19/22] cmake: only set lfs.url when using ssh URL --- Utilities/SetupForDevelopment.sh | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/Utilities/SetupForDevelopment.sh b/Utilities/SetupForDevelopment.sh index fdb043448..8d5486d09 100755 --- a/Utilities/SetupForDevelopment.sh +++ b/Utilities/SetupForDevelopment.sh @@ -29,16 +29,18 @@ git config alias.gitlab-sync '!bash Utilities/GitSetup/git-gitlab-sync' && echo "Set up git gitlab-sync" && true +# shellcheck disable=SC2034 SetupForDevelopment=1 -git config hooks.SetupForDevelopment ${SetupForDevelopment_VERSION} +# shellcheck disable=SC2154 +git config hooks.SetupForDevelopment "${SetupForDevelopment_VERSION}" # Setup VTK-m-specifc LFS config # -# Disable lfsurl if our origin points to the main repo -OriginURL=$(git remote get-url origin) -if [[ "$OriginURL" =~ ^(https://|git@)gitlab\.kitware\.com(/|:)vtk/vtk-m\.git$ ]] +# Only set lfs.url to the ssh url +OriginURL="$(git remote get-url origin)" +if [[ "$OriginURL" =~ ^git@gitlab\.kitware\.com:vtk/vtk-m\.git$ ]] then - # Disable this setting which overrides every remote/url lfs setting + # This setting overrides every remote/url lfs setting git config --local lfs.url "${OriginURL}" # Those settings are only available for newer git-lfs releases From 6df195ed0dd846b1220a051c4cc286c7de2306ab Mon Sep 17 00:00:00 2001 From: Vicente Bolea Date: Wed, 24 May 2023 20:01:27 -0400 Subject: [PATCH 20/22] Update file CONTRIBUTING.md --- CONTRIBUTING.md | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b95e7ba62..b1c94b31a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -19,7 +19,7 @@ Before you begin, perform initial setup: 3. Use Git to create a local clone of the main VTK repository: - $ git clone https://gitlab.kitware.com/vtk/vtk-m.git + $ git clone git@gitlab.kitware.com:vtk/vtk-m.git $ cd vtk-m The main repository will be configured as your `origin` remote. @@ -32,19 +32,24 @@ Before you begin, perform initial setup: This will prompt for your GitLab user name and configure a remote called `gitlab` to refer to it. -5. (Optional but highly recommended.) +5. (Required to use Git LFS.) + [Using the SSH URL for the origin remote] is needed to for Git LFS to work. + Do not forget to run `$ ./Utilities/SetupForDevelopment.sh` after changing + the remote URL. + +6. (Optional but highly recommended.) [Disabling git-lfs in your fork] is needed to add/modify git-lfs files. Find the setting to disable git-lfs in your fork through your fork web UI: Settings/General/Project Features/Git LFS; set it to off; and _save changes_. -6. (Optional but highly recommended.) +7. (Optional but highly recommended.) [Register with the VTK-m dashboard] on Kitware's CDash instance to better know how your code performs in regression tests. After registering and signing in, click on "All Dashboards" link in the upper left corner, scroll down and click "Subscribe to this project" on the right of VTK-m. -7. (Optional but highly recommended.) +8. (Optional but highly recommended.) [Sign up for the VTK-m mailing list] to communicate with other developers and users. From e71412c6457ba335af8fe1b61a7a620f9b4e9432 Mon Sep 17 00:00:00 2001 From: Vicente Bolea Date: Wed, 24 May 2023 20:02:26 -0400 Subject: [PATCH 21/22] Update file CONTRIBUTING.md --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b1c94b31a..3090d4cee 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -33,7 +33,7 @@ Before you begin, perform initial setup: called `gitlab` to refer to it. 5. (Required to use Git LFS.) - [Using the SSH URL for the origin remote] is needed to for Git LFS to work. + [Using the SSH URL for the origin remote] is needed for Git LFS to work. Do not forget to run `$ ./Utilities/SetupForDevelopment.sh` after changing the remote URL. From b6e61f9447376daa81093387244b492cc59ccf4c Mon Sep 17 00:00:00 2001 From: Kenneth Moreland Date: Mon, 29 May 2023 20:55:48 -0400 Subject: [PATCH 22/22] Fixed an issue with copying array from a disabled device The internal array copy has an optimization to use the device the array exists on to do the copy. However, if that device is disabled the copy would fail. This problem has been fixed. --- docs/changelog/copy-from-disabled-device.md | 5 +++++ vtkm/cont/internal/ArrayCopyUnknown.cxx | 3 ++- vtkm/cont/testing/UnitTestArrayCopy.cxx | 18 ++++++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 docs/changelog/copy-from-disabled-device.md diff --git a/docs/changelog/copy-from-disabled-device.md b/docs/changelog/copy-from-disabled-device.md new file mode 100644 index 000000000..c0657cdc3 --- /dev/null +++ b/docs/changelog/copy-from-disabled-device.md @@ -0,0 +1,5 @@ +# Fix an issue with copying array from a disabled device + +The internal array copy has an optimization to use the device the array +exists on to do the copy. However, if that device is disabled the copy +would fail. This problem has been fixed. diff --git a/vtkm/cont/internal/ArrayCopyUnknown.cxx b/vtkm/cont/internal/ArrayCopyUnknown.cxx index fdee4befc..bbeb16571 100644 --- a/vtkm/cont/internal/ArrayCopyUnknown.cxx +++ b/vtkm/cont/internal/ArrayCopyUnknown.cxx @@ -52,7 +52,8 @@ struct UnknownCopyOnDevice // by pulling out one of the component arrays and querying that. if (!this->Called && ((device == vtkm::cont::DeviceAdapterTagAny{}) || - (in.GetComponentArray(0).IsOnDevice(device)))) + (in.GetComponentArray(0).IsOnDevice(device) && + vtkm::cont::GetRuntimeDeviceTracker().CanRunOn(device)))) { vtkm::cont::Invoker invoke(device); invoke(CopyWorklet{}, in, out); diff --git a/vtkm/cont/testing/UnitTestArrayCopy.cxx b/vtkm/cont/testing/UnitTestArrayCopy.cxx index 4526f638b..0c36c12bb 100644 --- a/vtkm/cont/testing/UnitTestArrayCopy.cxx +++ b/vtkm/cont/testing/UnitTestArrayCopy.cxx @@ -199,6 +199,24 @@ void TryCopy() TestValues(input, output); } + { + std::cout << "unknown -> basic (different type, unsupported device)" << std::endl; + // Force the source to be on the Serial device. If the --vtkm-device argument was + // given with a different device (which is how ctest is set up if compiled with + // any device), then Serial will be turned off. + using SourceType = typename VTraits::template ReplaceComponentType; + auto rawInput = MakeInputArray(); + { + // Force moving the data to the Serial device. + vtkm::cont::Token token; + rawInput.PrepareForInput(vtkm::cont::DeviceAdapterTagSerial{}, token); + } + vtkm::cont::UnknownArrayHandle input = rawInput; + vtkm::cont::ArrayHandle output; + vtkm::cont::ArrayCopy(input, output); + TestValues(input, output); + } + // Test the copy methods in UnknownArrayHandle. Although this would be appropriate in // UnitTestUnknownArrayHandle, it is easier to test copies here. {