<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
Wed Jan 20 11:00:45 UTC 2016
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.
>
More information about the swing-dev
mailing list