Re: Review Request For 8160766: [TEST_BUG] java/awt/Focus/DisposedWindow

2016-11-03 Thread Ajit Ghaisas 
Looks good.

Regards,
Ajit

-Original Message-
From: Sergey Bylokhov 
Sent: Thursday, November 03, 2016 4:08 PM
To: Ambarish Rapte; Ajit Ghaisas; Semyon Sadetsky; awt-dev@openjdk.java.net
Subject: Re: Review Request For 8160766: [TEST_BUG] 
java/awt/Focus/DisposedWindow

HI, Ambarish.
Looks fine, but it seems that you need to make buttonReceivedFocus volatile, 
since you write/read it from the different thread. You can do this just before 
the push.

On 27.10.16 18:21, Ambarish Rapte wrote:
> Hi All,
>
>
>
> Please review this test bug fix.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8160766
>
>
>
> This is small fix to add waitForIdle & delay. 
> Webrev.00 is for reference of minimal changes.
>
> Webrev with minimum changes for review:
> http://cr.openjdk.java.net/~arapte/8160766/webrev.00/
>
>
>
> This is an applet based, so would be removing the applet.
>
> Final Webrev without applet:
> http://cr.openjdk.java.net/~arapte/8160766/webrev.01/
>
>
>
> Regards,
>
> Ambarish
>
>
>


--
Best regards, Sergey.

Re: [9] Review request for JDK-7153700: [macosx] add support for MouseMotionListener to the TrayIcon

2016-11-03 Thread Sergey Bylokhov 

Looks fine.

On 03.11.16 14:13, Manajit Halder wrote:

Hi Sergey,

Thank you for the review comment. Code is modified as per the comment.
Please review the modified webrev:

http://cr.openjdk.java.net/~mhalder/7153700/webrev.02/

Thanks,
Manajit


On 27-Oct-2016, at 5:05 pm, Sergey Bylokhov
mailto:sergey.bylok...@oracle.com>> wrote:

Hi, Manajit.

179 if (trackingArea) {
180 [self removeTrackingArea:trackingArea];
181 }
I think that the code above is also not necessary(Since this code is
executed only once). The pointer trackingArea is not initialized to
nil so it can contains some non-nil garbage, and calling
removeTrackingArea:trackingArea can cause a crash.

On 27.10.16 14:08, Manajit Halder wrote:

Hi Sergey,

Code is modified to correct a memory leak. Please review the modified
webrev:
_http://cr.openjdk.java.net/~mhalder/7153700/webrev.01/_

Thanks,
Manajit


On 21-Oct-2016, at 1:20 pm, Manajit Halder
mailto:manajit.hal...@oracle.com>
> wrote:

Hi All,

Kindly review the fix for JDK9.

Bug:
https://bugs.openjdk.java.net/browse/JDK-7153700

Webrev:
http://cr.openjdk.java.net/~mhalder/7153700/webrev.00/

Issue:
[macosx] add support for MouseMotionListener to the TrayIcon.

Fix:
Added MouseMotionListener support for TrayIcon on Mac OS X.

Regards,
Manajit





--
Best regards, Sergey.





--
Best regards, Sergey.

Re: [9] Review request for JDK-7153700: [macosx] add support for MouseMotionListener to the TrayIcon

2016-11-03 Thread Manajit Halder 
Hi Sergey,

Thank you for the review comment. Code is modified as per the comment. 
Please review the modified webrev:

http://cr.openjdk.java.net/~mhalder/7153700/webrev.02/ 


Thanks,
Manajit

> On 27-Oct-2016, at 5:05 pm, Sergey Bylokhov  
> wrote:
> 
> Hi, Manajit.
> 
> 179 if (trackingArea) {
> 180 [self removeTrackingArea:trackingArea];
> 181 }
> I think that the code above is also not necessary(Since this code is executed 
> only once). The pointer trackingArea is not initialized to nil so it can 
> contains some non-nil garbage, and calling removeTrackingArea:trackingArea 
> can cause a crash.
> 
> On 27.10.16 14:08, Manajit Halder wrote:
>> Hi Sergey,
>> 
>> Code is modified to correct a memory leak. Please review the modified
>> webrev:
>> _http://cr.openjdk.java.net/~mhalder/7153700/webrev.01/_ 
>> 
>> 
>> Thanks,
>> Manajit
>> 
>>> On 21-Oct-2016, at 1:20 pm, Manajit Halder >> 
>>> >> 
>>> wrote:
>>> 
>>> Hi All,
>>> 
>>> Kindly review the fix for JDK9.
>>> 
>>> Bug:
>>> https://bugs.openjdk.java.net/browse/JDK-7153700
>>> 
>>> Webrev:
>>> http://cr.openjdk.java.net/~mhalder/7153700/webrev.00/
>>> 
>>> Issue:
>>> [macosx] add support for MouseMotionListener to the TrayIcon.
>>> 
>>> Fix:
>>> Added MouseMotionListener support for TrayIcon on Mac OS X.
>>> 
>>> Regards,
>>> Manajit
>> 
> 
> 
> -- 
> Best regards, Sergey.

Re: 8138771: java.awt.image.AbstractMultiResolutionImage needs customized spec for methods of Image which it implements

2016-11-03 Thread Alexandr Scherbatiy 

The fix looks good to me.

Thanks,
Alexandr.

On 10/31/2016 9:31 PM, Jim Graham wrote:

Looks good.  +1

...jim

On 10/30/16 11:53 PM, Avik Niyogi wrote:

Hi All,
Please review the proposed specification for JDK9 including inputs 
from reviewer reviews.
*cr.openjdk.java.net/~aniyogi/8138771/webrev.05/* 


Thank you in advance.

With Regards,
Avik Niyogi
On 28-Oct-2016, at 1:18 am, Jim Graham > wrote:


Hi Avik,

My suggestion about adding a word "the" was not taken and a couple 
of other changes were made to the @return
statements which are not optimal.  Let's reset and use the following 
@return statements for each of the methods (to

mirror the way these are described in the Image base class):

getWidth() - @return the width of the base image, or -1 if the width 
is not yet known
getHeight() - @return the height of the base image, or -1 if the 
height is not yet known

getGraphics() - @return throws {@code UnsupportedOperationException}
getSource() - @return the image producer that produces the pixels 
for the base image
getProperty() - @return the value of the named property in the base 
image


(It would also be nice if the blank lines were the same in all of 
the doc comments.  Some comments have a couple of
blank lines to separate the javadoc sections and others have no 
blank lines.  But, that doesn't affect correctness, it

is just an easthetic issue...)

...jim

On 10/26/16 11:51 PM, Avik Niyogi wrote:

Hi All,

Please review the proposed specification for JDK9 including inputs 
from reviewer reviews.

*http://cr.openjdk.java.net/~aniyogi/8138771/webrev.04/*
Thank you in advance.

With Regards,
Avik Niyogi

On 27-Oct-2016, at 2:33 am, Jim Graham 

> wrote:

The "@return" tags should not start with "returns" in the text.

Also, in the @return for getProperty(), insert a word "the" as 
"the property of the base image"...


...jim

On 10/26/16 12:36 AM, Avik Niyogi wrote:

Hi All,

Please review the proposed specification for JDK9 including 
inputs from reviver reviews.


*cr.openjdk.java.net/~aniyogi/8138771/webrev.03/* 






Thank you in advance.

With Regards,
Avik Niyogi

Re: Review Request For 8160766: [TEST_BUG] java/awt/Focus/DisposedWindow

2016-11-03 Thread Sergey Bylokhov 

HI, Ambarish.
Looks fine, but it seems that you need to make buttonReceivedFocus 
volatile, since you write/read it from the different thread. You can do 
this just before the push.


On 27.10.16 18:21, Ambarish Rapte wrote:

Hi All,



Please review this test bug fix.

Bug: https://bugs.openjdk.java.net/browse/JDK-8160766



This is small fix to add waitForIdle & delay. Webrev.00
is for reference of minimal changes.

Webrev with minimum changes for review:
http://cr.openjdk.java.net/~arapte/8160766/webrev.00/



This is an applet based, so would be removing the applet.

Final Webrev without applet:
http://cr.openjdk.java.net/~arapte/8160766/webrev.01/



Regards,

Ambarish






--
Best regards, Sergey.

Re: Review Request JDK:-8162959 [HiDPI] screenshot artifacts using AWT Robot

2016-11-03 Thread Alexandr Scherbatiy 

On 11/2/2016 1:57 PM, Prem Balakrishnan wrote:


Hi Alexander,

Please review updated patch.

http://cr.openjdk.java.net/~pkbalakr/8162959/webrev.01/ 



Added a new public API “*Image createHiDPIScreenCapture(Rectangle 
screenRect)”.*


**Returns an ordinary screenshot(BufferedImage) if the UI scale is 1.**

Returns MultiResolutionImage for HiDPI display with two resolution 
variants,


1.Low Resolution/base image with user input width and height,  I have 
used interpolation algorithm for scaling , adapted from JavaFX Robot 
(http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/89a5de54b7dc),


2.High Resolution image with scaled width and height .

  - Please, check that the Robot.createScreenCapture(Rectangle) method 
returns a scaled down image on the HiDPI display

  - Probably existing Java methods can be used for an image scaling.
  For example, there is the Image.getScaledInstance(int width, int 
height, int hints) method.
  Or may be it is better just to create an image with required size and 
draw the high-resolution image into it using a specific scale and 
rendering hints.


  Thanks,
  Alexandr.


Regards,

Prem

*From:*Alexander Scherbatiy
*Sent:* Thursday, October 13, 2016 3:21 PM
*To:* Prem Balakrishnan; Sergey Bylokhov; awt-dev@openjdk.java.net
*Subject:* Re: Review Request JDK:-8162959 [HiDPI] screenshot 
artifacts using AWT Robot


On 06/10/16 15:28, Prem Balakrishnan wrote:

Hi*,*

**

Please review fix for JDK 9,

*Bug:*https://bugs.openjdk.java.net/browse/JDK-8162959

*Webrev:*http://cr.openjdk.java.net/~pkbalakr/8162959/webrev.00/


I have adapted the same fix as used for JavaFX Robot

*Bug: *JDK-8162783 .

*Patch: *http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/89a5de54b7dc

New Public API ” BufferedImage createScreenCapture(Rectangle
screenRect, boolean isHiDPI)”is added,

Which will have a parameter to specify if HiDPI.

  It is better to a add public method which returns MultiResolution 
image on HiDPI display and  consists of two resoltion variants

   - base image which has size requested by a user
   - high-resolution image which creates an original screen capture

  The proposed by your algorithm can be applied to the base image.
  For more details see JDK-8020618 [macosx] java.awt.Robot makes 
blurry screen captures on Retina


  Thanks,
  Alexandr.

  
Regards,

Prem

Re: Fix for JDK-8160146 : Resolve disabled GCC warning 'deprecated-declarations' for libawt_xawt

2016-11-03 Thread Sergey Bylokhov 

On 02.11.16 10:01, Ajit Ghaisas wrote:

Can I get another +1 for this fix?

+1



Regards,
Ajit

-Original Message-
From: Erik Joelsson
Sent: Thursday, October 27, 2016 3:30 PM
To: Ajit Ghaisas; build-...@openjdk.java.net; awt-dev@openjdk.java.net
Subject: Re: Fix for JDK-8160146 : Resolve disabled GCC warning 
'deprecated-declarations' for libawt_xawt

Looks good.

/Erik


On 2016-10-27 11:02, Ajit Ghaisas wrote:

Hi,



 Fix of HYPERLINK 
"https://bugs.openjdk.java.net/browse/JDK-8165232"JDK-8165232 has fixed the 
'deprecated' warnings from libawt_xawt.

 Now, only removing warning suppression from makefile remains. This webrev 
does it.



 http://cr.openjdk.java.net/~aghaisas/8160146/webrev.0/



 I have run JPRT and builds are successful.



 Request you to review.



Regards,

Ajit







--
Best regards, Sergey.

Re: [9] Review request for 8163101: dual-screen issue with JMenu, JPopupMenu

2016-11-03 Thread Alexander Zvegintsev 

+1

Thanks,
Alexander.

On 8/31/16 5:18 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

bug: https://bugs.openjdk.java.net/browse/JDK-8163101

webrev: http://cr.openjdk.java.net/~ssadetsky/8163101/webrev.00/

In the 8137571 the available screen area request

Toolkit.getDefaultToolkit().getScreenSize()

was replaced by

graphicsConfig.getBounds();

that returns a particular screen area which is not the same as the 
joint screen area in case of Xinerama multi-monitor configuration.


Reverting the previous way fixes the issue.

--Semyon

Re: [9] Review request for 8143914: Provide Mac-specific fullscreen support

2016-11-03 Thread Semyon Sadetsky 

+1

--Semyon


On 03.11.2016 03:20, Sergey Bylokhov wrote:
I guess we can extract some of the common logic in the fix, upto other 
reviewers, this looks fine to me.


On 25.10.16 19:25, Alexander Zvegintsev wrote:

Please see the updated webrev:

http://cr.openjdk.java.net/~azvegint/jdk/9/8143914/02/


Thanks,
Alexander.

On 10/11/16 5:00 PM, Alexander Zvegintsev wrote:

Hi Sergey,

Please see the updated webrev:

http://cr.openjdk.java.net/~azvegint/jdk/9/8143914/01/

As I can observe other windows  such as native dialogs doesn't have
fullscreen button.


On 10/2/16 5:52 PM, Sergey Bylokhov wrote:

Hi, Alexander.
In the fix you use eawt events which are generated by the same peer.
Why not use windowWill/DidEnterFullScreen +
windowWill/DidExitFullScreen?
Why the fix is for the Frames only, there are some limitations for
other windows, dialogs etc?

On 06.09.16 17:32, Alexander Zvegintsev wrote:

Hello,

please review the fix

http://cr.openjdk.java.net/~azvegint/jdk/9/8143914/00/

for the issue

https://bugs.openjdk.java.net/browse/JDK-8143914


This fix adds the green FullScreen button to a resizable frames by
default.

Previous maximize behavior is still accessible by holding Alt while
clicking on the green button.

setResizable is fixed because of two reasons:

Window will not be able to leave fullscreen if we remove resizable 
from

style bits(and even if we set it again)

Window will not have the green button(maximize/zoom or fullscreen) if
the window was initially not resizable.