<AWT Dev> [8] Review request for 7124213 and 7160627

[Sergey Bylokhov]

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/view_bug.do?bug_id=7160627
>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7124213
>>>> ** Webrev can be found at: 
>>>> http://cr.openjdk.java.net/~serb/7124213_7160627/webrev.00/
>>>>
>>>> -- 
>>>> Best regards, Sergey.
>>
>>


-- 
Best regards, Sergey.