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

Complete viewscene state #2285

Merged
merged 51 commits into from
Jun 30, 2021
Merged

Complete viewscene state #2285

merged 51 commits into from
Jun 30, 2021

Conversation

dcwhite
Copy link
Member

@dcwhite dcwhite commented Jun 1, 2021

Closes #1939

Also made the change to scoped enums. #1381

And M1 mac build works, at least headless.

@dcwhite dcwhite requested review from jab0707 and Sumientra June 3, 2021 19:50
@dcwhite
Copy link
Member Author

dcwhite commented Jun 4, 2021

Please provide some saved network files that demonstrate those settings not loading correctly.

@Sumientra
Copy link

Sumientra commented Jun 4, 2021

Tested on macOS 11.4 and 10.15.7. Same behavior.

Settings that did get saved:

  • background color
  • zoom
  • scale bar
  • window size
  • object selection settings
  • clipping plane settings (but the settings do not get applied until you uncheck and check the box)
  • orientation axes

Settings that did not get saved:

  • lights settings

@dcwhite
Copy link
Member Author

dcwhite commented Jun 4, 2021

Hmm, not sure why MacOS version is a factor here. Anyway, I can test on 11.3 now.

I don't think I hooked up the light settings yet, I'll check. And clipping state is a little flaky, as you found.

@dcwhite
Copy link
Member Author

dcwhite commented Jun 4, 2021

Oh, one problem with the screenshots--you're not using the updated version, that's the milestone build.

@jessdtate
Copy link
Contributor

I was finally able to test this. I'm getting a build Error:

/Users/jess/software/SCIRun/src/Modules/Legacy/Fields/InterfaceWithTetGenImpl.cc:341:25: error: unknown type
      name 'TETVOLMESH_E'
    FieldInformation fi(TETVOLMESH_E,CONSTANTDATA_E,DOUBLE_E);
                        ^
/Users/jess/software/SCIRun/src/Modules/Legacy/Fields/InterfaceWithTetGenImpl.cc:341:38: error: unknown type
      name 'CONSTANTDATA_E'
    FieldInformation fi(TETVOLMESH_E,CONSTANTDATA_E,DOUBLE_E);
                                     ^
/Users/jess/software/SCIRun/src/Modules/Legacy/Fields/InterfaceWithTetGenImpl.cc:341:53: error: unknown type
      name 'DOUBLE_E'
    FieldInformation fi(TETVOLMESH_E,CONSTANTDATA_E,DOUBLE_E);
                                                    ^
/Users/jess/software/SCIRun/src/Modules/Legacy/Fields/InterfaceWithTetGenImpl.cc:341:24: error: parentheses
      were disambiguated as a function declaration [-Werror,-Wvexing-parse]
    FieldInformation fi(TETVOLMESH_E,CONSTANTDATA_E,DOUBLE_E);
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/jess/software/SCIRun/src/Modules/Legacy/Fields/InterfaceWithTetGenImpl.cc:341:25: note: add a pair of
      parentheses to declare a variable
    FieldInformation fi(TETVOLMESH_E,CONSTANTDATA_E,DOUBLE_E);

@jessdtate
Copy link
Contributor

Also the lighting is different in the artifact that was generated. I'll investigate it some more tomorrow

@dcwhite
Copy link
Member Author

dcwhite commented Jun 18, 2021

Oops never built tetgen. I'll fix that today.

@jessdtate
Copy link
Contributor

looks like the light color doesn't initialize properly in a previously saved network. Once I click on the change light color button it fixes the lighting. without doing it, it only shows the ambient component

@dcwhite
Copy link
Member Author

dcwhite commented Jun 18, 2021

OK I moved some lighting init code around so maybe it's working differently. But the code I changed should not have been correct before. Was it working completely on master?

@jessdtate
Copy link
Contributor

It works fine on master, but the color was not save in the state. It works fine if it's a new network, of if the network has been saved with the newest version. It's only a problem with a network saved in an old version (master) and loaded in this (branch), so it's a pretty narrow case, albeit one that most people will see.

@jessdtate
Copy link
Contributor

This fixes the save from a new model, but it still sets to 0,0,0 with an old network. One thing I noticed is that an old module sets the colors as an int 0-255, but the new one is a float 0-1. I'm not sure I'd want to sacrifice accuracy, but this may be a problem for users

@jessdtate
Copy link
Contributor

maybe we can add a note somewhere obvious? like the red viewscene thing?

@dcwhite
Copy link
Member Author

dcwhite commented Jun 21, 2021

Can you attach a network with int colors saved? All of mine seem to be float.

@dcwhite
Copy link
Member Author

dcwhite commented Jun 21, 2021

I believe that was a change made a long time ago, like years.

@jessdtate
Copy link
Contributor

test_light_color_save.srn5.zip

This network was made on the branch, so the ints probably don't matter. It appears the color selector is returning ints, but the module handles it ok.

One thing I noticed is that I have to toggle the meta layer to get it to update the state info

@jessdtate
Copy link
Contributor

with old modules loaded in this branch, it is still defaulting to 0,0,0. Why would that be?

@dcwhite
Copy link
Member Author

dcwhite commented Jun 22, 2021

Right the metadata layer doesn't update until execute. That should be an easy improvement but do make an issue for it.

@dcwhite
Copy link
Member Author

dcwhite commented Jun 22, 2021

with old modules loaded in this branch, it is still defaulting to 0,0,0. Why would that be?

Can you provide an old network that does this? Or just use any old one with headlights turned on...hard to search for that in the xml files...

@jessdtate
Copy link
Contributor

test_light_color_save_II.srn5.zip

Here is a test case for the default settings that probably should be reverted to white

@jessdtate
Copy link
Contributor

test_light_color_save_III.srn5.zip

multiviewscene. One with non default that should be left alone

@dcwhite dcwhite merged commit 2168fff into master Jun 30, 2021
@dcwhite dcwhite deleted the complete-viewscene-state branch June 30, 2021 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Renderer/Visualization
  
Awaiting triage
Development

Successfully merging this pull request may close these issues.

ViewScene saved parameters are not applied when network is loaded
3 participants