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 >> <http://cr.openjdk.java.net/%7Eaniyogi/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("<path to the duke 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. > ---------------------- > > - "(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 >>> <alexandr.scherba...@oracle.com <mailto:alexandr.scherba...@oracle.com>> >>> 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 >>>> <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" >>>>>> >>>>>> <mailto:/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 >>>>>>> <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>> >>>>>>>>> <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/> >>>>>>>>> <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>> >>>>>>>>>> <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>> >>>>>>>>>>> <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 >>>>>>>>>>>> <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/> >>>>>>>>>>>> <http://cr.openjdk.java.net/%7Eaniyogi/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. >>>> >>> >> >