[8] Review request for 7124213 and 7160627 (original) (raw)

Sergey Bylokhov Sergey.Bylokhov at oracle.com
Wed Sep 12 07:08:13 PDT 2012

12.09.2012 17:51, Anthony Petrov wrote:

On 9/12/2012 5:42 PM, Sergey Bylokhov wrote:

12.09.2012 16:50, Anthony Petrov wrote

src/macosx/classes/sun/lwawt/LWComponentPeer.java

880 * empty. In the XPeers or WPeers we use some magic constants, but here we 881 * try to use something more useful,

Why can't we use "some magic constants" here, and the constant 1 in particular? This choice may be relevant to components that display some text, but e.g. for a container component using text-based metrics doesn't look right. Containers uses getP/getM methods from the LWCanvasPeer. Default implementation in LWComponentPeer is applicable for usual components only. Also, I see that "w" was used previously, and you're changing this to "0". Perhaps "W" might work best? Just because it is used in the XToolkit and Wtoolkit.(WTextAreaPeer/XTextAreaPeer). Should we extract this constant to a java.awt.peer.* interface then for consistency across all toolkits? Well, possibility to change this constant for different toolkits should be. src/macosx/classes/sun/lwawt/LWContainerPeer.java 43 abstract class LWContainerPeer<T extends Container, D extends_ _JComponent> 44 extends LWCanvasPeer<T, D> A Canvas peer implementation may be "heavier" since it can pull some graphics-related code which is unnecessary for simple containers. Most of the graphics code will be in the CGLGraphicsConfig. I see. But in the future we may want to modify the LWCanvasPeer, and this would also unintentionally affect the LWContainerPeer. I still suggest to extract all common code into a new abstract class. Nobody knows that will be in the future. But actually, if I'll move all this code to the separate class our LWCanvasPeer becomes noop. Note that in the WToolkit/XToolkit, container is subclass of X/W.CanvasPeer. From this point of view this is unification across toolkits. -- best regards, Anthony

-- best regards, Anthony On 9/11/2012 9:38 PM, Sergey Bylokhov wrote: Hi Everyone, Please review the fix for: 7124213 [macosx] pack() does ignore size of a component; doesn't on the other platforms. 7160627 [macosx] TextArea has wrong initial size

Description of main changes: All our components were split into 3 groups: - Uses getPreferedSize()/getMinimumSise from swing delegetes. - Uses its own size as a preferedSize/minimumSize. - Uses its own implementation. Notes: LWContainerPeer is subclass of LWCanvasPeer now. We can share buffers methods in LWCanvasPeer and LWWindowPeer. All magic/system constants were removed. Now we relies on default look and feel as much as possible. Bugs: http://bugs.sun.com/bugdatabase/viewbug.do?bugid=7160627 http://bugs.sun.com/bugdatabase/viewbug.do?bugid=7124213 ** Webrev can be found at: http://cr.openjdk.java.net/~serb/71242137160627/webrev.00/ -- Best regards, Sergey.

-- Best regards, Sergey.

More information about the awt-dev mailing list