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/~aniyogi/8146321/webrev.03>
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, >> 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 < >>>>>>>> <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>>> 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 >>>>>>>>> <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/~aniyogi/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. >> >
