<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
Wed Feb 3 08:47:52 UTC 2016
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.scherbatiy at 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.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 < <mailto:alexandr.scherbatiy at oracle.com>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 < <mailto:Sergey.Bylokhov at oracle.com>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( <mailto:/Users/avniyogi/Downloads/FeedbinNotifier-master/FeedbinNotifier/Images.xcassets/AppIcon.appiconset/icon_128x128 at 2x.png>"/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 < <mailto:avik.niyogi at oracle.com>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>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 < <mailto:avik.niyogi at oracle.com>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>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
>>>>>>>>>>>>> < <mailto:Sergey.Bylokhov at oracle.com>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>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 <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.
>>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20160203/c8aa11e8/attachment.html>
More information about the swing-dev
mailing list