<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 09:50:50 UTC 2016
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.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 < <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>>> 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 <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/~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.
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20160202/ace006ea/attachment.html>
More information about the swing-dev
mailing list