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

[#689] Reset cut is not updating view #692

Merged
merged 3 commits into from
Nov 8, 2024
Merged

Conversation

emiliocortina
Copy link
Collaborator

* @returns The current node.
*/
addConfig<T extends keyof PhoenixMenuConfigs>(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the plan here is to avoid type inference and make it explicit in the caller? Is there an advantage to this i.e. was this the cause of the problem?

(Just for my understanding)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@EdwardMoyse
Copy link
Collaborator

EdwardMoyse commented Oct 25, 2024

Hey Emilio - thank you so much for doing this! I confirm that the reset cut button now works as expected. However, I think the cuts no longer do anything (for some reason which is not obvious to me).

For example, go to https://hepsoftwarefoundation.org/phoenix/#/atlas and load the attached config file - the screen should go blank. But when I do this locally, the jets (the blue cones) are still visible.

image

I just did some debugging, and I still cannot see the problem. AFAICS the SceneManager::collectionFilter function is called correctly, and correctly sets all child objects to visible === false. Is it possible the SceneManager is being copied i.e. it is no longer a singleton and is modifying a Scene which is not the same as we see?

Less importantly, you can find out how to fix the CI failure
here

TL;DR: run yarn lint:fix

@EdwardMoyse
Copy link
Collaborator

Hmm. A bit more debugging, and I think the issue might be that cut dialog ownership is not correct - cuts work for the first Jet collection ("AntiKt4EMTopoJets_xAOD") but for the second ("AntiKt4LCTopoJets_xAOD") the collectionName in collectionFilter is still "AntiKt4EMTopoJets_xAOD", AND if both are visible, the cuts from the second apply to the first.

@EdwardMoyse
Copy link
Collaborator

Yeah, this is definitely it. In addObjectType for the first collection there is no configRangeSlider on the cuts. However, once we get to the second collection there is. So in this case, the new configRangeSlider is never created here:

 public getConfigRangeSlider(
    collectionFiltering: () => void,
  ): ConfigRangeSlider {
    if (this.configRangeSlider == undefined) {
      this.configRangeSlider = {
        type: 'rangeSlider',

emiliocortina and others added 3 commits November 8, 2024 11:30
HSF#689

Prettier fixes

Deep copy cuts, so that they work correctly

Add missing documentation for range slider
@EdwardMoyse EdwardMoyse merged commit c2b8b9e into HSF:main Nov 8, 2024
1 check passed
@emiliocortina
Copy link
Collaborator Author

This is great! I took a look a couple of days ago and I found it very weird that it was working for the tracks, but not for the jets

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

Successfully merging this pull request may close these issues.

2 participants