<Swing Dev> Review Request of 8146321: [macosx] JInternalFrame frame icon in wrong position on Mac L&F if icon is not ImageIcon
Alexandr Scherbatiy
alexandr.scherbatiy at oracle.com
Tue Feb 2 10:14:44 UTC 2016
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();
}
}
----------------------
- "(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?
Thanks,
Alexandr.
>
> With Regards,
> Avik Niyogi
>> On 02-Feb-2016, at 3:02 pm, Alexandr Scherbatiy
>> <alexandr.scherbatiy at oracle.com
>> <mailto:alexandr.scherbatiy at 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.Bylokhov at oracle.com <mailto:Sergey.Bylokhov at 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_128x128 at 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.scherbatiy at oracle.com
>>>>>> <mailto:alexandr.scherbatiy at oracle.com>
>>>>>> <mailto:alexandr.scherbatiy at 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.niyogi at oracle.com
>>>>>>>> <mailto:avik.niyogi at oracle.com>
>>>>>>>> <mailto:avik.niyogi at oracle.com>
>>>>>>>> <mailto:avik.niyogi at 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.niyogi at oracle.com
>>>>>>>>> <mailto:avik.niyogi at oracle.com>
>>>>>>>>> <mailto:avik.niyogi at 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.Bylokhov at oracle.com
>>>>>>>>>> <mailto:Sergey.Bylokhov at oracle.com>
>>>>>>>>>> <mailto:Sergey.Bylokhov at oracle.com>
>>>>>>>>>> <mailto:Sergey.Bylokhov at 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.
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20160202/a528bdcf/attachment.html>
More information about the swing-dev
mailing list