# Add vtkDataset::GetPoints function

**URL:** <https://discourse.vtk.org/t/add-vtkdataset-getpoints-function/10920>\
**Category:** Development\
**Created:** [March 8, 2023, 5:47pm UTC](https://discourse.vtk.org/t/add-vtkdataset-getpoints-function/10920 "2023-03-08T17:47:29Z")\
**Posts on this page:** 19\
**Page:** 1

<div class="post-metadata">

**Author:** ![spyridon97](https://discourse.vtk.org/user_avatar/discourse.vtk.org/spyridon97/32/7069_2.png) [@spyridon97](https://discourse.vtk.org/u/spyridon97)\
**Post date:** [March 8, 2023, 5:47pm UTC](https://discourse.vtk.org/t/add-vtkdataset-getpoints-function/10920/1 "2023-03-08T17:47:29Z")

</div>

Hello everyone

I would like to add a vtkDataSet::GetPoints function that computes the points array and return it.  
I would like to do that to avoid using GetPoint() since it performs a computation at each call for vtkImagaData and vtkRectilinearGrid, while GetPoints would allow me to compute the points once and then query them more than once.

There are 2 options for doing this

1. vtkSmartPointer vtkDataSet::GetPoints()
2. void vtkDataSet::GetPoints(vtkPoints\* points) like void vtkRectilinearGrid::GetPoints(vtkPoints\* points)

The first solution would have a conflict with vtkPoints\* vtkPointSet::GetPoints() but it would have 1 function for both.  
The second solution would have no conflict with vtkPoints\* vtkPointSet::GetPoints() but it would have 2 functions for the same thing for vtkPointSet subclasses.

Which one do you think is best?

---

<div class="post-metadata">

**Author:** ![will.schroeder](https://discourse.vtk.org/user_avatar/discourse.vtk.org/will.schroeder/32/233_2.png) [@will.schroeder](https://discourse.vtk.org/u/will.schroeder)\
**Post date:** [March 8, 2023, 5:49pm UTC](https://discourse.vtk.org/t/add-vtkdataset-getpoints-function/10920/2 "2023-03-08T17:49:58Z")

</div>

I would avoid extending the API and introducing smart pointers. I would choose #2.

---

<div class="post-metadata">

**Author:** ![dgobbi](https://discourse.vtk.org/user_avatar/discourse.vtk.org/dgobbi/32/18_2.png) [@dgobbi](https://discourse.vtk.org/u/dgobbi)\
**Post date:** [March 8, 2023, 6:33pm UTC](https://discourse.vtk.org/t/add-vtkdataset-getpoints-function/10920/3 "2023-03-08T18:33:41Z")

</div>

Where would you use the new GetPoints() function? Memory-wise, implicit geometry a major feature of vtkImageData and vtkRectilinearGrid, and any filter that generates vtkPoints for these dataset types is going to consume a lot of memory. The generated vtkPoints array will consume anywhere from 3 times up to more than 10 times as much memory as the vtkDataArray that holds the scalars.

---

<div class="post-metadata">

**Author:** ![spyridon97](https://discourse.vtk.org/user_avatar/discourse.vtk.org/spyridon97/32/7069_2.png) [@spyridon97](https://discourse.vtk.org/u/spyridon97)\
**Post date:** [March 8, 2023, 6:40pm UTC](https://discourse.vtk.org/t/add-vtkdataset-getpoints-function/10920/4 "2023-03-08T18:40:20Z")

</div>

I am thinking of utilize vtkImplicitArrays to reduce the memory cost and just have the x, y, z coordinates.

I would use it for in vtkExtractGeometry.

---

<div class="post-metadata">

**Author:** ![dgobbi](https://discourse.vtk.org/user_avatar/discourse.vtk.org/dgobbi/32/18_2.png) [@dgobbi](https://discourse.vtk.org/u/dgobbi)\
**Post date:** [March 8, 2023, 6:53pm UTC](https://discourse.vtk.org/t/add-vtkdataset-getpoints-function/10920/5 "2023-03-08T18:53:23Z")

</div>

Yes, it does make more sense if you have a vtkImplicitArray that computes the coordinates on demand. For some of my own algorithms, I wrote [vtkImagePointIterator](https://gitlab.kitware.com/vtk/vtk/-/blob/master/Imaging/Core/vtkImagePointIterator.h) to make it easier to write image-processing code that uses points. An implicit array could do pretty much the same thing, but with random-access.

---

<div class="post-metadata">

**Author:** ![spyridon97](https://discourse.vtk.org/user_avatar/discourse.vtk.org/spyridon97/32/7069_2.png) [@spyridon97](https://discourse.vtk.org/u/spyridon97)\
**Post date:** [March 8, 2023, 8:32pm UTC](https://discourse.vtk.org/t/add-vtkdataset-getpoints-function/10920/6 "2023-03-08T20:32:01Z")

</div>

I think my approach would be faster, and we could deprecate the [vtkImagePointIterator](https://gitlab.kitware.com/vtk/vtk/-/blob/master/Imaging/Core/vtkImagePointIterator.h) in favor of that.  
I plan to do the same for GetCellPoints.

---

<div class="post-metadata">

**Author:** ![Yohann\_Bearzi](https://discourse.vtk.org/user_avatar/discourse.vtk.org/yohann_bearzi/32/671_2.png) [@Yohann\_Bearzi](https://discourse.vtk.org/u/Yohann_Bearzi)\
**Post date:** [March 9, 2023, 4:38pm UTC](https://discourse.vtk.org/t/add-vtkdataset-getpoints-function/10920/7 "2023-03-09T16:38:22Z")

</div>

Why not hosting a `vtkPoints` inside each subclass or `vtkDataSet` and update it’s implicit definition upon chaning extents / origin, etc?  
`GetPoints` would become virtual.

---

<div class="post-metadata">

**Author:** ![spyridon97](https://discourse.vtk.org/user_avatar/discourse.vtk.org/spyridon97/32/7069_2.png) [@spyridon97](https://discourse.vtk.org/u/spyridon97)\
**Post date:** [March 9, 2023, 4:39pm UTC](https://discourse.vtk.org/t/add-vtkdataset-getpoints-function/10920/8 "2023-03-09T16:39:04Z")

</div>

That’s what i end up doing.

[https://gitlab.kitware.com/vtk/vtk/-/merge\_requests/10037](https://gitlab.kitware.com/vtk/vtk/-/merge_requests/10037)

---

<div class="post-metadata">

**Author:** ![Yohann\_Bearzi](https://discourse.vtk.org/user_avatar/discourse.vtk.org/yohann_bearzi/32/671_2.png) [@Yohann\_Bearzi](https://discourse.vtk.org/u/Yohann_Bearzi)\
**Post date:** [March 9, 2023, 4:41pm UTC](https://discourse.vtk.org/t/add-vtkdataset-getpoints-function/10920/9 "2023-03-09T16:41:12Z")

</div>

Yeah I think it looks better that way.

Problem now: you’re breaking backward compatibility for people who have custom `vtkDataSet`, so I think we should probably host a `vtkPoints` in `vtkDataSet` as well that would call `GetPoint` under the cover, and overwrite its behavior in subclasses.

---

<div class="post-metadata">

**Author:** ![spyridon97](https://discourse.vtk.org/user_avatar/discourse.vtk.org/spyridon97/32/7069_2.png) [@spyridon97](https://discourse.vtk.org/u/spyridon97)\
**Post date:** [March 9, 2023, 4:51pm UTC](https://discourse.vtk.org/t/add-vtkdataset-getpoints-function/10920/10 "2023-03-09T16:51:15Z")

</div>

We could just return nullptr in vtkPoints\* vtkDataSet::GetPoints() and let the other classes override, right?

---

<div class="post-metadata">

**Author:** ![Yohann\_Bearzi](https://discourse.vtk.org/user_avatar/discourse.vtk.org/yohann_bearzi/32/671_2.png) [@Yohann\_Bearzi](https://discourse.vtk.org/u/Yohann_Bearzi)\
**Post date:** [March 9, 2023, 5:07pm UTC](https://discourse.vtk.org/t/add-vtkdataset-getpoints-function/10920/11 "2023-03-09T17:07:07Z")

</div>

Oh that’s an interesting point.

What I’m afraid of is people that would assume that `vtkDataSet::GetPoints` returning `nullptr` means that there are no points, which would not be the case for custom subclasses. Similarly it would mean that we would still need to write a specific path using the `GetPoint` API in filters accepting `vtkDataSet` as input so they still work in all cases.

On the contrary, if `GetPoints` returns a legit implicit array returning the points that `GetPoint` would return, then I feel like we are getting ourselves away from the caveats I just described.

Any take on that @will.schroeder and @dgobbi ?

---

<div class="post-metadata">

**Author:** ![dgobbi](https://discourse.vtk.org/user_avatar/discourse.vtk.org/dgobbi/32/18_2.png) [@dgobbi](https://discourse.vtk.org/u/dgobbi)\
**Post date:** [March 9, 2023, 6:45pm UTC](https://discourse.vtk.org/t/add-vtkdataset-getpoints-function/10920/12 "2023-03-09T18:45:24Z")

</div>

I agree. If you change the behavior of a pre-existing method name like `GetPoints()`, there are certain expectations. It shouldn’t return _nullptr_ unless there are no points. People will not like it if vtkExtractGeometry and other filters suddenly stop working for their data.

It’s better to have a pure virtual method than a default implementation that returns _nullptr_. With a pure virtual method, people see the problems at compile-time and they know what they have to fix.

A default implementation that uses `vtkDataSet::GetPoint()` sounds like a reasonable solution.

---

<div class="post-metadata">

**Author:** ![Yohann\_Bearzi](https://discourse.vtk.org/user_avatar/discourse.vtk.org/yohann_bearzi/32/671_2.png) [@Yohann\_Bearzi](https://discourse.vtk.org/u/Yohann_Bearzi)\
**Post date:** [March 9, 2023, 6:55pm UTC](https://discourse.vtk.org/t/add-vtkdataset-getpoints-function/10920/13 "2023-03-09T18:55:55Z")

</div>

`GetPoints` could also potentially throw a warning advising overriding this method. I don’t think we would spam the terminal because it doesn’t get called that often, and developers would be aware of this API enhancement and could optimize their class if they feel like it. Explicitly talking about implicit arrays could point them toward the right path easily.

---

<div class="post-metadata">

**Author:** ![spyridon97](https://discourse.vtk.org/user_avatar/discourse.vtk.org/spyridon97/32/7069_2.png) [@spyridon97](https://discourse.vtk.org/u/spyridon97)\
**Post date:** [March 9, 2023, 6:57pm UTC](https://discourse.vtk.org/t/add-vtkdataset-getpoints-function/10920/14 "2023-03-09T18:57:31Z")

</div>

That’s a good Idea.

In the same MR i will also introduce vtkAbstractCellArray so that vtkCellArray and vtkImplicitCellArray (a new class).

These are kinda disturbing changes, but i hope that performance and code simplification will justify them.

---

<div class="post-metadata">

**Author:** ![Yohann\_Bearzi](https://discourse.vtk.org/user_avatar/discourse.vtk.org/yohann_bearzi/32/671_2.png) [@Yohann\_Bearzi](https://discourse.vtk.org/u/Yohann_Bearzi)\
**Post date:** [March 9, 2023, 7:01pm UTC](https://discourse.vtk.org/t/add-vtkdataset-getpoints-function/10920/15 "2023-03-09T19:01:36Z")

</div>

Array ranges with implicit arrays have a performance hit of ~25% if I remember correctly because they use the generic implementation. If we want performance, in addition of enhancing those core component APIs, it might be worth specializing array ranges for implicit arrays so we don’t have this performance hit anymore.

---

<div class="post-metadata">

**Author:** ![spyridon97](https://discourse.vtk.org/user_avatar/discourse.vtk.org/spyridon97/32/7069_2.png) [@spyridon97](https://discourse.vtk.org/u/spyridon97)\
**Post date:** [March 9, 2023, 7:14pm UTC](https://discourse.vtk.org/t/add-vtkdataset-getpoints-function/10920/16 "2023-03-09T19:14:00Z")

</div>

I will see what i can do. There is lots of room for improvement. We just need to be careful not breaking the API.

---

<div class="post-metadata">

**Author:** ![spyridon97](https://discourse.vtk.org/user_avatar/discourse.vtk.org/spyridon97/32/7069_2.png) [@spyridon97](https://discourse.vtk.org/u/spyridon97)\
**Post date:** [March 10, 2023, 2:08am UTC](https://discourse.vtk.org/t/add-vtkdataset-getpoints-function/10920/17 "2023-03-10T02:08:57Z")

</div>

I have a couple of updates in my MR but they can be summarized as below:

When `vtkImageData`/`vtkRectilinearGrid`/`vtkStructuredGrid` call `GetPoint`, `GetPointCells`, `GetCellType` queries, they perform a calculation on demand. Parts of this calculation are repeated for each query. To improve the performance of these we can precompute those parts and utilize them inside implicit Points, Cells and CellTypes structures.

Using the `vtkImplicitArray` capability, we have added implicit Points, Cells, and CellTypes structures. `vtkStructuredData` now has 3 new methods to create these implicit structures:

1. `vtkSmartPointer<vtkPoints> GetPoints(vtkDataArray* x, vtkDataArray* y, vtkDataArray* z, int dim[3])`
  1. `vtkPoints` contains an `vtkImplicitArray` as its data which utilizes, x, y, and z arrays and the dims to compute the implicit Points structure.

2. `vtkSmartPointer<vtkImplicitCellArray> GetCellArray(int dim[3], bool usePixelVoxelOrientation)`
  1. `vtkImplicitCellArray` is a new class to defines an implicit Cells structure for structured data. It utilizes the `vtkImplicitArray` capability. `vtkAbstractCellArray` (also a new class) is the parent class of both `vtkCellArray` and `vtkImplicitCellArray`.

3. `vtkSmartPointer<vtkConstantArray<int>> GetCellTypesArray(int dim[3], bool usePixelVoxelOrientation)`
  1. `vtkConstantArray<int>` is a `vtkImplicitArray` that is used to define an implicit CellTypes structure for structured data.

The above structures are used by:

1. `vtkImageData` uses the implicit Points, Cells, and CellTypes structures to implement `GetPoint`, `GetPointCells`, and `GetCellType` queries.
2. `vtkRectilinearGrid` uses the implicit Points, Cells, and CellTypes structures to implement `GetPoint`, `GetPointCells`, and `GetCellType` queries.
3. `vtkStructuredGrid` uses and CellTypes structures to implement `GetPointCells`, and `GetCellType` queries.

Thanks to the addition, the highly performant and specialized code for structured data in `vtkTableBasedClipDataSet` can be removed since these new implicit structures implement the same performant functionality.

Moreover, there is a new `vtkPoints* vtkDataSet::GetPoints()` function, which for `vtkImageData`, and `vtkRectilinearGrid` return their implicit structure. If the `vtkDataSet` subclass does not define override this function then an internal copy is created and returned which should not be modified.

Lastly, because of this implicit points structure, `void vtkRectilinearGrid::GetPoints(vtkPoints*)` has been deprecated.

---

<div class="post-metadata">

**Author:** ![dgobbi](https://discourse.vtk.org/user_avatar/discourse.vtk.org/dgobbi/32/18_2.png) [@dgobbi](https://discourse.vtk.org/u/dgobbi)\
**Post date:** [March 10, 2023, 9:18am UTC](https://discourse.vtk.org/t/add-vtkdataset-getpoints-function/10920/18 "2023-03-10T09:18:46Z")

</div>

> [@spyridon97](#):
>
> vtkSmartPointer GetPoints(vtkDataArray\* x, vtkDataArray\* y, vtkDataArray\* z, int dim[3])

Isn’t `dims[3]` redundant, since it’s simply the sizes of `x`, `y`, and `z`?

---

<div class="post-metadata">

**Author:** ![spyridon97](https://discourse.vtk.org/user_avatar/discourse.vtk.org/spyridon97/32/7069_2.png) [@spyridon97](https://discourse.vtk.org/u/spyridon97)\
**Post date:** [March 10, 2023, 3:29pm UTC](https://discourse.vtk.org/t/add-vtkdataset-getpoints-function/10920/19 "2023-03-10T15:29:16Z")

</div>

I think you are right! Nice catch