Feasibility of breaking change to vtkWindow interface

Development
1

Hello. While working to increase rendering efficiency of I noticed via valgrind’s callgrind tool that vtkWindow::GetSize() was taking up more time than expected. Caching the TileSize on change of Size/TileScale improved the rendering speed by a small amount.

However, there is currently no way to force the re-computation on change of Size/TileScale because of two reasons:

  1. The Size/TileScale members are protected, so all derived classes (which there are many) would have to do the re-computation after every change of Size/TileScale. My opinion is this is not feasible or reasonable.
  2. The GetSize/GetTileScale functions expose a non-const pointer to the Size/TileScale members, which makes them equivalent to public members. Any code is then free to update these variables at will (which does happen in current VTK code).

The standard way in C++ to fix this, is to make Size/TileSize and TileScale all private and have set/get functions that can properly enforce the invariant. I have merge request that does this, but am unsure as to the impact to the greater VTK community and how to properly make a breaking change to the interface of such a fundamental class. (Note: new functions cannot merely be added, because the current GetSize method allows unfettered access to the internals).

Is it worth pursuing this change, or are the costs too high for benefit?

Link to merge request: https://gitlab.kitware.com/vtk/vtk/-/merge_requests/8401

2

The breaks are usually not worth it. I would say to instead add new methods with sensible defaults that forward to existing implementations. Callers can be updated to use the new API in VTK and ParaView, but the old APIs should continue working for at least one release cycle to give callers time to update to the new API before ripping it out from underneath them.

Does it mean that we lose access to the “nice” names that exist now? Yes. But decreasing the slope of the upgrade mountain is worth it (otherwise you get folks still on VTK 5.x because it works for them and the API changes are just churn).

3

Understood. I will pull out the changes in the MR unrelated to the vtkWindow changes into a separate MR so those can be put through separately. I will close MR 8401 and work on making non-breaking API updates as you suggest.