<Swing Dev> Review request for 8015748: JProgressbar with Aqua LaF ignores JProgressbar#applyComponentOrientation(ComponentOrientation.RIGHT_TO_LEFT) call
Sergey Bylokhov
Sergey.Bylokhov at oracle.com
Wed Jan 20 16:18:41 UTC 2016
On 20/01/16 18:45, Avik Niyogi wrote:
> The fix is for Aqua Look and Feel. It works after the fix on JDK9
correct, it is for aqua and it works on jdk9. My point was that it works
without the fix also, this is incorrect behavior, it should not pass
before the fix.
>
>> On 20-Jan-2016, at 6:02 pm, Sergey Bylokhov <Sergey.Bylokhov at oracle.com> wrote:
>>
>> Hi, Avik.
>> The test still pass before the fix, but it should fail.
>>
>> On 20/01/16 14:00, Alexander Scherbatiy 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
>>>> *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/>
>>>>
>>>> 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>> 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/>
>>>>
>>>> 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>> 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>> 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/>
>>>>
>>>> <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.
>>>>
>>>
>>
>>
>> --
>> Best regards, Sergey.
>
--
Best regards, Sergey.
More information about the swing-dev
mailing list