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