search

openmc-dev / plotter

Native plotting GUI for model design and verification
MIT License
43 stars 17 forks source link

Allow user to change default resolution from command line #140

Closed paulromano closed 3 weeks ago

paulromano commented 2 months ago

Relatively small PR adding ability to change the default resolution with a -r/--resolution switch from the command line.

Closes #139

pshriwise commented 2 months ago

Really nice idea when dealing with large models or DAGMC models. What would you think of updating the current view's resolution as well if this flag is passed? Then the view and plot settings from a previous session can be maintained when opening the plotter.

paulromano commented 2 months ago

I'm a little confused by your suggestion. The view and plot settings from a previous session are preserved even if the -r flag is passed since it only affects the view settings if no plot_settings.pkl file was present. Can you elaborate?

pshriwise commented 2 months ago

I guess when I opened #138, I was thinking that the resolution value would be applied to the image seen when opening the application which could be the default view or the last view from a previous session. The idea here being if the plotter is generally taking a long time to open (regardless of whether it's loading the default view or not), the value can be provided to reduce the loading time for the GUI to get a first image and start the session.

To leverage this feature for a previous session, a user would have to discard the last view of a previous session, which for large models may have taken some time to locate, using the -e flag. They could save this as a view file, but the current image resolution is also stored in that file, so I don't that that's a solution either.

If the resolution is low, it's quick to move around in any model and get back to the last view from another session, but all-in-all I wouldn't mind seeing this setting apply to the initial image rather than the default PlotView object.

paulromano commented 2 months ago

@pshriwise Thanks for the explanation. I've just updated the branch so that a specified resolution can apply to a restored view.