search

saalfeldlab / paintera

GNU General Public License v2.0
100 stars 17 forks source link

Maximizing current view doesn't work #350

Closed igorpisarev closed 4 years ago

igorpisarev commented 4 years ago

It seems that in recent versions the M shortcut started acting the same way as Shift+M: instead of maximizing the current view it puts it side by side with the 3D view.

igorpisarev commented 4 years ago

After some debugging I found that for some reason these JavaFX bindings are not updated when the observed properties change its values: https://github.com/saalfeldlab/paintera/blob/master/src/main/java/org/janelia/saalfeldlab/fx/ortho/GridConstraintsManager.java#L104-L110 If I minimize, firstRowHeight becomes 50.0 which should correspond to MaximizedRow.NONE, but somehow the maximizedRow binding stays in its previous state MaximizedRow.BOTTOM. So far I don't see why it wouldn't work as expected :confused:

igorpisarev commented 4 years ago

I figured out why the bindings didn't work: they were initialized at the time of creating an instance of GridConstraintsManager and referred to the SimpleDoubleProperty instances that were also initialized at the same time. However, deserialization happened only after the GridConstraintsManager object was initially created, so the SimpleDoubleProperty instances were replaced with the deserialized objects behind the scenes, while the bindings were still referring to the old dummy objects.

For now I think the fix in #351 makes sense, since these bindings are not used anywhere anyway. But it's easy to make the same mistake again, and it's very misleading and not obvious from the code, so it would be nice to find a more robust generalized solution for this, such as serializing only the values and not the actual property instances that hold these values.