Remove deprecated code before PCL 1.13.0 release

[theoniko]

Fixes #5313

[larshg]

Seems we need to upgrade 18.04 to use VTK 7 first.

pcl/.ci/azure-pipelines/env.yml

Line 47 in a92da8b

VTK_VERSION: 6

Should be available: https://packages.ubuntu.com/bionic/libvtk7-dev

[mvieth]

To be honest I don't really understand why OpenGL compatibility should be deprecated/removed.
PR #4065 by @SunBlack introduced the deprecation, claiming that we don't loose any compatibility by dropping support of the old backend, and that Ubuntu 18.04 already has OpenGL2 in its repo. However VTK 6.3 in Ubuntu 18.04 and 20.04 both still use OpenGL. So if we remove support for OpenGL, we effectively remove support for VTK 6.3 in Ubuntu, for which I don't see a reason. When a user installs VTK from a package manager (not self compiled), they don't really have a choice which backend they get. And even if the OpenGL backend is not supported in newer VTK versions, I don't see why we have to do anything on the PCL side: for people using VTK from package managers, they will automatically get OpenGL2 for newer VTK versions, and for people self-compiling VTK, they will get warnings or won't be able to build VTK if they try to use OpenGL with newer VTK versions.
So I would propose to un-deprecate the OpenGL backend and continue supporting it. Thoughts @larshg @SunBlack ?

[mvieth]

@theoniko Oh and there are some more deprecated things in filters and registration (see the issue). Would you also remove those, please?

[SunBlack]

The idea behind dropping the old backend is to simplify the code by removing #if VTK_RENDERING_BACKEND_OPENGL_VERSION < 2, as I don't think any maintainer here really checks every relevant change regarding VTK to see if it still works with the old backend.

But in general I think: If you absolutely want to upgrade to PCL 1.13.0, you should also be able to switch to e.g. VTK 7 or in case of doubt build VTK yourself, if you insist on historical in combinations with current systems ;-)

[mvieth]

Ok, that sounds reasonable. Then we have to update the 18.04 docker image to VTK 7, as Lars suggested, but in a separate pull request. @theoniko would you also be interested to do that? And please remove the deprecated function in bfgs.h

[theoniko]

@mvieth: PR is not yet ready anyway. I can give it a try to update 18.04 docker image to VTK 7 with a new pr.

[SunBlack]

@theoniko : Can you also removing all checks for #if VTK_RENDERING_BACKEND_OPENGL_VERSION < 2 as you removed the sipport for this in CMake as well?

[mvieth]

@theoniko Are you still working on this?

[theoniko]

@mvieth: I am quite busy these days. I will try to have a look at the weekend. Is there a fixed date plan for release of 1.13.0

[mvieth]

@mvieth: I am quite busy these days. I will try to have a look at the weekend. Is there a fixed date plan for release of 1.13.0

Not a totally fixed date, but somewhat soon, the PCL 1.12.0 release was over a year ago. And we will definitely have to remove the deprecated stuff before the release. If you currently don't have much time, it's also fine if you remove everything but the stuff related to VTK and the VTK rendering backend (and revert the changes in pcl_find_vtk.cmake). As soon as this PR is in a good state, we can merge it, and someone else can remove the remaining things.

[theoniko]

@mvieth , @SunBlack In Cmakelist under visualization folder do i need to update anything to this line? At first look, i do not think so but could you confirm

[mvieth]

@mvieth , @SunBlack In Cmakelist under visualization folder do i need to update anything to this line? At first look, i do not think so but could you confirm

No, I would also say that this can stay as it is

[SunBlack]

@mvieth , @SunBlack In Cmakelist under visualization folder do i need to update anything to this line? At first look, i do not think so but could you confirm

No, I would also say that this can stay as it is

Same here. The code looks like it is generally needed for VTK6 under 18.04 and has nothing to do with the legacy backend. You could test what happens if you remove it, but I think you can leave it as it is, since probably the support of VTK6 will be gone with the end of Ubuntu 18.04 next year anyway.

[mvieth]

Please also remove the deprecated function in bfgs.h

[mvieth]

Looks good to me now. Thank you!

@larshg and @SunBlack Are you also happy with the changes?

[larshg]

Only a minor change, else looks good 👍

[larshg]

Thanks for the contribution! 👍