only keep the QUIET flag for OpenGL package search

[christian-rauch]

For finding OpenGL, both QUIET and REQUIRED are defined. This is contradicting. Since OPENGL_FOUND is explicitly checked, the OpenGL packages can be optional and thus only set to QUIET.

I am also adding the components OpenGL EGL for "modern OpenGL" (see https://cmake.org/cmake/help/latest/module/FindOpenGL.html).

Edit:
Commit 0e0f2c4 added REQUIRED to QUIET.

[christian-rauch]

@larshg I fixed the macOS build by removing the EGL component.

[mvieth]

@christian-rauch Could you explain what this pull request is intended to fix? For example, do you experience a build error without this change?
I don't really see why QUIET and REQUIRED are contradicting. I would understand it as: don't print any messages, unless the package cannot be found, then throw a fatal error.
Also: does adding COMPONENTS OpenGL still have an effect or is that only what remains of the experiment with EGL?

[christian-rauch]

For example, do you experience a build error without this change?

I am getting a "build error" when OpenGL is not installed.

I don't really see why QUIET and REQUIRED are contradicting. I would understand it as: don't print any messages, unless the package cannot be found, then throw a fatal error.

You can use both in parallel, but I am not sure what the effect is. REQUIRED means that the build will fail with a message when the dependency is not present. QUIET is usually used in conjunction with _FOUND to check if a dependency is present or not and consequently change the build behaviour. The check for OPENGL_FOUND highly suggests that OpenGL is not required and that the build depends on if this is present or not. So also having this REQUIRED does not make sense. Building with out OpenGL works just fine so the REQUIRED is not required.

Also: does adding COMPONENTS OpenGL still have an effect or is that only what remains of the experiment with EGL?

Yes, this is the way to check for the "modern" OpenGL stack (https://cmake.org/cmake/help/latest/module/FindOpenGL.html) where the OpenGL and windowing implementation are separated.

[mvieth]

I am getting a "build error" when OpenGL is not installed.

Are you aware of the option WITH_OPENGL?

pcl/CMakeLists.txt

Line 393 in c1835f4

option(WITH_OPENGL "Support for OpenGL" TRUE)

If you turn that off, you should be able to build successfully even if OpenGL is not installed.
So I guess the question is: if WITH_OPENGL=TRUE, should the build fail if OpenGL is not installed or should it be treated as optional? I think I would tend towards the former. However, we could create a third option next to TRUE and FALSE that means "use OpenGL if available and build PCL components depending on it, but if it is not available, quietly continue". Would be interesting to hear your opinion on the usefulness of this

You can use both in parallel, but I am not sure what the effect is. REQUIRED means that the build will fail with a message when the dependency is not present. QUIET is usually used in conjunction with _FOUND to check if a dependency is present or not and consequently change the build behaviour. The check for OPENGL_FOUND highly suggests that OpenGL is not required and that the build depends on if this is present or not. So also having this REQUIRED does not make sense. Building with out OpenGL works just fine so the REQUIRED is not required.

Which check for OPENGL_FOUND do you mean? The one directly below in pcl_find_gl.cmake is only to set OPENGL_IS_A_FRAMEWORK.

Yes, this is the way to check for the "modern" OpenGL stack (https://cmake.org/cmake/help/latest/module/FindOpenGL.html) where the OpenGL and windowing implementation are separated.

Unless I misunderstood the documentation of FindOpenGL, simply adding COMPONENTS OpenGL does not have an effect if PCL uses OPENGL_LIBRARIES, and does not use the target OpenGL::OpenGL?

[christian-rauch]

Are you aware of the option WITH_OPENGL?

I didn't know of WITH_OPENGL before. Thanks.

if WITH_OPENGL=TRUE, should the build fail if OpenGL is not installed or should it be treated as optional?

If OpenGL is requested (WITH_OPENGL=ON) and it is not available, the built should fail. It would be useful to have a message that suggests setting WITH_OPENGL to OFF when OpenGL cannot be found.

Which check for OPENGL_FOUND do you mean? The one directly below in pcl_find_gl.cmake is only to set OPENGL_IS_A_FRAMEWORK.

Still, checking for OPENGL_FOUND suggests that OpenGL is optional. I think the cmake scripts should either require OpenGL and fail otherwise, or make OpenGL optional and check OPENGL_FOUND to make components optional or show messages.

Unless I misunderstood the documentation of FindOpenGL, simply adding COMPONENTS OpenGL does not have an effect if PCL uses OPENGL_LIBRARIES, and does not use the target OpenGL::OpenGL?

That's correct. I overlooked that PCL is using the old/legacy OPENGL_LIBRARIES in a couple of places. This should probably be replaced by OpenGL::OpenGL + OpenGL::EGL.

[christian-rauch]

@mvieth Can you review this again?