Reorganize and beautify project generation for IDEs

[SergioRAgostinho]

It follows most of the recommendations mentioned in #2673 (comment) , creating a native look and feel for IDE users.

Closes #2680 .

[taketwo]

I understand the idea to make target names consistent with the defaults of the generator, but on the other hand I don't like that the same target is named differently depending on the generator. What about keeping the old names and creating aliases? No strong opinion here, just throwing in an idea.

[SergioRAgostinho]

What about keeping the old names and creating aliases? No strong opinion here, just throwing in an idea.

The code will look cleaner. Most of these folder are filled with so many targets that having one more makes no difference. The only questionable case is the uninstall target, it will look a little funky.

[taketwo]

Hm, true. Well, forget about this idea then.

[taketwo]

Looking at the screenshot you've just referenced, I wonder if names like EXAMPLES_BUILD are more consistent. ALL_BUILD builds everything, EXAMPLES_BUILD builds examples.

[SergioRAgostinho]

Looking at the screenshot you've just referenced, I wonder if names like EXAMPLES_BUILD are more consistent. ALL_BUILD builds everything, EXAMPLES_BUILD builds examples.

Valid point. I will adopt it. Summarizing everything

• Make use of USE_FOLDERS (which exists by defaults) instead of USE_PROJECT_FOLDERS
• Set USE_FOLDERS to on by default. It only has an effect on IDE generators anyway
• Get rid of unnecessary USE_PROJECT_FOLDERS checks if possible
• Rename compound targets to *_BUILD

[taketwo]

Sounds good except to the first item.

USE_FOLDERS is not an option or variable, it's a property. As such, it can not be set through command line options and is not exposed in the ccmake interface. If we want to let the user decide whether he wants it or not, we have to create on option. If we do so, I'd propose to call it PCL_USE_FOLDERS. But maybe we don't want to make this configurable at all, don't know why someone would turn it off anyway.

[SergioRAgostinho]

Let's be opinionated and use it by default. There's no disadvantage for adopting it.

[SergioRAgostinho]

I just completed the items from the last review. I need some help from windows users. I need an updated replacement for

https://github.com/PointCloudLibrary/pcl/raw/master/doc/tutorials/content/images/windows/pcl_solution_with_projects_folder.png

No need for the side-by-side image without project folders because it is unconditionally turned on now.

@UnaNancyOwen, @claudiofantacci or @SunBlack whenever one of you guys remembers it, would you mind sending me an updated version?

Edit:
After the review we can decide on the squash strategy.

[claudiofantacci]

I’ll test MSVS on Monday and send you a screen asap 👍🏻

[claudiofantacci]

Ok, here we go!
Here follow 7 pictures of my MSVS 2017 solution view. I enabled all build options resulting in 303 projects.

[SergioRAgostinho]

Thank you.

At first sight I'm confused that there's a target called pcl_apps under Libraries. But I'm not sure I want to address this on the current PR.

[taketwo]

Is there a reason to have specific versions here? I'd prefer a future-proof check if possible.

[SergioRAgostinho]

No. It was a literal copy paste from the supported generators list. I'll change it to "Visual Studio*".

[taketwo]

LGTM. Do you plan to add the image(s) from Claudio to the tutorial? (Maybe an edited/shortened version.)

[SergioRAgostinho]

That's the idea. Hopefully some GIMP-fu to add some \vdots here and there.

[SergioRAgostinho]

I just noticed this

pcl/apps/CMakeLists.txt

Lines 215 to 221 in 356ea4f

	set(LIB_NAME "pcl_${SUBSYS_NAME}")
	PCL_ADD_LIBRARY("${LIB_NAME}" "${SUBSYS_NAME}" ${srcs} ${impl_incs} ${incs})
	target_link_libraries("${LIB_NAME}" pcl_common pcl_io pcl_filters pcl_visualization pcl_segmentation pcl_surface pcl_features pcl_sample_consensus pcl_search)
	PCL_MAKE_PKGCONFIG("${LIB_NAME}" "${SUBSYS_NAME}" "${SUBSYS_DESC}" "" "" "" "" "")
	# Install include files
	PCL_ADD_INCLUDES("${SUBSYS_NAME}" "${SUBSYS_NAME}" ${incs})
	PCL_ADD_INCLUDES("${SUBSYS_NAME}" "${SUBSYS_NAME}/impl" ${impl_incs})

I'm inclined to replace it with regular executable targets. Currently it's compiling source files from apparently two different applications into a PCL Library "target" called pcl_apps. 😕

[taketwo]

Disclaimer: this is a speculation.

Maybe the idea is that some of the apps include reusable components that e.g. combine functionality from several modules. Then a user can link with pcl_apps and reuse them.

[SergioRAgostinho]

I'll keep that in mind. If that is indeed the case, I'll create the target without resorting to a PCL_ADD_LIBRARY call. That one should be limited to actual modules.

[SergioRAgostinho]

The end result.

[SergioRAgostinho]

I'm not going to extend any more functionalities on this PR. In terms of squash strategy I believe the only important is to keep the tools relocation commit separate from the rest. I'm also open to keep the unconditional project folder activation as a separate commit.

[taketwo]

I'd be fine with two commits, don't think unconditional project activation needs a separate one.

[claudiofantacci]

Great! 🚀

[taketwo]

Thanks!