<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 12:25:35 UTC 2016
On 2/2/2016 3:41 AM, Avik Niyogi wrote:
> Hi Alexander,
>
>> On 02-Feb-2016, at 3:44 pm, Alexandr Scherbatiy
>> <alexandr.scherbatiy at oracle.com
>> <mailto: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.
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
>>>> <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/f23b9b6e/attachment.html>
More information about the swing-dev
mailing list