diff --git a/data/baseline/filter/contour-uniform-boundaries.png b/data/baseline/filter/contour-uniform-boundaries.png new file mode 100644 index 000000000..193c76848 --- /dev/null +++ b/data/baseline/filter/contour-uniform-boundaries.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:25d61b72591e1ecd66a70e1005cb31a5d37b0bba90bec42288dd41118801000e +size 29986 diff --git a/docs/changelog/flying-edges-z-boundary-fix.md b/docs/changelog/flying-edges-z-boundary-fix.md new file mode 100644 index 000000000..6d9a5752c --- /dev/null +++ b/docs/changelog/flying-edges-z-boundary-fix.md @@ -0,0 +1,14 @@ +# Fixed Flying Edges Crash + +There was a bug in VTK-m's flying edges algorithm (in the contour filter +for uniform structured data) that cause the code to read an index from +uninitialized memory. This in turn caused memory reads from an +inappropriate address that could cause bad values, failed assertions, or +segmentation faults. + +The problem was caused by a misidentification of edges at the positive z +boundary. Due to a typo, the z index was being compared to the length in +the y dimension. Thus, the problem would only occur in the case where the y +and z dimensions were of different sizes and the contour would go through +the positive z boundary of the data, which was missing our test cases. + diff --git a/vtkm/filter/contour/testing/RenderTestContourFilter.cxx b/vtkm/filter/contour/testing/RenderTestContourFilter.cxx index 1eb787a74..9d037e049 100644 --- a/vtkm/filter/contour/testing/RenderTestContourFilter.cxx +++ b/vtkm/filter/contour/testing/RenderTestContourFilter.cxx @@ -10,11 +10,13 @@ #include #include +#include #include #include #include #include +#include #include #include @@ -70,6 +72,40 @@ void TestContourFilterUniform() result, "pointvar", "filter/contour-uniform.png", testOptions); } +void TestContourFilterUniformBoundaries() +{ + std::cout << "Generate Image for Contour filter on a uniform grid that goes through boundaries" + << std::endl; + // There was a bug in flying edges that did not identify boundaries when the dimension + // sizes were not the same. + + vtkm::cont::DataSetBuilderUniform dsb; + vtkm::cont::DataSet dataSet = + dsb.Create({ 9, 5, 3 }, vtkm::Vec3f_64{ 0.0, 0.0, 0.0 }, vtkm::Vec3f_64{ 0.125, 0.25, 0.5 }); + + std::string fieldName = "pointvar"; + vtkm::filter::field_transform::PointElevation elevation; + elevation.SetLowPoint(1.0, 0.0, 0.0); + elevation.SetHighPoint(0.0, 1.0, 1.0); + elevation.SetOutputFieldName(fieldName); + elevation.SetUseCoordinateSystemAsField(true); + dataSet = elevation.Execute(dataSet); + + vtkm::filter::contour::Contour contour; + contour.SetGenerateNormals(true); + contour.SetMergeDuplicatePoints(true); + contour.SetIsoValues({ 0.25, 0.5, 0.75 }); + contour.SetActiveField(fieldName); + vtkm::cont::DataSet result = contour.Execute(dataSet); + + result.PrintSummary(std::cout); + + //Y axis Flying Edge algorithm has subtle differences at a couple of boundaries + vtkm::rendering::testing::RenderTestOptions testOptions; + vtkm::rendering::testing::RenderTest( + result, fieldName, "filter/contour-uniform-boundaries.png", testOptions); +} + void TestContourFilterTangle() { std::cout << "Generate Image for Contour filter on a uniform tangle grid" << std::endl; @@ -97,6 +133,7 @@ void TestContourFilterTangle() void TestContourFilter() { TestContourFilterUniform(); + TestContourFilterUniformBoundaries(); TestContourFilterTangle(); TestContourFilterWedge(); } diff --git a/vtkm/filter/contour/worklet/contour/FlyingEdges.h b/vtkm/filter/contour/worklet/contour/FlyingEdges.h index 42af4639f..b9f605124 100644 --- a/vtkm/filter/contour/worklet/contour/FlyingEdges.h +++ b/vtkm/filter/contour/worklet/contour/FlyingEdges.h @@ -34,17 +34,7 @@ template vtkm::Id extend_by(vtkm::cont::ArrayHandle& handle, vtkm::Id size) { vtkm::Id oldLen = handle.GetNumberOfValues(); - if (oldLen == 0) - { - handle.Allocate(size); - } - else - { - vtkm::cont::ArrayHandle tempHandle; - tempHandle.Allocate(oldLen + size); - vtkm::cont::Algorithm::CopySubRange(handle, 0, oldLen, tempHandle); - handle = tempHandle; - } + handle.Allocate(oldLen + size, vtkm::CopyFlag::On); return oldLen; } } diff --git a/vtkm/filter/contour/worklet/contour/FlyingEdgesHelpers.h b/vtkm/filter/contour/worklet/contour/FlyingEdgesHelpers.h index aafe9ec49..7a03a0e46 100644 --- a/vtkm/filter/contour/worklet/contour/FlyingEdgesHelpers.h +++ b/vtkm/filter/contour/worklet/contour/FlyingEdgesHelpers.h @@ -15,6 +15,12 @@ #include +#include +#include + +#include +#include + namespace vtkm { namespace worklet diff --git a/vtkm/filter/contour/worklet/contour/FlyingEdgesPass4Common.h b/vtkm/filter/contour/worklet/contour/FlyingEdgesPass4Common.h index b598725a6..059eb63d0 100644 --- a/vtkm/filter/contour/worklet/contour/FlyingEdgesPass4Common.h +++ b/vtkm/filter/contour/worklet/contour/FlyingEdgesPass4Common.h @@ -185,7 +185,7 @@ struct Pass4TrimState { boundaryStatus[AxisToSum::zindex] += FlyingEdges3D::MinBoundary; } - if (ijk[AxisToSum::zindex] >= (pdims[AxisToSum::yindex] - 2)) + if (ijk[AxisToSum::zindex] >= (pdims[AxisToSum::zindex] - 2)) { boundaryStatus[AxisToSum::zindex] += FlyingEdges3D::MaxBoundary; } diff --git a/vtkm/filter/contour/worklet/contour/FlyingEdgesPass4X.h b/vtkm/filter/contour/worklet/contour/FlyingEdgesPass4X.h index 49cccf4c3..99cf6550a 100644 --- a/vtkm/filter/contour/worklet/contour/FlyingEdgesPass4X.h +++ b/vtkm/filter/contour/worklet/contour/FlyingEdgesPass4X.h @@ -15,8 +15,11 @@ #include +#include #include +#include + namespace vtkm { namespace worklet