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

With Regards,
Avik Niyogi
> On 20-Jan-2016, at 10:35 pm, Sergey Bylokhov <sergey.bylok...@oracle.com> 
> 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
>>> <alexandr.scherba...@oracle.com
>>> <mailto:alexandr.scherba...@oracle.com>> 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 <avik.niy...@oracle.com
>>>>> <mailto:avik.niy...@oracle.com> <mailto:avik.niy...@oracle.com>> wrote:
>>>>> 
>>>>> Hi Sergey,
>>>>> 
>>>>> Please review the webrev taking inputs as per the discussion before:
>>>>> http://cr.openjdk.java.net/~aniyogi/8146321/webrev.01/
>>>>> <http://cr.openjdk.java.net/%7Eaniyogi/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 <avik.niy...@oracle.com
>>>>>> <mailto:avik.niy...@oracle.com> <mailto:avik.niy...@oracle.com>> 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
>>>>>>> <sergey.bylok...@oracle.com <mailto:sergey.bylok...@oracle.com>
>>>>>>> <mailto:sergey.bylok...@oracle.com>> 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/
>>>>>>>> <http://cr.openjdk.java.net/%7Eaniyogi/8146321/webrev.00/>
>>>>>>>> 
>>>>>>>> *Issue:*
>>>>>>>> Under the Mac Look&Feel, 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.
>>>>>> 
>>>>> 
>>>> 
>>> 
>> 
> 
> 
> -- 
> Best regards, Sergey.

Reply via email to