Improve build times by reducing included headers

[darksylinc]

Summary

Ogre2Includes.hh is a very convenient header but it includes too many
Ogre headers, which heavily affects build times.

For certain .cc files compilation time was reduced by 1.2 seconds on an
i7 7700 @3.60Ghz

A full build was of ign-rendering5 went from 1m 57s to 1m 46s (I only
touched ogre2, not ogre; a full rebuild of ign-rendering5 includes
both; optix was not included in my config).

This amounts to an 11 second improvement for a full build meaning a 9.4%
reduction (more if we consider ogre1 module is affecting measuring)

The solution is to simply include the Ogre headers that are needed, instead
of relying on Ogre2Includes.hh; and making sure Ogre2Includes.hh is never
included in a *.hh file.

Checklist

• Signed all commits for DCO

This is a simple header inclusion change; it does not alter runtime functionality.
Note that merging this change into other branches such as the WIP 2.2 might cause a few compiler errors as there are fewer headers included. But those build errors should be trivial fix.

[darksylinc]

I had to resubmit the PR because it was accidentally done onto the wrong branch, which triggered all CIs to fail.

[chapulina]

Nice! I just have a question about a non-header change.

[chapulina]

This PR is exposing some compiler warnings on Windows coming from upstream. We had suppressed those on 206 with pragmas on Ogre2Includes.hh. Now that we're using more granular includes, I think we'll need to suppress the warnings separately on each file.

[darksylinc]

Ahh I see.

I'll have to compile on Windows and see those warnings. If they're trivial I may be able to fix upstream

[chapulina]

I'll have to compile on Windows and see those warnings

You should be able to see them on CI here:

https://build.osrfoundation.org/job/ign_rendering-pr-win/1547/msbuild/

If they're trivial I may be able to fix upstream

That would be awesome to get on the 2.2 version that we're about to build against, if possible 😃

[iche033]

That would be awesome to get on the 2.2 version that we're about to build against, if possible 

+1

since ign-rendering4 (and ign-rendering5) depends on 2.1, we'll probably still need to add those pragma's in this PR around places where we include ogre headers.

looks like this PR is branched out of main when it was ign-rendering5, can you retarget to the ign-rendering5 branch?

[darksylinc]

• I've re-targetted the PR to ign-rendering5 as requested
• Added #pragmas to disable warnings coming from Ogre headers in MSVC
• Fixed warnings in upstream
• I don't know why MSVC claims these 5 new warnings given that these warnings should've been in the previous version as well.

[iche033]

For the 5 windows warnings, I think we can just give it the same treatment as we did in the ogre 1.x case:
https://github.com/ignitionrobotics/ign-rendering/blob/cd6e89f0e528b27955bc33c4e37ad7f3b9df6e25/ogre/include/ignition/rendering/ogre/OgreRenderTargetMaterial.hh#L29

here's a diff

diff --git a/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTargetMaterial.hh b/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTargetMaterial.hh
index e2e4cbd3..a5fbb197 100644
--- a/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTargetMaterial.hh
+++ b/ogre2/include/ignition/rendering/ogre2/Ogre2RenderTargetMaterial.hh
@@ -31,6 +31,11 @@
   #pragma warning(pop)
 #endif
 
+#ifdef _MSC_VER
+ #pragma warning(push)
+ #pragma warning(disable:4275)
+#endif
+
 namespace ignition
 {
   namespace rendering
@@ -106,4 +111,8 @@ namespace ignition
   }
 }
 
+#ifdef _MSC_VER
+ #pragma warning(pop)
+#endif
+
 #endif

[darksylinc]

Updated again. The windows CI now passes!

[chapulina]

Nice!