<AWT Dev> Request for Review: 7170655 frame size change does not follow label font change

[Jonathan Lu]

Hi Charles,

Verified, thanks!

Regards
- Jonathan

On 05/30/2012 11:02 AM, Charles Lee wrote:
> Hi Jonathan,
>
> The patch is committed @
>
> Changeset: 3c9adc88956d
> Author:    luchsh
> Date:      2012-05-30 10:58 +0800
> URL:http://hg.openjdk.java.net/jdk8/awt/jdk/rev/3c9adc88956d
>
> 7170655: Frame size does not follow font size change with XToolkit
> Reviewed-by: serb, art
>
> Please verify it and thank you all for reviewing the patch.
>
> On 05/29/2012 01:23 PM, Jonathan Lu wrote:
>> Hi Artem,
>>
>> Thanks for review!
>>
>> On 05/28/2012 09:12 PM, Artem Ananiev wrote:
>>> Hi, Jonathan,
>>>
>>> the fix looks fine.
>>>
>>> The test requires some corrections, though:
>>>
>>> 1. "@bug 7150655" should be replaced with "@bug 7170655".
>> Sorry for the typo.
>>>
>>> 2. Most of the tests in test/java/awt/ have some "meaningful" names, 
>>> e.g. LabelFontToAffectFrameSize, so every developer can easily 
>>> understand what a test is about.
>>>
>> In the new patch, I've renamed it to "ResizeAfterSetFont".
>>
>>> 3. Instead of Thread.sleep() calls, I would recommend to use the 
>>> test.java.awt.regtesthelpers.Util class. An example can be found here:
>>>
>>> test/java/awt/Window/WindowType/WindowType.java
>>
>> Thanks for the reference, I've updated it in the new patch, see
>>
>> http://cr.openjdk.java.net/~luchsh/7170655_4/
>>
>> Best regards!
>> - Jonathan
>>
>>>
>>> Thanks,
>>>
>>> Artem
>>>
>>> On 5/28/2012 9:17 AM, Jonathan Lu wrote:
>>>> Hello Sergey,
>>>>
>>>> On 05/26/2012 03:32 AM, Sergey Bylokhov wrote:
>>>>> Hi, Jonathan.
>>>>> Fix looks good to me. Just note that one more reviewer needed.
>>>>
>>>> Thanks for review.
>>>>
>>>> Can anybody please help to take another look?
>>>>
>>>>>
>>>>> Thanks for the fix.
>>>>
>>>> Best regards!
>>>> - Jonathan
>>>>> 24.05.2012 11:50, Jonathan Lu wrote:
>>>>>> Hello Sergey,
>>>>>>
>>>>>> I made one automatic jtreg test case and put it to following link
>>>>>> along with the patch.
>>>>>> http://cr.openjdk.java.net/~luchsh/7170655_3/
>>>>>>
>>>>>> Could you please take a look?
>>>>>>
>>>>>> Thanks!
>>>>>> - Jonathan
>>>>>>
>>>>>> On 05/23/2012 08:34 PM, Sergey Bylokhov wrote:
>>>>>>> Hi, Jonathan,
>>>>>>> Fix looks good. But I guess this bug was not caught by the
>>>>>>> regression tests, so new test should be added(automatic test is
>>>>>>> better).
>>>>>>>
>>>>>>> 23.05.2012 14:15, Jonathan Lu wrote:
>>>>>>>> Hi Sergey,
>>>>>>>>
>>>>>>>> On 05/22/2012 10:23 PM, Sergey Bylokhov wrote:
>>>>>>>>> Hi, Jonathan,
>>>>>>>>> Looks like this bug is duplicate of
>>>>>>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7161865
>>>>>>>>> And the reason of the issue is that we post UpdateEvent in
>>>>>>>>> target.repaint() in XLabelPeer.setFont(), but we should paint
>>>>>>>>> native part of the component in place[1] and then post 
>>>>>>>>> PaintEvent[2].
>>>>>>>>>
>>>>>>>>> - To fix [1] we can replace target.repaint() to repaint() in
>>>>>>>>> setFont(). Does it solve your issue?
>>>>>>>>
>>>>>>>> Yes, this resolves my problem, and here's the updated patch from
>>>>>>>> your suggestion,
>>>>>>>> http://cr.openjdk.java.net/~luchsh/7170655_2/
>>>>>>>>
>>>>>>>> Thanks a lot!
>>>>>>>> - Jonathan
>>>>>>>>
>>>>>>>>>
>>>>>>>>> 22.05.2012 11:47, Jonathan Lu wrote:
>>>>>>>>>> Hi awt-dev,
>>>>>>>>>>
>>>>>>>>>> Here's a patch for bug 7170655, could anybody please help to 
>>>>>>>>>> take
>>>>>>>>>> a look?
>>>>>>>>>> http://cr.openjdk.java.net/~luchsh/7170655/
>>>>>>>>>>
>>>>>>>>>> The problem is that painting event from EDT during painting
>>>>>>>>>> handling does not get processed immediately, so leave a lag to
>>>>>>>>>> the user. My solution here is to coalesce and dispatch the new
>>>>>>>>>> paint event right after it was posted. This happens only to AWT
>>>>>>>>>> components.
>>>>>>>>>>
>>>>>>>>>> Thanks!
>>>>>>>>>> -Jonathan
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>
>
>