<Swing Dev> Review request for 8015748: JProgressbar with Aqua LaF ignores JProgressbar#applyComponentOrientation(ComponentOrientation.RIGHT_TO_LEFT) call

Tue Jan 19 15:31:40 UTC 2016

  The fix looks good to me.

  Thanks,
  Alexandr.

On 19/01/16 15:27, Avik Niyogi wrote:
> Hi All,
> A gentle reminder. Please review my code changes as mentioned in the 
> webrev below as available in the link in the mail trail.
>
> With Regards,
> Avik Niyogi
>
>> On 18-Jan-2016, at 11:34 am, Avik Niyogi <avik.niyogi at oracle.com 
>> <mailto:avik.niyogi at oracle.com>> wrote:
>>
>> Hi All, Please find the changes as provided with incorporation of 
>> inputs:
>> http://cr.openjdk.java.net/~aniyogi/8015748/webrev.06/ 
>> <http://cr.openjdk.java.net/%7Eaniyogi/8015748/webrev.06/>
>>
>> With Regards,
>> Avik Niyogi
>>
>>> On 14-Jan-2016, at 10:57 pm, Sergey Bylokhov 
>>> <Sergey.Bylokhov at oracle.com <mailto:Sergey.Bylokhov at oracle.com>> wrote:
>>>
>>> Probably I missed something but why we need two tests? Note that the 
>>> manual test is not marked as manual, which means that it will be run 
>>> during the regular run?(even if -a option is provided to jtreg). 
>>> Please check your other review requests for this issue.
>>>
>>> moreover on my system JProgressBarOrientationManualTest.java simply 
>>> passed, and JProgressBarOrientationRobotTest.java failed even after 
>>> the fix. Please recheck.
>>>
>>> On 14/01/16 13:11, Avik Niyogi wrote:
>>>> Hi All,
>>>> Please find the changes as provided with incorporation of inputs:
>>>> http://cr.openjdk.java.net/~aniyogi/8015748/webrev.05/ 
>>>> <http://cr.openjdk.java.net/%7Eaniyogi/8015748/webrev.05/>
>>>>
>>>> With Regards,
>>>> Avik Niyogi
>>>>> On 14-Jan-2016, at 3:18 pm, Alexander Scherbatiy
>>>>> <alexandr.scherbatiy at oracle.com 
>>>>> <mailto:alexandr.scherbatiy at oracle.com>
>>>>> <mailto:alexandr.scherbatiy at oracle.com>> wrote:
>>>>>
>>>>> On 1/14/2016 8:18 AM, Avik Niyogi wrote:
>>>>>> Hi All,
>>>>>> Please find changes as provided with incorporation of inputs:
>>>>>> http://cr.openjdk.java.net/~aniyogi/8015748/webrev.04/ 
>>>>>> <http://cr.openjdk.java.net/%7Eaniyogi/8015748/webrev.04/>
>>>>>> <http://cr.openjdk.java.net/%7Eaniyogi/8015748/webrev.04/>
>>>>>
>>>>>  It is better to restore the graphics transform after the progress
>>>>> bar is painted and before the paintString call because the a method
>>>>> that calls AquaProgressBarUI.paint(Graphics) can rely that the
>>>>> graphics transform is unchanged.
>>>>> In your fix the graphics transform is not restored if
>>>>> progressBar.isStringPainted() returns false.
>>>>>
>>>>> Thanks,
>>>>> Alexandr.
>>>>>
>>>>>>
>>>>>> With Regards,
>>>>>> Avik Niyogi
>>>>>>> On 13-Jan-2016, at 7:02 pm, Alexander Scherbatiy
>>>>>>> <alexandr.scherbatiy at oracle.com 
>>>>>>> <mailto:alexandr.scherbatiy at oracle.com>
>>>>>>> <mailto:alexandr.scherbatiy at oracle.com>
>>>>>>> <mailto:alexandr.scherbatiy at oracle.com>> wrote:
>>>>>>>
>>>>>>> On 1/13/2016 9:28 AM, Avik Niyogi wrote:
>>>>>>>> Hi All,
>>>>>>>> Please find changes as provided with incorporation of inputs:
>>>>>>>> http://cr.openjdk.java.net/~aniyogi/8015748/webrev.03/ 
>>>>>>>> <http://cr.openjdk.java.net/%7Eaniyogi/8015748/webrev.03/>
>>>>>>>> <http://cr.openjdk.java.net/%7Eaniyogi/8015748/webrev.03/>
>>>>>>>> <http://cr.openjdk.java.net/%7Eaniyogi/8015748/webrev.03/>
>>>>>>>>
>>>>>>>
>>>>>>> It looks like a string on a vertical progress bar with the right to
>>>>>>> left orientation will be mirrored.
>>>>>>> Did you try just restore the scale/translate transform after the
>>>>>>> painter.paint() call? Will it help in such case?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Alexandr.
>>>>>>>
>>>>>>>> With Regards,
>>>>>>>> Avik Niyogi
>>>>>>>>> On 12-Jan-2016, at 11:49 pm, Alexander Scherbatiy
>>>>>>>>> <alexandr.scherbatiy at oracle.com 
>>>>>>>>> <mailto:alexandr.scherbatiy at oracle.com>
>>>>>>>>> <mailto:alexandr.scherbatiy at oracle.com>
>>>>>>>>> <mailto:alexandr.scherbatiy at oracle.com>
>>>>>>>>> <mailto:alexandr.scherbatiy at oracle.com>> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> - there was the comment below that it is better to revert the
>>>>>>>>> transform back after the painter.paint() call
>>>>>>>>> - according to the comment from the
>>>>>>>>> http://mail.openjdk.java.net/pipermail/swing-dev/2016-January/005262.html
>>>>>>>>>
>>>>>>>>> It is true that a filled progress bar has different colors because
>>>>>>>>> of animation under Aqua L&F.
>>>>>>>>> However, it is possible to compare colors before a progress bar
>>>>>>>>> was filled and after that to check that the progress bar is filled
>>>>>>>>> from the correct side.
>>>>>>>>> For example let's set a progress bar value to 0 and get its color
>>>>>>>>> from 5/6 of the progress bar width
>>>>>>>>>   progress bar: [_________o__]  // get a color at point o
>>>>>>>>> Now set the progress bar value to 30 and get a color at the same
>>>>>>>>> point.
>>>>>>>>> If colors are the same then  the progress bar is filled from left
>>>>>>>>> to the right [||||_____o__].
>>>>>>>>> If colors are different then the progress bar is filled from the
>>>>>>>>> right to the left [________|o||] .
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Alexandr.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 12/01/16 13:34, Avik Niyogi wrote:
>>>>>>>>>> Hi All,
>>>>>>>>>>
>>>>>>>>>> Please find the code changes in fix as with the inputs received
>>>>>>>>>> for the same.
>>>>>>>>>> http://cr.openjdk.java.net/~aniyogi/8015748/webrev.02/ 
>>>>>>>>>> <http://cr.openjdk.java.net/%7Eaniyogi/8015748/webrev.02/>
>>>>>>>>>> <http://cr.openjdk.java.net/%7Eaniyogi/8015748/webrev.02/>
>>>>>>>>>> <http://cr.openjdk.java.net/%7Eaniyogi/8015748/webrev.02/>
>>>>>>>>>>
>>>>>>>>>> With Regards,
>>>>>>>>>> Avik Niyogi
>>>>>>>>>>
>>>>>>>>>>> On 11-Jan-2016, at 3:55 pm, Semyon Sadetsky
>>>>>>>>>>> <semyon.sadetsky at oracle.com 
>>>>>>>>>>> <mailto:semyon.sadetsky at oracle.com><mailto:semyon.sadetsky at oracle.com>
>>>>>>>>>>> <mailto:semyon.sadetsky at oracle.com>
>>>>>>>>>>> <mailto:semyon.sadetsky at oracle.com>> wrote:
>>>>>>>>>>>
>>>>>>>>>>> Hi Avik,
>>>>>>>>>>>
>>>>>>>>>>> Shouldn't the graphics transformation be restored before the
>>>>>>>>>>> paintString() call?
>>>>>>>>>>>
>>>>>>>>>>> It seems to me that left/right insets need to be swapped for
>>>>>>>>>>> right-to-left painting with mirroring graphics transformation.
>>>>>>>>>>>
>>>>>>>>>>> --Semyon
>>>>>>>>>>>
>>>>>>>>>>> On 1/5/2016 1:22 PM, Avik Niyogi wrote:
>>>>>>>>>>>> Hi All,
>>>>>>>>>>>> Please find webrev with inputs as provided:
>>>>>>>>>>>> http://cr.openjdk.java.net/~aniyogi/8015748/webrev.01/ 
>>>>>>>>>>>> <http://cr.openjdk.java.net/%7Eaniyogi/8015748/webrev.01/>
>>>>>>>>>>>> <http://cr.openjdk.java.net/%7Eaniyogi/8015748/webrev.01/>
>>>>>>>>>>>> With Regards,
>>>>>>>>>>>> Avik Niyogi
>>>>>>>>>>>>
>>>>>>>>>>>>> On 23-Dec-2015, at 7:29 pm, Alexander Scherbatiy
>>>>>>>>>>>>> <alexandr.scherbatiy at oracle.com 
>>>>>>>>>>>>> <mailto:alexandr.scherbatiy at oracle.com>
>>>>>>>>>>>>> <mailto:alexandr.scherbatiy at oracle.com>
>>>>>>>>>>>>> <mailto:alexandr.scherbatiy at oracle.com>> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> - please check that the progress bar string
>>>>>>>>>>>>> (progressBar.setString()/setStringPainted()) is painted 
>>>>>>>>>>>>> correctly.
>>>>>>>>>>>>> - is it possible to write an automated test for the fix?
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>> Alexandr.
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 12/21/2015 11:47 AM, Avik Niyogi wrote:
>>>>>>>>>>>>>> Hi All,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Kindly review the bug fix for JDK 9.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> *Bug:*
>>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8015748
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> *Webrev:*
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~aniyogi/8015748/webrev.00/
>>>>>>>>>>>>>> <http://cr.openjdk.java.net/%7Eaniyogi/8015748/webrev.00/>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> *Issue:*
>>>>>>>>>>>>>> The manual test:
>>>>>>>>>>>>>> Swing_JProgressbar/Manual/ProgressBarLAFTests/ProgressBarLAFTest1
>>>>>>>>>>>>>> in testsuite
>>>>>>>>>>>>>> http://sqe-hg.us.oracle.com/hg/index.cgi/testbase/javase/functional/7/swing
>>>>>>>>>>>>>> fails
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> *Cause:*
>>>>>>>>>>>>>> Due to not honouring of RIGHT_TO_LEFT parameter for
>>>>>>>>>>>>>> setOrientation method applied for a JProgressBar for the
>>>>>>>>>>>>>> AquaLookAndFeel only,
>>>>>>>>>>>>>> the progressBar does not have the ability to grow from right
>>>>>>>>>>>>>> to left. This issue was verified to exist only in
>>>>>>>>>>>>>> AquaLookAndFeel for JProgressBar.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> *Fix:*
>>>>>>>>>>>>>> Added implementation for the check of RIGHT_TO_LEFT
>>>>>>>>>>>>>> ComponentOrientation and verified with other combination
>>>>>>>>>>>>>> orientation with available
>>>>>>>>>>>>>> Horizontal and Vertical orientations as provided from before.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> With Regards,
>>>>>>>>>>>>>> Avik Niyogi
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>> --
>>> Best regards, Sergey.
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20160119/eba60bf6/attachment.html>