<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 09:32:30 UTC 2016
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> <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/ed3fbd37/attachment.html>
More information about the swing-dev
mailing list