Hi All, Please review the code changes made as per the inputs provided: http://cr.openjdk.java.net/~aniyogi/8146321/webrev.04 <http://cr.openjdk.java.net/~aniyogi/8146321/webrev.04>
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 >>> <alexandr.scherba...@oracle.com <mailto: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. > > 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 < >>>>> <mailto:alexandr.scherba...@oracle.com>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 < >>>>>>> <mailto:sergey.bylok...@oracle.com>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( >>>>>>>> <mailto:/Users/avniyogi/Downloads/FeedbinNotifier-master/FeedbinNotifier/Images.xcassets/AppIcon.appiconset/icon_128x...@2x.png>"/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 < >>>>>>>>>>> <mailto:avik.niy...@oracle.com>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>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 < >>>>>>>>>>>> <mailto:avik.niy...@oracle.com>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>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 >>>>>>>>>>>>> < <mailto:sergey.bylok...@oracle.com>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>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 >>>>>>>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8146321> >>>>>>>>>>>>>> >>>>>>>>>>>>>> *Webrev:* >>>>>>>>>>>>>> >>>>>>>>>>>>>> <http://cr.openjdk.java.net/%7Eaniyogi/8146321/webrev.00/>http://cr.openjdk.java.net/~aniyogi/8146321/webrev.00/ >>>>>>>>>>>>>> <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. >>>>>> >>>>> >>>> >>> >> >
