Can we assume that a vtkCell will hold a vtkPoints object with a double type?

All the vtkCell subclasses when created rhwy instantiate a vtkPoints object with VTK_DOUBLE, which means that the vtkDataArray holding the points of a cell will be a vtkDoubleArray.

// Construct cell.
  this->Points = vtkPoints::New(VTK_DOUBLE);

The vtkPoints object can technically be altered after creating the vtkCell by changing its type. Practically though, most of the times (if not every time) a vtkCell is acquired by using vtkDataSet->GetCell(), then the cell information is used in some way, and then the cell is deallocated, and the next cell is acquired again.

Functions such as EvaluateLocation and EvaluatePosition could greatly benefit by assuming that a vtkCell will always hold points of double type, since copying of coordinates point will not be needed.
vtkTetra, vtkHexahedron, vtkPyramid, vtkWedge also already assume this for the EvaluatePosition operation.

Also, other functions such as this->Points->GetPoints(cell->PointIds, cell->Points);, which is found in vtkUnstructuredGrid::GetCell(), could also be improved performance wise if we knew a vtkCell will always hold points of double type.

We have two options going forward:

  1. If we can’t make such an assumption, we need to remove the existing assumption from vtkTetra, vtkHexahedron, vtkPyramid, vtkWedge.
  2. If we can make such an assumption, we need to enforce such assumption or at least document that EvaluatePosition/EvaluateLocation will not work when the data type is not double.

FYI @ben.boeckel @will.schroeder @dgobbi @cory.quammen @mwestphal @dcthomp

I don’t see what allowing vtkCell to hold point of other than double buys. It’s not like huge data structures of vtkCell are created where memory would become an issue. To some extent, vtkCell is acting like an interface object, packaging up data into a form that can then be operated on - it serves as a bridge to potentially heavy data like arrays of points and connectivity. As Spiros points out, performance could be improved be making double points a requirement - this is a very good thing. So make the double assumption and document it etc.

1 Like

If there is strong benefix for limiting to double, then it makes sense. However a clean deprecation will be needed.

What kind of depreciation do you have in mind ? I was thinking of adding a note to the documentation and printing a warning in case the type is different than double.

Nevermind, you mean the only way to have a non-double vtkCell is by creating your own type, is that correct ?

In that case documentation should be enough indeed.

There are at least two vtkCell APIs that allow setting points of a different type:

void Initialize(int npts, const vtkIdType *pts, vtkPoints *p);
void Initialize(int npts, vtkPoints *p);

So type checking would have to be added to these. I’d also suggest grep VTK to see where these are used. If there are any places in VTK where a vtkCell is initialized to use the vtkPoints of the dataset itself, then that also restricts the point type of that dataset.

Edit: One example, vtkPLYReader generates float points for its output dataset, and uses vtkCell::Initialize() to attach these points to a vtkPolygon cell.

1 Like