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

Modularize inspector #596

Merged
merged 10 commits into from
Jul 2, 2024
Merged

Modularize inspector #596

merged 10 commits into from
Jul 2, 2024

Conversation

skalarproduktraum
Copy link
Member

This PR modularises the code for the inspector:

  • one can define inspector groups for different node types, such that the properties for Atmosphere are handled via AtmosphereProperties, which extends InteractiveInspectorCommand
  • InteractiveInspectorCommand can have again extensions for specific UI types, this is currently available for Swing as SwingInspectorInteractiveCommandExtension

@smlpt
Copy link
Contributor

smlpt commented Jun 11, 2024

I tested this and I think I found a bug: when you scroll down the inspector and change a value in any of the spinner fields, it will move to the top of the panel, requiring you to scroll down again to change the value again.

@skalarproduktraum
Copy link
Member Author

@smlpt 🧲😂

I'll check that and fix it, thanks for the report 👍

@moreApi moreApi removed their request for review June 11, 2024 15:39
@skalarproduktraum
Copy link
Member Author

@smlpt This should be fixed now. Can you confirm?

@skalarproduktraum skalarproduktraum force-pushed the enhancement/inspector-modularization branch from 26e9a49 to 4db185a Compare June 12, 2024 15:33
Copy link
Member

@kephale kephale left a comment

Choose a reason for hiding this comment

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

Can you please double check how this works with the Text demo? I'm getting exceptions: Color parameter outside of expected range: Red Green

Also, volume timeseries is good, but volumes without a time axis have an incorrect timepoint slider:
image

During this testing I also tested the atmosphere shader, and it didnt work (on main either, but it did work when the PR was merged :/)

@smlpt
Copy link
Contributor

smlpt commented Jun 14, 2024

@smlpt This should be fixed now. Can you confirm?
Yes, editing values works properly now.

@kephale atmosphere works on my side, both in this branch and on main. Although I also noticed that something about the blend mode or depth test must be wrong, since it is visible in front of geometrical objects (which is not the case with the AtmosphereExample in scenery), also observed by @moreApi.

I also noticed a bug in my Atmosphere input: the azimuth is supposed to go from 0 to 360° according to the properties parameter. But setSunPositionFromTime returns values in range -180-180°. So line 108 should be changed to be + PI instead. Should I create a branch + PR for this one line? Or do we want to fix it in this branch?

@kephale
Copy link
Member

kephale commented Jun 19, 2024

@smlpt i'm verifying that the atmosphere is working after a gradle clean 😊 I'd suggest doing the azimuth fix in a separate PR

Otherwise the other requested changes still seem to be needed.

@smlpt
Copy link
Contributor

smlpt commented Jun 21, 2024

Atmosphere range has been fixed in this PR

@skalarproduktraum skalarproduktraum force-pushed the enhancement/inspector-modularization branch from 090abe7 to 2418496 Compare July 2, 2024 14:50
@skalarproduktraum
Copy link
Member Author

@kephale Known issues should be fixed, good to go from my side 👍

@kephale
Copy link
Member

kephale commented Jul 2, 2024

When there is a single timepoint volume but the inspector is collapsed i get:

[INFO] Input field timepoint of type Integer not found, therefore it can't be removed.
[INFO] Input field playPause of type Button not found, therefore it can't be removed.
[INFO] Input field playSpeed of type Integer not found, therefore it can't be removed.

@skalarproduktraum skalarproduktraum force-pushed the enhancement/inspector-modularization branch from 2418496 to 3451df5 Compare July 2, 2024 15:12
@kephale kephale merged commit 5ceadb8 into main Jul 2, 2024
4 checks passed
@kephale kephale deleted the enhancement/inspector-modularization branch July 2, 2024 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants