With VTK-9 coming, I wonder if we should rename QVTKOpenGLWidget again, as the new version was not so much used after all as it has some specific failings, while it’s only advantage is to support stereo rendering.
As some have remarked it, this suggest the the newly named QVTKOpenGLStereoWidget will never be adopted as a generic widget. I personally am fine with that. So difficulties present with it are inherent to it’s design and can’t be solved on our side, but only on Qt/GraphicDriverStack which we have no control of. As this design is not preferred by the Qt community, I’m not hopeful for these issues to be fixed.
As a person who is usually averse to making naming changes, I think the current names are confusing at best and the suggested names are much clearer. So +1 from me.
I think the rename makes sense too, so +1 from me.
The only thing I want to ask: Is it conceivable that there may be some other functionality/context besides stereo rendering for which you would choose the QVTKOpenGLStereoWidget? If so, is there some other name that would be more fitting, something alluding to the way the widget is implemented maybe?
If it will only ever be a choice for stereo rendering, and only stereo rendering, then the name is good.
The reasoning behind the QVTK prefix was that it indicated to the user/developer that it is a QObject subclass. Also, the API style would follow Qt’s rather than VTK’s, thus methods named with lowerCase etc etc.
QVTKOpenGLWidget sounds good, but usually the “OpenGL” word is not included in class names that users should normally use (only in low-level implementation classes). So, QVTKWidget, QVTKRenderWidget, QVTKRenderWindowWidget names would be somewhat more appropriate.
Yep, I understand the logic. vtk* ==> typically inherits vtkObject/vtkObjectBase, QVTK ==> inherits QObject. I believe it’s discouraged to use the Q* prefix for classes outside of Qt itself though (even if the VTK probably makes this OK).
In my suggestion vtkQWidget, I thought that even if it’s not inheriting vtkObject, it is a VTK class, so has the vtk* prefix, and it’s a QWidget, hence the QWidget in the name (the Q included to not be confused with VTK’s own widgets), and the OpenGL has been left out since it’s an implementation detail.
But the more I think of it, it’s probably water under the bridge, with the QVTK* prefix used for a bunch of classes in VTK at this point. So let’s keep it that way!
The OpenGL part is here to not confuse with the old QVTKWidget that as not been removed yet.
Maybe it is time to remove it, but we can’t use this name either.
Also, let’s not use the WIndow term, as it is heavily charged in Qt.
For QVTKOpenGLWidget :
QVTKStereoWidget
QVTKStereoRenderWidget
QVTKOpenGLStereoWidget
For QVTKOpenGLNativeWidget :
no rename
QVTKNativeWidget
QVTKRenderWidget
Any other suggestions ? Which suggestions would you prefer ?
Since we derive from QOpenGLWidget, we should definitely keep OpenGL in the name.
At some point, I expect that we will have an OpenGL and a Vulkan rendering backend and we will have difficulty to differentiate them without OpenGL and Vulkan prefixes.
At some point we could have higher-level classes that dispatch to the right implementation (such as vtkRenderer etc.), but since we aren’t there yet but dealing directly with the OpenGL implementation, including OpenGL is okay and even necessary in my opinion.
Also, VTK class names usually read left-to-right from more specific to less specific (e.g. vtkOpenGLRenderWindow vs. vtkRenderWindow, vtkOpenGLPolyDataMapper vs. vtkPolyDataMapper, etc.). In keeping with that convention, I would vote for none of the options above but instead propose
VTK users don’t care what underlying technology (OpenGL, OpenGLES, Vulkan, Metal, etc.) should be used to create a VTK render window in a Qt widget. Would you really want to create #ifdefs around instantiation of the render window in all Qt VTK tests, examples, and applications?
If you want, you can still expose specialized classes that allow users to choose a specific technology, but the QVTKRenderWidget should just work, with the best underlying available technology automatically selected.
That seems like a desirable goal, but that is beyond a simple renaming of the existing classes. The question at hand is what to rename these very OpenGL-specific widgets. They both rely on a vtkGenericOpenGLRenderWindow internally.
If you want to add that implementation-agnostic level above this now and advise people to use that, I have no objection. But as for a simple rename operation of these classes, I am arguing OpenGL should stay in the name.
I fully agree, we should have low-level classes that have OpenGL in their names. We should just not advertise to users that these low-level classes exist (don’t use in any examples and minimize their usage in tests) because we know that in a few years we will have alternative implementations.
It is about the same as with vtkActor / vtkOpenGLActor. Only advanced users should instantiate vtkOpenGLActor explicitly, everyone else should just use vtkActor and rely on VTK factory (or any other suitable mechanism) to instantiate the best implementation.
We should add the new implementation-agnostic classes at the same time we rename the existing classes. This allows users to switch to the final class names in one step instead of renaming classes in tests, examples, user code now and again in 1-2 years. Yes, it may require 1-2 extra days of work for a VTK developer now, but it may save an hour or more for hundreds of VTK-based projects.
Right now the poll is mixing both names for a high-level factory mechanism and an implementation-specific renaming, and that I think is confusing matters.
We should first get consensus that a factory mechanism/generic name for these widgets is the way to go, then figure out what their names should be. Agreed?
