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 <http://cr.openjdk.java.net/%7Eaniyogi/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 <sergey.bylok...@oracle.com <mailto: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>
<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> <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/>
<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> <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>
<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