Hi Anton,
Thanks for the clarification. Yes, I agree that there are many cases to
consider wrt. layout. What concerns me though, is a period of time
between a call to setContent() and a moment the componentAdded() handler
is invoked. Is it possible that the component's pref/min/max sizes
change during this period? If so,
1) Should we call the content.*SizeChanged() methods again after adding
the listeners in the componentAdded() handler?, and
2) Do we really have to call the *SizeChanged() methods in the
setContent() method? Is the pref/min/max sizes information really needed
during the period between setContent() and componentAdded() calls?
Both these questions directly relate to the JDK part of the fix.
--
best regards,
Anthony
On 08/25/2013 07:29 PM, Anton V. Tarasov wrote:
On 19.07.2013 19:05, Anthony Petrov wrote:
The updated fix looks fine. Thanks.
I'd only suggest to test whether the initial content.*SizeChanged()
calls in the setContent() are enough, or they should be
moved/duplicated somewhere else in case creating and showing of a
SwingNode instance is stretched in time, so as to make sure the FX
code always operates with valid values when the SwingNode gets added
to a live scene or gets modified while still being inserted into the
scene.
What you're talking about relates to the javafx part of the fix (which
I'll submit for review separately via the javafx review tool, and I'll
add you as a reviewer =).
Just to answer your question. The layout data is stored in SwingNode.
When it's added to scene after the content has been set on it, it will
return valid values via its pref/max/minWidth/Heigh() methods which its
containing layout pane will ask for while laying it out as a newly added
node. If a content is set/replaced on already visible SwingNode, it will
ask for relayout via calling impl_notifyLayoutBoundsChanged(). If you're
suggesting to test it, let me leave that to SQE as they have "layout" in
their SwingNode test plan. Because layout means quite a lot testcases...
Thanks,
Anton.
--
best regards,
Anthony
On 08/25/2013 06:53 PM, Anton V. Tarasov wrote:
Hi Anthony,
Thanks for the review.
On 19.07.2013 17:43, Anthony Petrov wrote:
Hi Anton,
The fix looks good overall.
I'm just not sure about the exact symmetry between the setContent()
method and the componentRemoved() listener wrt. the layoutSizeListener
adding/removing. In theory, the setContent() may be called several
times with different (or same) content objects. Similarly, the
contentPane may in theory be added or removed manually several times
(or even transferred between different JLFs).
setContent() is the API method which we don't control (and which indeed,
in theory, may be called several times with any content).
But the content pane is what we manage internally. It's tight to the JLF
instance and follows its life cycle. (Yes, a developer may hack it, but
this is strongly discouraged =)
Perhaps both adding and removing the layoutSizeListener should happen
in componentAdded() and componentRemoved() correspondingly?
I like the idea. Here's the new version:
http://cr.openjdk.java.net/~ant/JDK-8020927/webrev.1
Thanks,
Anton.
--
best regards,
Anthony
On 07/19/2013 04:27 PM, Anton V. Tarasov wrote:
Please, review a fix.
jira: https://jbs.oracle.com/bugs/browse/JDK-8020927
webrev: http://cr.openjdk.java.net/~ant/JDK-8020927/webrev.0
Layout bounds notifications are added to internal JLightweightFrame
API.
(Just FYI, related fx changes are here:
http://cr.openjdk.java.net/~ant/RT-30650/webrev.0)
Thanks,
Anton.
