Add accessor functions and functionality needed by CMS event display.

[osschar]

Parts of this were accessed through redefining private/protected as public
which, besides of being a nasty hack, does not work with gcc-5.

TColor had a static bool member fgInitDone that is now a local static in InitializeColors(). I just added a bool argument force=kFALSE as this was a minimal change. If desired, I can do the following:
. Split InitializeColors() into initial part that does the check is-init-done and the actual initialization code that is private.
. Introduce new static function InitializeColorsForce() that skips the check.
This way the interface to InitializeColors() will not change.

[pcanal]

It is not obvious what is the use case for re-initializing the TColor. Could you add a comment giving some context?

[osschar]

This is for full-framework use-case where we start root in batch mode. When the request to start up TApplication & GUI arrives we have to set up things again. TStryle objects get deleted and even if we recreate them, the colors do not because the init_done is set to true ... so we had this workaround to enforce this reinitialization.

[osschar]

Oh, I remember now ... there is ~TApplication called somewhere from cmssw, let me try to find it.

[pcanal]

Why is deleting the TStyle also deleting the TColor (such that they need recreating)?

[osschar]

I guess because it's in gROOT->GetListOfColors() ... and ~TApplication calls gROOT->EndOfProcessCleanups();

[osschar]

Looking at it some more ... we create a new instance of TRint that replaces the default TApplication:
https://github.com/cms-sw/cmssw/blob/68b4ccfc9d6f7b0ae0980969622686957e61fc9a/Fireworks/FWInterface/src/FWFFHelper.cc#L45
The TApplication constructor then deletes the default TApplication. Apparently TStyle and TColors only got deleted starting from 5.34.18, before this all worked.

Should we try figuring this one out?

[pcanal]

I don't see a commit between 5.34.17 and 5.34.18 that would explain the change of behaviors.
But either way, there is indeed a semantic problem and we need to update the "deletes the default TApplication" to be softer (i.e. not call EndOfProcessCleanups() ) ....

[osschar]

CMS probably skipped some versions before 5.34.18 ... and we only noticed it at 18.

I'd guess this is the guy:
2bcf0d7

[pcanal]

skipped some versions before 5.34.18 .
Fair enough.
I'd guess this is the guy:
Close enough :)
It is indeed needed .. but we need to update the "deletes the default TApplication" to be softer (i.e. not call EndOfProcessCleanups() ) ....

[osschar]

OK, I'll remove the TColor changes from the PR then.

Let me know when you have the fix, I'll try it in fireworks.

[davidlt]

ping^1

[pcanal]

The changes have been pushed onto the master. We still need to come up with a solution for TColor (I.e. we need to update the "deletes the default TApplication" to be softer (i.e. not call EndOfProcessCleanups() )

[davidlt]

Just for reference, commit: https://root.cern.ch/gitweb?p=root.git;a=commit;h=e1503ba36e99dab94f281f75cc504025689493c5

That's a good progress, we might have GCC 5.2.0 and GCC 6.0.0 builds after all ;)

Any ideas on ETA for TColor?

[pcanal]

TColor: I recommend using osschar's fix locally to move forward.

[davidlt]

We are not that burning, I prefer upstream solution instead out of tree patches. GCC 5.2+ is scheduled for CMSSW_8_0_X. TColor is now the last piece keeping us from cleaning up #define private public (it breaks GNU C++ standard library headers).

[pcanal]

@osschar, can you double check that your TColor issue is solved by the commit I just pushed:

• 8dd71a9 - (13 seconds ago) Do not delete resource when replacing default TApplication. — Philippe Canal (HEAD, origin/master, origin/HEAD, master)

[osschar]

I'm away this week, it's near impossible to run full-framework event display remotely. I'll try this next Monday.

[davidlt]

ping, reminder ;)

[osschar]

Yup, i R back ... on to it now :)

[davidlt]

@osschar do we have a verdict? Are we fine between ROOT + CMS Fireworks? No more any #define private public or #define protected public are needed?

[osschar]

Yup ... @pcanal's change works. I was able to remove reinitialization of TColor array. Note that it doesn't merge cleanly into 6.02, you also need to take 8068d47

[davidlt]

@pcanal would it be possible to get these changes at least in 6.04 for now?

[osschar]

@pcanal do you want me to make separate PRs for 6.02 and 6.04 branches?

[davidlt]

Actually, yes would be nice to have in 6.02 also, because 6.04 are taking time and not fully working yet for CMSSW.

[pcanal]

@osschar: yes, it would nice to have two new PR.
I am closing this one has it is per se completed
Thanks.

[osschar]

OK, deal, will try to do it later today. Thanks for your help!