<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
Wed Feb 3 14:08:11 UTC 2016
On 2/3/2016 12:47 AM, Avik Niyogi wrote:
> 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/%7Eaniyogi/8146321/webrev.04>
326 g2.translate(0, 0);
The translation to the zero vector leaves the coordinate system the same.
327 float xScaleFactor = (float) sMaxIconWidth /
icon.getIconWidth();
It is better to use double values because the graphics transform
methods use them.
332 g2.scale(scaleFactorAspectRatio, scaleFactorAspectRatio);
333 g2.translate(x / scaleFactorAspectRatio, y /
scaleFactorAspectRatio);
Is it possible to use the translation first and the scale the second?
I this case where no need to re-scale translation coordinates.
Thanks,
Alexandr.
>
> With Regards,
> Avik Niyogi
>> On 02-Feb-2016, at 5:55 pm, Alexandr Scherbatiy
>> <alexandr.scherbatiy at oracle.com
>> <mailto: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
>>>>>> <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> 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>> 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>> 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/20160203/e3e30867/attachment.html>
More information about the swing-dev
mailing list