-
Notifications
You must be signed in to change notification settings - Fork 413
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
Fixed problem in 3 Layouts where Dimension reused between calls to getPreferredSize and getScrollSize #2229
base: master
Are you sure you want to change the base?
Conversation
I'm not sure if the command code in the span button is still there, it might be problematic. Moving the field into the method is probably wrong and could have a huge impact on performance. It might make sense to have two fields instead of one though. |
Since there are no comments explaining the code it's difficult to me to know if there are good reasons for the current implementation. getPreferredSize gets recalculated on each call, so it shouldn't add a performance penalty. Also, since since the more recent layouts do it the same was as my change I reasoned the old layouts just never got updated. In any case, the current implementation causes the really tricky side effect I described, so something needs to be done. |
No, preferred size is always cached. There are a lot of dependencies on this as this is pretty deep in. |
I meant that it is recalculated on each call to Layout.getpreferredSize() but, I won't pretend to an expert on this But as I said, most layouts e.g. LayeredLayout, does create a new Dimension in each call to getPreferredSize, so I simply assumed that that was also a valid implementation and it fixes the issue. Anyway, I hope the real issue that led me to make this change was clear? If not, let me know, and I'll be happy to create an example to reproduce the issue. |
That's a pathway that is called when layout is forced. getPreferredSize in component always caches |
Here's the code to reproduce the issue. As you can see when you click the button to hide the Container it doesn't work. Hopefully this helps illustrate the issue. The reason is what I tried to describe above. NB. You need to click the button several times to see issue, first time it behaves correctly, but not after that.
|
I think it the same problem I mentionned here : #2228 |
Any update on this (the issue with setHidden not working is currently preventing me from building my app for a device)? Could you reproduce the issue based my example above? |
I suggest moving that to an issue. Adding a scrollable container to the north is a mistake as the border layout always gives the full requested height to the north/south containers. |
Border, Box and Flow layouts (contrary to all other layouts) reuse the same Dimension for calls to getPreferredSize and getScrollSize. This creates an issue since the same dim instance gets used both as preferredSize and scrollSize which creates side effects when preferredSize is forced to (0,0) in setHidden(true). (Took me several days to debug why a hidden container with BoxLayout kept becoming visible after being set hidden)
(Sorry for the Commit history, still struggling with using Git correctly)