<Swing Dev> Review request for 8015748: JProgressbar with Aqua LaF ignores JProgressbar#applyComponentOrientation(ComponentOrientation.RIGHT_TO_LEFT) call
Alexander Scherbatiy
alexandr.scherbatiy at oracle.com
Fri Mar 18 21:16:58 UTC 2016
The fix looks good to me.
Thanks,
Alexandr.
On 21/01/16 07:49, Avik Niyogi wrote:
> Hi All,
> Please review my code change with inputs received:
> http://cr.openjdk.java.net/~aniyogi/8015748/webrev.08/
> <http://cr.openjdk.java.net/%7Eaniyogi/8015748/webrev.08/>
> With Regards,
> Avik Niyogi
>
>> On 21-Jan-2016, at 8:30 am, Avik Niyogi <avik.niyogi at oracle.com
>> <mailto:avik.niyogi at oracle.com>> wrote:
>>
>> Hi Sergey,
>> The JTreg will pass, but the errors are posted to the log and not as
>> an interrupt as it would prematurely terminate execution of entire
>> test case for other look and feels if done so.
>>
>> With Regards,
>> Avik Niyogi
>>
>>> On 21-Jan-2016, at 8:26 am, Avik Niyogi <avik.niyogi at oracle.com
>>> <mailto:avik.niyogi at oracle.com>> wrote:
>>>
>>> Hi Sergey,
>>> This is the *log* of the test
>>> *JProgressBarOrientationRobotTest.java* after doing a *make
>>> java.desktop* after commenting out my code changes in
>>> *AquaProgressBarUI.java* :
>>>
>>> run:
>>> [Metal]: LTR orientation test passed
>>> [Metal]: RTL orientation test passed
>>> [Nimbus]: LTR orientation test passed
>>> [Nimbus]: RTL orientation test passed
>>> [CDE/Motif]: LTR orientation test passed
>>> [CDE/Motif]: RTL orientation test passed
>>> [Mac OS X]: LTR orientation test passed
>>> [Mac OS X]: [Error]: LTR orientation test failed
>>> [Mac OS X]: [Error]: LTR orientation test failed
>>> BUILD SUCCESSFUL (total time: 31 seconds)
>>>
>>> With Regards,
>>> Avik Niyogi
>>>
>>>> On 20-Jan-2016, at 4:30 pm, Alexander Scherbatiy
>>>> <alexandr.scherbatiy at oracle.com
>>>> <mailto:alexandr.scherbatiy at oracle.com>> wrote:
>>>>
>>>>
>>>> The fix looks good to me.
>>>>
>>>> Thanks,
>>>> Alexandr.
>>>>
>>>> On 1/20/2016 12:47 PM, Rajeev Chamyal wrote:
>>>>>
>>>>> Looks good to me.
>>>>>
>>>>> Regards,
>>>>>
>>>>> Rajeev Chamyal
>>>>>
>>>>> *From:*Avik Niyogi
>>>>> *Sent:* 20 January 2016 12:23
>>>>> *To:* Rajeev Chamyal; Alexander Scherbatiy; Sergey Bylokhov
>>>>> *Cc:* swing-dev at openjdk.java.net <mailto:swing-dev at openjdk.java.net>
>>>>> *Subject:* Re: <Swing Dev> Review request for 8015748:
>>>>> JProgressbar with Aqua LaF ignores
>>>>> JProgressbar#applyComponentOrientation(ComponentOrientation.RIGHT_TO_LEFT)
>>>>> call
>>>>>
>>>>> Hi All,
>>>>>
>>>>> Please review the code changes made as with inputs for the webrev:
>>>>> http://cr.openjdk.java.net/~aniyogi/8015748/webrev.07/
>>>>> <http://cr.openjdk.java.net/%7Eaniyogi/8015748/webrev.07/>
>>>>> <http://cr.openjdk.java.net/%7Eaniyogi/8015748/webrev.07/>
>>>>>
>>>>> With Regards,
>>>>>
>>>>> Avik Niyogi
>>>>>
>>>>> On 20-Jan-2016, at 10:40 am, Rajeev Chamyal
>>>>> <rajeev.chamyal at oracle.com <mailto:rajeev.chamyal at oracle.com>
>>>>> <mailto:rajeev.chamyal at oracle.com>> wrote:
>>>>>
>>>>> Hello Avik,
>>>>>
>>>>> All exception caught during test should mark the test as failed.
>>>>> For example not able to set any LAF should also be considered as
>>>>> test failure.
>>>>>
>>>>> Regards,
>>>>>
>>>>> Rajeev Chamyal
>>>>>
>>>>> *From:*Avik Niyogi
>>>>> *Sent:*20 January 2016 10:20
>>>>> *To:*Rajeev Chamyal
>>>>> *Cc:*Alexander Scherbatiy; Sergey Bylokhov
>>>>> *Subject:*Re: <Swing Dev> Review request for 8015748: JProgressbar
>>>>> with Aqua LaF ignores
>>>>> JProgressbar#applyComponentOrientation(ComponentOrientation.RIGHT_TO_LEFT)
>>>>> call
>>>>>
>>>>> Hi Rajeev and Sergey,
>>>>>
>>>>> A gentle reminder. Kindly request to complete the pending review
>>>>> of my code changes in the webrev:
>>>>> http://cr.openjdk.java.net/~aniyogi/8015748/webrev.06/
>>>>> <http://cr.openjdk.java.net/%7Eaniyogi/8015748/webrev.06/>
>>>>> <http://cr.openjdk.java.net/%7Eaniyogi/8015748/webrev.06/>
>>>>>
>>>>> Thank you in advance.
>>>>>
>>>>> With Regards,
>>>>>
>>>>> Avik Niyogi
>>>>>
>>>>> On 19-Jan-2016, at 9:01 pm, Alexander Scherbatiy
>>>>> <alexandr.scherbatiy at oracle.com
>>>>> <mailto:alexandr.scherbatiy at oracle.com>
>>>>> <mailto:alexandr.scherbatiy at oracle.com>> wrote:
>>>>>
>>>>>
>>>>> 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>
>>>>> <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/>
>>>>> <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>
>>>>> <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/>
>>>>> <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>
>>>>> <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/>
>>>>> <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>
>>>>> <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/>
>>>>> <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>
>>>>> <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/>
>>>>> <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>
>>>>> <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/>
>>>>> <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>
>>>>> <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/>
>>>>> <http://cr.openjdk.java.net/%7Eaniyogi/8015748/webrev.00/>
>>>>> <http://cr.openjdk.java.net/%7Eaniyogi/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/20160319/df65ef32/attachment.html>
More information about the swing-dev
mailing list