Skip to content

Refactor a bit the TrackSlimmingTool to separate the logic needing the const_cast

The original documentation of the tool claims that

/**This method 'skims' interesting information from the passed track, and creates 
a new one with cloned copies of this information
@param track A reference to the track to be skimmed. It will not be modified in any way.
@return A 'slimmed' version of 'track', where exactly what information is copied depends on 
how the tool is configured*/
virtual Trk::Track* slim(const Trk::Track& track) const = 0

In reality the existence of m_setPersistificationHints means there is another path where the passed track is modified (involving const_cast of the attached TrackStateOnSurfaces) and a nullptr is returned.

In this MR:

  • Add std::unique_ptr<Trk::Track> slimCopy(const Trk::Track& track); which implements the original intention. m_setPersistificationHints does not enter when this interface is used.

  • The virtual Trk::Track* slim(const Trk::Track& track); stays. If m_setPersistificationHints is not set it just calls the new method above. When m_setPersistificationHints is set it falls through to the old behaviour of setting Hints on the TrackStateOnSurfaces of the Track ,still doing the const_cast.

  • Update the comments in the headers to reflect the issue.

  • Mark things with the thread safety checker. The old interface since it includes const_cast is marked as non-const safe and clients should be careful when setting the m_setPersistificationHints and/or the context it is called.

  • Alternatively, for the originally intended case one can just use the new interface directly which is thread_safe

A couple of helpers methods were added to reduce a bit code duplication where it was obvious it can be avoided. As we now have 2 methods rather than 1.

Edited by Christos Anastopoulos

Merge request reports