Re: Review Request of 8146321: [macosx] JInternalFrame frame icon in wrong position on Mac L if icon is not ImageIcon

2016-02-09 Thread Rajeev Chamyal 
The fix looks good to me.

 

Regards,

Rajeev Chamyal

 

From: Avik Niyogi 
Sent: 09 February 2016 11:43
To: Sergey Bylokhov; Alexandr Scherbatiy; Rajeev Chamyal
Cc: swing-dev@openjdk.java.net
Subject: Re:  Review Request of 8146321: [macosx] JInternalFrame 
frame icon in wrong position on Mac L if icon is not ImageIcon

 

A gentle reminder. Please review webrev with changes as per the inputs provided:

HYPERLINK 
"http://cr.openjdk.java.net/~aniyogi/8146321/webrev.06/"cr.openjdk.java.net/~aniyogi/8146321/webrev.06/

 

With Regards,

Avik Niyogi

On 08-Feb-2016, at 12:42 pm, Avik Niyogi mailto:avik.niy...@oracle.com"avik.niy...@oracle.com> wrote:

 

Hi Sergey,

Please review webrev with changes as per the inputs provided:

HYPERLINK 
"http://cr.openjdk.java.net/~aniyogi/8146321/webrev.06/"cr.openjdk.java.net/~aniyogi/8146321/webrev.06/

 

With Regards,

Avik Niyogi

 

On 08-Feb-2016, at 2:29 am, Sergey Bylokhov mailto:sergey.bylok...@oracle.com"sergey.bylok...@oracle.com> wrote:

 

Both updated methods have a typo, 0 will be returned if 
frame.getFrameIcon()==null:

   protected int getIconWidth(final JInternalFrame frame) {
   int width = 0;

   Icon icon = frame.getFrameIcon();
   if (icon == null) {
   icon = UIManager.getIcon("InternalFrame.icon");
   }
   else {
   width = Math.min(icon.getIconWidth(), sMaxIconWidth);
   }

   return width;
   }

   protected int getIconHeight(final JInternalFrame frame) {
   int height = 0;

   Icon icon = frame.getFrameIcon();
   if (icon == null) {
   icon = UIManager.getIcon("InternalFrame.icon");
   }
   else{
   height = Math.min(icon.getIconHeight(), sMaxIconHeight);
   }

   return height;
   }

Note that getIconWidth/getIconHeight should return width/height of the icon, 
but this is not true anymore, because paintTitleIcon() now use the aspect 
ratio, which can produce the different Width/Height for the same icon.


On 04.02.16 12:26, Alexandr Scherbatiy wrote:




  The fix looks good to me.

  Thanks,
  Alexandr.

On 2/3/2016 8:51 PM, Avik Niyogi wrote:



Hi All,
Please review the code changes made as per the inputs provided:
http://cr.openjdk.java.net/~aniyogi/8146321/webrev.05/


With Regards,
Avik Niyogi




On 03-Feb-2016, at 7:38 pm, Alexandr Scherbatiy
mailto:alexandr.scherba...@oracle.com"alexandr.scherba...@oracle.com
> wrote:

On 2/3/2016 12:47 AM, Avik Niyogi wrote:



Hi All,
Please review the code changes made as per the inputs provided:
http://cr.openjdk.java.net/~aniyogi/8146321/webrev.04


 326 g2.translate(0, 0);
 The translation to the zero vector leaves the coordinate system the
same.

 327 float xScaleFactor = (float) sMaxIconWidth /
icon.getIconWidth();
 It is better to use double values because the graphics transform
methods use them.

 332 g2.scale(scaleFactorAspectRatio,
scaleFactorAspectRatio);
 333 g2.translate(x / scaleFactorAspectRatio, y /
scaleFactorAspectRatio);
 Is it possible to use the translation first and the scale the
second? I this case where no need to re-scale translation coordinates.

 Thanks,
 Alexandr.





With Regards,
Avik Niyogi



On 02-Feb-2016, at 5:55 pm, Alexandr Scherbatiy
<HYPERLINK 
"mailto:alexandr.scherba...@oracle.com"alexandr.scherba...@oracle.com>
wrote:

On 2/2/2016 3:41 AM, Avik Niyogi wrote:



Hi Alexander,




On 02-Feb-2016, at 3:44 pm, Alexandr Scherbatiy
mailto:alexandr.scherba...@oracle.com"alexandr.scherba...@oracle.com> wrote:

On 2/2/2016 1:50 AM, Avik Niyogi wrote:



Hi All,
Please review the code changes made as per the inputs provided:
HYPERLINK 
"http://cr.openjdk.java.net/~aniyogi/8146321/webrev.03"cr.openjdk.java.net/~aniyogi/8146321/webrev.03



-  Will it work with custom implementation of the Icon interface
which just draws an image?
 For example:
--
public class DukeIcon implements Icon {

   private BufferedImage dukeImage;

   public DukeIcon() throws IOException {
   dukeImage = ImageIO.read(new File(""));
   }

   @Override
   public void paintIcon(Component c, Graphics g, int x, int y) {
   g.drawImage(dukeImage, x, y, c);
   }

   @Override
   public int getIconWidth() {
   return dukeImage.getWidth();
   }

   @Override
   public int getIconHeight() {
   return dukeImage.getHeight();
   }
}


This is a limitation for custom Icons because they will need to
use toe drawImage with appropriate implementation.
To fix that will be a major change and may need change in the
implementation of drawImage method.


 It looks like the code below from the fix doesn't work for the
ImageIcon because x and y are now scaled. Is it possible to apply
some other

Re: Review Request of 8146321: [macosx] JInternalFrame frame icon in wrong position on Mac L if icon is not ImageIcon

2016-02-09 Thread Sergey Bylokhov 

On 08.02.16 10:12, Avik Niyogi wrote:

Hi Sergey,
Please review webrev with changes as per the inputs provided:
cr.openjdk.java.net/~aniyogi/8146321/webrev.06/



Looks fine.



With Regards,
Avik Niyogi


On 08-Feb-2016, at 2:29 am, Sergey Bylokhov
> wrote:

Both updated methods have a typo, 0 will be returned if
frame.getFrameIcon()==null:

   protected int getIconWidth(final JInternalFrame frame) {
   int width = 0;

   Icon icon = frame.getFrameIcon();
   if (icon == null) {
   icon = UIManager.getIcon("InternalFrame.icon");
   }
   else {
   width = Math.min(icon.getIconWidth(), sMaxIconWidth);
   }

   return width;
   }

   protected int getIconHeight(final JInternalFrame frame) {
   int height = 0;

   Icon icon = frame.getFrameIcon();
   if (icon == null) {
   icon = UIManager.getIcon("InternalFrame.icon");
   }
   else{
   height = Math.min(icon.getIconHeight(), sMaxIconHeight);
   }

   return height;
   }

Note that getIconWidth/getIconHeight should return width/height of the
icon, but this is not true anymore, because paintTitleIcon() now use
the aspect ratio, which can produce the different Width/Height for the
same icon.


On 04.02.16 12:26, Alexandr Scherbatiy wrote:


  The fix looks good to me.

  Thanks,
  Alexandr.

On 2/3/2016 8:51 PM, Avik Niyogi wrote:

Hi All,
Please review the code changes made as per the inputs provided:
http://cr.openjdk.java.net/~aniyogi/8146321/webrev.05/


With Regards,
Avik Niyogi


On 03-Feb-2016, at 7:38 pm, Alexandr Scherbatiy

> wrote:

On 2/3/2016 12:47 AM, Avik Niyogi wrote:

Hi All,
Please review the code changes made as per the inputs provided:
http://cr.openjdk.java.net/~aniyogi/8146321/webrev.04


 326 g2.translate(0, 0);
 The translation to the zero vector leaves the coordinate system the
same.

 327 float xScaleFactor = (float) sMaxIconWidth /
icon.getIconWidth();
 It is better to use double values because the graphics transform
methods use them.

 332 g2.scale(scaleFactorAspectRatio,
scaleFactorAspectRatio);
 333 g2.translate(x / scaleFactorAspectRatio, y /
scaleFactorAspectRatio);
 Is it possible to use the translation first and the scale the
second? I this case where no need to re-scale translation coordinates.

 Thanks,
 Alexandr.



With Regards,
Avik Niyogi

On 02-Feb-2016, at 5:55 pm, Alexandr Scherbatiy
<alexandr.scherba...@oracle.com
>
wrote:

On 2/2/2016 3:41 AM, Avik Niyogi wrote:

Hi Alexander,


On 02-Feb-2016, at 3:44 pm, Alexandr Scherbatiy
> wrote:

On 2/2/2016 1:50 AM, Avik Niyogi wrote:

Hi All,
Please review the code changes made as per the inputs provided:
cr.openjdk.java.net/~aniyogi/8146321/webrev.03




-  Will it work with custom implementation of the Icon interface
which just draws an image?
 For example:
--
public class DukeIcon implements Icon {

   private BufferedImage dukeImage;

   public DukeIcon() throws IOException {
   dukeImage = ImageIO.read(new File(""));
   }

   @Override
   public void paintIcon(Component c, Graphics g, int x, int y) {
   g.drawImage(dukeImage, x, y, c);
   }

   @Override
   public int getIconWidth() {
   return dukeImage.getWidth();
   }

   @Override
   public int getIconHeight() {
   return dukeImage.getHeight();
   }
}


This is a limitation for custom Icons because they will need to
use toe drawImage with appropriate implementation.
To fix that will be a major change and may need change in the
implementation of drawImage method.


 It looks like the code below from the fix doesn't work for the
ImageIcon because x and y are now scaled. Is it possible to apply
some other transformations (may be some translations) to the
graphics to draw the image at the right position?
---
334 g2.scale((float) sMaxIconWidth /
icon.getIconWidth(),
335 (float) sMaxIconWidth /
icon.getIconHeight());
336 icon.paintIcon(frame, g2, x, y);
---

--

- "(icon != null && (icon instanceof Icon))"
 Could the check to null also be omitted here?
 In other words, are the "(icon != null && (icon instanceof
Icon))" and "(icon instanceof Icon)" checks return the same result?


If we remove the check, the cases where custom ImageIcon have no
images will fail.

Re: Review Request of 8146321: [macosx] JInternalFrame frame icon in wrong position on Mac L if icon is not ImageIcon

2016-02-08 Thread Avik Niyogi 
A gentle reminder. Please review webrev with changes as per the inputs provided:
cr.openjdk.java.net/~aniyogi/8146321/webrev.06/ 


With Regards,
Avik Niyogi
> On 08-Feb-2016, at 12:42 pm, Avik Niyogi  wrote:
> 
> Hi Sergey,
> Please review webrev with changes as per the inputs provided:
> cr.openjdk.java.net/~aniyogi/8146321/webrev.06/ 
> 
> 
> With Regards,
> Avik Niyogi
> 
>> On 08-Feb-2016, at 2:29 am, Sergey Bylokhov > > wrote:
>> 
>> Both updated methods have a typo, 0 will be returned if 
>> frame.getFrameIcon()==null:
>> 
>>protected int getIconWidth(final JInternalFrame frame) {
>>int width = 0;
>> 
>>Icon icon = frame.getFrameIcon();
>>if (icon == null) {
>>icon = UIManager.getIcon("InternalFrame.icon");
>>}
>>else {
>>width = Math.min(icon.getIconWidth(), sMaxIconWidth);
>>}
>> 
>>return width;
>>}
>> 
>>protected int getIconHeight(final JInternalFrame frame) {
>>int height = 0;
>> 
>>Icon icon = frame.getFrameIcon();
>>if (icon == null) {
>>icon = UIManager.getIcon("InternalFrame.icon");
>>}
>>else{
>>height = Math.min(icon.getIconHeight(), sMaxIconHeight);
>>}
>> 
>>return height;
>>}
>> 
>> Note that getIconWidth/getIconHeight should return width/height of the icon, 
>> but this is not true anymore, because paintTitleIcon() now use the aspect 
>> ratio, which can produce the different Width/Height for the same icon.
>> 
>> 
>> On 04.02.16 12:26, Alexandr Scherbatiy wrote:
>>> 
>>>   The fix looks good to me.
>>> 
>>>   Thanks,
>>>   Alexandr.
>>> 
>>> On 2/3/2016 8:51 PM, Avik Niyogi wrote:
 Hi All,
 Please review the code changes made as per the inputs provided:
 http://cr.openjdk.java.net/~aniyogi/8146321/webrev.05/ 
 
 >
 
 With Regards,
 Avik Niyogi
 
> On 03-Feb-2016, at 7:38 pm, Alexandr Scherbatiy
> 
>  >> wrote:
> 
> On 2/3/2016 12:47 AM, Avik Niyogi wrote:
>> Hi All,
>> Please review the code changes made as per the inputs provided:
>> http://cr.openjdk.java.net/~aniyogi/8146321/webrev.04 
>> 
>> > >
>  326 g2.translate(0, 0);
>  The translation to the zero vector leaves the coordinate system the
> same.
> 
>  327 float xScaleFactor = (float) sMaxIconWidth /
> icon.getIconWidth();
>  It is better to use double values because the graphics transform
> methods use them.
> 
>  332 g2.scale(scaleFactorAspectRatio,
> scaleFactorAspectRatio);
>  333 g2.translate(x / scaleFactorAspectRatio, y /
> scaleFactorAspectRatio);
>  Is it possible to use the translation first and the scale the
> second? I this case where no need to re-scale translation coordinates.
> 
>  Thanks,
>  Alexandr.
> 
>> 
>> With Regards,
>> Avik Niyogi
>>> On 02-Feb-2016, at 5:55 pm, Alexandr Scherbatiy
>>> <>> >alexandr.scherba...@oracle.com 
>>> >
>>> wrote:
>>> 
>>> On 2/2/2016 3:41 AM, Avik Niyogi wrote:
 Hi Alexander,
 
> On 02-Feb-2016, at 3:44 pm, Alexandr Scherbatiy
>  > wrote:
> 
> On 2/2/2016 1:50 AM, Avik Niyogi wrote:
>> Hi All,
>> Please review the code changes made as per the inputs provided:
>> cr.openjdk.java.net/~aniyogi/8146321/webrev.03 
>> 
>> > >
> 
> -  Will it work with custom implementation of the Icon interface
> which just draws an image?
>  For example:
> --
> public class DukeIcon implements Icon {
> 
>private BufferedImage dukeImage;
> 
>public DukeIcon() throws IOException {
>dukeImage = ImageIO.read(new

Re: Review Request of 8146321: [macosx] JInternalFrame frame icon in wrong position on Mac L if icon is not ImageIcon

2016-02-07 Thread Sergey Bylokhov 
Both updated methods have a typo, 0 will be returned if 
frame.getFrameIcon()==null:


protected int getIconWidth(final JInternalFrame frame) {
int width = 0;

Icon icon = frame.getFrameIcon();
if (icon == null) {
icon = UIManager.getIcon("InternalFrame.icon");
}
else {
width = Math.min(icon.getIconWidth(), sMaxIconWidth);
}

return width;
}

protected int getIconHeight(final JInternalFrame frame) {
int height = 0;

Icon icon = frame.getFrameIcon();
if (icon == null) {
icon = UIManager.getIcon("InternalFrame.icon");
}
else{
height = Math.min(icon.getIconHeight(), sMaxIconHeight);
}

return height;
}

Note that getIconWidth/getIconHeight should return width/height of the 
icon, but this is not true anymore, because paintTitleIcon() now use the 
aspect ratio, which can produce the different Width/Height for the same 
icon.



On 04.02.16 12:26, Alexandr Scherbatiy wrote:


   The fix looks good to me.

   Thanks,
   Alexandr.

On 2/3/2016 8:51 PM, Avik Niyogi wrote:

Hi All,
Please review the code changes made as per the inputs provided:
http://cr.openjdk.java.net/~aniyogi/8146321/webrev.05/


With Regards,
Avik Niyogi


On 03-Feb-2016, at 7:38 pm, Alexandr Scherbatiy
> wrote:

On 2/3/2016 12:47 AM, Avik Niyogi wrote:

Hi All,
Please review the code changes made as per the inputs provided:
http://cr.openjdk.java.net/~aniyogi/8146321/webrev.04


  326 g2.translate(0, 0);
  The translation to the zero vector leaves the coordinate system the
same.

  327 float xScaleFactor = (float) sMaxIconWidth /
icon.getIconWidth();
  It is better to use double values because the graphics transform
methods use them.

  332 g2.scale(scaleFactorAspectRatio,
scaleFactorAspectRatio);
  333 g2.translate(x / scaleFactorAspectRatio, y /
scaleFactorAspectRatio);
  Is it possible to use the translation first and the scale the
second? I this case where no need to re-scale translation coordinates.

  Thanks,
  Alexandr.



With Regards,
Avik Niyogi

On 02-Feb-2016, at 5:55 pm, Alexandr Scherbatiy
<alexandr.scherba...@oracle.com>
wrote:

On 2/2/2016 3:41 AM, Avik Niyogi wrote:

Hi Alexander,


On 02-Feb-2016, at 3:44 pm, Alexandr Scherbatiy
 wrote:

On 2/2/2016 1:50 AM, Avik Niyogi wrote:

Hi All,
Please review the code changes made as per the inputs provided:
cr.openjdk.java.net/~aniyogi/8146321/webrev.03



 -  Will it work with custom implementation of the Icon interface
which just draws an image?
  For example:
 --
 public class DukeIcon implements Icon {

private BufferedImage dukeImage;

public DukeIcon() throws IOException {
dukeImage = ImageIO.read(new File(""));
}

@Override
public void paintIcon(Component c, Graphics g, int x, int y) {
g.drawImage(dukeImage, x, y, c);
}

@Override
public int getIconWidth() {
return dukeImage.getWidth();
}

@Override
public int getIconHeight() {
return dukeImage.getHeight();
}
}


This is a limitation for custom Icons because they will need to
use toe drawImage with appropriate implementation.
To fix that will be a major change and may need change in the
implementation of drawImage method.


  It looks like the code below from the fix doesn't work for the
ImageIcon because x and y are now scaled. Is it possible to apply
some other transformations (may be some translations) to the
graphics to draw the image at the right position?
---
 334 g2.scale((float) sMaxIconWidth /
icon.getIconWidth(),
 335 (float) sMaxIconWidth /
icon.getIconHeight());
 336 icon.paintIcon(frame, g2, x, y);
---

--

- "(icon != null && (icon instanceof Icon))"
  Could the check to null also be omitted here?
  In other words, are the "(icon != null && (icon instanceof
Icon))" and "(icon instanceof Icon)" checks return the same result?


If we remove the check, the cases where custom ImageIcon have no
images will fail.


The provided example  should work with the check: "(icon
instanceof Icon)" in the same way as with the check "(icon != null
&& (icon instanceof Icon))" because the used icon is not null and
it implements Icon interface, should not it?

 Thanks,
 Alexandr.


Example:


import java.awt.*;
import javax.swing.*;

public class JInternalFrameBug {

   public static void main(String[] args) {
SwingUtilities.invokeLater(
new Runnable() {
   public void run() {

Re: Review Request of 8146321: [macosx] JInternalFrame frame icon in wrong position on Mac L if icon is not ImageIcon

2016-02-07 Thread Avik Niyogi 
Hi Sergey,
Please review webrev with changes as per the inputs provided:
cr.openjdk.java.net/~aniyogi/8146321/webrev.06/ 


With Regards,
Avik Niyogi

> On 08-Feb-2016, at 2:29 am, Sergey Bylokhov  
> wrote:
> 
> Both updated methods have a typo, 0 will be returned if 
> frame.getFrameIcon()==null:
> 
>protected int getIconWidth(final JInternalFrame frame) {
>int width = 0;
> 
>Icon icon = frame.getFrameIcon();
>if (icon == null) {
>icon = UIManager.getIcon("InternalFrame.icon");
>}
>else {
>width = Math.min(icon.getIconWidth(), sMaxIconWidth);
>}
> 
>return width;
>}
> 
>protected int getIconHeight(final JInternalFrame frame) {
>int height = 0;
> 
>Icon icon = frame.getFrameIcon();
>if (icon == null) {
>icon = UIManager.getIcon("InternalFrame.icon");
>}
>else{
>height = Math.min(icon.getIconHeight(), sMaxIconHeight);
>}
> 
>return height;
>}
> 
> Note that getIconWidth/getIconHeight should return width/height of the icon, 
> but this is not true anymore, because paintTitleIcon() now use the aspect 
> ratio, which can produce the different Width/Height for the same icon.
> 
> 
> On 04.02.16 12:26, Alexandr Scherbatiy wrote:
>> 
>>   The fix looks good to me.
>> 
>>   Thanks,
>>   Alexandr.
>> 
>> On 2/3/2016 8:51 PM, Avik Niyogi wrote:
>>> Hi All,
>>> Please review the code changes made as per the inputs provided:
>>> http://cr.openjdk.java.net/~aniyogi/8146321/webrev.05/ 
>>> 
>>> >> >
>>> 
>>> With Regards,
>>> Avik Niyogi
>>> 
 On 03-Feb-2016, at 7:38 pm, Alexandr Scherbatiy
 
 >> wrote:
 
 On 2/3/2016 12:47 AM, Avik Niyogi wrote:
> Hi All,
> Please review the code changes made as per the inputs provided:
> http://cr.openjdk.java.net/~aniyogi/8146321/webrev.04 
> 
>  >
  326 g2.translate(0, 0);
  The translation to the zero vector leaves the coordinate system the
 same.
 
  327 float xScaleFactor = (float) sMaxIconWidth /
 icon.getIconWidth();
  It is better to use double values because the graphics transform
 methods use them.
 
  332 g2.scale(scaleFactorAspectRatio,
 scaleFactorAspectRatio);
  333 g2.translate(x / scaleFactorAspectRatio, y /
 scaleFactorAspectRatio);
  Is it possible to use the translation first and the scale the
 second? I this case where no need to re-scale translation coordinates.
 
  Thanks,
  Alexandr.
 
> 
> With Regards,
> Avik Niyogi
>> On 02-Feb-2016, at 5:55 pm, Alexandr Scherbatiy
>> <> >alexandr.scherba...@oracle.com 
>> >
>> wrote:
>> 
>> On 2/2/2016 3:41 AM, Avik Niyogi wrote:
>>> Hi Alexander,
>>> 
 On 02-Feb-2016, at 3:44 pm, Alexandr Scherbatiy
 > wrote:
 
 On 2/2/2016 1:50 AM, Avik Niyogi wrote:
> Hi All,
> Please review the code changes made as per the inputs provided:
> cr.openjdk.java.net/~aniyogi/8146321/webrev.03 
> 
>  >
 
 -  Will it work with custom implementation of the Icon interface
 which just draws an image?
  For example:
 --
 public class DukeIcon implements Icon {
 
private BufferedImage dukeImage;
 
public DukeIcon() throws IOException {
dukeImage = ImageIO.read(new File(">>> image>"));
}
 
@Override
public void paintIcon(Component c, Graphics g, int x, int y) {
g.drawImage(dukeImage, x, y, c);
}
 
@Override
public int getIconWidth() {
return dukeImage.getWidth();
}
 
@Override
public int getIconHeight() {
return dukeImage.getHeight();

Re: Review Request of 8146321: [macosx] JInternalFrame frame icon in wrong position on Mac L if icon is not ImageIcon

2016-02-04 Thread Alexandr Scherbatiy 


  The fix looks good to me.

  Thanks,
  Alexandr.

On 2/3/2016 8:51 PM, Avik Niyogi wrote:

Hi All,
Please review the code changes made as per the inputs provided:
http://cr.openjdk.java.net/~aniyogi/8146321/webrev.05/ 



With Regards,
Avik Niyogi

On 03-Feb-2016, at 7:38 pm, Alexandr Scherbatiy 
> wrote:


On 2/3/2016 12:47 AM, Avik Niyogi wrote:

Hi All,
Please review the code changes made as per the inputs provided:
http://cr.openjdk.java.net/~aniyogi/8146321/webrev.04 


  326 g2.translate(0, 0);
  The translation to the zero vector leaves the coordinate system the 
same.


  327 float xScaleFactor = (float) sMaxIconWidth / 
icon.getIconWidth();
  It is better to use double values because the graphics transform 
methods use them.


  332 g2.scale(scaleFactorAspectRatio, 
scaleFactorAspectRatio);
  333 g2.translate(x / scaleFactorAspectRatio, y / 
scaleFactorAspectRatio);
  Is it possible to use the translation first and the scale the 
second? I this case where no need to re-scale translation coordinates.


  Thanks,
  Alexandr.



With Regards,
Avik Niyogi
On 02-Feb-2016, at 5:55 pm, Alexandr Scherbatiy 
> wrote:


On 2/2/2016 3:41 AM, Avik Niyogi wrote:

Hi Alexander,

On 02-Feb-2016, at 3:44 pm, Alexandr Scherbatiy 
 wrote:


On 2/2/2016 1:50 AM, Avik Niyogi wrote:

Hi All,
Please review the code changes made as per the inputs provided:
cr.openjdk.java.net/~aniyogi/8146321/webrev.03 



 -  Will it work with custom implementation of the Icon interface 
which just draws an image?

  For example:
 --
 public class DukeIcon implements Icon {

private BufferedImage dukeImage;

public DukeIcon() throws IOException {
dukeImage = ImageIO.read(new File("image>"));

}

@Override
public void paintIcon(Component c, Graphics g, int x, int y) {
g.drawImage(dukeImage, x, y, c);
}

@Override
public int getIconWidth() {
return dukeImage.getWidth();
}

@Override
public int getIconHeight() {
return dukeImage.getHeight();
}
}


This is a limitation for custom Icons because they will need to 
use toe drawImage with appropriate implementation.
To fix that will be a major change and may need change in the 
implementation of drawImage method.


  It looks like the code below from the fix doesn't work for the 
ImageIcon because x and y are now scaled. Is it possible to apply 
some other transformations (may be some translations) to the 
graphics to draw the image at the right position?

---
 334 g2.scale((float) sMaxIconWidth / 
icon.getIconWidth(),
 335 (float) sMaxIconWidth / 
icon.getIconHeight());

 336 icon.paintIcon(frame, g2, x, y);
---

--

- "(icon != null && (icon instanceof Icon))"
  Could the check to null also be omitted here?
  In other words, are the "(icon != null && (icon instanceof 
Icon))" and "(icon instanceof Icon)" checks return the same result?


If we remove the check, the cases where custom ImageIcon have no 
images will fail.


The provided example  should work with the check: "(icon 
instanceof Icon)" in the same way as with the check "(icon != null 
&& (icon instanceof Icon))" because the used icon is not null and 
it implements Icon interface, should not it?


 Thanks,
 Alexandr.


Example:


import java.awt.*;
import javax.swing.*;

public class JInternalFrameBug {

   public static void main(String[] args) {
SwingUtilities.invokeLater(
new Runnable() {
   public void run() {
  try {
 UIManager.setLookAndFeel("com.apple.laf.AquaLookAndFeel"); }
catch(Exception e) {
 System.out.println("This is not a Mac.");
 return;
  }
  JFrame f = new JFrame();
f.setSize(500, 500);
JDesktopPane dtp = new JDesktopPane();
JInternalFrame jif = new JInternalFrame();
jif.setTitle("Test");
jif.setFrameIcon(
new ImageIcon() {
   public int getIconWidth() {
  return 16;
   }
   public int getIconHeight() {
  return 16;
   }
   public void paintIcon(Component c, Graphics g, int x, int y) {
g.setColor(java.awt.Color.green);
  g.fillRect(x, y, 16, 16);
   }
});
jif.setSize(400, 400);
jif.setVisible(true);
dtp.add(jif);

f.getContentPane().setLayout(new BorderLayout());
f.getContentPane().add(dtp, "Center");
f.setVisible(true);
   }
});
   }
}


Thanks,
  Alexandr.



With Regards,
Avik Niyogi
On 02-Feb-2016, at 3:02 pm, Alexandr Scherbatiy 
 wrote:


On 2/1/2016 11:25 PM, Avik Niyogi

Re: Review Request of 8146321: [macosx] JInternalFrame frame icon in wrong position on Mac L if icon is not ImageIcon

2016-02-03 Thread Avik Niyogi 
Hi All,
Please review the code changes made as per the inputs provided:
http://cr.openjdk.java.net/~aniyogi/8146321/webrev.04 


With Regards,
Avik Niyogi
> On 02-Feb-2016, at 5:55 pm, Alexandr Scherbatiy 
>  wrote:
> 
> On 2/2/2016 3:41 AM, Avik Niyogi wrote:
>> Hi Alexander,
>> 
>>> On 02-Feb-2016, at 3:44 pm, Alexandr Scherbatiy 
>>> > 
>>> wrote:
>>> 
>>> On 2/2/2016 1:50 AM, Avik Niyogi wrote:
 Hi All,
 Please review the code changes made as per the inputs provided:
 cr.openjdk.java.net/~aniyogi/8146321/webrev.03 
 
>>>  -  Will it work with custom implementation of the Icon interface which 
>>> just draws an image?
>>>   For example:
>>>  --
>>>  public class DukeIcon implements Icon {
>>> 
>>> private BufferedImage dukeImage;
>>> 
>>> public DukeIcon() throws IOException {
>>> dukeImage = ImageIO.read(new File(""));
>>> }
>>> 
>>> @Override
>>> public void paintIcon(Component c, Graphics g, int x, int y) {
>>> g.drawImage(dukeImage, x, y, c);
>>> }
>>> 
>>> @Override
>>> public int getIconWidth() {
>>> return dukeImage.getWidth();
>>> }
>>> 
>>> @Override
>>> public int getIconHeight() {
>>> return dukeImage.getHeight();
>>> }
>>> }
>> 
>> This is a limitation for custom Icons because they will need to use toe 
>> drawImage with appropriate implementation.
>> To fix that will be a major change and may need change in the implementation 
>> of drawImage method.
> 
>   It looks like the code below from the fix doesn't work for the ImageIcon 
> because x and y are now scaled. Is it possible to apply some other 
> transformations (may be some translations) to the graphics to draw the image 
> at the right position?
> ---
>  334 g2.scale((float) sMaxIconWidth / icon.getIconWidth(),
>  335 (float) sMaxIconWidth / icon.getIconHeight());
>  336 icon.paintIcon(frame, g2, x, y);
> ---
>>> --
>>> 
>>> - "(icon != null && (icon instanceof Icon))"
>>>   Could the check to null also be omitted here?
>>>   In other words, are the "(icon != null && (icon instanceof Icon))" and 
>>> "(icon instanceof Icon)" checks return the same result?
>>> 
>> If we remove the check, the cases where custom ImageIcon have no images will 
>> fail.
> 
> The provided example  should work with the check: "(icon instanceof 
> Icon)" in the same way as with the check "(icon != null && (icon instanceof 
> Icon))" because the used icon is not null and it implements Icon interface, 
> should not it?
> 
>  Thanks,
>  Alexandr.
> 
>> Example:
>> 
>> 
>> import java.awt.*;
>> import javax.swing.*;
>> 
>> public class JInternalFrameBug {
>> 
>>public static void main(String[] args) {
>>   SwingUtilities.invokeLater(
>> new Runnable() {
>>public void run() {
>>   try {
>>  
>> UIManager.setLookAndFeel("com.apple.laf.AquaLookAndFeel"); }
>>   catch(Exception e) {
>>  System.out.println("This is not a Mac.");
>>  return;
>>   }
>>   JFrame f = new JFrame();
>>   f.setSize(500, 500);
>>   JDesktopPane dtp = new JDesktopPane();
>>   JInternalFrame jif = new JInternalFrame();
>>   jif.setTitle("Test");
>>   jif.setFrameIcon(
>> new ImageIcon() {
>>public int getIconWidth() {
>>   return 16;
>>}
>>public int getIconHeight() {
>>   return 16;
>>}
>>public void paintIcon(Component c, Graphics g, 
>> int x, int y) {
>>   g.setColor(java.awt.Color.green);
>>   g.fillRect(x, y, 16, 16);
>>}
>> });
>>   jif.setSize(400, 400);
>>   jif.setVisible(true);
>>   dtp.add(jif);
>> 
>>   f.getContentPane().setLayout(new BorderLayout());
>>   f.getContentPane().add(dtp, "Center");
>>   f.setVisible(true);
>>}
>> });
>>}
>> }
>> 
>>>   Thanks,
>>>   Alexandr.
>>> 
 
 With Regards,
 Avik Niyogi
> On 02-Feb-2016, at 3:02 pm, Alexandr Scherbatiy < 
> alexandr.scherba...@oracle.com 
> > wrote:
> 
> On 2/1/2016 11:25 PM, Avik Niyogi wrote:
>> Hi All,

Re: Review Request of 8146321: [macosx] JInternalFrame frame icon in wrong position on Mac L if icon is not ImageIcon

2016-02-03 Thread Alexandr Scherbatiy 

On 2/3/2016 12:47 AM, Avik Niyogi wrote:

Hi All,
Please review the code changes made as per the inputs provided:
http://cr.openjdk.java.net/~aniyogi/8146321/webrev.04 


  326 g2.translate(0, 0);
  The translation to the zero vector leaves the coordinate system the same.

  327 float xScaleFactor = (float) sMaxIconWidth / 
icon.getIconWidth();
  It is better to use double values because the graphics transform 
methods use them.


  332 g2.scale(scaleFactorAspectRatio, scaleFactorAspectRatio);
  333 g2.translate(x / scaleFactorAspectRatio, y / 
scaleFactorAspectRatio);
  Is it possible to use the translation first and the scale the second? 
I this case where no need to re-scale translation coordinates.


  Thanks,
  Alexandr.



With Regards,
Avik Niyogi
On 02-Feb-2016, at 5:55 pm, Alexandr Scherbatiy 
> wrote:


On 2/2/2016 3:41 AM, Avik Niyogi wrote:

Hi Alexander,

On 02-Feb-2016, at 3:44 pm, Alexandr Scherbatiy 
> wrote:


On 2/2/2016 1:50 AM, Avik Niyogi wrote:

Hi All,
Please review the code changes made as per the inputs provided:
cr.openjdk.java.net/~aniyogi/8146321/webrev.03 



 -  Will it work with custom implementation of the Icon interface 
which just draws an image?

  For example:
 --
 public class DukeIcon implements Icon {

private BufferedImage dukeImage;

public DukeIcon() throws IOException {
dukeImage = ImageIO.read(new File(""));
}

@Override
public void paintIcon(Component c, Graphics g, int x, int y) {
g.drawImage(dukeImage, x, y, c);
}

@Override
public int getIconWidth() {
return dukeImage.getWidth();
}

@Override
public int getIconHeight() {
return dukeImage.getHeight();
}
}


This is a limitation for custom Icons because they will need to use 
toe drawImage with appropriate implementation.
To fix that will be a major change and may need change in the 
implementation of drawImage method.


  It looks like the code below from the fix doesn't work for the 
ImageIcon because x and y are now scaled. Is it possible to apply 
some other transformations (may be some translations) to the graphics 
to draw the image at the right position?

---
 334 g2.scale((float) sMaxIconWidth / 
icon.getIconWidth(),
 335 (float) sMaxIconWidth / 
icon.getIconHeight());

 336 icon.paintIcon(frame, g2, x, y);
---

--

- "(icon != null && (icon instanceof Icon))"
  Could the check to null also be omitted here?
  In other words, are the "(icon != null && (icon instanceof 
Icon))" and "(icon instanceof Icon)" checks return the same result?


If we remove the check, the cases where custom ImageIcon have no 
images will fail.


The provided example  should work with the check: "(icon 
instanceof Icon)" in the same way as with the check "(icon != null && 
(icon instanceof Icon))" because the used icon is not null and it 
implements Icon interface, should not it?


 Thanks,
 Alexandr.


Example:


import java.awt.*;
import javax.swing.*;

public class JInternalFrameBug {

   public static void main(String[] args) {
  SwingUtilities.invokeLater(
new Runnable() {
   public void run() {
  try {
 UIManager.setLookAndFeel("com.apple.laf.AquaLookAndFeel"); }
  catch(Exception e) {
 System.out.println("This is not a Mac.");
 return;
  }
  JFrame f = new JFrame();
  f.setSize(500, 500);
  JDesktopPane dtp = new JDesktopPane();
  JInternalFrame jif = new JInternalFrame();
jif.setTitle("Test");
  jif.setFrameIcon(
new ImageIcon() {
   public int getIconWidth() {
  return 16;
   }
   public int getIconHeight() {
  return 16;
   }
   public void paintIcon(Component c, 
Graphics g, int x, int y) {

g.setColor(java.awt.Color.green);
g.fillRect(x, y, 16, 16);
   }
});
  jif.setSize(400, 400);
jif.setVisible(true);
  dtp.add(jif);

f.getContentPane().setLayout(new BorderLayout());
f.getContentPane().add(dtp, "Center");
f.setVisible(true);
   }
});
   }
}


  Thanks,
  Alexandr.



With Regards,
Avik Niyogi
On 02-Feb-2016, at 3:02 pm, Alexandr Scherbatiy 
 wrote:


On 2/1/2016 11:25 PM, Avik Niyogi wrote:

Hi All,
Please

Re: Review Request of 8146321: [macosx] JInternalFrame frame icon in wrong position on Mac L if icon is not ImageIcon

2016-02-03 Thread Sergey Bylokhov 

On 02.02.16 15:25, Alexandr Scherbatiy wrote:

 The provided example  should work with the check: "(icon instanceof
Icon)" in the same way as with the check "(icon != null && (icon
instanceof Icon))" because the used icon is not null and it implements
Icon interface, should not it?


Note that "icon instanceof Icon" in "(icon != null && (icon instanceof 
Icon))" is unnecessary because icon is Icon already. Only null check is 
necessary.


--
Best regards, Sergey.

Re: Review Request of 8146321: [macosx] JInternalFrame frame icon in wrong position on Mac L if icon is not ImageIcon

2016-02-03 Thread Avik Niyogi 
Hi All,
Please review the code changes made as per the inputs provided:
http://cr.openjdk.java.net/~aniyogi/8146321/webrev.05/ 


With Regards,
Avik Niyogi

> On 03-Feb-2016, at 7:38 pm, Alexandr Scherbatiy 
>  wrote:
> 
> On 2/3/2016 12:47 AM, Avik Niyogi wrote:
>> Hi All,
>> Please review the code changes made as per the inputs provided:
>> http://cr.openjdk.java.net/~aniyogi/8146321/webrev.04 
>>   326 
>> g2.translate(0, 0);
>   The translation to the zero vector leaves the coordinate system the same.
> 
>   327 float xScaleFactor = (float) sMaxIconWidth / 
> icon.getIconWidth();
>   It is better to use double values because the graphics transform methods 
> use them.
> 
>   332 g2.scale(scaleFactorAspectRatio, scaleFactorAspectRatio);
>   333 g2.translate(x / scaleFactorAspectRatio, y / 
> scaleFactorAspectRatio);
>   Is it possible to use the translation first and the scale the second? I 
> this case where no need to re-scale translation coordinates.
> 
>   Thanks,
>   Alexandr.
> 
>> 
>> With Regards,
>> Avik Niyogi
>>> On 02-Feb-2016, at 5:55 pm, Alexandr Scherbatiy 
>>> > 
>>> wrote:
>>> 
>>> On 2/2/2016 3:41 AM, Avik Niyogi wrote:
 Hi Alexander,
 
> On 02-Feb-2016, at 3:44 pm, Alexandr Scherbatiy < 
> alexandr.scherba...@oracle.com 
> > wrote:
> 
> On 2/2/2016 1:50 AM, Avik Niyogi wrote:
>> Hi All,
>> Please review the code changes made as per the inputs provided:
>> cr.openjdk.java.net/~aniyogi/8146321/webrev.03 
>> 
>  -  Will it work with custom implementation of the Icon interface which 
> just draws an image?
>   For example:
>  --
>  public class DukeIcon implements Icon {
> 
> private BufferedImage dukeImage;
> 
> public DukeIcon() throws IOException {
> dukeImage = ImageIO.read(new File(""));
> }
> 
> @Override
> public void paintIcon(Component c, Graphics g, int x, int y) {
> g.drawImage(dukeImage, x, y, c);
> }
> 
> @Override
> public int getIconWidth() {
> return dukeImage.getWidth();
> }
> 
> @Override
> public int getIconHeight() {
> return dukeImage.getHeight();
> }
> }
 
 This is a limitation for custom Icons because they will need to use toe 
 drawImage with appropriate implementation.
 To fix that will be a major change and may need change in the 
 implementation of drawImage method.
>>> 
>>>   It looks like the code below from the fix doesn't work for the ImageIcon 
>>> because x and y are now scaled. Is it possible to apply some other 
>>> transformations (may be some translations) to the graphics to draw the 
>>> image at the right position?
>>> ---
>>>  334 g2.scale((float) sMaxIconWidth / icon.getIconWidth(),
>>>  335 (float) sMaxIconWidth / icon.getIconHeight());
>>>  336 icon.paintIcon(frame, g2, x, y);
>>> ---
> --
> 
> - "(icon != null && (icon instanceof Icon))"
>   Could the check to null also be omitted here?
>   In other words, are the "(icon != null && (icon instanceof Icon))" and 
> "(icon instanceof Icon)" checks return the same result?
> 
 If we remove the check, the cases where custom ImageIcon have no images 
 will fail.
>>> 
>>> The provided example  should work with the check: "(icon instanceof 
>>> Icon)" in the same way as with the check "(icon != null && (icon instanceof 
>>> Icon))" because the used icon is not null and it implements Icon interface, 
>>> should not it?
>>> 
>>>  Thanks,
>>>  Alexandr.
>>> 
 Example:
 
 
 import java.awt.*;
 import javax.swing.*;
 
 public class JInternalFrameBug {
 
public static void main(String[] args) {
   SwingUtilities.invokeLater(
 new Runnable() {
public void run() {
   try {
  
 UIManager.setLookAndFeel("com.apple.laf.AquaLookAndFeel"); }
   catch(Exception e) {
  System.out.println("This is not a Mac.");
  return;
   }
   JFrame f = new JFrame();
   f.setSize(500, 500);
   JDesktopPane dtp = new JDesktopPane();
   JInternalFrame jif = new JInternalFrame();
   jif.setTitle("Test");

Re: Review Request of 8146321: [macosx] JInternalFrame frame icon in wrong position on Mac L if icon is not ImageIcon

2016-02-02 Thread Alexandr Scherbatiy 

On 2/1/2016 11:25 PM, Avik Niyogi wrote:

Hi All,
Please review the code changes made as per inputs provided:
cr.openjdk.java.net/~aniyogi/8146321/webrev.02 



   - is it possible to skip the ImageIcon parsing and handle it as 
others icons (may be applying some translation to the graphics)?

   -   "(icon instanceof ImageIcon || icon instanceof Icon):
 ImageIcons is also Icon so the whole condition should be the same 
as just (icon instanceof Icon)


  Thanks,
  Alexandr.



With Regards,
Avik Niyogi
On 20-Jan-2016, at 10:35 pm, Sergey Bylokhov 
> wrote:


On 20/01/16 18:43, Avik Niyogi wrote:

if ((icon.getIconWidth() > sMaxIconWidth
|| icon.getIconHeight() > sMaxIconHeight)) {
final Graphics2D g2 = (Graphics2D) g;
final AffineTransform savedAT = g2.getTransform();
g2.scale((float)sMaxIconWidth/icon.getIconWidth(),
(float)sMaxIconWidth/icon.getIconHeight());
icon.paintIcon(frame, g2, x, y);
g2.setTransform(savedAT);
}


This code does not take into account that x,y whcih are passed to the 
paintIcon should be adjusted, because after the scale they contain 
incorrect starting points.




then for a test code with the following:

import java.awt.*;
import javax.swing.*;

public class JInternalFrameBug {

   public static void main(String[] args) {
  SwingUtilities.invokeLater(
new Runnable() {
   public void run() {
  try {

 UIManager.setLookAndFeel("com.apple.laf.AquaLookAndFeel"); }
  catch(Exception e) {
 System.out.println("This is not a Mac.");
 return;
  }
  JFrame f = new JFrame();
  f.setSize(400, 400);
  JDesktopPane dtp = new JDesktopPane();
  JInternalFrame jif = new JInternalFrame();
  jif.setTitle("Test");
  jif.setFrameIcon(new
ImageIcon("/Users/avniyogi/Downloads/FeedbinNotifier-master/FeedbinNotifier/Images.xcassets/AppIcon.appiconset/icon_128x...@2x.png"));
  jif.setSize(200, 200);
  jif.setVisible(true);
  dtp.add(jif);

  f.getContentPane().setLayout(new BorderLayout());
  f.getContentPane().add(dtp, "Center");
  f.setVisible(true);
   }
});
   }
}

results in this:




On 20-Jan-2016, at 4:42 pm, Alexander Scherbatiy

> wrote:

On 1/20/2016 7:59 AM, Avik Niyogi wrote:

Hi All,
A gentle reminder, please review my code changes as in the webrev
below in the mail trail.
With Regards,
Avik Niyogi
On 18-Jan-2016, at 3:01 pm, Avik Niyogi 
 > 
wrote:


Hi Sergey,

Please review the webrev taking inputs as per the discussion before:
http://cr.openjdk.java.net/~aniyogi/8146321/webrev.01/ 





  The idea was to scale the graphics that the drawn icon fits to the
target sizes.
  Something like:
 
 g2d.scale(targetIconWidth/icon.getWidth(),
targetIconHeight/icon.getHeight());
 icon.paintIcon(frame, g2d, x, y);
-

 This should work not only for ImageIcon but for any type of icons.

 Thanks,
 Alexandr.


With Regards,
Avik Niyogi

On 14-Jan-2016, at 12:55 pm, Avik Niyogi 
 > 
wrote:


Hi Sergey,
I have verified it with the test case as well. If a test case
overrides these methods to imply a change with icons larger than
16x16 it will show that for ImageIcon and Icon as before. The
resize of ImageIcon is only in case it has an image file that it
will try to fit. I had a similar query myself and have found out
that *getImage()* exists for ImageIcon class only and not Icon 
class.

Example:
private static void createImageIconUI(final String 
lookAndFeelString)

  throws Exception {
SwingUtilities.invokeAndWait(new Runnable() {
  @Override
  public void run() {
  desktopPane = new JDesktopPane();
  internalFrame = new JInternalFrame();
  frame = new JFrame();
internalFrame.setTitle(lookAndFeelString);
  titleImageIcon = new ImageIcon() {
  @Override
  public int getIconWidth() {
  return 16;
  }

  @Override
  public int getIconHeight() {
  return 16;
  }

Re: Review Request of 8146321: [macosx] JInternalFrame frame icon in wrong position on Mac L if icon is not ImageIcon

2016-02-02 Thread Avik Niyogi 
Hi Alexander,

> On 02-Feb-2016, at 3:44 pm, Alexandr Scherbatiy 
>  wrote:
> 
> On 2/2/2016 1:50 AM, Avik Niyogi wrote:
>> Hi All,
>> Please review the code changes made as per the inputs provided:
>> cr.openjdk.java.net/~aniyogi/8146321/webrev.03 
>> 
>  -  Will it work with custom implementation of the Icon interface which just 
> draws an image?
>   For example:
>  --
>  public class DukeIcon implements Icon {
> 
> private BufferedImage dukeImage;
> 
> public DukeIcon() throws IOException {
> dukeImage = ImageIO.read(new File(""));
> }
> 
> @Override
> public void paintIcon(Component c, Graphics g, int x, int y) {
> g.drawImage(dukeImage, x, y, c);
> }
> 
> @Override
> public int getIconWidth() {
> return dukeImage.getWidth();
> }
> 
> @Override
> public int getIconHeight() {
> return dukeImage.getHeight();
> }
> }

This is a limitation for custom Icons because they will need to use toe 
drawImage with appropriate implementation.
To fix that will be a major change and may need change in the implementation of 
drawImage method.
> --
> 
> - "(icon != null && (icon instanceof Icon))"
>   Could the check to null also be omitted here?
>   In other words, are the "(icon != null && (icon instanceof Icon))" and 
> "(icon instanceof Icon)" checks return the same result?
> 
If we remove the check, the cases where custom ImageIcon have no images will 
fail.
Example:


import java.awt.*;
import javax.swing.*;

public class JInternalFrameBug {

   public static void main(String[] args) {
  SwingUtilities.invokeLater(
new Runnable() {
   public void run() {
  try {
 UIManager.setLookAndFeel("com.apple.laf.AquaLookAndFeel"); 
}
  catch(Exception e) {
 System.out.println("This is not a Mac.");
 return;
  }
  JFrame f = new JFrame();
  f.setSize(500, 500);
  JDesktopPane dtp = new JDesktopPane();
  JInternalFrame jif = new JInternalFrame();
  jif.setTitle("Test");
  jif.setFrameIcon(
new ImageIcon() {
   public int getIconWidth() {
  return 16;
   }
   public int getIconHeight() {
  return 16;
   }
   public void paintIcon(Component c, Graphics g, int 
x, int y) {
  g.setColor(java.awt.Color.green);
  g.fillRect(x, y, 16, 16);
   }
});
  jif.setSize(400, 400);
  jif.setVisible(true);
  dtp.add(jif);

  f.getContentPane().setLayout(new BorderLayout());
  f.getContentPane().add(dtp, "Center");
  f.setVisible(true);
   }
});
   }
}

>   Thanks,
>   Alexandr.
> 
>> 
>> With Regards,
>> Avik Niyogi
>>> On 02-Feb-2016, at 3:02 pm, Alexandr Scherbatiy 
>>> > 
>>> wrote:
>>> 
>>> On 2/1/2016 11:25 PM, Avik Niyogi wrote:
 Hi All,
 Please review the code changes made as per inputs provided:
 cr.openjdk.java.net/~aniyogi/8146321/webrev.02 
 
>>>- is it possible to skip the ImageIcon parsing and handle it as others 
>>> icons (may be applying some translation to the graphics)?
>>>-   "(icon instanceof ImageIcon || icon instanceof Icon):
>>>  ImageIcons is also Icon so the whole condition should be the same as 
>>> just (icon instanceof Icon)
>>> 
>>>   Thanks,
>>>   Alexandr.
>>> 
 
 With Regards,
 Avik Niyogi
> On 20-Jan-2016, at 10:35 pm, Sergey Bylokhov  > wrote:
> 
> On 20/01/16 18:43, Avik Niyogi wrote:
>> if ((icon.getIconWidth() > sMaxIconWidth
>> || icon.getIconHeight() > sMaxIconHeight)) {
>> final Graphics2D g2 = (Graphics2D) g;
>> final AffineTransform savedAT = g2.getTransform();
>> g2.scale((float)sMaxIconWidth/icon.getIconWidth(),
>> (float)sMaxIconWidth/icon.getIconHeight());
>> icon.paintIcon(frame, g2, x, y);
>> g2.setTransform(savedAT);
>> }
> 
> This code does not take into account that x,y whcih are passed to the 
> paintIcon should be adjusted, because after the scale they contain 
> incorrect starting points.
> 
>> 
>> then for a test code with the

Re: Review Request of 8146321: [macosx] JInternalFrame frame icon in wrong position on Mac L if icon is not ImageIcon

2016-02-02 Thread Alexandr Scherbatiy 

On 2/2/2016 3:41 AM, Avik Niyogi wrote:

Hi Alexander,

On 02-Feb-2016, at 3:44 pm, Alexandr Scherbatiy 
> wrote:


On 2/2/2016 1:50 AM, Avik Niyogi wrote:

Hi All,
Please review the code changes made as per the inputs provided:
cr.openjdk.java.net/~aniyogi/8146321/webrev.03 



 -  Will it work with custom implementation of the Icon interface 
which just draws an image?

  For example:
 --
 public class DukeIcon implements Icon {

private BufferedImage dukeImage;

public DukeIcon() throws IOException {
dukeImage = ImageIO.read(new File(""));
}

@Override
public void paintIcon(Component c, Graphics g, int x, int y) {
g.drawImage(dukeImage, x, y, c);
}

@Override
public int getIconWidth() {
return dukeImage.getWidth();
}

@Override
public int getIconHeight() {
return dukeImage.getHeight();
}
}


This is a limitation for custom Icons because they will need to use 
toe drawImage with appropriate implementation.
To fix that will be a major change and may need change in the 
implementation of drawImage method.


  It looks like the code below from the fix doesn't work for the 
ImageIcon because x and y are now scaled. Is it possible to apply some 
other transformations (may be some translations) to the graphics to draw 
the image at the right position?

---
 334 g2.scale((float) sMaxIconWidth / icon.getIconWidth(),
 335 (float) sMaxIconWidth / icon.getIconHeight());
 336 icon.paintIcon(frame, g2, x, y);
---

--

- "(icon != null && (icon instanceof Icon))"
  Could the check to null also be omitted here?
  In other words, are the "(icon != null && (icon instanceof Icon))" 
and "(icon instanceof Icon)" checks return the same result?


If we remove the check, the cases where custom ImageIcon have no 
images will fail.


The provided example  should work with the check: "(icon instanceof 
Icon)" in the same way as with the check "(icon != null && (icon 
instanceof Icon))" because the used icon is not null and it implements 
Icon interface, should not it?


 Thanks,
 Alexandr.


Example:


import java.awt.*;
import javax.swing.*;

public class JInternalFrameBug {

   public static void main(String[] args) {
  SwingUtilities.invokeLater(
new Runnable() {
   public void run() {
  try {
 UIManager.setLookAndFeel("com.apple.laf.AquaLookAndFeel"); }
  catch(Exception e) {
 System.out.println("This is not a Mac.");
 return;
  }
  JFrame f = new JFrame();
  f.setSize(500, 500);
  JDesktopPane dtp = new JDesktopPane();
  JInternalFrame jif = new JInternalFrame();
  jif.setTitle("Test");
  jif.setFrameIcon(
new ImageIcon() {
   public int getIconWidth() {
  return 16;
   }
   public int getIconHeight() {
  return 16;
   }
   public void paintIcon(Component c, Graphics 
g, int x, int y) {

g.setColor(java.awt.Color.green);
  g.fillRect(x, y, 16, 16);
   }
});
  jif.setSize(400, 400);
  jif.setVisible(true);
  dtp.add(jif);

  f.getContentPane().setLayout(new BorderLayout());
  f.getContentPane().add(dtp, "Center");
  f.setVisible(true);
   }
});
   }
}


  Thanks,
  Alexandr.



With Regards,
Avik Niyogi
On 02-Feb-2016, at 3:02 pm, Alexandr Scherbatiy 
> wrote:


On 2/1/2016 11:25 PM, Avik Niyogi wrote:

Hi All,
Please review the code changes made as per inputs provided:
cr.openjdk.java.net/~aniyogi/8146321/webrev.02 



   - is it possible to skip the ImageIcon parsing and handle it as 
others icons (may be applying some translation to the graphics)?

   -   "(icon instanceof ImageIcon || icon instanceof Icon):
 ImageIcons is also Icon so the whole condition should be the 
same as just (icon instanceof Icon)


  Thanks,
  Alexandr.



With Regards,
Avik Niyogi
On 20-Jan-2016, at 10:35 pm, Sergey Bylokhov 
> 
wrote:


On 20/01/16 18:43, Avik Niyogi wrote:

if ((icon.getIconWidth() > sMaxIconWidth
|| icon.getIconHeight() > sMaxIconHeight)) {
final Graphics2D g2 = (Graphics2D) g;

Re: Review Request of 8146321: [macosx] JInternalFrame frame icon in wrong position on Mac L if icon is not ImageIcon

2016-02-01 Thread Avik Niyogi 
Hi All,
Please review the code changes made as per inputs provided:
cr.openjdk.java.net/~aniyogi/8146321/webrev.02 


With Regards,
Avik Niyogi
> On 20-Jan-2016, at 10:35 pm, Sergey Bylokhov  
> wrote:
> 
> On 20/01/16 18:43, Avik Niyogi wrote:
>> if ((icon.getIconWidth() > sMaxIconWidth
>> || icon.getIconHeight() > sMaxIconHeight)) {
>> final Graphics2D g2 = (Graphics2D) g;
>> final AffineTransform savedAT = g2.getTransform();
>> g2.scale((float)sMaxIconWidth/icon.getIconWidth(),
>> (float)sMaxIconWidth/icon.getIconHeight());
>> icon.paintIcon(frame, g2, x, y);
>> g2.setTransform(savedAT);
>> }
> 
> This code does not take into account that x,y whcih are passed to the 
> paintIcon should be adjusted, because after the scale they contain incorrect 
> starting points.
> 
>> 
>> then for a test code with the following:
>> 
>> import java.awt.*;
>> import javax.swing.*;
>> 
>> public class JInternalFrameBug {
>> 
>>public static void main(String[] args) {
>>   SwingUtilities.invokeLater(
>> new Runnable() {
>>public void run() {
>>   try {
>> 
>>  UIManager.setLookAndFeel("com.apple.laf.AquaLookAndFeel"); }
>>   catch(Exception e) {
>>  System.out.println("This is not a Mac.");
>>  return;
>>   }
>>   JFrame f = new JFrame();
>>   f.setSize(400, 400);
>>   JDesktopPane dtp = new JDesktopPane();
>>   JInternalFrame jif = new JInternalFrame();
>>   jif.setTitle("Test");
>>   jif.setFrameIcon(new
>> ImageIcon("/Users/avniyogi/Downloads/FeedbinNotifier-master/FeedbinNotifier/Images.xcassets/AppIcon.appiconset/icon_128x...@2x.png"));
>>   jif.setSize(200, 200);
>>   jif.setVisible(true);
>>   dtp.add(jif);
>> 
>>   f.getContentPane().setLayout(new BorderLayout());
>>   f.getContentPane().add(dtp, "Center");
>>   f.setVisible(true);
>>}
>> });
>>}
>> }
>> 
>> results in this:
>> 
>> 
>> 
>>> On 20-Jan-2016, at 4:42 pm, Alexander Scherbatiy
>>> >> > wrote:
>>> 
>>> On 1/20/2016 7:59 AM, Avik Niyogi wrote:
 Hi All,
 A gentle reminder, please review my code changes as in the webrev
 below in the mail trail.
 With Regards,
 Avik Niyogi
> On 18-Jan-2016, at 3:01 pm, Avik Niyogi   > wrote:
> 
> Hi Sergey,
> 
> Please review the webrev taking inputs as per the discussion before:
> http://cr.openjdk.java.net/~aniyogi/8146321/webrev.01/
> 
>>> 
>>>   The idea was to scale the graphics that the drawn icon fits to the
>>> target sizes.
>>>   Something like:
>>>  
>>>  g2d.scale(targetIconWidth/icon.getWidth(),
>>> targetIconHeight/icon.getHeight());
>>>  icon.paintIcon(frame, g2d, x, y);
>>> -
>>> 
>>>  This should work not only for ImageIcon but for any type of icons.
>>> 
>>>  Thanks,
>>>  Alexandr.
> 
> With Regards,
> Avik Niyogi
> 
>> On 14-Jan-2016, at 12:55 pm, Avik Niyogi >  > wrote:
>> 
>> Hi Sergey,
>> I have verified it with the test case as well. If a test case
>> overrides these methods to imply a change with icons larger than
>> 16x16 it will show that for ImageIcon and Icon as before. The
>> resize of ImageIcon is only in case it has an image file that it
>> will try to fit. I had a similar query myself and have found out
>> that *getImage()* exists for ImageIcon class only and not Icon class.
>> Example:
>> private static void createImageIconUI(final String lookAndFeelString)
>>   throws Exception {
>> SwingUtilities.invokeAndWait(new Runnable() {
>>   @Override
>>   public void run() {
>>   desktopPane = new JDesktopPane();
>>   internalFrame = new JInternalFrame();
>>   frame = new JFrame();
>> internalFrame.setTitle(lookAndFeelString);
>>   titleImageIcon = new ImageIcon() {
>>   @Override
>>   public int getIconWidth() {
>>   return 16;
>>   }
>> 
>>   @Override
>>   public int getIconHeight() {
>>   return 16;
>>   }
>> 
>>   @Override

Re: Review Request of 8146321: [macosx] JInternalFrame frame icon in wrong position on Mac L if icon is not ImageIcon

2016-01-20 Thread Sergey Bylokhov 

On 20/01/16 18:43, Avik Niyogi wrote:

if ((icon.getIconWidth() > sMaxIconWidth
 || icon.getIconHeight() > sMaxIconHeight)) {
 final Graphics2D g2 = (Graphics2D) g;
 final AffineTransform savedAT = g2.getTransform();
 g2.scale((float)sMaxIconWidth/icon.getIconWidth(),
 (float)sMaxIconWidth/icon.getIconHeight());
 icon.paintIcon(frame, g2, x, y);
 g2.setTransform(savedAT);
 }


This code does not take into account that x,y whcih are passed to the 
paintIcon should be adjusted, because after the scale they contain 
incorrect starting points.




then for a test code with the following:

import java.awt.*;
import javax.swing.*;

public class JInternalFrameBug {

public static void main(String[] args) {
   SwingUtilities.invokeLater(
 new Runnable() {
public void run() {
   try {

  UIManager.setLookAndFeel("com.apple.laf.AquaLookAndFeel"); }
   catch(Exception e) {
  System.out.println("This is not a Mac.");
  return;
   }
   JFrame f = new JFrame();
   f.setSize(400, 400);
   JDesktopPane dtp = new JDesktopPane();
   JInternalFrame jif = new JInternalFrame();
   jif.setTitle("Test");
   jif.setFrameIcon(new
ImageIcon("/Users/avniyogi/Downloads/FeedbinNotifier-master/FeedbinNotifier/Images.xcassets/AppIcon.appiconset/icon_128x...@2x.png"));
   jif.setSize(200, 200);
   jif.setVisible(true);
   dtp.add(jif);

   f.getContentPane().setLayout(new BorderLayout());
   f.getContentPane().add(dtp, "Center");
   f.setVisible(true);
}
 });
}
}

results in this:




On 20-Jan-2016, at 4:42 pm, Alexander Scherbatiy
> wrote:

On 1/20/2016 7:59 AM, Avik Niyogi wrote:

Hi All,
A gentle reminder, please review my code changes as in the webrev
below in the mail trail.
With Regards,
Avik Niyogi

On 18-Jan-2016, at 3:01 pm, Avik Niyogi  > wrote:

Hi Sergey,

Please review the webrev taking inputs as per the discussion before:
http://cr.openjdk.java.net/~aniyogi/8146321/webrev.01/



   The idea was to scale the graphics that the drawn icon fits to the
target sizes.
   Something like:
  
  g2d.scale(targetIconWidth/icon.getWidth(),
targetIconHeight/icon.getHeight());
  icon.paintIcon(frame, g2d, x, y);
 -

  This should work not only for ImageIcon but for any type of icons.

  Thanks,
  Alexandr.


With Regards,
Avik Niyogi


On 14-Jan-2016, at 12:55 pm, Avik Niyogi  > wrote:

Hi Sergey,
I have verified it with the test case as well. If a test case
overrides these methods to imply a change with icons larger than
16x16 it will show that for ImageIcon and Icon as before. The
resize of ImageIcon is only in case it has an image file that it
will try to fit. I had a similar query myself and have found out
that *getImage()* exists for ImageIcon class only and not Icon class.
Example:
private static void createImageIconUI(final String lookAndFeelString)
   throws Exception {
SwingUtilities.invokeAndWait(new Runnable() {
   @Override
   public void run() {
   desktopPane = new JDesktopPane();
   internalFrame = new JInternalFrame();
   frame = new JFrame();
internalFrame.setTitle(lookAndFeelString);
   titleImageIcon = new ImageIcon() {
   @Override
   public int getIconWidth() {
   return 16;
   }

   @Override
   public int getIconHeight() {
   return 16;
   }

   @Override
   public void paintIcon(
Component c, Graphics g, int x, int y) {
g.setColor(java.awt.Color.black);
*g.fillRect(x, y, 50, 50);*
   }
   };
internalFrame.setFrameIcon(titleImageIcon);
internalFrame.setSize(500, 200);
internalFrame.setVisible(true);
desktopPane.add(internalFrame);

frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
frame.getContentPane().setLayout(new BorderLayout());
frame.getContentPane().add(desktopPane, "Center");
frame.setSize(500, 500);
frame.setLocationRelativeTo(null);
frame.setVisible(true);
frame.toFront();
   }
   });
   }
In this case the ImageIcon will NOT be resized to 16 x 16 even
before my fix was implemented. That resize as shown in code is
*only for Image files to fit in default

Re: Review Request of 8146321: [macosx] JInternalFrame frame icon in wrong position on Mac L if icon is not ImageIcon

2016-01-20 Thread Alexander Scherbatiy 

On 1/20/2016 7:59 AM, Avik Niyogi wrote:

Hi All,
A gentle reminder, please review my code changes as in the webrev 
below in the mail trail.

With Regards,
Avik Niyogi
On 18-Jan-2016, at 3:01 pm, Avik Niyogi > wrote:


Hi Sergey,

Please review the webrev taking inputs as per the discussion before:
http://cr.openjdk.java.net/~aniyogi/8146321/webrev.01/ 



The idea was to scale the graphics that the drawn icon fits to the 
target sizes.

Something like:
   
   g2d.scale(targetIconWidth/icon.getWidth(), 
targetIconHeight/icon.getHeight());

   icon.paintIcon(frame, g2d, x, y);
  -

   This should work not only for ImageIcon but for any type of icons.

   Thanks,
   Alexandr.


With Regards,
Avik Niyogi

On 14-Jan-2016, at 12:55 pm, Avik Niyogi > wrote:


Hi Sergey,
I have verified it with the test case as well. If a test case 
overrides these methods to imply a change with icons larger than 
16x16 it will show that for ImageIcon and Icon as before. The resize 
of ImageIcon is only in case it has an image file that it will try 
to fit. I had a similar query myself and have found out that 
*getImage()* exists for ImageIcon class only and not Icon class.

Example:
private static void createImageIconUI(final String lookAndFeelString)
throws Exception {
SwingUtilities.invokeAndWait(new Runnable() {
@Override
public void run() {
desktopPane = new JDesktopPane();
internalFrame = new JInternalFrame();
frame = new JFrame();
internalFrame.setTitle(lookAndFeelString);
titleImageIcon = new ImageIcon() {
@Override
public int getIconWidth() {
return 16;
}

@Override
public int getIconHeight() {
return 16;
}

@Override
public void paintIcon(
Component c, Graphics g, int x, int y) {
g.setColor(java.awt.Color.black);
*g.fillRect(x, y, 50, 50);*
}
};
internalFrame.setFrameIcon(titleImageIcon);
internalFrame.setSize(500, 200);
internalFrame.setVisible(true);
desktopPane.add(internalFrame);

frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
frame.getContentPane().setLayout(new BorderLayout());
frame.getContentPane().add(desktopPane, "Center");
frame.setSize(500, 500);
frame.setLocationRelativeTo(null);
frame.setVisible(true);
frame.toFront();
}
});
}
In this case the ImageIcon will NOT be resized to 16 x 16 even 
before my fix was implemented. That resize as shown in code is *only 
for Image files to fit in default ImageIcon* size as returned from 
the getIconHeight() and getIconWidth() methods. If user changes the 
ImageIcon size as in the example, that is upto to user to choose to 
do so and no control exists to prevent that. *So, with or without my 
fix, this behaviour will be same for ImageIcon and Icon custom 
instances.*

Hope this clarifies the query.

With Regards,
Avik Niyogi

On 14-Jan-2016, at 11:36 am, Sergey Bylokhov 
> wrote:


Hi, Avik.
In the fix you update getIconWidth() and getIconHeight, so now we 
take the Icon into account. but it seems if the Icon is bigger that 
the maximum size it will not be resided to 16x16, right?


On 14/01/16 07:49, Avik Niyogi wrote:

Hi All,

Kindly review the bug fix for JDK 9.

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

*Webrev:*
http://cr.openjdk.java.net/~aniyogi/8146321/webrev.00/ 



*Issue:*
Under the Mac Look, if an icon type other than an ImageIcon 
is used

in JInternalFrame.setFrameIcon(),
the icon will show up in the wrong position.

*Cause:*
the "instanceof Icon" was not checked for. Also, customs ImageIcon 
with

color fill (and no image URL) which would
have resulted in null value for resized ImageIcon image was not well
handled.

*Fix:*
All places in Aqua LAF where "instanceof Icon” (and not just ImageIcon
class) is required,
inputs were added after significant analyses.
Check for null for getImage was done to remove a Null Pointer 
Exception.


With Regards,
Avik Niyogi



--
Best regards, Sergey.

Re: Review Request of 8146321: [macosx] JInternalFrame frame icon in wrong position on Mac L if icon is not ImageIcon

2016-01-19 Thread Avik Niyogi 
Hi All,
A gentle reminder, please review my code changes as in the webrev below in the 
mail trail.
With Regards,
Avik Niyogi
> On 18-Jan-2016, at 3:01 pm, Avik Niyogi  wrote:
> 
> Hi Sergey,
> 
> Please review the webrev taking inputs as per the discussion before:
> http://cr.openjdk.java.net/~aniyogi/8146321/webrev.01/ 
> 
> 
> With Regards,
> Avik Niyogi
> 
>> On 14-Jan-2016, at 12:55 pm, Avik Niyogi > > wrote:
>> 
>> Hi Sergey,
>> I have verified it with the test case as well. If a test case overrides 
>> these methods to imply a change with icons larger than 16x16 it will show 
>> that for ImageIcon and Icon as before. The resize of ImageIcon is only in 
>> case it has an image file that it will try to fit. I had a similar query 
>> myself and have found out that getImage() exists for ImageIcon class only 
>> and not Icon class.
>> Example:
>> private static void createImageIconUI(final String lookAndFeelString)
>> throws Exception {
>> SwingUtilities.invokeAndWait(new Runnable() {
>> @Override
>> public void run() {
>> desktopPane = new JDesktopPane();
>> internalFrame = new JInternalFrame();
>> frame = new JFrame();
>> internalFrame.setTitle(lookAndFeelString);
>> titleImageIcon = new ImageIcon() {
>> @Override
>> public int getIconWidth() {
>> return 16;
>> }
>> 
>> @Override
>> public int getIconHeight() {
>> return 16;
>> }
>> 
>> @Override
>> public void paintIcon(
>> Component c, Graphics g, int x, int y) {
>> g.setColor(java.awt.Color.black);
>> g.fillRect(x, y, 50, 50);
>> }
>> };
>> internalFrame.setFrameIcon(titleImageIcon);
>> internalFrame.setSize(500, 200);
>> internalFrame.setVisible(true);
>> desktopPane.add(internalFrame);
>> 
>> frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
>> frame.getContentPane().setLayout(new BorderLayout());
>> frame.getContentPane().add(desktopPane, "Center");
>> frame.setSize(500, 500);
>> frame.setLocationRelativeTo(null);
>> frame.setVisible(true);
>> frame.toFront();
>> }
>> });
>> }
>> In this case the ImageIcon will NOT be resized to 16 x 16 even before my fix 
>> was implemented. That resize as shown in code is only for Image files to fit 
>> in default ImageIcon size as returned from the getIconHeight() and 
>> getIconWidth() methods. If user changes the ImageIcon size as in the 
>> example, that is upto to user to choose to do so and no control exists to 
>> prevent that. So, with or without my fix, this behaviour will be same for 
>> ImageIcon and Icon custom instances.
>> Hope this clarifies the query. 
>> 
>> With Regards,
>> Avik Niyogi
>> 
>>> On 14-Jan-2016, at 11:36 am, Sergey Bylokhov >> > wrote:
>>> 
>>> Hi, Avik.
>>> In the fix you update getIconWidth() and getIconHeight, so now we take the 
>>> Icon into account. but it seems if the Icon is bigger that the maximum size 
>>> it will not be resided to 16x16, right?
>>> 
>>> On 14/01/16 07:49, Avik Niyogi wrote:
 Hi All,
 
 Kindly review the bug fix for JDK 9.
 
 *Bug:*
 https://bugs.openjdk.java.net/browse/JDK-8146321 
 
 
 *Webrev:*
 http://cr.openjdk.java.net/~aniyogi/8146321/webrev.00/ 
 
 
 *Issue:*
 Under the Mac Look, if an icon type other than an ImageIcon is used
 in JInternalFrame.setFrameIcon(),
 the icon will show up in the wrong position.
 
 *Cause:*
 the "instanceof Icon" was not checked for. Also, customs ImageIcon with
 color fill (and no image URL) which would
 have resulted in null value for resized ImageIcon image was not well
 handled.
 
 *Fix:*
 All places in Aqua LAF where "instanceof Icon” (and not just ImageIcon
 class) is required,
 inputs were added after significant analyses.
 Check for null for getImage was done to remove a Null Pointer Exception.
 
 With Regards,
 Avik Niyogi
>>> 
>>> 
>>> -- 
>>> Best regards, Sergey.
>> 
>

Re: Review Request of 8146321: [macosx] JInternalFrame frame icon in wrong position on Mac L if icon is not ImageIcon

2016-01-18 Thread Avik Niyogi 
Hi Sergey,

Please review the webrev taking inputs as per the discussion before:
http://cr.openjdk.java.net/~aniyogi/8146321/webrev.01/ 


With Regards,
Avik Niyogi

> On 14-Jan-2016, at 12:55 pm, Avik Niyogi  wrote:
> 
> Hi Sergey,
> I have verified it with the test case as well. If a test case overrides these 
> methods to imply a change with icons larger than 16x16 it will show that for 
> ImageIcon and Icon as before. The resize of ImageIcon is only in case it has 
> an image file that it will try to fit. I had a similar query myself and have 
> found out that getImage() exists for ImageIcon class only and not Icon class.
> Example:
> private static void createImageIconUI(final String lookAndFeelString)
> throws Exception {
> SwingUtilities.invokeAndWait(new Runnable() {
> @Override
> public void run() {
> desktopPane = new JDesktopPane();
> internalFrame = new JInternalFrame();
> frame = new JFrame();
> internalFrame.setTitle(lookAndFeelString);
> titleImageIcon = new ImageIcon() {
> @Override
> public int getIconWidth() {
> return 16;
> }
> 
> @Override
> public int getIconHeight() {
> return 16;
> }
> 
> @Override
> public void paintIcon(
> Component c, Graphics g, int x, int y) {
> g.setColor(java.awt.Color.black);
> g.fillRect(x, y, 50, 50);
> }
> };
> internalFrame.setFrameIcon(titleImageIcon);
> internalFrame.setSize(500, 200);
> internalFrame.setVisible(true);
> desktopPane.add(internalFrame);
> 
> frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
> frame.getContentPane().setLayout(new BorderLayout());
> frame.getContentPane().add(desktopPane, "Center");
> frame.setSize(500, 500);
> frame.setLocationRelativeTo(null);
> frame.setVisible(true);
> frame.toFront();
> }
> });
> }
> In this case the ImageIcon will NOT be resized to 16 x 16 even before my fix 
> was implemented. That resize as shown in code is only for Image files to fit 
> in default ImageIcon size as returned from the getIconHeight() and 
> getIconWidth() methods. If user changes the ImageIcon size as in the example, 
> that is upto to user to choose to do so and no control exists to prevent 
> that. So, with or without my fix, this behaviour will be same for ImageIcon 
> and Icon custom instances.
> Hope this clarifies the query. 
> 
> With Regards,
> Avik Niyogi
> 
>> On 14-Jan-2016, at 11:36 am, Sergey Bylokhov > > wrote:
>> 
>> Hi, Avik.
>> In the fix you update getIconWidth() and getIconHeight, so now we take the 
>> Icon into account. but it seems if the Icon is bigger that the maximum size 
>> it will not be resided to 16x16, right?
>> 
>> On 14/01/16 07:49, Avik Niyogi wrote:
>>> Hi All,
>>> 
>>> Kindly review the bug fix for JDK 9.
>>> 
>>> *Bug:*
>>> https://bugs.openjdk.java.net/browse/JDK-8146321 
>>> 
>>> 
>>> *Webrev:*
>>> http://cr.openjdk.java.net/~aniyogi/8146321/webrev.00/
>>> 
>>> *Issue:*
>>> Under the Mac Look, if an icon type other than an ImageIcon is used
>>> in JInternalFrame.setFrameIcon(),
>>> the icon will show up in the wrong position.
>>> 
>>> *Cause:*
>>> the "instanceof Icon" was not checked for. Also, customs ImageIcon with
>>> color fill (and no image URL) which would
>>> have resulted in null value for resized ImageIcon image was not well
>>> handled.
>>> 
>>> *Fix:*
>>> All places in Aqua LAF where "instanceof Icon” (and not just ImageIcon
>>> class) is required,
>>> inputs were added after significant analyses.
>>> Check for null for getImage was done to remove a Null Pointer Exception.
>>> 
>>> With Regards,
>>> Avik Niyogi
>> 
>> 
>> -- 
>> Best regards, Sergey.
>

Re: Review Request of 8146321: [macosx] JInternalFrame frame icon in wrong position on Mac L if icon is not ImageIcon

2016-01-13 Thread Sergey Bylokhov 

Hi, Avik.
In the fix you update getIconWidth() and getIconHeight, so now we take 
the Icon into account. but it seems if the Icon is bigger that the 
maximum size it will not be resided to 16x16, right?


On 14/01/16 07:49, Avik Niyogi wrote:

Hi All,

Kindly review the bug fix for JDK 9.

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

*Webrev:*
http://cr.openjdk.java.net/~aniyogi/8146321/webrev.00/

*Issue:*
Under the Mac Look, if an icon type other than an ImageIcon is used
in JInternalFrame.setFrameIcon(),
the icon will show up in the wrong position.

*Cause:*
the "instanceof Icon" was not checked for. Also, customs ImageIcon with
color fill (and no image URL) which would
have resulted in null value for resized ImageIcon image was not well
handled.

*Fix:*
All places in Aqua LAF where "instanceof Icon” (and not just ImageIcon
class) is required,
inputs were added after significant analyses.
Check for null for getImage was done to remove a Null Pointer Exception.

With Regards,
Avik Niyogi



--
Best regards, Sergey.

Re: Review Request of 8146321: [macosx] JInternalFrame frame icon in wrong position on Mac L if icon is not ImageIcon

2016-01-13 Thread Avik Niyogi 
Hi Sergey,
I have verified it with the test case as well. If a test case overrides these 
methods to imply a change with icons larger than 16x16 it will show that for 
ImageIcon and Icon as before. The resize of ImageIcon is only in case it has an 
image file that it will try to fit. I had a similar query myself and have found 
out that getImage() exists for ImageIcon class only and not Icon class.
Example:
private static void createImageIconUI(final String lookAndFeelString)
throws Exception {
SwingUtilities.invokeAndWait(new Runnable() {
@Override
public void run() {
desktopPane = new JDesktopPane();
internalFrame = new JInternalFrame();
frame = new JFrame();
internalFrame.setTitle(lookAndFeelString);
titleImageIcon = new ImageIcon() {
@Override
public int getIconWidth() {
return 16;
}

@Override
public int getIconHeight() {
return 16;
}

@Override
public void paintIcon(
Component c, Graphics g, int x, int y) {
g.setColor(java.awt.Color.black);
g.fillRect(x, y, 50, 50);
}
};
internalFrame.setFrameIcon(titleImageIcon);
internalFrame.setSize(500, 200);
internalFrame.setVisible(true);
desktopPane.add(internalFrame);

frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
frame.getContentPane().setLayout(new BorderLayout());
frame.getContentPane().add(desktopPane, "Center");
frame.setSize(500, 500);
frame.setLocationRelativeTo(null);
frame.setVisible(true);
frame.toFront();
}
});
}
In this case the ImageIcon will NOT be resized to 16 x 16 even before my fix 
was implemented. That resize as shown in code is only for Image files to fit in 
default ImageIcon size as returned from the getIconHeight() and getIconWidth() 
methods. If user changes the ImageIcon size as in the example, that is upto to 
user to choose to do so and no control exists to prevent that. So, with or 
without my fix, this behaviour will be same for ImageIcon and Icon custom 
instances.
Hope this clarifies the query. 

With Regards,
Avik Niyogi

> On 14-Jan-2016, at 11:36 am, Sergey Bylokhov  
> wrote:
> 
> Hi, Avik.
> In the fix you update getIconWidth() and getIconHeight, so now we take the 
> Icon into account. but it seems if the Icon is bigger that the maximum size 
> it will not be resided to 16x16, right?
> 
> On 14/01/16 07:49, Avik Niyogi wrote:
>> Hi All,
>> 
>> Kindly review the bug fix for JDK 9.
>> 
>> *Bug:*
>> https://bugs.openjdk.java.net/browse/JDK-8146321
>> 
>> *Webrev:*
>> http://cr.openjdk.java.net/~aniyogi/8146321/webrev.00/
>> 
>> *Issue:*
>> Under the Mac Look, if an icon type other than an ImageIcon is used
>> in JInternalFrame.setFrameIcon(),
>> the icon will show up in the wrong position.
>> 
>> *Cause:*
>> the "instanceof Icon" was not checked for. Also, customs ImageIcon with
>> color fill (and no image URL) which would
>> have resulted in null value for resized ImageIcon image was not well
>> handled.
>> 
>> *Fix:*
>> All places in Aqua LAF where "instanceof Icon” (and not just ImageIcon
>> class) is required,
>> inputs were added after significant analyses.
>> Check for null for getImage was done to remove a Null Pointer Exception.
>> 
>> With Regards,
>> Avik Niyogi
> 
> 
> -- 
> Best regards, Sergey.

Review Request of 8146321: [macosx] JInternalFrame frame icon in wrong position on Mac L if icon is not ImageIcon

2016-01-13 Thread Avik Niyogi 
Hi All,

Kindly review the bug fix for JDK 9.

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


Webrev:
http://cr.openjdk.java.net/~aniyogi/8146321/webrev.00/ 


Issue:
Under the Mac Look, if an icon type other than an ImageIcon is used in 
JInternalFrame.setFrameIcon(),
the icon will show up in the wrong position. 

Cause:
the "instanceof Icon" was not checked for. Also, customs ImageIcon with color 
fill (and no image URL) which would 
have resulted in null value for resized ImageIcon image was not well handled.

Fix:
All places in Aqua LAF where "instanceof Icon” (and not just ImageIcon class) 
is required, 
inputs were added after significant analyses.
Check for null for getImage was done to remove a Null Pointer Exception.

With Regards,
Avik Niyogi