Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New visualisation by reference edits alpha value of referenced VisAttr #1117

Closed
simonge opened this issue May 25, 2023 · 13 comments
Closed

New visualisation by reference edits alpha value of referenced VisAttr #1117

simonge opened this issue May 25, 2023 · 13 comments

Comments

@simonge
Copy link

simonge commented May 25, 2023

Setting the values of a new VisAttr by reference to another by name, then changing some results in the alpha of the original being edited. All other original attributes remain unchanged.

e.g. The alpha of AnlGray is changed to 0.3 by the following line
<vis name="TransparentGray" alpha="0.3" ref="AnlGray" showDaughters="true" visible="true"/>

I believe the problem is in the function VisAttr::argb from Objects.cpp
alpha = o.alpha;
may be better as
alpha = o.alpha();

@simonge simonge added the bug label May 25, 2023
@simonge
Copy link
Author

simonge commented May 26, 2023

The bug appears to be more complicated than I thought at first. Trying to further understand my problem

@MarkusFrankATcernch
Copy link
Contributor

MarkusFrankATcernch commented May 26, 2023

This is far more complex, because VisAttrs are only partially represented in ROOT by TColor.
The rest is embedded in the inheritance structure of TGeoVolume.

This would leave you with a very difficult mechanism to properly update all dependent objects if
you would change alpha as you would like in the code snippet above.

@simonge
Copy link
Author

simonge commented May 26, 2023

What I now see is the transparency of seemingly unrelated detector components being set by a different VisAttr. This is dependant on the order the detectors were included in the xml description.

@MarkusFrankATcernch
Copy link
Contributor

To me it looks like ROOT inherits the transparency from the parent volume.
This means the alpha of a n-level daughter is the product of it's own alpha with all alphas of the parent path.
This makes sense if you think of subdetector (or elements thereof) as individual modules you simply group together.
Anyhow, this would not be addressed by your suggestion.

@MarkusFrankATcernch
Copy link
Contributor

@simonge What should we do with this issue?
I would propose to close it. There is simply no solution to it.
The internals of TGeo are simply not prepared to handle visualization attributes like this.
Hence there is nothing one could do here from outside.

@simonge
Copy link
Author

simonge commented Jul 18, 2023

Feel free to close it. I still need to follow up on where the issue is coming from, might be somewhere in ROOT rather than DD4hep. The order/whether detectors are included shouldn't really be able to effect the visualisation of other unrelated detectors.

@MarkusFrankATcernch
Copy link
Contributor

The order how subdetectors are included should not affect the visualization.
According to my experience it also does not.
What matters however is the hierarchy.
Alpha values of parents (transparency) also affects the transparency of the children of these parents.

@simonge
Copy link
Author

simonge commented Jul 19, 2023

I have replicated this with a minimum example with in both the geant4 and ROOTJS geometry visualisation (see attached pictures)

In the example xml file linked here, just swapping around the order Tube1 and Tube2 are in the file (or swapping their colours) should demonstrate the effect. Hopefully should just be able to use it out of the box.

Link to xml

Geant4Compare
RootJSCompare

@MarkusFrankATcernch
Copy link
Contributor

MarkusFrankATcernch commented Jul 19, 2023

@simonge Thanks a lot for your help!
I will try to reproduce it with pure ROOT (no dd4hep).
Otherwise I cannot file a bug report if the problem is not in dd4hep itself.
I am afraid it will take a bit.

Just for completeness: Do you get the same result with geoDisplay ?

@MarkusFrankATcernch
Copy link
Contributor

I discussed with the ROOT guys. To handle the transparency for volumes properly changes in ROOT shall be required.
This will take a while -- and once I made the MR in ROOT a new ROOT version -- I am afraid.

@MarkusFrankATcernch
Copy link
Contributor

This PR root-project/root#13402 should eventually fix the problem.

@simonge
Copy link
Author

simonge commented Aug 7, 2023

That's great thanks.
I hadn't noticed the link with the material, changing it seems to fix the problem in the ROOTJS viewer but has no obvious change in the behaviour in Geant4

@MarkusFrankATcernch
Copy link
Contributor

Assumed fixed with latest ROOT. Closed due to no reaction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants