From dca352b751b1c21c7aebbe5d6a9b7bd81d70789c Mon Sep 17 00:00:00 2001
From: Utkarsh Ayachit <utkarsh.ayachit@kitware.com>
Date: Wed, 5 Feb 2020 10:29:59 -0500
Subject: [PATCH] vtkExtractCells fixes
vtkExtractCells changed the cell ids filled by the user in its
RequestData. This was incorrect. It had the potential of dropping cell
ids provided is the input dataset had fewer elements for an earlier
iteration.
Adding API to vtkExtractCells to skip sorting and uniquifying operations
if its already known that the ids are sorted and unique.
---
Filters/Extraction/vtkExtractCells.cxx | 103 ++++++++-------------
Filters/Extraction/vtkExtractCells.h | 13 +++
Filters/Extraction/vtkExtractSelection.cxx | 1 +
3 files changed, 54 insertions(+), 63 deletions(-)
diff --git a/Filters/Extraction/vtkExtractCells.cxx b/Filters/Extraction/vtkExtractCells.cxx
index 71e08a5bf54..384ba912d30 100644
--- a/Filters/Extraction/vtkExtractCells.cxx
+++ b/Filters/Extraction/vtkExtractCells.cxx
@@ -34,8 +34,6 @@
#include "vtkUnsignedCharArray.h"
#include "vtkUnstructuredGrid.h"
-vtkStandardNewMacro(vtkExtractCells);
-
#include <algorithm>
#include <numeric>
#include <vector>
@@ -152,39 +150,36 @@ private:
class vtkExtractCellsSTLCloak
{
+ vtkTimeStamp SortTime;
+
public:
std::vector<vtkIdType> CellIds;
- vtkTimeStamp ModifiedTime;
- vtkTimeStamp SortTime;
+ std::pair<typename std::vector<vtkIdType>::const_iterator,
+ typename std::vector<vtkIdType>::const_iterator>
+ CellIdsRange;
FastPointMap PointMap;
- void Modified() { this->ModifiedTime.Modified(); }
-
- inline bool IsPrepared() const
+ vtkIdType Prepare(vtkIdType numInputCells, vtkExtractCells* self)
{
- return this->ModifiedTime.GetMTime() < this->SortTime.GetMTime();
- }
+ assert(numInputCells > 0);
- void Prepare(vtkIdType numInputCells)
- {
- if (!this->IsPrepared())
+ if (self->GetAssumeSortedAndUniqueIds() == false && (self->GetMTime() > this->SortTime))
{
vtkSMPTools::Sort(this->CellIds.begin(), this->CellIds.end());
auto last = std::unique(this->CellIds.begin(), this->CellIds.end());
- auto first = this->CellIds.begin();
-
- // These lines clamp the ids to the number of cells in the
- // dataset. Otherwise segfaults occur when cellIds are greater than the
- // number of input cells, in particular when cellId==numInputCells.
- auto clampLast = std::find(first, last, numInputCells);
- last = (clampLast != last ? clampLast : last);
-
- this->CellIds.resize(std::distance(first, last));
+ this->CellIds.erase(last, this->CellIds.end());
this->SortTime.Modified();
}
+
+ this->CellIdsRange =
+ std::make_pair(std::lower_bound(this->CellIds.begin(), this->CellIds.end(), 0),
+ std::upper_bound(this->CellIds.begin(), this->CellIds.end(), (numInputCells - 1)));
+ return static_cast<vtkIdType>(
+ std::distance(this->CellIdsRange.first, this->CellIdsRange.second));
}
};
+vtkStandardNewMacro(vtkExtractCells);
//----------------------------------------------------------------------------
vtkExtractCells::vtkExtractCells()
{
@@ -218,19 +213,10 @@ void vtkExtractCells::AddCellList(vtkIdList* l)
return;
}
+ auto& cellIds = this->CellList->CellIds;
const vtkIdType* inputBegin = l->GetPointer(0);
const vtkIdType* inputEnd = inputBegin + inputSize;
-
- const std::size_t oldSize = this->CellList->CellIds.size();
- const std::size_t newSize = oldSize + static_cast<std::size_t>(inputSize);
- this->CellList->CellIds.resize(newSize);
-
- auto outputBegin = this->CellList->CellIds.begin();
- std::advance(outputBegin, oldSize);
-
- std::copy(inputBegin, inputEnd, outputBegin);
-
- this->CellList->Modified();
+ std::copy(inputBegin, inputEnd, std::back_inserter(cellIds));
this->Modified();
}
@@ -250,13 +236,7 @@ void vtkExtractCells::SetCellIds(const vtkIdType* ptr, vtkIdType numValues)
void vtkExtractCells::AddCellIds(const vtkIdType* ptr, vtkIdType numValues)
{
auto& cellIds = this->CellList->CellIds;
- const auto oldsize = cellIds.size();
- cellIds.resize(oldsize + numValues);
-
- auto start = cellIds.begin();
- std::advance(start, oldsize);
- std::copy(ptr, ptr + numValues, start);
- this->CellList->Modified();
+ std::copy(ptr, ptr + numValues, std::back_inserter(cellIds));
this->Modified();
}
@@ -270,19 +250,11 @@ void vtkExtractCells::AddCellRange(vtkIdType from, vtkIdType to)
}
// This range specification is inconsistent with C++. Left for backward
- // compatibility reasons.
- const vtkIdType inputSize = to - from + 1; // +1 to include 'to'
- const std::size_t oldSize = this->CellList->CellIds.size();
- const std::size_t newSize = oldSize + static_cast<std::size_t>(inputSize);
+ // compatibility reasons. Add 1 to `to` to make it consistent.
+ ++to;
- this->CellList->CellIds.resize(newSize);
-
- auto outputBegin = this->CellList->CellIds.begin() + oldSize;
- auto outputEnd = this->CellList->CellIds.begin() + newSize;
-
- std::iota(outputBegin, outputEnd, from);
-
- this->CellList->Modified();
+ auto& cellIds = this->CellList->CellIds;
+ std::generate_n(std::back_inserter(cellIds), (to - from), [&from]() { return from++; });
this->Modified();
}
@@ -308,11 +280,8 @@ int vtkExtractCells::RequestData(vtkInformation* vtkNotUsed(request),
return 1;
}
- // Sort/uniquify the cell ids if needed.
- vtkIdType numCellsInput = input->GetNumberOfCells();
- this->CellList->Prepare(numCellsInput);
-
- vtkIdType numCells = static_cast<vtkIdType>(this->CellList->CellIds.size());
+ const vtkIdType numCellsInput = input->GetNumberOfCells();
+ const vtkIdType numCells = this->CellList->Prepare(numCellsInput, this);
if (numCells == numCellsInput)
{
this->Copy(input, output);
@@ -435,9 +404,10 @@ vtkIdType vtkExtractCells::ReMapPointIds(vtkDataSet* grid)
if (!this->InputIsUgrid)
{
vtkNew<vtkIdList> ptIds;
-
- for (vtkIdType cellId : this->CellList->CellIds)
+ const auto& range = this->CellList->CellIdsRange;
+ for (auto iter = range.first; iter != range.second; ++iter)
{
+ const auto cellId = (*iter);
grid->GetCellPoints(cellId, ptIds);
vtkIdType npts = ptIds->GetNumberOfIds();
@@ -462,8 +432,10 @@ vtkIdType vtkExtractCells::ReMapPointIds(vtkDataSet* grid)
this->SubSetUGridCellArraySize = 0;
this->SubSetUGridFacesArraySize = 0;
- for (vtkIdType cellId : this->CellList->CellIds)
+ const auto& range = this->CellList->CellIdsRange;
+ for (auto iter = range.first; iter != range.second; ++iter)
{
+ const auto cellId = (*iter);
if (cellId > maxid)
{
continue;
@@ -516,7 +488,8 @@ vtkIdType vtkExtractCells::ReMapPointIds(vtkDataSet* grid)
//----------------------------------------------------------------------------
void vtkExtractCells::CopyCellsDataSet(vtkDataSet* input, vtkUnstructuredGrid* output)
{
- output->Allocate(static_cast<vtkIdType>(this->CellList->CellIds.size()));
+ const auto& range = this->CellList->CellIdsRange;
+ output->Allocate(static_cast<vtkIdType>(std::distance(range.first, range.second)));
vtkCellData* oldCD = input->GetCellData();
vtkCellData* newCD = output->GetCellData();
@@ -536,8 +509,9 @@ void vtkExtractCells::CopyCellsDataSet(vtkDataSet* input, vtkUnstructuredGrid* o
vtkNew<vtkIdList> cellPoints;
- for (vtkIdType cellId : this->CellList->CellIds)
+ for (auto iter = range.first; iter != range.second; ++iter)
{
+ const auto cellId = (*iter);
input->GetCellPoints(cellId, cellPoints);
for (vtkIdType i = 0; i < cellPoints->GetNumberOfIds(); i++)
@@ -585,7 +559,8 @@ void vtkExtractCells::CopyCellsUnstructuredGrid(vtkDataSet* input, vtkUnstructur
origMap->Delete();
}
- vtkIdType numCells = static_cast<vtkIdType>(this->CellList->CellIds.size());
+ const auto& range = this->CellList->CellIdsRange;
+ const auto numCells = static_cast<vtkIdType>(std::distance(range.first, range.second));
vtkNew<vtkCellArray> cellArray; // output
vtkNew<vtkIdTypeArray> newcells;
@@ -605,8 +580,9 @@ void vtkExtractCells::CopyCellsUnstructuredGrid(vtkDataSet* input, vtkUnstructur
vtkIdType maxid = ugrid->GetNumberOfCells();
bool havePolyhedron = false;
- for (vtkIdType oldCellId : this->CellList->CellIds)
+ for (auto iter = range.first; iter != range.second; ++iter)
{
+ const auto oldCellId = (*iter);
if (oldCellId >= maxid)
{
continue;
@@ -690,4 +666,5 @@ void vtkExtractCells::PrintSelf(ostream& os, vtkIndent indent)
{
this->Superclass::PrintSelf(os, indent);
os << indent << "ExtractAllCells: " << this->ExtractAllCells << endl;
+ os << indent << "AssumeSortedAndUniqueIds: " << this->AssumeSortedAndUniqueIds << endl;
}
diff --git a/Filters/Extraction/vtkExtractCells.h b/Filters/Extraction/vtkExtractCells.h
index c03871c8dd7..f10d4dfdbb3 100644
--- a/Filters/Extraction/vtkExtractCells.h
+++ b/Filters/Extraction/vtkExtractCells.h
@@ -81,11 +81,23 @@ public:
* If all cells are being extracted, this filter can use fast path to speed up
* the extraction. In that case, one can set this flag to true. When set to
* true, cell ids added via the various methods are simply ignored.
+ * Defaults to false.
*/
vtkSetMacro(ExtractAllCells, bool);
vtkGetMacro(ExtractAllCells, bool);
vtkBooleanMacro(ExtractAllCells, bool);
//@}
+
+ //@{
+ /**
+ * If the cell ids specified are already sorted and unique, then set this to
+ * true to avoid the filter from doing time-consuming sorts and uniquification
+ * operations. Defaults to false.
+ */
+ vtkSetMacro(AssumeSortedAndUniqueIds, bool);
+ vtkGetMacro(AssumeSortedAndUniqueIds, bool);
+ vtkBooleanMacro(AssumeSortedAndUniqueIds, bool);
+ //@}
protected:
int RequestData(vtkInformation*, vtkInformationVector**, vtkInformationVector*) override;
int FillInputPortInformation(int port, vtkInformation* info) override;
@@ -104,6 +116,7 @@ protected:
vtkIdType SubSetUGridFacesArraySize = 0;
bool InputIsUgrid = false;
bool ExtractAllCells = false;
+ bool AssumeSortedAndUniqueIds = false;
private:
vtkExtractCells(const vtkExtractCells&) = delete;
diff --git a/Filters/Extraction/vtkExtractSelection.cxx b/Filters/Extraction/vtkExtractSelection.cxx
index 5469f94b13b..a0ef0dd1b10 100644
--- a/Filters/Extraction/vtkExtractSelection.cxx
+++ b/Filters/Extraction/vtkExtractSelection.cxx
@@ -510,6 +510,7 @@ void vtkExtractSelection::ExtractSelectedCells(
ids.push_back(cc);
}
}
+ extractor->SetAssumeSortedAndUniqueIds(true);
extractor->SetCellIds(&ids.front(), static_cast<vtkIdType>(ids.size()));
}
--
GitLab