Re: [9] Review request for 8132503: [macosx] Chinese full stop symbol cannot be entered with Pinyin IM on OS X
On 10/29/2015 7:08 PM, Anton Litvinov wrote: Hello Alexander, The second version of the fix, which addresses your suggestion concerning introduction of a separate method checking, if the "unichar" belongs to certain Unicode blocks, was created. For this purpose the method "-(BOOL) isCodePointInUnicodeBlockNeedingIMEvent: (unichar) codePoint;" was introduced. Could you please review the second version of the fix. Webrev: http://cr.openjdk.java.net/~alitvinov/8132503/jdk9/webrev.01 The fix looks good to me. Thanks, Alexandr. Thank you, Anton On 10/29/2015 3:09 PM, Anton Litvinov wrote: Hello Alexander, Thank you for review of this fix. Responses to your questions are following. 1) I also was not able to find any methods available in "java.lang.Character", "java.lang.Character.UnicodeBlock", "java.lang.Character.Subset" classes or API in Cocoa which would allow to get minimal and maximal code point for the given Unicode block. I do not think that resuming work on RFE JDK-7057645 created in 2011 will help to resolve this particular bug, because it will not be possible to introduce a new API in JDK 8, for which this bug was originally filed. 2) Yes, I think that "aString" can have more than 1 Unicode character, since it is described as "The text to insert" in the documentation of the method "(void)insertText:(id)aString replacementRange:(NSRange)" (https://developer.apple.com/library/mac/documentation/Cocoa/Reference/NSTextInputClient_Protocol/#//apple_ref/occ/intfm/NSTextInputClient/insertText:replacementRange:) And, if "aString" contains more than 1 Unicode character, this case was already taken into account by "(utf16Length > 2)" check. "AWTView.m" without this fix: 893 if ([self hasMarkedText] || !fProcessingKeystroke || (utf16Length > 2)) { "AWTView.m" with this fix: 894 if (utf16Length > 2) { 895 aStringIsComplex = YES; 3) Concerning creation of a separate function for checking, if "unichar" belongs to certain Unicode blocks. Did you mean addition of the possible function which would just check, if "unichar" is in any of the Unicode blocks hard coded in the function? Or did you mean the function which would just check, if "unichar" belongs to 1 Unicode block, whose identifier is transferred to the function, for example, as some "enum" value? Thank you, Anton On 10/28/2015 5:21 PM, Alexander Scherbatiy wrote: On 10/27/2015 1:18 PM, Anton Litvinov wrote: Hello, Could you please review the following fix for the bug. Bug: https://bugs.openjdk.java.net/browse/JDK-8132503 Webrev: http://cr.openjdk.java.net/~alitvinov/8132503/jdk9/webrev.00 The bug consists in the fact that after the fix for JDK-8068283, when IDEOGRAPHIC FULL STOP character "。" is entered from a keyboard using Pinyin IM, "java.awt.event.InputMethodEvent" is not generated and FULL STOP character "." is entered in "javax.swing.JTextArea" component. The solution adds the additional check to "if" condition, which was edited by the fix for JDK-8068283, "if ([self hasMarkedText] || !fProcessingKeystroke || (utf16Length > 2)) {" in the method "- (void) insertText:(id) replacementRange:(NSRange)" from the file "jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTView.m". This additional check defines, whether the analyzed code point belongs to Unicode code points range "U+3000 – U+303F" ("CJK Symbols and Punctuation"), which contains "。" character and, if it is so, generates "InputMethodEvent". It was interested for me does Cocoa or Java allow to get a minimal and maximal character for the given Unicode Block. I was not able to find how to do it in Cocoa. Java has an open RFE JDK-7057645 Add methods to Character.UnicodeBlock (returning first & last codepoints, the list of blocks). May be the current issue can be one more valid use case for the RFE. I have just few comments: - aString is treated as NSString. Can it have more than one Unicode character? - It could be better to move the check that a unichar belongs to a separate function that can be easily extended later for new Unicode Blocks. Thanks, Alexandr. It was verified in a local environment that the regression test from the fix for JDK-8068283 does not fail with this fix also. Thank you, Anton
Re: [9] Review request for 8132503: [macosx] Chinese full stop symbol cannot be entered with Pinyin IM on OS X
On 10/27/2015 1:18 PM, Anton Litvinov wrote: Hello, Could you please review the following fix for the bug. Bug: https://bugs.openjdk.java.net/browse/JDK-8132503 Webrev: http://cr.openjdk.java.net/~alitvinov/8132503/jdk9/webrev.00 The bug consists in the fact that after the fix for JDK-8068283, when IDEOGRAPHIC FULL STOP character "。" is entered from a keyboard using Pinyin IM, "java.awt.event.InputMethodEvent" is not generated and FULL STOP character "." is entered in "javax.swing.JTextArea" component. The solution adds the additional check to "if" condition, which was edited by the fix for JDK-8068283, "if ([self hasMarkedText] || !fProcessingKeystroke || (utf16Length > 2)) {" in the method "- (void) insertText:(id) replacementRange:(NSRange)" from the file "jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTView.m". This additional check defines, whether the analyzed code point belongs to Unicode code points range "U+3000 – U+303F" ("CJK Symbols and Punctuation"), which contains "。" character and, if it is so, generates "InputMethodEvent". It was interested for me does Cocoa or Java allow to get a minimal and maximal character for the given Unicode Block. I was not able to find how to do it in Cocoa. Java has an open RFE JDK-7057645 Add methods to Character.UnicodeBlock (returning first & last codepoints, the list of blocks). May be the current issue can be one more valid use case for the RFE. I have just few comments: - aString is treated as NSString. Can it have more than one Unicode character? - It could be better to move the check that a unichar belongs to a separate function that can be easily extended later for new Unicode Blocks. Thanks, Alexandr. It was verified in a local environment that the regression test from the fix for JDK-8068283 does not fail with this fix also. Thank you, Anton
Re: RFR: JDK-5108778 Too many instances of java.lang.Boolean created in Java application(macos)
I have pushed your fix to the JDK 9: http://hg.openjdk.java.net/jdk9/client/jdk/rev/deb544c19dec Thanks, Alexandr. On 10/21/2015 8:59 PM, Sebastian Sickelmann wrote: On 10/20/2015 12:23 PM, Alexander Scherbatiy wrote: On 10/13/2015 10:32 PM, Sebastian Sickelmann wrote: On 10/08/2015 01:06 PM, Alexander Scherbatiy wrote: Are you going to push the fix as part of other fixes for different JDK areas or as a separate fix? In the second case you need to file a new bug for it. I think the patches in the other areas are much bigger than this(macosx-port-dev) one. I think I should handle the changes separated for each mailing-list/repository. Should i than create a sub-task in JBS for each mailing-list/repository who have reviewed the change? Do I have to recreate the webrev for the new ticket-number? Just create a new issue in java.awt area and send the bug id. You can copy the same approved webrev under the new bug id if you wish. Hi, i create a subtask JDK-8139754 for this. Unfortunatly i cannot scp to cr.openjdk.java.net anymore. I somehow "managed" that the server does not accept my publickey anymore. I mailed to k...@openjdk.java.net (found that on another thread) but don't get an answer yet. Is there anotherway to update my public key on cr.openjdk.java.net? My username is sebastian Thanks, Sebastian Thanks, Alexandr. Thanks, Sebastian Thanks, Alexandr.
Re: RFR: JDK-5108778 Too many instances of java.lang.Boolean created in Java application(macos)
On 10/13/2015 10:32 PM, Sebastian Sickelmann wrote: On 10/08/2015 01:06 PM, Alexander Scherbatiy wrote: Are you going to push the fix as part of other fixes for different JDK areas or as a separate fix? In the second case you need to file a new bug for it. I think the patches in the other areas are much bigger than this(macosx-port-dev) one. I think I should handle the changes separated for each mailing-list/repository. Should i than create a sub-task in JBS for each mailing-list/repository who have reviewed the change? Do I have to recreate the webrev for the new ticket-number? Just create a new issue in java.awt area and send the bug id. You can copy the same approved webrev under the new bug id if you wish. Thanks, Alexandr. Thanks, Sebastian Thanks, Alexandr.
Re: Bug: SystemFlavorMap.addFlavorForUnencodedNative ineffective on MacOS
Hello Mike, The initial Clipboard support in CToolkit in Mac OS X [1] recognizes only NSPasteboard data types that have "JAVA_DATAFLAVOR:" prefix: - +jlong indexForFormat(NSString *format) { ... +// If we don't recognize the format, but it's a Java "custom" format register it +if (returnValue == -1 && ([format hasPrefix:@"JAVA_DATAFLAVOR:"]) ) { +returnValue = registerFormatWithPasteboard(format); +} - What was the reason that Java supports only these java custom formats on Mac OS X? See also issue [2]: JDK-8136781 SystemFlavorMap.addFlavorForUnencodedNative is ineffective on MacOS [1] http://hg.openjdk.java.net/macosx-port/macosx-port/jdk/rev/dfe77a31519f [2] https://bugs.openjdk.java.net/browse/JDK-8136781 Thanks, Alexandr. On 9/18/2015 9:18 PM, Eirik Bakke wrote: Hi, macosx-port-dev. There seems to be a bug in the MacOS implementation underlying SystemFlavorMap.addFlavorForUnencodedNative. Where would the best place to report this be? Is this the relevant mailing list? The bug report follows: My Java application allows users to paste selections from Microsoft Excel. The data is retrieved using Excel's binary BIFF8 format rather than in plain text format in order to reliably detect date formatting, preserve numerical precision, and such. On Windows, getting to the relevant clipboard InputStream can be achieved by calling SystemFlavorMap.addUnencodedNativeForFlavor to map a new DataFlavor to a native data type identifier. On MacOS, however, there seems to be a bug that renders SystemFlavorMap.addUnencodedNativeForFlavor ineffective. Looking at the JDK source code, the bug seems to be that sun.lwawt.macosx.CDataTransferer.registerFormatWithPasteboard is never called when SystemFlavorMap.addUnencodedNativeForFlavor/ addFlavorForUnencodedNative is used to register a new native clipboard data format. This leads Java_sun_lwawt_macosx_CClipboard_getClipboardFormats in macosx/native/sun/awt/CClipboard.m to never return formats of the new type, because indexForFormat (in CDataTransferer.m) will always return -1. The observation above lead me to a workaround for the bug, which is to call sun.awt.datatransfer.DataTransferer.getInstance().getFormatsForFlavor(myNewFlavor, (SystemFlavorMap) SystemFlavorMap.getDefaultFlavorMap()) once after first mapping the flavor using addUnencodedNativeForFlavor and addFlavorForUnencodedNative. This works because the call to getFormatsForFlavor forces DataTransferer.getFormatForNativeAsLong to be called with the new native data type identifier, which in turn causes registerFormatWithPasteboard to be called in CDataTransferer.m. I'm worried that the workaround will cease to work in the future, however, since it relies on an obscure side-effect of a method in a private API from the sun.awt package. I've assembled a minimal example here: https://gist.github.com/eirikbakke/e8022f9f8d26c28eecd7 -- Eirik
Re: RFR: JDK-5108778 Too many instances of java.lang.Boolean created in Java application(macos)
The fix looks good to me. Note that there are usually necessary to have at least two reviewers for a fix in Open JDK. Are you going to push the fix as part of other fixes for different JDK areas or as a separate fix? In the second case you need to file a new bug for it. Thanks, Alexandr. On 10/7/2015 10:59 PM, Sebastian Sickelmann wrote: Please find the updated webrev at: http://cr.openjdk.java.net/~sebastian/5108778/macos/webrev.00/ For some general discussion on regression-tests for this please find the thread in discuss[0][1] and for the general suggestion to make more wrapper-type-constructors deprecated find [2] at core-libs-dev. [0] http://mail.openjdk.java.net/pipermail/discuss/2015-September/003804.html [1] http://mail.openjdk.java.net/pipermail/discuss/2015-October/003805.html [2] http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-October/035642.html -- Sebastian On 10/06/2015 03:06 PM, Alexander Scherbatiy wrote: On 10/3/2015 5:40 AM, Stuart Marks wrote: On 9/28/15 2:03 AM, Alexander Scherbatiy wrote: Is it possible to use autoboxing in the fix instead of Boolean.valueOf(someBooleanVariable)? Hi, These cases are all interesting because they end up requiring boxed Boolean values, whether explicitly or implicitly via autoboxing: 1. AquaTabbedPaneUI.java -- use of boxed Boolean as a tri-state (!) 2. CAccessibility.java -- return value from Callable 3. LWCToolkit.java -- value in Properties (like Map) For #2 and #3 I think auto-boxing instead of valueOf() is just fine. I guess using Boolean.TRUE or Boolean.FALSE instead of true or false is ok, as it's a micro-optimization to prevent autoboxing from generating a call to Boolean.valueOf(true|false). I'm sure the JIT compiler will inline such calls, so this speeds up interpreted code a tiny bit at the expense of a little verbosity in the code. The explicit calls to Boolean.valueOf(some boolean expression) I think can simply be replaced with autoboxing in all cases. The weird one is in AquaTabbedPaneUI.java, which has protected Boolean isDefaultFocusReceiver = null; I do not mean to change the isDefaultFocusReceivertype type to boolean. It was just interesting are there pros and cons to use a short version with autoboxing for assignment: isDefaultFocusReceiver = defaultFocusReceiver != null && defaultFocusReceiver.equals(component); instead of the long version: isDefaultFocusReceiver = Boolean.valueOf(defaultFocusReceiver != null && defaultFocusReceiver.equals(component)); Thanks, Alexandr. Ugh! It would be nice to get rid of null as a marker for uninitialized state, but that might require too much analysis to show that the change would be correct. This is, I think, the only case where the code has to be explicit about dealing with a boxed Boolean object that might be null, as opposed to true or false. The only place that really has to do that is isDefaultFocusReceiver(), which checks for null up front. I'm not sure that using Boolean.valueOf() and Boolean.booleanValue() in the rest of the method are really helpful in denoting that this is a boxed, nullable Boolean though. So I'd switch to autoboxing here too. s'marks
Re: RFR: JDK-5108778 Too many instances of java.lang.Boolean created in Java application
On 10/3/2015 5:40 AM, Stuart Marks wrote: On 9/28/15 2:03 AM, Alexander Scherbatiy wrote: Is it possible to use autoboxing in the fix instead of Boolean.valueOf(someBooleanVariable)? Hi, These cases are all interesting because they end up requiring boxed Boolean values, whether explicitly or implicitly via autoboxing: 1. AquaTabbedPaneUI.java -- use of boxed Boolean as a tri-state (!) 2. CAccessibility.java -- return value from Callable 3. LWCToolkit.java -- value in Properties (like Map) For #2 and #3 I think auto-boxing instead of valueOf() is just fine. I guess using Boolean.TRUE or Boolean.FALSE instead of true or false is ok, as it's a micro-optimization to prevent autoboxing from generating a call to Boolean.valueOf(true|false). I'm sure the JIT compiler will inline such calls, so this speeds up interpreted code a tiny bit at the expense of a little verbosity in the code. The explicit calls to Boolean.valueOf(some boolean expression) I think can simply be replaced with autoboxing in all cases. The weird one is in AquaTabbedPaneUI.java, which has protected Boolean isDefaultFocusReceiver = null; I do not mean to change the isDefaultFocusReceivertype type to boolean. It was just interesting are there pros and cons to use a short version with autoboxing for assignment: isDefaultFocusReceiver = defaultFocusReceiver != null && defaultFocusReceiver.equals(component); instead of the long version: isDefaultFocusReceiver = Boolean.valueOf(defaultFocusReceiver != null && defaultFocusReceiver.equals(component)); Thanks, Alexandr. Ugh! It would be nice to get rid of null as a marker for uninitialized state, but that might require too much analysis to show that the change would be correct. This is, I think, the only case where the code has to be explicit about dealing with a boxed Boolean object that might be null, as opposed to true or false. The only place that really has to do that is isDefaultFocusReceiver(), which checks for null up front. I'm not sure that using Boolean.valueOf() and Boolean.booleanValue() in the rest of the method are really helpful in denoting that this is a boxed, nullable Boolean though. So I'd switch to autoboxing here too. s'marks
Re: RFR: JDK-5108778 Too many instances of java.lang.Boolean created in Java application
On 9/26/2015 6:42 PM, Sebastian Sickelmann wrote: Hello, my name is Sebastian Sickelmann and i signed the OCA. Actually I am searching through the JBS for low hanging fruits. Right now i am looking through the openjdk-sources and try to evaluate if i can make something about JDK-5108778. As i am not an author, i am actually not able to host webrevs on cr.openjdk.java.net. Is there someone who would support at hosting the macosx-part of JDK-5108778 on cr.openjdk.java.net for reviewing? Thank you for the contribution. Here is the created link: http://cr.openjdk.java.net/~alexsch/sebastian.sickelmann/5108778/webrev.00/ I placed the macosx part in my dropbox at: https://dl.dropboxusercontent.com/u/43692695/oss-patches/openjdk/jdk-5108778/macosx_0/webrev/index.html or as zip: https://dl.dropboxusercontent.com/u/43692695/oss-patches/openjdk/jdk-5108778/macosx_0/webrev.zip Is it possible to use autoboxing in the fix instead of Boolean.valueOf(someBooleanVariable)? It is required to have automated tests for a fix where it is possible. Can an automated test be written which checks that number of Boolean valueas are not too much? The issue 5108778 is too broad. Are you going to fix it only in JDK client area? If so, you can file a specific issue on it. Unfortunately I am not able to compile or run the tests for this on my machine because I don't have a macosx running on my hardware. I compiled the code and at least it does not have errors. Maybe someone can support here, too? Is it possible to use a virtual machine on your desktop? Thanks, Alexandr. -- Sebastian
Re: java.lang.NoSuchMethodError: createImageUsingNativeSize error in 1.8.0_45
On 8/18/2015 10:48 AM, Hendrik Schreiber wrote: On Jul 24, 2015, at 16:38, Paul Taylor wrote: On 23/07/2015 12:46, Alexander Scherbatiy wrote: It looks like known issue JDK-8077016 [macosx] Image transfer through System Clipboard is broken in 1.8.0_40 on Mac OS X https://bugs.openjdk.java.net/browse/JDK-8077016 and it fails this test https://bugs.openjdk.java.net/browse/JDK-8037371 so begs the question how did it get released in the first place ! The bug has been marked as resolved for 9. It’s unfortunately still present in 8u51. Does anybody know: Will this be fixed in the next 8u version? It is backported to JDK 8u as part of the fix: JDK-8132248 [macosx] Test closed/java/awt/dnd/ImageTransferTest/ImageTransferTest.html fails https://bugs.openjdk.java.net/browse/JDK-8132248 Thanks, Alexandr. Thanks, -hendrik
Re: Public API for internal Swing classes.
Hello Koen, Are you using the isEnabled(Object sender) method just to separate a logic that checks that an action needs to be executed from the action execution in the same way as it it done in the UIAction? Could you file an enhancement on it and provide a simple use case: http://bugreport.java.com/bugreport Thanks, Alexandr. On 7/27/2015 4:13 PM, Van Den Borre, Koen wrote: Hey, We are using sun.swing.UIAction in a custom ListUI where we override the following method and use the sender object: @Override public boolean isEnabled(Object sender) Regards, Koen On 27 Jul 2015, at 14:30, Alexander Scherbatiy wrote: According to the JEP 200: The Modular JDK (see http://openjdk.java.net/jeps/200) we expect that the standard Java SE modules will not export any internal packages. It means that classes from internal packages (like sun.swing) will not be accessible. For example: sun.swing.FilePane sun.swing.SwingUtilities2 sun.swing.sun.swing.plaf.synth.SynthIcon and others. Please, let us known if you are using the internal Swing API and it is not possible to replace it by public API. There are some known requests: JDK-8132119 Provide public API for text related methods in SwingUtilities2 https://bugs.openjdk.java.net/browse/JDK-8132119 JDK-8132120 Provide public API for screen menu bar support on MacOS https://bugs.openjdk.java.net/browse/JDK-8132120 JDK-6274842 RFE: Provide a means for a custom look and feel to use desktop font antialiasing settings. https://bugs.openjdk.java.net/browse/JDK-6274842 If you don't know if you use these types (because you use 3rd party jars) you can use the JDK 8 "jdeps" tool to find such dependencies :- ~/jdk1.8/bin/jdeps Usage: jdeps where can be a pathname to a .class file, a directory, a JAR file, or a fully-qualified class name Thanks, Alexandr.
Public API for internal Swing classes.
According to the JEP 200: The Modular JDK (see http://openjdk.java.net/jeps/200) we expect that the standard Java SE modules will not export any internal packages. It means that classes from internal packages (like sun.swing) will not be accessible. For example: sun.swing.FilePane sun.swing.SwingUtilities2 sun.swing.sun.swing.plaf.synth.SynthIcon and others. Please, let us known if you are using the internal Swing API and it is not possible to replace it by public API. There are some known requests: JDK-8132119 Provide public API for text related methods in SwingUtilities2 https://bugs.openjdk.java.net/browse/JDK-8132119 JDK-8132120 Provide public API for screen menu bar support on MacOS https://bugs.openjdk.java.net/browse/JDK-8132120 JDK-6274842 RFE: Provide a means for a custom look and feel to use desktop font antialiasing settings. https://bugs.openjdk.java.net/browse/JDK-6274842 If you don't know if you use these types (because you use 3rd party jars) you can use the JDK 8 "jdeps" tool to find such dependencies :- ~/jdk1.8/bin/jdeps Usage: jdeps where can be a pathname to a .class file, a directory, a JAR file, or a fully-qualified class name Thanks, Alexandr.
Re: java.lang.NoSuchMethodError: createImageUsingNativeSize error in 1.8.0_45
It looks like known issue JDK-8077016 [macosx] Image transfer through System Clipboard is broken in 1.8.0_40 on Mac OS X https://bugs.openjdk.java.net/browse/JDK-8077016 Thanks, Alexandr. On 7/23/2015 2:32 PM, Alexander Scherbatiy wrote: Could you file an issue on it: http://bugreport.java.com/bugreport Thanks, Alexandr. On 7/23/2015 2:27 PM, Paul Taylor wrote: On 23/07/2015 11:19, Paul Taylor wrote: This code that was used in some circumstances for dealing with single images dragged and drop from certain webbrowsers (firefox) gave no issues in 1.8.0_25 image = (Image) trans.getTransferData("image/x-java-image;class=java.awt.Image"); but now in 1.8.0_45 causing java.lang.NoSuchMethodError: createImageUsingNativeSize at sun.lwawt.macosx.CDataTransferer.getImageForByteStream(Native Method) at sun.lwawt.macosx.CDataTransferer.platformImageBytesToImage(CDataTransferer.java:238) at sun.awt.datatransfer.DataTransferer.translateBytes(DataTransferer.java:1659) at sun.lwawt.macosx.CDataTransferer.translateBytes(CDataTransferer.java:142) at sun.awt.dnd.SunDropTargetContextPeer.getTransferData(SunDropTargetContextPeer.java:269) at sun.awt.datatransfer.TransferableProxy.getTransferData(TransferableProxy.java:73) at java.awt.dnd.DropTargetContext$TransferableProxy.getTransferData(DropTargetContext.java:376) at com.jthink.jaikoz.draganddrop.ImageHandler.createImageCell(ImageHandler.java:30) Is this a bug in the new version of OSX Java or am I simply doing something wrong, is there a simple workaround ? Paul It seems to be https://bugs.openjdk.java.net/browse/JDK-8077016?page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#issue-tabs but breaking use of the clipboard for copy and pate of images is a massive bug dont you think, surely this should be a priority 1 bug as we are not talking about some obscure functionality here ? Paul
Re: java.lang.NoSuchMethodError: createImageUsingNativeSize error in 1.8.0_45
Could you file an issue on it: http://bugreport.java.com/bugreport Thanks, Alexandr. On 7/23/2015 2:27 PM, Paul Taylor wrote: On 23/07/2015 11:19, Paul Taylor wrote: This code that was used in some circumstances for dealing with single images dragged and drop from certain webbrowsers (firefox) gave no issues in 1.8.0_25 image = (Image) trans.getTransferData("image/x-java-image;class=java.awt.Image"); but now in 1.8.0_45 causing java.lang.NoSuchMethodError: createImageUsingNativeSize at sun.lwawt.macosx.CDataTransferer.getImageForByteStream(Native Method) at sun.lwawt.macosx.CDataTransferer.platformImageBytesToImage(CDataTransferer.java:238) at sun.awt.datatransfer.DataTransferer.translateBytes(DataTransferer.java:1659) at sun.lwawt.macosx.CDataTransferer.translateBytes(CDataTransferer.java:142) at sun.awt.dnd.SunDropTargetContextPeer.getTransferData(SunDropTargetContextPeer.java:269) at sun.awt.datatransfer.TransferableProxy.getTransferData(TransferableProxy.java:73) at java.awt.dnd.DropTargetContext$TransferableProxy.getTransferData(DropTargetContext.java:376) at com.jthink.jaikoz.draganddrop.ImageHandler.createImageCell(ImageHandler.java:30) Is this a bug in the new version of OSX Java or am I simply doing something wrong, is there a simple workaround ? Paul It seems to be https://bugs.openjdk.java.net/browse/JDK-8077016?page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#issue-tabs but breaking use of the clipboard for copy and pate of images is a massive bug dont you think, surely this should be a priority 1 bug as we are not talking about some obscure functionality here ? Paul
Re: Odd behavior on Max OSX 10.10 with retina display and external monitor.
Could you create an issue on it: http://bugreport.java.com/bugreport Thanks, Alexandr. On 6/27/2015 9:39 AM, AJ Gregory wrote: That was a typo. It was tested with latest 1.8.0_45. -Aj On Friday, June 26, 2015, Hendrik Schreiber wrote: On Jun 27, 2015, at 00:36, AJ Gregory > wrote: Using latest Java 1.8.0_25 or Java 1.7.0_80 I'm seeing some odd behavior on Max OSX 10.10 when using a retina laptop and an external monitor. The latest Java 8 is Java 1.8.0_45 not 25. Perhaps you should make sure you see the same behavior with the current version. Cheers, -hendrik
Re: Odd behavior with transparent window getting shadow after toggling visibility
On 11/25/2014 10:47 PM, AJ Gregory wrote: If you run the class below on a RETINA MacBook Pro (OSX 10.10) the first time you see the JWindow it's a white circle with NO shadow, but then later when it calls setVisible(false) and setVisible(true) the circle has a shadow when it's made visible again which isn't right... I can't reproduce on non-retina (OSX 10.9) so wondering if it's a retina only issue? I can reproduce it on non-retina OSX 10.10 and it is not reproduced on my retina OSX 10.9. It seems that the problem relates to the OSX 10.10. Could you file an issue on it: http://bugs.java.com Thanks, Alexandr. Same behavior for both Java 7u71 and 8u25... Anybody else experience this and have a work around? Seems like a bug for sure unless I'm missing something... import javax.swing.*; import java.awt.*; import java.awt.event.ActionEvent; import java.awt.event.ActionListener; public class TestMacShadow { public static void main(String[] args) { EventQueue.invokeLater(new Runnable() { public void run() { final JWindow window = new JWindow() { public void paint(Graphics g) { g.setColor(Color.WHITE); g.fillOval(0, 0, getWidth(), getHeight()); } }; window.setBackground(new Color(0, 0, 0, 0)); window.setLocation(new Point(100, 100)); window.setSize(new Dimension(300, 300)); window.setAlwaysOnTop(true); window.setVisible(true); Timer t = new Timer(2000, new ActionListener() { public void actionPerformed(ActionEvent actionEvent) { window.setVisible(!window.isVisible()); } }); t.setRepeats(true); t.start(); } }); } }
Re: [OpenJDK 2D-Dev] [9] Review request for 8029339 Custom MultiResolution image support on HiDPI displays
Just friendly remainder. Thanks, Alexandr. On 7/8/2014 7:02 PM, Alexander Scherbatiy wrote: Hi Phil, Could you review the fix? Thanks, Alexandr. On 6/11/2014 7:18 PM, Alexander Scherbatiy wrote: Hi Phil , I just prepared a simple FAQ about the Custom MultiResolution image API. Hope it will be helpful. 1. Scale naming convention for high-resolution images. Different OSes use different "scale" naming convention for high-resolution images: Mac OS X: image.ext, im...@2x.ext Windows: image.scale-100.ext, image.scale-140.ext, image.scale-180.ext Q: Does "scale" naming convention supported in JDK? A: Mac OS X "scale" naming convention are supported in JDK 8u20 (see JDK-8011059) It is planned to support the Windows "scale" naming convention as well. Q. How does it work in JDK? A. Bundle image.ext and im...@2x.ext images with your app on Mac OS X and call Toolkit.getImage(...) method: Image image = Toolkit.getDefaultToolkit().getImage("image.ext"); Graphics2D g2d = // get graphics g2d.drawImage(image, 0, 0, null) SunGraphics2D automatically queries and draws the provided high-resolution image. Q: There are different "scale" naming conventions on Mac OS X and Windows. May be it is better to have unified "scale" naming conventions for all OSes in Java like image[java-scale-Nx].ext? A: It seems reasonable and can be filled as a new JDK enhancement. Q: Does using "scale" naming conventions solves all problems. A: There are tasks like image processing/programmatically generated images/loading images from non-standard sources that can't be solved with predefined set of images. Q: Are there any tools that support these tasks? A: Cocoa API contains NSImage that allows to work with image representations: addRepresentation/removeRepresentation/representations JDK uses these methods to get/set multi-resolution images for the Native system (see sun.lwawt.macosx.CImage class). 2. Graphics2D Q: How SunGraphics2D deals with multi-resolution images? A: SunGraphics2D queries a resolution variant using DPI scale factors and transformed base image sizes // logicalDPIX, logicalDPIY - DPI scale factors // destImageWidth, destImageHeight - transformed base image sizes including DPI scale factors multiResolutionImage.getResolutionVariant(logicalDPIX, logicalDPIY, destImageWidth, destImageHeight); Q: Which algorithm multi-resolution image is used in getResolutionVariant(...) method? A: ToolkitImage returned by toolkit.loadImage() method should behave like the native system. It means that it should use transformed image sizes on Mac OS X and only DPI scale factors on Windows. it looks like: - // logicalDPIX, logicalDPIY - DPI scale factors // destImageWidth, destImageHeight - transformed base image sizes including DPI scale factors public Image getResolutionVariant(float logicalDPIX, float logicalDPIY, float destImageWidth, float destImageHeight) { if (Mac OS X) { return resolution variant best fitted to the destImageWidth and destImageHeight } else if (Windows){ return resolution variant best fitted to the logicalDPIX and logicalDPIY scale factors } } - 3. Custom multi-resolution image. Q: The custom multi-resolution image should be able to return an image according to the requested transformed image size and DPI scale factors. Is it enough? A: There are task like setting custom cursor that require to get all resolution variants. So the custom multi-resolution image should also contain the getResolutionVariants(): Q: Should the custom multi-resolution image be class or interface? A: There is ToolkitImage that should also have resolution variants. It is not possible to extend it from MultiResolutionImage class. The current proposal introduces the MultiResolutionImage as an interface. Q: MultiResolutionImage interface sounds strange for me. A: The better name can be suggested. Q: What does the Custom MultiResolution image API suggest? A: The current proposal provides MultiResolutionImage interface with the following methods: --- Image getResolutionVariant(float logicalDPIX, float logicalDPIY, float destImageWidth, float destImageHeight); List getResolutionVariants(); --- and AbstractMultiResolutionImage class. See samples below. 4. Memory cost Q: Can the the implementation be "lazy"? A: SunGraphics2D does not require full list of resolution variants. It queries only the image with necessary resolution. It means that resolution variants can b
Re: Horizontal scrolling not possible with touchpad with Metal and Nimbus L&F when both scrollbars are visible
Please, file the bug report in http://bugreport.java.com/bugreport Thanks, Alexandr. On 7/25/2014 10:34 AM, Robert Krüger wrote: Hi, horizontal scrolling in a JScrollPane does not work with a touchpad with Metal and Nimbus Look & Feel when both scrollbars are visible. It does work with Aqua. This little example demonstrates the problem: import javax.swing.*; public class TestTreeScrolling { public static void main(String[] args) throws Exception { for (UIManager.LookAndFeelInfo info : UIManager.getInstalledLookAndFeels()) { if (info.getName().startsWith("Nimbus")) { UIManager.setLookAndFeel(info.getClassName()); break; } } final JFrame frame = new JFrame(TestTreeScrolling.class.getSimpleName()); frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE); final JScrollPane scrollPane = new JScrollPane(new JTree()); scrollPane.setHorizontalScrollBarPolicy(JScrollPane.HORIZONTAL_SCROLLBAR_AS_NEEDED); scrollPane.setVerticalScrollBarPolicy(JScrollPane.VERTICAL_SCROLLBAR_AS_NEEDED); frame.getContentPane().add(scrollPane); frame.pack(); frame.setVisible(true); } } Expand the tree a bit and resize the window so that both scrollbars are visible. Making a horizontal scroll gesture on the touchpad results in vertical scrolling. Tested systematically with JDKs 6_51 and 8_11 but observed with all production releases of 8 before as well. Is this a known bug? Is there a known workaround (hacking the Look & Feel is also an option because we override it in our application anyway)? There must be as IntelliJ Idea does not have this problem and they are not using Aqua but I have failed to find it in their code. Does it make sense to file a bug report and if so where? Best, Robert
Re: [9] Review request for 8040291 [macosx] Http-Images are not fully loaded when using ImageIcon
Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8040291/webrev.03/ - The test checks that the resolution-variant observer is called at least once. Thanks, Alexandr. On 5/21/2014 2:50 PM, Alexander Scherbatiy wrote: Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8040291/webrev.02/ - The isRVObserevr typo is fixed On 5/20/2014 7:32 PM, Petr Pchelko wrote: Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8040291/webrev.01 - The getRVSize() method from the SunToolkit is fixed Thank you for the update, but I still have questions about the test: 1. There’s a typo in isRVObserevr. 2. Do we need the isRVObserevr method? How could it be that this method return false in this test and it’s a correct behavior and not a bug? I see it’s only possible from SunToolkit.prepareImage if bits are ERROR | ABORT. Couldn’t we get rid of this method? Walking up the stack doesn’t look like the most reliable solution.. The ImageObserver is used for two purposes in the Toolkit prepareImage() method: - to query which part of the image should be loaded - to notify an object about which image information is available To prepare a multi-resolution image it needs to prepare its resolution variant as well. The only way to know which part of the resolution variant should be loaded is requesting the base image observer. It leads that there will be a notification for the object that contains the resolution variant instead of the base image. To improve it the MultiResolutionToolkitImage wraps the base image observer for the resolution variant. The object gets notification which contains only the base image and updated sizes after that. The another case which is fixed in this issue is that the resolution variant can be loaded first and it notifies the object that the image is loaded. The fix reduces the resolution variant info flags so the object should wait for image loading notification from the base image. The test needs to check that the wrapped observer from the resolution variant does not send more info than the original image has already had. The isRVObserver() just checks that the observer is called from the resolution variant and not is from the base image. Thanks, Alexandr. With best regards. Petr. On May 20, 2014, at 7:07 PM, Alexander Scherbatiy wrote: Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8040291/webrev.01 - The getRVSize() method from the SunToolkit is fixed On 5/20/2014 6:46 PM, Petr Pchelko wrote: Hello, Alexander. SunToolkit:876 size == -1 ? -1 : size What’s going on here? Isn’t this equal to just size? I believe is’t wrong and the size should be multiplied by 2 somewhere? If the method is wrong how does the test pass? The test passes because it uses SunToolkit.prepareImage() method with the -1 size. It also uses the image observer that requires to load all image bits. Thanks, Alexandr. With best regards. Petr. On May 20, 2014, at 6:32 PM, Alexander Scherbatiy wrote: Hello, Could you look at the fix? Thanks, Alexandr. On 4/30/2014 6:34 PM, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8040291 webrev: http://cr.openjdk.java.net/~alexsch/8040291/webrev.00 The SunToolkit.prepareResolutionVariant() method wraps the base image observer and passes it to the resolution variant. It leads that the resolution variant notifies the base image about info changing. When the base image is loaded by the MediaTracker and the resolution variant is loaded first it calls the base image observer and wrongly finishes the base image loading. The fix passes the reduced resolution variant info flags to the base image so the base image loading is finished only after notifiing by the original base image observer. Thanks, Alexandr.
Re: [9] Review request for 8040291 [macosx] Http-Images are not fully loaded when using ImageIcon
Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8040291/webrev.02/ - The isRVObserevr typo is fixed On 5/20/2014 7:32 PM, Petr Pchelko wrote: Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8040291/webrev.01 - The getRVSize() method from the SunToolkit is fixed Thank you for the update, but I still have questions about the test: 1. There’s a typo in isRVObserevr. 2. Do we need the isRVObserevr method? How could it be that this method return false in this test and it’s a correct behavior and not a bug? I see it’s only possible from SunToolkit.prepareImage if bits are ERROR | ABORT. Couldn’t we get rid of this method? Walking up the stack doesn’t look like the most reliable solution.. The ImageObserver is used for two purposes in the Toolkit prepareImage() method: - to query which part of the image should be loaded - to notify an object about which image information is available To prepare a multi-resolution image it needs to prepare its resolution variant as well. The only way to know which part of the resolution variant should be loaded is requesting the base image observer. It leads that there will be a notification for the object that contains the resolution variant instead of the base image. To improve it the MultiResolutionToolkitImage wraps the base image observer for the resolution variant. The object gets notification which contains only the base image and updated sizes after that. The another case which is fixed in this issue is that the resolution variant can be loaded first and it notifies the object that the image is loaded. The fix reduces the resolution variant info flags so the object should wait for image loading notification from the base image. The test needs to check that the wrapped observer from the resolution variant does not send more info than the original image has already had. The isRVObserver() just checks that the observer is called from the resolution variant and not is from the base image. Thanks, Alexandr. With best regards. Petr. On May 20, 2014, at 7:07 PM, Alexander Scherbatiy wrote: Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8040291/webrev.01 - The getRVSize() method from the SunToolkit is fixed On 5/20/2014 6:46 PM, Petr Pchelko wrote: Hello, Alexander. SunToolkit:876 size == -1 ? -1 : size What’s going on here? Isn’t this equal to just size? I believe is’t wrong and the size should be multiplied by 2 somewhere? If the method is wrong how does the test pass? The test passes because it uses SunToolkit.prepareImage() method with the -1 size. It also uses the image observer that requires to load all image bits. Thanks, Alexandr. With best regards. Petr. On May 20, 2014, at 6:32 PM, Alexander Scherbatiy wrote: Hello, Could you look at the fix? Thanks, Alexandr. On 4/30/2014 6:34 PM, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8040291 webrev: http://cr.openjdk.java.net/~alexsch/8040291/webrev.00 The SunToolkit.prepareResolutionVariant() method wraps the base image observer and passes it to the resolution variant. It leads that the resolution variant notifies the base image about info changing. When the base image is loaded by the MediaTracker and the resolution variant is loaded first it calls the base image observer and wrongly finishes the base image loading. The fix passes the reduced resolution variant info flags to the base image so the base image loading is finished only after notifiing by the original base image observer. Thanks, Alexandr.
Re: [9] Review request for 8040291 [macosx] Http-Images are not fully loaded when using ImageIcon
Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8040291/webrev.01 - The getRVSize() method from the SunToolkit is fixed On 5/20/2014 6:46 PM, Petr Pchelko wrote: Hello, Alexander. SunToolkit:876 size == -1 ? -1 : size What’s going on here? Isn’t this equal to just size? I believe is’t wrong and the size should be multiplied by 2 somewhere? If the method is wrong how does the test pass? The test passes because it uses SunToolkit.prepareImage() method with the -1 size. It also uses the image observer that requires to load all image bits. Thanks, Alexandr. With best regards. Petr. On May 20, 2014, at 6:32 PM, Alexander Scherbatiy wrote: Hello, Could you look at the fix? Thanks, Alexandr. On 4/30/2014 6:34 PM, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8040291 webrev: http://cr.openjdk.java.net/~alexsch/8040291/webrev.00 The SunToolkit.prepareResolutionVariant() method wraps the base image observer and passes it to the resolution variant. It leads that the resolution variant notifies the base image about info changing. When the base image is loaded by the MediaTracker and the resolution variant is loaded first it calls the base image observer and wrongly finishes the base image loading. The fix passes the reduced resolution variant info flags to the base image so the base image loading is finished only after notifiing by the original base image observer. Thanks, Alexandr.
Re: [9] Review request for 8040291 [macosx] Http-Images are not fully loaded when using ImageIcon
Hello, Could you look at the fix? Thanks, Alexandr. On 4/30/2014 6:34 PM, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8040291 webrev: http://cr.openjdk.java.net/~alexsch/8040291/webrev.00 The SunToolkit.prepareResolutionVariant() method wraps the base image observer and passes it to the resolution variant. It leads that the resolution variant notifies the base image about info changing. When the base image is loaded by the MediaTracker and the resolution variant is loaded first it calls the base image observer and wrongly finishes the base image loading. The fix passes the reduced resolution variant info flags to the base image so the base image loading is finished only after notifiing by the original base image observer. Thanks, Alexandr.
Re: [OpenJDK 2D-Dev] [9] Review request for 8029339 Custom MultiResolution image support on HiDPI displays
Hi Phil, On 5/16/2014 9:12 PM, Phil Race wrote: I think Jim was looking at this. I am not sure if you yet answered all his questions/concerns. There's a lot of API here and it will take more time than I have right now just to get my head around it so do not expect a quick answer. 1. Why is there no javadoc on the new API on Toolkit ? It was decided to split the original issue on two parts: - this fix adds only MultiResolutionImage interface and AbstractMultiResolutionImage class. Here is the webrev for it: http://cr.openjdk.java.net/~alexsch/8029339/webrev.05/ - the Toolkit related API is moved to the separate issue Could you review the current fix: http://cr.openjdk.java.net/~alexsch/8029339/webrev.05/ 2. What kinds of classes are expected to implement MultiResolutionImage Application ones or platform ones ? Both. - Application: A developer can provide a set of images with different resolutions to create a multi-resolution image. An image with best-fitting resolution will be drawn on HiDPI display. - Platform: we used it to support Aqua L&F on HiDPI displays. 3. can you better explain all these parameters : 49 * @param logicalDPIX the logical horizontal DPI of the desktop. 50 * @param logicalDPIY the logical vertical DPI of the desktop. 51 * @param baseImageWidth the width of the base image. 52 * @param baseImageHeight the height of the base image. 53 * @param destImageWidth the width of the destination image. 54 * @param destImageHeight the height of the destination image. 55 * @return image resolution variant. Could we postpone it to the CCC request? 4.public List getResolutionVariants(); So this implies a fixed, known ahead of time set of images ? Why is it required to have this API ? How will anyone be able to tell which is which and use the list ? Here are some usages from the JDK code: - AquaImagefactory.getAppIconCompositedOn(final Image background) The original multi-resolution image is used to create another multi-resolution image with the background - AquaUtils.generateLightenedImage(Image image, ImageFilter filter) The original multi-resolution image is used to create lightening multi-resolution image - CImage.createFromImage(final Image image) Resolution variants from a multi-resolution image are used to create an NSImage - CCustomCursor: it is possible set a custom cursor which contains resolution variants to the native system Usually the getResolutionVariants() method is used to create one multi-resolution image based on the another multi-resolution image. 5. Why is the rendering hint needed ? Someone can manually switch off the multi-resolution image drawing from graphics so only the base image will be drawn. It is useful for the performance reason. There is a choice to draw the high-resolution image slowly or the low-resolution image faster. Thanks, Alexandr. -phil. On 5/16/2014 9:16 AM, Alexander Scherbatiy wrote: Hi Phil, I need a reviewer from the 2d group for the fix. Could you take a look at the fix and review it? Thanks, Alexandr. On 5/12/2014 6:35 PM, Alexander Scherbatiy wrote: There was a long thread about the image with sub-pixel resolution drawing on Mac OS X: http://mail.openjdk.java.net/pipermail/macosx-port-dev/2013-April/005559.html It was pointed out that Icon images that can be programmatically generated also need to have HiDPI support: http://mail.openjdk.java.net/pipermail/macosx-port-dev/2013-April/005566.html http://mail.openjdk.java.net/pipermail/macosx-port-dev/2013-April/005569.html All requests about Mac OS X HiDPI support were included to the umbrella issue: 7124410 [macosx] Lion HiDPI support https://bugs.openjdk.java.net/browse/JDK-7124410 Thanks, Alexandr. On 4/25/2014 6:45 PM, Alexander Scherbatiy wrote: On 4/25/2014 2:17 AM, Jim Graham wrote: Hi Alexandr, I asked for who was requesting these facilities and you responded with the solution you are planning to provide. I don't care what the solution looks like if we have nobody asking for the feature - I am asking who is asking for these capabilities? This is the request from the AWT team for the HiDPI support. Thanks, Alexandr. ...jim On 4/4/14 4:53 AM, Alexander Scherbatiy wrote: On 4/3/2014 2:23 AM, Jim Graham wrote: Hi Alexandr, The back and forth is getting confusing here, so I thought I'd try to summarize and start fresh(ish): 1. We need to support @2x internally for MacOS compatibility (done). 2. We will need to support _DPI images for Win-DPI compatibility (TBD). 3. Customers may have their own collection of images to bundle together into an MR image (working on that here). What is the push for this? Is this simply parity with Mac interfaces? --
Re: [OpenJDK 2D-Dev] [9] Review request for 8029339 Custom MultiResolution image support on HiDPI displays
Hi Phil, I need a reviewer from the 2d group for the fix. Could you take a look at the fix and review it? Thanks, Alexandr. On 5/12/2014 6:35 PM, Alexander Scherbatiy wrote: There was a long thread about the image with sub-pixel resolution drawing on Mac OS X: http://mail.openjdk.java.net/pipermail/macosx-port-dev/2013-April/005559.html It was pointed out that Icon images that can be programmatically generated also need to have HiDPI support: http://mail.openjdk.java.net/pipermail/macosx-port-dev/2013-April/005566.html http://mail.openjdk.java.net/pipermail/macosx-port-dev/2013-April/005569.html All requests about Mac OS X HiDPI support were included to the umbrella issue: 7124410 [macosx] Lion HiDPI support https://bugs.openjdk.java.net/browse/JDK-7124410 Thanks, Alexandr. On 4/25/2014 6:45 PM, Alexander Scherbatiy wrote: On 4/25/2014 2:17 AM, Jim Graham wrote: Hi Alexandr, I asked for who was requesting these facilities and you responded with the solution you are planning to provide. I don't care what the solution looks like if we have nobody asking for the feature - I am asking who is asking for these capabilities? This is the request from the AWT team for the HiDPI support. Thanks, Alexandr. ...jim On 4/4/14 4:53 AM, Alexander Scherbatiy wrote: On 4/3/2014 2:23 AM, Jim Graham wrote: Hi Alexandr, The back and forth is getting confusing here, so I thought I'd try to summarize and start fresh(ish): 1. We need to support @2x internally for MacOS compatibility (done). 2. We will need to support _DPI images for Win-DPI compatibility (TBD). 3. Customers may have their own collection of images to bundle together into an MR image (working on that here). What is the push for this? Is this simply parity with Mac interfaces? -- Image[] resolutionVariants = // get sorted by sizes array of resolution variants; Image mrImage = Toolkit.getDefaultToolkit().createMRImage(baseImageIndex, resolutionVariants); -- Here is the proposed patch: http://cr.openjdk.java.net/~alexsch/8029339/webrev.04/ 4. Customers may want to synthetically generate images at arbitrary resolutions (a variation that is impacting this solution). What is the push for this? -- Image mrImage = Toolkit.getDefaultToolkit().createMRImage(baseImageWidth, baseImageHeight, new float[][]{{100, 100}, {150, 150}, {200, 200}}, // resolution variants sizes (rvWidth, rvHeight) -> { /* generate a resolution variant */ }); -- 5. I'm guessing that customers might want to override the logic to choose from among multiple resolutions. That came from me based on seeing Mac and Win using different selection logic and our history of developers split between those wanting cross-platform consistency and those wanting consistency with native apps on each platform. Also, the needs of an animator may differ from the needs of a resolution-settable-document editor as to how dynamically the images shift between resolution variants. -- Image[] resolutionVariants = // get sorted by sizes array of resolution variants; Image mrImage = ImageResolutionHelper.createMRImage( (rvWidth, rvHeight, resolutionVariants) -> { /*use a custom logic to choose a resolution variant from an array of images*/}, (logicalDPI, baseImageSize, destImageSize) -> destImageSize, // calculate the custom aware resolution variant size baseImageIndex, resolutionVariants); -- or just extend the CustomMultiResolutionImage which has Image as the parent class: public class CustomMultiResolutionImage extends AbstractMultiResolutionImage { @Override public Image getResolutionVariant(float logicalDPIX, float logicalDPIY, float baseImageWidth, float baseImageHeight, float destImageWidth, float destImageHeight) { // return a resolution variant based on the given logical DPI, // base image size, or destination image size } @Override public List getResolutionVariants() { // return a list of resolution variants } @Override protected Image getBaseImage() { // return the base image } } Is that a fair summary of all of the considerations so far, or did I miss something? I think it should cover the main needs. Thanks, Alexandr. ...jim On 3/27/14 7:43 AM, Alexander Scherbatiy wrote: Below are some thoughts about TK.createMRImage(...) method On 3/24/2014 4:52 PM, Alexander Scherbatiy wrote: Hello, Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8029339/webrev.03/ - baseImageWidth/Height arguments are added to the getResolutionVariant(...) met
Re: [OpenJDK 2D-Dev] [9] Review request for 8029339 Custom MultiResolution image support on HiDPI displays
There was a long thread about the image with sub-pixel resolution drawing on Mac OS X: http://mail.openjdk.java.net/pipermail/macosx-port-dev/2013-April/005559.html It was pointed out that Icon images that can be programmatically generated also need to have HiDPI support: http://mail.openjdk.java.net/pipermail/macosx-port-dev/2013-April/005566.html http://mail.openjdk.java.net/pipermail/macosx-port-dev/2013-April/005569.html All requests about Mac OS X HiDPI support were included to the umbrella issue: 7124410 [macosx] Lion HiDPI support https://bugs.openjdk.java.net/browse/JDK-7124410 Thanks, Alexandr. On 4/25/2014 6:45 PM, Alexander Scherbatiy wrote: On 4/25/2014 2:17 AM, Jim Graham wrote: Hi Alexandr, I asked for who was requesting these facilities and you responded with the solution you are planning to provide. I don't care what the solution looks like if we have nobody asking for the feature - I am asking who is asking for these capabilities? This is the request from the AWT team for the HiDPI support. Thanks, Alexandr. ...jim On 4/4/14 4:53 AM, Alexander Scherbatiy wrote: On 4/3/2014 2:23 AM, Jim Graham wrote: Hi Alexandr, The back and forth is getting confusing here, so I thought I'd try to summarize and start fresh(ish): 1. We need to support @2x internally for MacOS compatibility (done). 2. We will need to support _DPI images for Win-DPI compatibility (TBD). 3. Customers may have their own collection of images to bundle together into an MR image (working on that here). What is the push for this? Is this simply parity with Mac interfaces? -- Image[] resolutionVariants = // get sorted by sizes array of resolution variants; Image mrImage = Toolkit.getDefaultToolkit().createMRImage(baseImageIndex, resolutionVariants); -- Here is the proposed patch: http://cr.openjdk.java.net/~alexsch/8029339/webrev.04/ 4. Customers may want to synthetically generate images at arbitrary resolutions (a variation that is impacting this solution). What is the push for this? -- Image mrImage = Toolkit.getDefaultToolkit().createMRImage(baseImageWidth, baseImageHeight, new float[][]{{100, 100}, {150, 150}, {200, 200}}, // resolution variants sizes (rvWidth, rvHeight) -> { /* generate a resolution variant */ }); -- 5. I'm guessing that customers might want to override the logic to choose from among multiple resolutions. That came from me based on seeing Mac and Win using different selection logic and our history of developers split between those wanting cross-platform consistency and those wanting consistency with native apps on each platform. Also, the needs of an animator may differ from the needs of a resolution-settable-document editor as to how dynamically the images shift between resolution variants. -- Image[] resolutionVariants = // get sorted by sizes array of resolution variants; Image mrImage = ImageResolutionHelper.createMRImage( (rvWidth, rvHeight, resolutionVariants) -> { /*use a custom logic to choose a resolution variant from an array of images*/}, (logicalDPI, baseImageSize, destImageSize) -> destImageSize, // calculate the custom aware resolution variant size baseImageIndex, resolutionVariants); -- or just extend the CustomMultiResolutionImage which has Image as the parent class: public class CustomMultiResolutionImage extends AbstractMultiResolutionImage { @Override public Image getResolutionVariant(float logicalDPIX, float logicalDPIY, float baseImageWidth, float baseImageHeight, float destImageWidth, float destImageHeight) { // return a resolution variant based on the given logical DPI, // base image size, or destination image size } @Override public List getResolutionVariants() { // return a list of resolution variants } @Override protected Image getBaseImage() { // return the base image } } Is that a fair summary of all of the considerations so far, or did I miss something? I think it should cover the main needs. Thanks, Alexandr. ...jim On 3/27/14 7:43 AM, Alexander Scherbatiy wrote: Below are some thoughts about TK.createMRImage(...) method On 3/24/2014 4:52 PM, Alexander Scherbatiy wrote: Hello, Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8029339/webrev.03/ - baseImageWidth/Height arguments are added to the getResolutionVariant(...) method - dest image sizes are reverted to included DPI scale - AbstractMultiResolutionImage is added. It needs only to implement only 3 methods from the AbstractMultiResolutionImage class
[9] Review request for 8040291 [macosx] Http-Images are not fully loaded when using ImageIcon
Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8040291 webrev: http://cr.openjdk.java.net/~alexsch/8040291/webrev.00 The SunToolkit.prepareResolutionVariant() method wraps the base image observer and passes it to the resolution variant. It leads that the resolution variant notifies the base image about info changing. When the base image is loaded by the MediaTracker and the resolution variant is loaded first it calls the base image observer and wrongly finishes the base image loading. The fix passes the reduced resolution variant info flags to the base image so the base image loading is finished only after notifiing by the original base image observer. Thanks, Alexandr.
Re: [9] Review request for 8040279 [macosx] Do not use the base image in the MultiResolutionBufferedImage constructor
Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8040279/webrev.01/ The createBufferedImage method is renamed to createImage in the CImage class. Thanks, Alexandr. On 4/21/2014 2:36 PM, Petr Pchelko wrote: Hello, Alexander. Just one minor comment: In CImage createBufferedImage method does not create a BufferredImage any more, so it worths renaming. With best regards. Petr. On 15.04.2014, at 18:36, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8040279 webrev: http://cr.openjdk.java.net/~alexsch/8040279/webrev.00 MultiResolutionBufferedImage extends BufferedImage. In this case it needs to use the base image to write it to the BufferedImage. The fix renames MultiResolutionBufferedImage to MultiResolutionCahcedImage and does not use the BufferedImage as the super class. The base image is created and cached by demand. CImage now always returns MultiResolutionCahcedImage. All requested resolution variants are scaled by NSImage. Thanks, Alexandr.
[9] Review request for 8040279 [macosx] Do not use the base image in the MultiResolutionBufferedImage constructor
Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8040279 webrev: http://cr.openjdk.java.net/~alexsch/8040279/webrev.00 MultiResolutionBufferedImage extends BufferedImage. In this case it needs to use the base image to write it to the BufferedImage. The fix renames MultiResolutionBufferedImage to MultiResolutionCahcedImage and does not use the BufferedImage as the super class. The base image is created and cached by demand. CImage now always returns MultiResolutionCahcedImage. All requested resolution variants are scaled by NSImage. Thanks, Alexandr.
Re: Checkmark in JCheckBoxMenuItem not HiDPI/Retina capable
Thank you for the report. I am able to reproduce the issue in JDK8u20 build 05 and it should have been already fixed in the build 09. Thanks, Alexandr. On 4/13/2014 6:55 PM, Eirik Bakke wrote: Huh, in Hendrik's case it seems he gets an even-larger checkmark even in retina mode (judging from the font size in Hendrik's screenshot). In my case I was _not_ running in retina mode. From: Hendrik Schreiber mailto:h...@tagtraum.com>> Date: Sunday, April 13, 2014 at 4:08 AM To: "macosx-port-dev@openjdk.java.net <mailto:macosx-port-dev@openjdk.java.net> X" <mailto:macosx-port-dev@openjdk.java.net>>, Alexander Scherbatiy mailto:alexandr.scherba...@oracle.com>> Cc: Eirik Bakke mailto:eba...@mit.edu>> Subject: Re: Checkmark in JCheckBoxMenuItem not HiDPI/Retina capable On Apr 11, 2014, at 18:30, Eirik Bakke <mailto:eba...@mit.edu>> wrote: > Hmm, now I cannot reproduce the bug. I think it might have disappeared > when I disconnected and reconnected my external monitor. I will file it > once I manage to reproduce it consistently. It does not depend on an external monitor, as I don't one and have observed this, too (screenshot of a menu attached). I've seen this in a menu, that is not in the system menu bar, but in the app window itself. This happened when running an ancient version of HPJmeter. Just double-clicked on the jar, and I was able to observe the issue. ps ax | grep java 4351 ?? U 0:03.88 /Library/Internet Plug-ins/JavaAppletPlugin.plugin/Contents/Home/bin/java -jar /Users/hendrik/repository/texte/performance/downloads/hpjmeter/HPjmeter.jar 4414 s004 R+ 0:00.00 grep java mankell:~ hendrik$ /Library/Internet\ Plug-ins/JavaAppletPlugin.plugin/Contents/Home/bin/java -version java version "1.8.0_20-ea" Java(TM) SE Runtime Environment (build 1.8.0_20-ea-b05) Java HotSpot(TM) 64-Bit Server VM (build 25.20-b05, mixed mode) Hope this helps to nail it down. -hendrik PS: I have attached the jar to illustrate the point, please don't re-distribute
Re: Checkmark in JCheckBoxMenuItem not HiDPI/Retina capable
On 4/10/2014 8:24 PM, Eirik Bakke wrote: I'm running the NetBeans IDE 8.0 on Java: 1.8.0_20-ea. Just saw a giant over-sized checkbox in a popup menu (see attached). Unfortunately the openjdk alias removes attachments from the mail. Could you resend the image and add me to the cc list. Could it be related to the retina patch mentioned in the thread below? The patch was only pushed yesterday to the JDK 8u-dev: http://hg.openjdk.java.net/jdk8u/jdk8u-dev/jdk/rev/c696bf320614 Could you create the issue on it: http://bugreport.java.com/bugreport/ Thanks, Alexandr. I don't see oversized checkmarks in the main menus. The oversized checkmark is still present in the popup menu after I restart the IDE. -- Eirik On 3/21/14, 10:50 AM, "Hendrik Schreiber" wrote: On Mar 21, 2014, at 15:40, Alexander Scherbatiy wrote: Thank you for the report. I have created an issue on it: 8038113 [macosx] JTree icon is not rendered in high resolution on Retina https://bugs.openjdk.java.net/browse/JDK-8038113 Thanks, Alexandr. Thanks! -hendrik
[9] Review request for 8039001 [macosx] Textfields in dialogs are disabled after using F3 on Mac OS X to select a window
Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8039001 webrev: http://cr.openjdk.java.net/~alexsch/8039001/webrev.00 NSRunningApplication active property is false when windowDidBecomeKey notification is received after switching from the Mission Control on Mac OS X to the modal dialog. The fix adds the isAppActive propery to the ApplicationDelegate and change it after the applicationWillBecomeActive/applicationWillResignActive notifications to track the application active state. It is difficult to write an automated test because it is hard to find the application coordinates in the Mission Control. Thanks, Alexandr.
Re: [9] Review request for 8038113 [macosx] JTree icon is not rendered in high resolution on Retina
Hello, Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8038113/webrev.02 - CachableJRSUIIcon explicitly implements Icon interface now - getOptimizedImage() method is removed from the CachingScalingIcon because NSImage always scales an image to the requested size - @Override annotation is added. Thanks, Alexandr. On 4/2/2014 4:37 PM, Petr Pchelko wrote: Hello, Alexander. Could you please add the @Override to the paintIcon method. Also I suggest to reuse the paintIcon in the createIcon. With best regards. Petr. On 02.04.2014, at 16:20, Alexander Scherbatiy wrote: Hello, Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8038113/webrev.01/ The CachableJRSUIIcon creates an image twice: in createImage() and in the AquaPainter.paint() methods. The fix paints the icon directly from the AquaPainter.paint() method which uses the properly scaled image. Thanks, Alexandr. On 3/26/2014 6:15 PM, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8038113 webrev: http://cr.openjdk.java.net/~alexsch/8038113/webrev.00 MultiResolution image is used to create resolution variants for the JTree icons. The fix assumes that AquaPainter uses the graphics transform for the image size calculation an so depends on the fix for issue 8032667 [macosx] Components cannot be rendered in http://mail.openjdk.java.net/pipermail/awt-dev/2014-March/007370.html Thanks, Alexandr.
Re: [9] Review request for 8038113 [macosx] JTree icon is not rendered in high resolution on Retina
Hello, Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8038113/webrev.01/ The CachableJRSUIIcon creates an image twice: in createImage() and in the AquaPainter.paint() methods. The fix paints the icon directly from the AquaPainter.paint() method which uses the properly scaled image. Thanks, Alexandr. On 3/26/2014 6:15 PM, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8038113 webrev: http://cr.openjdk.java.net/~alexsch/8038113/webrev.00 MultiResolution image is used to create resolution variants for the JTree icons. The fix assumes that AquaPainter uses the graphics transform for the image size calculation an so depends on the fix for issue 8032667 [macosx] Components cannot be rendered in http://mail.openjdk.java.net/pipermail/awt-dev/2014-March/007370.html Thanks, Alexandr.
Re: [9] Review Request: 8029196 Focus border of JButton.buttonType=roundRect is cut off
The fix looks good for me. Thanks, Alexandr. On 4/1/2014 4:53 PM, Sergey Bylokhov wrote: On 4/1/14 4:33 PM, Petr Pchelko wrote: Hello, Sergey. Does the fix work for retina thin focuses and non-retina thick? yes, it works in both cases. With best regards. Petr. On 01.04.2014, at 15:46, Sergey Bylokhov wrote: Hello. Please review the fix for jdk 9. JRS draws all component in specified bounds, but if a component is in focus, the frame of focus can exceed the limit of the specified bounds. The focusable components take this into account in the insets, but we cut the focus anyway, because we create the buffer image in bounds size. In the fix this image was increased by the focus size in each direction. Bug: https://bugs.openjdk.java.net/browse/JDK-8029196 Webrev can be found at: http://cr.openjdk.java.net/~serb/8029196/webrev.00 -- Best regards, Sergey.
Re: [9] Review request for 8032667 [macosx] Components cannot be rendered in HiDPI to BufferedImage
Hello, Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8032667/webrev.03/ - The automated test which checks that painting a component to 2x image and painting the component to 1x -> 2x image gives different result. Thanks, Alexandr. On 3/26/2014 5:54 PM, Sergey Bylokhov wrote: Hi, Alexander. The fix looks fine to me. Probably the test can be automated? before the fix COMP->imagex1->imagex2 and the COMP->imagex2 should be the same, and after the fix it should be different? On 3/26/14 5:43 PM, Alexander Scherbatiy wrote: Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8032667/webrev.02 The array of sizes is only used for the getResolutionVariants() method. The images are scaled according to the mapper function. The current fix creates only one size for the resolution variants list in case if they are not passed to the constructor. Thanks, Alexandr. On 3/26/2014 4:30 PM, Sergey Bylokhov wrote: Hello, Alexander. I think it will not work if the user set scale=1.5? On 3/26/14 4:18 PM, Alexander Scherbatiy wrote: Hello, Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8032667/webrev.01/ - MultiResolution image is used instead of image width/height scaling - Bounds are used in the key instead of the scale factor. Thanks, Alexandr. On 3/25/2014 8:26 PM, Sergey Bylokhov wrote: Hello, Alexander. You cannot skip scalfactor as a key, because images with different scale are different. On 3/25/14 8:10 PM, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8032667 webrev: http://cr.openjdk.java.net/~alexsch/8032667/webrev.00 High resolution image width and height were calculated using only scale factor in the AquaPainter class. The fix calculates image width and height based on the graphics transform. Thanks, Alexandr.
[9] Review request for 8036882 [macosx] Native memory leak in Java_sun_lwawt_macosx_CImage_nativeGetNSImageRepresentationSizes
Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8036882 webrev: http://cr.openjdk.java.net/~alexsch/8036882/webrev.00 autorelease is added for [[NSMutableArray alloc] init] call. There is no a test for the fix because it is hard to write it. Thanks, Alexandr.
[9] Review request for 8038113 [macosx] JTree icon is not rendered in high resolution on Retina
Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8038113 webrev: http://cr.openjdk.java.net/~alexsch/8038113/webrev.00 MultiResolution image is used to create resolution variants for the JTree icons. The fix assumes that AquaPainter uses the graphics transform for the image size calculation an so depends on the fix for issue 8032667 [macosx] Components cannot be rendered in http://mail.openjdk.java.net/pipermail/awt-dev/2014-March/007370.html Thanks, Alexandr.
Re: [9] Review request for 8032667 [macosx] Components cannot be rendered in HiDPI to BufferedImage
Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8032667/webrev.02 The array of sizes is only used for the getResolutionVariants() method. The images are scaled according to the mapper function. The current fix creates only one size for the resolution variants list in case if they are not passed to the constructor. Thanks, Alexandr. On 3/26/2014 4:30 PM, Sergey Bylokhov wrote: Hello, Alexander. I think it will not work if the user set scale=1.5? On 3/26/14 4:18 PM, Alexander Scherbatiy wrote: Hello, Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8032667/webrev.01/ - MultiResolution image is used instead of image width/height scaling - Bounds are used in the key instead of the scale factor. Thanks, Alexandr. On 3/25/2014 8:26 PM, Sergey Bylokhov wrote: Hello, Alexander. You cannot skip scalfactor as a key, because images with different scale are different. On 3/25/14 8:10 PM, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8032667 webrev: http://cr.openjdk.java.net/~alexsch/8032667/webrev.00 High resolution image width and height were calculated using only scale factor in the AquaPainter class. The fix calculates image width and height based on the graphics transform. Thanks, Alexandr.
Re: [9] Review request for 8032667 [macosx] Components cannot be rendered in HiDPI to BufferedImage
Hello, Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8032667/webrev.01/ - MultiResolution image is used instead of image width/height scaling - Bounds are used in the key instead of the scale factor. Thanks, Alexandr. On 3/25/2014 8:26 PM, Sergey Bylokhov wrote: Hello, Alexander. You cannot skip scalfactor as a key, because images with different scale are different. On 3/25/14 8:10 PM, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8032667 webrev: http://cr.openjdk.java.net/~alexsch/8032667/webrev.00 High resolution image width and height were calculated using only scale factor in the AquaPainter class. The fix calculates image width and height based on the graphics transform. Thanks, Alexandr.
[9] Review request for 8032667 [macosx] Components cannot be rendered in HiDPI to BufferedImage
Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8032667 webrev: http://cr.openjdk.java.net/~alexsch/8032667/webrev.00 High resolution image width and height were calculated using only scale factor in the AquaPainter class. The fix calculates image width and height based on the graphics transform. Thanks, Alexandr.
Re: Checkmark in JCheckBoxMenuItem not HiDPI/Retina capable
Thank you for the report. I have created an issue on it: 8038113 [macosx] JTree icon is not rendered in high resolution on Retina https://bugs.openjdk.java.net/browse/JDK-8038113 Thanks, Alexandr. On 3/21/2014 1:36 PM, Hendrik Schreiber wrote: On Jan 17, 2014, at 13:07, Hendrik Schreiber wrote: On Jan 13, 2014, at 12:54, Hendrik Schreiber wrote: Hey Sergey, bug is filed under "(Bug ID: 9009584 ) - Checkmarks of JCheckBoxMenuItems aren't rendered in high res on Retina" While at it, you might also want to take a look at "(Bug ID: 9009344 ) - JPopupMenus in Swing don't have rounded corners on OS X". Sergey, I just realized that sub-menues in JPopupMenus also uses a low res arrow ">". I guess this is part of the same issue. Perhaps you want to add a comment to the original bug 9009584... Hey Guys, I am very pleased to see that http://bugs.java.com/bugdatabase/view_bug.do?bug_id=8031573 was addressed in JDK8u20-ea. Unfortunately, at least one icon was (partially) missed - the expand tree control (that little triangle). It is rendered in HiDPI, when you click on it, but in its regular appearance it is low res. The problem occurs on OS X 10.9.2 with java version "1.8.0_20-ea" Java(TM) SE Runtime Environment (build 1.8.0_20-ea-b05) Java HotSpot(TM) 64-Bit Server VM (build 25.20-b05, mixed mode) Cheers, -hendrik Source code to illustrate the issue - a plain vanilla JTree. Just play with the expansion triangles. import javax.swing.*; import java.awt.*; public class TreeExpanders { public static void main(String[] args) { final JFrame frame = new JFrame(); final JTree tree = new JTree(); frame.getContentPane().setLayout(new BorderLayout()); frame.getContentPane().add(tree, BorderLayout.CENTER); SwingUtilities.invokeLater(new Runnable() { @Override public void run() { frame.setBounds(200, 200, 200, 200); frame.setVisible(true); } }); } }
Re: [9] Review request for 8035069 [macosx] Loading resolution variants by demand
Hello, Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8035069/webrev.01 On 3/13/2014 12:21 PM, Petr Pchelko wrote: Hello, Alexander. 1. As Sergey always says, could you please split the long lines. 2. Instead of the MultiResolutionImageMapper you could use a BiFunction 3. About the ImageCache. As it's uses an AppContext, could you please mention in the JavaDoc that is must be used from the thread with an AppContext only? 1. 2. and 3 are updated. 4. I don't really like that you are duplicating the RecyclableSingleton class. May be it's better to make also move it out from com.apple.laf and reuse? I have added the getSoftReferenceValue(Object key, Supplier supplier) method to the AppContext class. It should reduce the code duplication. 5. Looks like the old ImageCache contained the following lines: 116 if (state.is(JRSUIConstants.Animating.YES)) { 117 return false; 118 } I agree that these are probably not needed, but could you please verify that? Also after these were removed the ImageCache.setImage never returns false, so it could be made void. Thank you for pointing it out. This is the necessary check for the animated images in the Aqua L&F. I just forgot to move it to the AquaPainter. I have found one more issue that ImageIcon preloads images by calling image.getProperty("comment", imageObserver) where the imageObserver is usually null. The MultiResolutionBufferedImage created non-preloaded resolution variants and they were not shown because JMenuItem as an image observer returns false for the image loading. This is described in the issue 8037405 JMenuItem should check L&F icons in the image observer https://bugs.openjdk.java.net/browse/JDK-8037405 It seems as a common problem so I added resolution variants preloading to the MultiResolutionBufferedImage. Thanks, Alexandr. With best regards. Petr. On 12.03.2014, at 19:03, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8035069 webrev: http://cr.openjdk.java.net/~alexsch/8035069/webrev.00 Image resolution variants are generated by request and cached in the ImageCache. - ImageCache is refactored to store different type of images and moved to sun.awt.image package. - An object is used as the cache key instead of hash code to prevent inetsection of hash codes for different type of images. - The base image for MultiResolutionBufferedImage is not cached and used for the hash code calculation in the getResolutionVariant method. Thanks, Alexandr.
[9] Review request for 8035069 [macosx] Loading resolution variants by demand
Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8035069 webrev: http://cr.openjdk.java.net/~alexsch/8035069/webrev.00 Image resolution variants are generated by request and cached in the ImageCache. - ImageCache is refactored to store different type of images and moved to sun.awt.image package. - An object is used as the cache key instead of hash code to prevent inetsection of hash codes for different type of images. - The base image for MultiResolutionBufferedImage is not cached and used for the hash code calculation in the getResolutionVariant method. Thanks, Alexandr.
[9] Review request for 8036598 [macosx] Create AquaIcons from image representations
Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8036598 webrev: http://cr.openjdk.java.net/~alexsch/8036598/webrev.00 The fix creates an AquaIcon based on resolution variants. It becomes possible after the fix 8033534 where CImage returns a MultiResolution image based on NSImage representations. Thanks, Alexandr.
Re: [9] Review request for 8033534 Get MultiResolution image from native system
Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8033534/webrev.05/ - JNU_CHECK_EXCEPTION_RETURN(env, NULL) after the SetObjectArrayElement call. Hope that the exception will never be thrown in the production code. Thanks, Alexandr. On 3/4/2014 4:12 PM, Sergey Bylokhov wrote: On 3/4/14 3:53 PM, Petr Pchelko wrote: I'm fine with the fix. Hello, Alexander. In CImage.m:430 - Do we really want to describe and clear the exception? May be it's better to simply return NULL and let Java handle the exception? This made sense when you were continuing the loop, but now doesn't. The exception is cleared because it should not be thrown on the Java side. For example the Toolkit.getDefaultToolkit().getImage("NSImage://NSApplicationIcon") call should not throw an exception in case if nativeGetNSImageRepresentationSizes() call fails. It should just return an Image without resolution variants. The ExceptionDescribe is left just for debugging purposes. In real life this will never happen, so I'm fine with any decision) Just CHECK_EXCEPTION_RETUEN looks nicer than these 3 lines. I agree. If call to nativeGetNSImageRepresentationSizes will fail, will mean that some error exists in our code and it will be better to fail fast in this case, just return and throw and exception on java side. With best regards. Petr. On 04.03.2014, at 15:39, Alexander Scherbatiy wrote: On 3/4/2014 3:30 PM, Petr Pchelko wrote: Hello, Alexander. In CImage.m:430 - Do we really want to describe and clear the exception? May be it's better to simply return NULL and let Java handle the exception? This made sense when you were continuing the loop, but now doesn't. The exception is cleared because it should not be thrown on the Java side. For example the Toolkit.getDefaultToolkit().getImage("NSImage://NSApplicationIcon") call should not throw an exception in case if nativeGetNSImageRepresentationSizes() call fails. It should just return an Image without resolution variants. The ExceptionDescribe is left just for debugging purposes. Thanks, Alexandr. With best regards. Petr. On 04.03.2014, at 15:04, Alexander Scherbatiy wrote: Hello, Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8033534/webrev.04 - NULL is returned for all cases from the nativeGetNSImageRepresentationsCount method - long lines are split - SetObjectArrayElement can throw ArrayIndexOutOfBoundsException and ArrayStoreException exceptions. We do not expect neither of them because the same length and type is used for the array creation and element settings. I updated the exception handling to return NULL if an exception occurs. Thanks, Alexandr. On 3/3/2014 11:48 PM, Sergey Bylokhov wrote: Hi, Alexander. nativeGetNSImageRepresentationsCount three times return different values in case of error (0, NULL, nil). What exception we expect from the SetObjectArrayElement and why we continue in this case? Also please split soo long lines in these files. On 2/26/14 6:40 PM, Alexander Scherbatiy wrote: Hello, Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8033534/webrev.03/ On 2/26/2014 4:54 PM, Petr Pchelko wrote: Hello, Alexander. I have a couple of comments: 1. You could replace the first loop with indexOfObjectPassingTest method.. Not sure if this would look cleaner, up to you. Updated. There is one more way to use one loop instead of two. All of them does not look simple. 2. I suppose JNFNewObjectArray could throw the OOM and we would get a parfait warning, could you please add CHECK_NULL_RETURN after it. CHECK_NULL_RETURN is added. 3. In CImage.java you are setting the currentImageIndex to the biggest image representation smaller that the one requested and this representation would be used as a base one in the MultiResolutionBufferedImage. However in MultiResolutionBufferedImage getResolutionVariant you are returning the smallest variant bigger than the requested one. Is this correct? I think that it is correct. Assume that an image with size 300x300 is requested but there are only resolution variants with sizes [250x250] and [350x350]. The resolution variant with [350x350] size is used as the base image. Now we need to draw the image to region [300x300]. The resolution variant with size [350x350] will be used from the MultiResolution image. Thanks, Alexandr. Thank you. With best regards. Petr. On 26.02.2014, at 16:08, Alexander Scherbatiy wrote: Hello, Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8033534/webrev.02/ This is the same fix. The only difference is that the MultiResolutionBufferedImage class is used from the fix JDK-8035069. Thanks, Alexandr. On 2/10/2014 7:05 PM, Scott Palmer wrote: Just to be clear, "the
Re: [9] Review request for 8033534 Get MultiResolution image from native system
On 3/4/2014 3:30 PM, Petr Pchelko wrote: Hello, Alexander. In CImage.m:430 - Do we really want to describe and clear the exception? May be it's better to simply return NULL and let Java handle the exception? This made sense when you were continuing the loop, but now doesn't. The exception is cleared because it should not be thrown on the Java side. For example the Toolkit.getDefaultToolkit().getImage("NSImage://NSApplicationIcon") call should not throw an exception in case if nativeGetNSImageRepresentationSizes() call fails. It should just return an Image without resolution variants. The ExceptionDescribe is left just for debugging purposes. Thanks, Alexandr. With best regards. Petr. On 04.03.2014, at 15:04, Alexander Scherbatiy wrote: Hello, Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8033534/webrev.04 - NULL is returned for all cases from the nativeGetNSImageRepresentationsCount method - long lines are split - SetObjectArrayElement can throw ArrayIndexOutOfBoundsException and ArrayStoreException exceptions. We do not expect neither of them because the same length and type is used for the array creation and element settings. I updated the exception handling to return NULL if an exception occurs. Thanks, Alexandr. On 3/3/2014 11:48 PM, Sergey Bylokhov wrote: Hi, Alexander. nativeGetNSImageRepresentationsCount three times return different values in case of error (0, NULL, nil). What exception we expect from the SetObjectArrayElement and why we continue in this case? Also please split soo long lines in these files. On 2/26/14 6:40 PM, Alexander Scherbatiy wrote: Hello, Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8033534/webrev.03/ On 2/26/2014 4:54 PM, Petr Pchelko wrote: Hello, Alexander. I have a couple of comments: 1. You could replace the first loop with indexOfObjectPassingTest method.. Not sure if this would look cleaner, up to you. Updated. There is one more way to use one loop instead of two. All of them does not look simple. 2. I suppose JNFNewObjectArray could throw the OOM and we would get a parfait warning, could you please add CHECK_NULL_RETURN after it. CHECK_NULL_RETURN is added. 3. In CImage.java you are setting the currentImageIndex to the biggest image representation smaller that the one requested and this representation would be used as a base one in the MultiResolutionBufferedImage. However in MultiResolutionBufferedImage getResolutionVariant you are returning the smallest variant bigger than the requested one. Is this correct? I think that it is correct. Assume that an image with size 300x300 is requested but there are only resolution variants with sizes [250x250] and [350x350]. The resolution variant with [350x350] size is used as the base image. Now we need to draw the image to region [300x300]. The resolution variant with size [350x350] will be used from the MultiResolution image. Thanks, Alexandr. Thank you. With best regards. Petr. On 26.02.2014, at 16:08, Alexander Scherbatiy wrote: Hello, Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8033534/webrev.02/ This is the same fix. The only difference is that the MultiResolutionBufferedImage class is used from the fix JDK-8035069. Thanks, Alexandr. On 2/10/2014 7:05 PM, Scott Palmer wrote: Just to be clear, "the image representations are chosen to be closest to the requested size" is not accurate. This change returns the smallest image representation that is greater than or equal to the requested size. (Which I believe is the correct thing to do.) A smaller image representation may be closer to the requested size, but it makes more sense to return a larger image since scaling down to size should produce better results than scaling up. Scott On Mon, Feb 10, 2014 at 8:53 AM, Alexander Scherbatiy mailto:alexandr.scherba...@oracle.com>> wrote: Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8033534/webrev.01 <http://cr.openjdk.java.net/%7Ealexsch/8033534/webrev.01> - The image representations are chosen to be closest to the requested size. Thanks, Alexandr. On 2/4/2014 5:00 PM, Sergey Bylokhov wrote: Hi, Alexander. I think that getResolutionVariant should return an image which is close as much as possible to the requested size. On 04.02.2014 16:42, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8033534 webrev: http://cr.openjdk.java.net/~alexsch/8033534/webrev.00 <http://cr.openjdk.java.net/%7Ealexsch/8033534/webrev.00> - The method that gets a sorted array of NSImage representaion pixel sizes for the given image size is added
Re: [9] Review request for 8033534 Get MultiResolution image from native system
Hello, Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8033534/webrev.04 - NULL is returned for all cases from the nativeGetNSImageRepresentationsCount method - long lines are split - SetObjectArrayElement can throw ArrayIndexOutOfBoundsException and ArrayStoreException exceptions. We do not expect neither of them because the same length and type is used for the array creation and element settings. I updated the exception handling to return NULL if an exception occurs. Thanks, Alexandr. On 3/3/2014 11:48 PM, Sergey Bylokhov wrote: Hi, Alexander. nativeGetNSImageRepresentationsCount three times return different values in case of error (0, NULL, nil). What exception we expect from the SetObjectArrayElement and why we continue in this case? Also please split soo long lines in these files. On 2/26/14 6:40 PM, Alexander Scherbatiy wrote: Hello, Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8033534/webrev.03/ On 2/26/2014 4:54 PM, Petr Pchelko wrote: Hello, Alexander. I have a couple of comments: 1. You could replace the first loop with indexOfObjectPassingTest method.. Not sure if this would look cleaner, up to you. Updated. There is one more way to use one loop instead of two. All of them does not look simple. 2. I suppose JNFNewObjectArray could throw the OOM and we would get a parfait warning, could you please add CHECK_NULL_RETURN after it. CHECK_NULL_RETURN is added. 3. In CImage.java you are setting the currentImageIndex to the biggest image representation smaller that the one requested and this representation would be used as a base one in the MultiResolutionBufferedImage. However in MultiResolutionBufferedImage getResolutionVariant you are returning the smallest variant bigger than the requested one. Is this correct? I think that it is correct. Assume that an image with size 300x300 is requested but there are only resolution variants with sizes [250x250] and [350x350]. The resolution variant with [350x350] size is used as the base image. Now we need to draw the image to region [300x300]. The resolution variant with size [350x350] will be used from the MultiResolution image. Thanks, Alexandr. Thank you. With best regards. Petr. On 26.02.2014, at 16:08, Alexander Scherbatiy wrote: Hello, Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8033534/webrev.02/ This is the same fix. The only difference is that the MultiResolutionBufferedImage class is used from the fix JDK-8035069. Thanks, Alexandr. On 2/10/2014 7:05 PM, Scott Palmer wrote: Just to be clear, "the image representations are chosen to be closest to the requested size" is not accurate. This change returns the smallest image representation that is greater than or equal to the requested size. (Which I believe is the correct thing to do.) A smaller image representation may be closer to the requested size, but it makes more sense to return a larger image since scaling down to size should produce better results than scaling up. Scott On Mon, Feb 10, 2014 at 8:53 AM, Alexander Scherbatiy <mailto:alexandr.scherba...@oracle.com>> wrote: Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8033534/webrev.01 <http://cr.openjdk.java.net/%7Ealexsch/8033534/webrev.01> - The image representations are chosen to be closest to the requested size. Thanks, Alexandr. On 2/4/2014 5:00 PM, Sergey Bylokhov wrote: Hi, Alexander. I think that getResolutionVariant should return an image which is close as much as possible to the requested size. On 04.02.2014 16:42, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8033534 webrev: http://cr.openjdk.java.net/~alexsch/8033534/webrev.00 <http://cr.openjdk.java.net/%7Ealexsch/8033534/webrev.00> - The method that gets a sorted array of NSImage representaion pixel sizes for the given image size is added - A MultiResolution image is created if an NSImage has several representations for the given image size Thanks, Alexandr. -- Best regards, Sergey.
Re: [9] Review request for 8033534 Get MultiResolution image from native system
Hello, Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8033534/webrev.03/ On 2/26/2014 4:54 PM, Petr Pchelko wrote: Hello, Alexander. I have a couple of comments: 1. You could replace the first loop with indexOfObjectPassingTest method.. Not sure if this would look cleaner, up to you. Updated. There is one more way to use one loop instead of two. All of them does not look simple. 2. I suppose JNFNewObjectArray could throw the OOM and we would get a parfait warning, could you please add CHECK_NULL_RETURN after it. CHECK_NULL_RETURN is added. 3. In CImage.java you are setting the currentImageIndex to the biggest image representation smaller that the one requested and this representation would be used as a base one in the MultiResolutionBufferedImage. However in MultiResolutionBufferedImage getResolutionVariant you are returning the smallest variant bigger than the requested one. Is this correct? I think that it is correct. Assume that an image with size 300x300 is requested but there are only resolution variants with sizes [250x250] and [350x350]. The resolution variant with [350x350] size is used as the base image. Now we need to draw the image to region [300x300]. The resolution variant with size [350x350] will be used from the MultiResolution image. Thanks, Alexandr. Thank you. With best regards. Petr. On 26.02.2014, at 16:08, Alexander Scherbatiy wrote: Hello, Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8033534/webrev.02/ This is the same fix. The only difference is that the MultiResolutionBufferedImage class is used from the fix JDK-8035069. Thanks, Alexandr. On 2/10/2014 7:05 PM, Scott Palmer wrote: Just to be clear, "the image representations are chosen to be closest to the requested size" is not accurate. This change returns the smallest image representation that is greater than or equal to the requested size. (Which I believe is the correct thing to do.) A smaller image representation may be closer to the requested size, but it makes more sense to return a larger image since scaling down to size should produce better results than scaling up. Scott On Mon, Feb 10, 2014 at 8:53 AM, Alexander Scherbatiy mailto:alexandr.scherba...@oracle.com>> wrote: Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8033534/webrev.01 <http://cr.openjdk.java.net/%7Ealexsch/8033534/webrev.01> - The image representations are chosen to be closest to the requested size. Thanks, Alexandr. On 2/4/2014 5:00 PM, Sergey Bylokhov wrote: Hi, Alexander. I think that getResolutionVariant should return an image which is close as much as possible to the requested size. On 04.02.2014 16:42, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8033534 webrev: http://cr.openjdk.java.net/~alexsch/8033534/webrev.00 <http://cr.openjdk.java.net/%7Ealexsch/8033534/webrev.00> - The method that gets a sorted array of NSImage representaion pixel sizes for the given image size is added - A MultiResolution image is created if an NSImage has several representations for the given image size Thanks, Alexandr.
Re: [9] Review request for 8033534 Get MultiResolution image from native system
Hello, Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8033534/webrev.02/ This is the same fix. The only difference is that the MultiResolutionBufferedImage class is used from the fix JDK-8035069. Thanks, Alexandr. On 2/10/2014 7:05 PM, Scott Palmer wrote: Just to be clear, "the image representations are chosen to be closest to the requested size" is not accurate. This change returns the smallest image representation that is greater than or equal to the requested size. (Which I believe is the correct thing to do.) A smaller image representation may be closer to the requested size, but it makes more sense to return a larger image since scaling down to size should produce better results than scaling up. Scott On Mon, Feb 10, 2014 at 8:53 AM, Alexander Scherbatiy <mailto:alexandr.scherba...@oracle.com>> wrote: Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8033534/webrev.01 <http://cr.openjdk.java.net/%7Ealexsch/8033534/webrev.01> - The image representations are chosen to be closest to the requested size. Thanks, Alexandr. On 2/4/2014 5:00 PM, Sergey Bylokhov wrote: Hi, Alexander. I think that getResolutionVariant should return an image which is close as much as possible to the requested size. On 04.02.2014 16:42, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8033534 webrev: http://cr.openjdk.java.net/~alexsch/8033534/webrev.00 <http://cr.openjdk.java.net/%7Ealexsch/8033534/webrev.00> - The method that gets a sorted array of NSImage representaion pixel sizes for the given image size is added - A MultiResolution image is created if an NSImage has several representations for the given image size Thanks, Alexandr.
Re: [9] Review request for 8031573 [macosx] Checkmarks of JCheckBoxMenuItems aren't rendered in high resolution on Retina
On 2/24/2014 5:59 PM, Hendrik Schreiber wrote: Hey guys, will this fix cover JTree folder icons as well? I.e. javax.swing.UIManager.getIcon("Tree.closedIcon") returns something that is rendered in HiDPI on a a HiDPI display? This should have been already fixed as part of the issue 8024926 [macosx] AquaIcon HiDPI support https://bugs.openjdk.java.net/browse/JDK-8024926 Thanks, Alexandr. Or would that be a separate issue? Thanks, -hendrik On Feb 24, 2014, at 14:48, Petr Pchelko wrote: Hello, Alexander. The fix looks good to me. With best regards. Petr. On 18.02.2014, at 16:20, Sergey Bylokhov wrote: Hi, Alexander. The fix looks good then. On 17.02.2014 18:38, Alexander Scherbatiy wrote: On 2/14/2014 3:16 PM, Sergey Bylokhov wrote: On 2/14/14 2:32 PM, Alexander Scherbatiy wrote: On 2/14/2014 2:12 AM, Sergey Bylokhov wrote: Hi, Alexander. Did you check option of loading of the picture on demand?Since most of the time x2 version is useless on non hdpi and vice versa. Yes but in this particular case menu items will be painted in one particular scale only. I have created the separate issue on it: 8035069 [macosx] Loading resolution variants by demand https://bugs.openjdk.java.net/browse/JDK-8035069 Thanks, Alexandr. It's not quite true. MacOSX choses a necessary image representation based on the current transformations. Setting current transformation to scale 2x leads that the high resolution image is drawn even on non HiDPI display. There is a similar mechanism for the MultiResolution toolkit images. The base image is drawn in case if the high-resolution image has not been loaded yet. It has an issue that if there is no one more repaint event the image with high resolution is not shown. I would suggest to move this topic to a separate issue. Thanks, Alexandr. On 13.02.2014 18:04, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8031573 webrev: http://cr.openjdk.java.net/~alexsch/8031573/webrev.00 The NSMenu* system icons are templates and do not have image representations. The fix retrieves images with original and double size from an NSImage and put them to a MultiResolution image. The fix also adds sun.awt.image.MultiResolutionBufferedImage class which can be used uniformly for a Multiresolution image creation. The fix is independent of the fix 8033534 Get MultiResolution image from native system http://mail.openjdk.java.net/pipermail/awt-dev/2014-February/006991.html because CImage.createImageFromName(imageName) never returns a MultiResolution image for templates. But the fix 8033534 can be updated to use the MultiResolutionBufferedImage. Thanks, Alexandr. -- Best regards, Sergey.
Re: [9] Review request for 8031573 [macosx] Checkmarks of JCheckBoxMenuItems aren't rendered in high resolution on Retina
On 2/14/2014 3:16 PM, Sergey Bylokhov wrote: On 2/14/14 2:32 PM, Alexander Scherbatiy wrote: On 2/14/2014 2:12 AM, Sergey Bylokhov wrote: Hi, Alexander. Did you check option of loading of the picture on demand?Since most of the time x2 version is useless on non hdpi and vice versa. Yes but in this particular case menu items will be painted in one particular scale only. I have created the separate issue on it: 8035069 [macosx] Loading resolution variants by demand https://bugs.openjdk.java.net/browse/JDK-8035069 Thanks, Alexandr. It's not quite true. MacOSX choses a necessary image representation based on the current transformations. Setting current transformation to scale 2x leads that the high resolution image is drawn even on non HiDPI display. There is a similar mechanism for the MultiResolution toolkit images. The base image is drawn in case if the high-resolution image has not been loaded yet. It has an issue that if there is no one more repaint event the image with high resolution is not shown. I would suggest to move this topic to a separate issue. Thanks, Alexandr. On 13.02.2014 18:04, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8031573 webrev: http://cr.openjdk.java.net/~alexsch/8031573/webrev.00 The NSMenu* system icons are templates and do not have image representations. The fix retrieves images with original and double size from an NSImage and put them to a MultiResolution image. The fix also adds sun.awt.image.MultiResolutionBufferedImage class which can be used uniformly for a Multiresolution image creation. The fix is independent of the fix 8033534 Get MultiResolution image from native system http://mail.openjdk.java.net/pipermail/awt-dev/2014-February/006991.html because CImage.createImageFromName(imageName) never returns a MultiResolution image for templates. But the fix 8033534 can be updated to use the MultiResolutionBufferedImage. Thanks, Alexandr.
Re: [9] Review request for 8031573 [macosx] Checkmarks of JCheckBoxMenuItems aren't rendered in high resolution on Retina
On 2/14/2014 2:12 AM, Sergey Bylokhov wrote: Hi, Alexander. Did you check option of loading of the picture on demand?Since most of the time x2 version is useless on non hdpi and vice versa. It's not quite true. MacOSX choses a necessary image representation based on the current transformations. Setting current transformation to scale 2x leads that the high resolution image is drawn even on non HiDPI display. There is a similar mechanism for the MultiResolution toolkit images. The base image is drawn in case if the high-resolution image has not been loaded yet. It has an issue that if there is no one more repaint event the image with high resolution is not shown. I would suggest to move this topic to a separate issue. Thanks, Alexandr. On 13.02.2014 18:04, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8031573 webrev: http://cr.openjdk.java.net/~alexsch/8031573/webrev.00 The NSMenu* system icons are templates and do not have image representations. The fix retrieves images with original and double size from an NSImage and put them to a MultiResolution image. The fix also adds sun.awt.image.MultiResolutionBufferedImage class which can be used uniformly for a Multiresolution image creation. The fix is independent of the fix 8033534 Get MultiResolution image from native system http://mail.openjdk.java.net/pipermail/awt-dev/2014-February/006991.html because CImage.createImageFromName(imageName) never returns a MultiResolution image for templates. But the fix 8033534 can be updated to use the MultiResolutionBufferedImage. Thanks, Alexandr.
[9] Review request for 8031573 [macosx] Checkmarks of JCheckBoxMenuItems aren't rendered in high resolution on Retina
Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8031573 webrev: http://cr.openjdk.java.net/~alexsch/8031573/webrev.00 The NSMenu* system icons are templates and do not have image representations. The fix retrieves images with original and double size from an NSImage and put them to a MultiResolution image. The fix also adds sun.awt.image.MultiResolutionBufferedImage class which can be used uniformly for a Multiresolution image creation. The fix is independent of the fix 8033534 Get MultiResolution image from native system http://mail.openjdk.java.net/pipermail/awt-dev/2014-February/006991.html because CImage.createImageFromName(imageName) never returns a MultiResolution image for templates. But the fix 8033534 can be updated to use the MultiResolutionBufferedImage. Thanks, Alexandr.
Re: [9] Review request for 8033534 Get MultiResolution image from native system
Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8033534/webrev.01 - The image representations are chosen to be closest to the requested size. Thanks, Alexandr. On 2/4/2014 5:00 PM, Sergey Bylokhov wrote: Hi, Alexander. I think that getResolutionVariant should return an image which is close as much as possible to the requested size. On 04.02.2014 16:42, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8033534 webrev: http://cr.openjdk.java.net/~alexsch/8033534/webrev.00 - The method that gets a sorted array of NSImage representaion pixel sizes for the given image size is added - A MultiResolution image is created if an NSImage has several representations for the given image size Thanks, Alexandr.
[9] Review request for 8033534 Get MultiResolution image from native system
Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8033534 webrev: http://cr.openjdk.java.net/~alexsch/8033534/webrev.00 - The method that gets a sorted array of NSImage representaion pixel sizes for the given image size is added - A MultiResolution image is created if an NSImage has several representations for the given image size Thanks, Alexandr.
Re: Nimbus color differences between JDK versions
Hi Robert, Could you send a sample that draws a JTree and set necessary icons/colors in the Nimbus L&F to investigate the issue? Thanks, Alexandr. On 1/30/2014 12:52 PM, Robert Krüger wrote: Hi, we are in the process of migrating an application from Apple JDK 6 to OpenJDK 8, which is going pretty well thanks to the progress that's been made in recent months by the Oracle team (thanks for that). Now we discovered that for the time being we will have to deploy with a bundled Oracle JRE 7 (currently testing v. 1.7.0_51). When we started our application with that runtime, we encountered significant problems with UI colors (see attached screen shots comparing a tree in the three mentioned runtimes). Now, the color difference between Apple JDK and Open JDK is not the main problem but the one with Oracle JRE 7 certainly is because in Nimbus (which we use with some color modifications), the color changes don't seem to be consistent leading to the effect that you see in the tree example. Before I put lots of "if (jdkVersion == X) modify colors ..." in our code and adjust this by trial and error, is this known and is there a better approach for solving this systematically? Here are the screenshots as links (if they don't make it through as attachments): https://dl.dropboxusercontent.com/u/33388463/colors-jdk6-s.png https://dl.dropboxusercontent.com/u/33388463/colors-jdk7-s.png https://dl.dropboxusercontent.com/u/33388463/colors-jdk8-s.png Thanks, Robert
Re: Drawing HiDPI component to BufferedImage
Hi Hendrik, The API that allows to create custom images for HiDPI displays is only under discussion now. It can be found in the thread: http://mail.openjdk.java.net/pipermail/awt-dev/2014-January/006775.html It also contains a link to the patch that allows SunGraphics2D class to correctly draw multi-resolution images. You can try to apply the patch to the JDK 9 and check that it works for your case. Thanks, Alexandr. On 1/24/2014 12:05 PM, Hendrik Schreiber wrote: Hey, to animate a component, I usually draw it to a BufferedImage and then draw that image to the glass pane, where I can easily move it around without having to completely re-render the component all the time. Unfortunately, this technique does not work well on Retina/HiDPI displays, as I can't seem to be able to convince any components to render in high resolution to a BufferedImage. Is there a way to paint components to BufferedImages in high resolution? I'd like something like: final Image image = component.createImage(component.getWidth(), component.getHeight()); component.paint(image.getGraphics()); // then go on to use the image... to work properly. Thanks, -hendrik
Setting a component orientation in JRSUI
Hello, Below is the code that shows that setting a component orientation for the JProgressBar in Aqua L&F does not work. My assumption was that the Direction should be properly set to the painter state in the AquaProgressBarUI like: - public void paint(final Graphics g, final JComponent c) { ... painter.state.set(c.getComponentOrientation().isLeftToRight() ? Direction.RIGHT : Direction.LEFT); } - but it does not work. The painter calls the JRSUI library to show components on Mac OS X. Is there any specification for the JRSUI library? What is the right way to set the component direction in the JRSUI. Thanks, Alexandr. -- Code Sample -- import java.awt.*; import java.awt.event.*; import javax.swing.*; public class JProgressBarOrientationTest { static boolean leftToRight = true; public static void main(String[] args) throws Exception { SwingUtilities.invokeLater(new Runnable() { @Override public void run() { JFrame frame = new JFrame(); frame.setSize(300, 200); final JProgressBar progressBar = new JProgressBar(0, 100); progressBar.setValue(30); progressBar.setComponentOrientation(ComponentOrientation.LEFT_TO_RIGHT); JButton button = new JButton("Change orientation"); button.addActionListener(new ActionListener() { @Override public void actionPerformed(ActionEvent e) { leftToRight = !leftToRight; progressBar.setComponentOrientation(leftToRight ? ComponentOrientation.LEFT_TO_RIGHT : ComponentOrientation.RIGHT_TO_LEFT); } }); JPanel panel = new JPanel(new BorderLayout()); panel.add(progressBar, BorderLayout.CENTER); panel.add(button, BorderLayout.SOUTH); frame.getContentPane().add(panel); frame.setVisible(true); } }); } } -
Re: [8] Review request for 8028212 Custom cursor HiDPI support
Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8028212/webrev.01/ - CImage.createFromImage() method is updated to handle MultiResolutionImage - resizeRepresentations() method is moved from Creator to the CImage class - nativeResizeImageRepresentations method is renamed to nativeResizeNSImageRepresentations in the CImage class - ImageReprese-M-tations typo is fixed in the CImage native code Thanks, Alexandr. On 12/4/2013 8:24 PM, Anthony Petrov wrote: Thanks for the update, Alexander. I understand. So, my only concerns that remain are: 1. The membership of the resizeImageRepresentations() method. 2. The encapsulation of the logic handling the Image/MultiResolutionImage case (see at the bottom of my original message). -- best regards, Anthony On 12/04/2013 06:59 PM, Alexander Scherbatiy wrote: On 12/3/2013 9:06 PM, Anthony Petrov wrote: Hi Alexander, If we go with this fix, I suggest to move the CImage.Creator.resizeImageRepresentations() to the CImage class and make it a member method, so that you don't need to pass a CImage reference to it as an argument. I will update this. Also, there's the CImage.resize() method already. Why does it not work for us? Having a specification for both methods (or for one, if the second is unneeded) might be helpful. I do not know why, but SetNSImageSize ( [i setScalesWhenResized:TRUE], [i setSize:NSMakeSize(w, h)]) just does not work in this case. The high resolution representation is not chosen on my Mac OS X with enabled Quartz Debug. I tried to narrow the problem and write the cocoa code. It is described in https://bugs.openjdk.java.net/browse/JDK-8028212 Thanks, Alexandr. However, I'm not sure if we really want to resize each representation of an NSImage object to the same size. Why would we want to do that? In fact, one of the representations might already have the correct size, and we could use just that whenever we need it w/o wasting resources on resizing each of them. If there's no representations of suitable size, then we should choose the closest one and resize just it to the desired size. Or am I misunderstanding anything? Also, in CCustomCursor.getImageData(), could we somehow encapsulate a part (or all) of the Image vs. MultiResolutionImage logic in the CImage.Creator class? PS. I'm not really an expert in Image handling code. I'd suggest someone from the 2D team to review this as well. Maybe Jim could help? -- best regards, Anthony On 12/03/2013 08:32 PM, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8028212 webrev: http://cr.openjdk.java.net/~alexsch/8028212/webrev.00 - MultiResolutionImage interface is used from the fix 8011059 - NSImage with representations are created for the multi-resolution cursor - NSImage representations are rescaled to the base cursor size Thanks, Alexandr.
Re: [8] Review request for 8024926 [macosx] AquaIcon HiDPI support
Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8024926/webrev.02/ - the graphics object is disposed in the AquaImageFactory.MultiResolutionIconImage constructor Thanks, Alexandr. On 12/3/2013 8:39 PM, Anthony Petrov wrote: Hi Alexander, The Graphics obtained at line 519 in AquaImageFactory.java is never dispose()'d. Other than that, the fix looks good to me (although I'm not an expert in Aqua L&F or HiDPI support code). -- best regards, Anthony On 12/03/2013 08:11 PM, Alexander Scherbatiy wrote: Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8024926/webrev.01/ - MultiResolutionImage interface is used from the fix 8011059 - Only icons with resolution 1x and 2x are created. The real Mac OS X system icon have more resolutions. The full fix requires retrieving and handling all NSImage representations. It can be addressed for the next release. Thanks, Alexandr. On 10/29/2013 8:47 PM, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8024926 webrev: http://cr.openjdk.java.net/~alexsch/8024926/webrev.00 The fix returns a high resolution system icon in the overridden getScaledInstance(width, height, hints) method. The fix relies on the fix for the issue JDK-8011059 [macosx] Make JDK demos look perfect on retina displays: http://mail.openjdk.java.net/pipermail/awt-dev/2013-October/006133.html - getScaledInstance(width, height, hints) method is used for the image drawing when IMAGE_SCALING hints are enabled - LWCToolkit.ScalableToolkitImage class is public Thanks, Alexandr.
Re: [8] Review request for 8028212 Custom cursor HiDPI support
On 12/3/2013 9:06 PM, Anthony Petrov wrote: Hi Alexander, If we go with this fix, I suggest to move the CImage.Creator.resizeImageRepresentations() to the CImage class and make it a member method, so that you don't need to pass a CImage reference to it as an argument. I will update this. Also, there's the CImage.resize() method already. Why does it not work for us? Having a specification for both methods (or for one, if the second is unneeded) might be helpful. I do not know why, but SetNSImageSize ( [i setScalesWhenResized:TRUE], [i setSize:NSMakeSize(w, h)]) just does not work in this case. The high resolution representation is not chosen on my Mac OS X with enabled Quartz Debug. I tried to narrow the problem and write the cocoa code. It is described in https://bugs.openjdk.java.net/browse/JDK-8028212 Thanks, Alexandr. However, I'm not sure if we really want to resize each representation of an NSImage object to the same size. Why would we want to do that? In fact, one of the representations might already have the correct size, and we could use just that whenever we need it w/o wasting resources on resizing each of them. If there's no representations of suitable size, then we should choose the closest one and resize just it to the desired size. Or am I misunderstanding anything? Also, in CCustomCursor.getImageData(), could we somehow encapsulate a part (or all) of the Image vs. MultiResolutionImage logic in the CImage.Creator class? PS. I'm not really an expert in Image handling code. I'd suggest someone from the 2D team to review this as well. Maybe Jim could help? -- best regards, Anthony On 12/03/2013 08:32 PM, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8028212 webrev: http://cr.openjdk.java.net/~alexsch/8028212/webrev.00 - MultiResolutionImage interface is used from the fix 8011059 - NSImage with representations are created for the multi-resolution cursor - NSImage representations are rescaled to the base cursor size Thanks, Alexandr.
Re: [OpenJDK 2D-Dev] [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays
Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8011059/webrev.14 - Observer and rvObserver are used to get width/hight from image/rvImage in SunGraphics2D - width and height are rounded up in the image observer wrapper - width and height are also rescaled for the SOMEBITS, FRAMEBITS , and ALLBITS infoflags Thanks, Alexandr. On 12/3/2013 9:49 PM, Jim Graham wrote: Hi Alexander, There is one last thing that I think I forgot to mention in SG2D that might make some other comments I made make more sense. There is no observer registered on the resolution variant in SG2D.drawHiDPIImage() in the case where the resolution variant hasn't been loaded yet. Basically, the lines at 3099,3100 will trigger the variant to load, but there is no observer in those calls to trace it back to the guy who needs to call drawImage() again. So, the only thing I think that needs to be done is that the observer needs to be wrapped and handed in to those calls to getWidth/Height(observer). The rest of that method looks fine - the regular variant will be used (and will trigger repaints via the code that calls into drawImage()) until the base image dimensions are known enough to trigger the getResolutionVariant() code, and then we might continue to use the regular version until the resolution variant at least knows its dimensions, and that is all OK, but we need to start using the observer wrapper on the resolution variant starting at lines 3099,3100 in order to get the repaints to keep happening for that version of the image. Arguably, in addition, the unwrapped observer probably could be used on lines 3089, 3090 when you get the dimensions of the base image, but since the base image will later be handed to the drawImage pipeline, the observer will be registered there anyway, so that isn't a bug. But, the wrapped observer needs to be used on 3099,3100 or we may never repaint with the resolution variant (it will be a race condition based on how fast the regular and hiDPI images load). More comments below, but that is the only remaining blocker that I can see... On 12/3/13 3:48 AM, Alexander Scherbatiy wrote: On 12/3/2013 1:16 AM, Jim Graham wrote: On 12/2/13 4:55 AM, Alexander Scherbatiy wrote: On 11/30/2013 3:27 AM, Jim Graham wrote: Hi Alexander, I suppose the wrapping solution works for ImageObservers, but I think the suggestion I gave in recent emails was to simply modify the newInfo method (and a couple of other methods) to deliver the same information with no caches, no hashmaps/lookups, no wrapping the observer, and no wrapping with soft references. Keep in mind that observers are typically references to Component objects so any delay in processing the soft references could keep relatively large component hierarchies in play (if they are parents). It should work well for a first draft, though. It seems that just updating the newInfo method is not enough. There were 5 or 6 places that called imageUpdate when I did a quick search and most of the calls went through newInfo. They'd all have to be updated. Other than that, I'm not sure why it would not be enough? Consider the following scenario. There are image.png and im...@2x.png files on the disk. Image image1 = Toolkit.getImage("image.png"); // load as multi-resolution image Image image2 = Toolkit.getImage("im...@2x.png"); // load the image from cache toolkit.prepareImage(image2,.., imageObserver2); The image2 has image1 as the base image so it rescale its coordinates/dimension and passes the base instead of self to the imageObserver2 which does not look correct. I see your point now. I had thought that they would be cached separately, but I see now that both are inserted into the hash directly. That allows sharing if they access both manually, but it complicates the observer issue. I don't think I would have bothered with sharing the Image instance with a manual reference to the @2x in that case, but we should be able to handle both in a future bug fix and hopefully also get rid of wrappers, but it would take some surgery on the drawImage pipeline and the record keeping in the observer lists. The existing solution will work fine for @2x images and allow sharing so it is good to go for now (modulo the one issue with using the wrapper for the getWidth()/getHeight() I mentioned above). Also, why does ObserverCache exist? Couldn't the cache just be a static field on MRToolkitImage? MRToolkitImage can be used in drawImage(Image,..,ImageObserver) method always with null observer. So the is no need to create the observer cache or use a synchronization during the cache initialization. Maybe I'm missing something about your answer here, but I think you may have misunderstood my question. You placed the field that holds the reference to the cache inside an inner class. I didn&
[8] Review request for 8028212 Custom cursor HiDPI support
Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8028212 webrev: http://cr.openjdk.java.net/~alexsch/8028212/webrev.00 - MultiResolutionImage interface is used from the fix 8011059 - NSImage with representations are created for the multi-resolution cursor - NSImage representations are rescaled to the base cursor size Thanks, Alexandr.
Re: [8] Review request for 8024926 [macosx] AquaIcon HiDPI support
Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8024926/webrev.01/ - MultiResolutionImage interface is used from the fix 8011059 - Only icons with resolution 1x and 2x are created. The real Mac OS X system icon have more resolutions. The full fix requires retrieving and handling all NSImage representations. It can be addressed for the next release. Thanks, Alexandr. On 10/29/2013 8:47 PM, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8024926 webrev: http://cr.openjdk.java.net/~alexsch/8024926/webrev.00 The fix returns a high resolution system icon in the overridden getScaledInstance(width, height, hints) method. The fix relies on the fix for the issue JDK-8011059 [macosx] Make JDK demos look perfect on retina displays: http://mail.openjdk.java.net/pipermail/awt-dev/2013-October/006133.html - getScaledInstance(width, height, hints) method is used for the image drawing when IMAGE_SCALING hints are enabled - LWCToolkit.ScalableToolkitImage class is public Thanks, Alexandr.
Re: [OpenJDK 2D-Dev] [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays
Hello, Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8011059/webrev.13/ - observer cache is returned back - resolution variant x/y/width/height are divided by 2 in the observer wrapper On 12/3/2013 1:16 AM, Jim Graham wrote: Hi Alexander, There must have been a miscommunication. While I think the cache has some issues that worry me, it is absolutely needed if you are going to wrap the observer. You can't just delete it and re-wrap the observer on every call because that will create a number of problems. If you are going to use a wrapper solution then we will need to live with the potential issues of a cache. Note that the underlying ImageReps store lists of observers to be notified. You will be growing that list every time a call is made on an image because each new wrapper looks like a new observer. For an animated GIF the number of drawImage calls and associated wrappers could be infinite (since they are never fully loaded). Also, for each new wrapper, an additional callback is performed. This will drive the performance of an animated GIF into the ground after a time when thousands of growing callbacks are issued for each frame. If you are going to use observer wrappers then you must cache the wrapper so you reuse the same wrapper on every new call. On 12/2/13 4:55 AM, Alexander Scherbatiy wrote: Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8011059/webrev.12/ - Image observers are not cached now for the resolution variants - base image width and height are checked in the wrapped image observer On 11/30/2013 3:27 AM, Jim Graham wrote: Hi Alexander, I suppose the wrapping solution works for ImageObservers, but I think the suggestion I gave in recent emails was to simply modify the newInfo method (and a couple of other methods) to deliver the same information with no caches, no hashmaps/lookups, no wrapping the observer, and no wrapping with soft references. Keep in mind that observers are typically references to Component objects so any delay in processing the soft references could keep relatively large component hierarchies in play (if they are parents). It should work well for a first draft, though. It seems that just updating the newInfo method is not enough. There were 5 or 6 places that called imageUpdate when I did a quick search and most of the calls went through newInfo. They'd all have to be updated. Other than that, I'm not sure why it would not be enough? Consider the following scenario. There are image.png and im...@2x.png files on the disk. Image image1 = Toolkit.getImage("image.png"); // load as multi-resolution image Image image2 = Toolkit.getImage("im...@2x.png"); // load the image from cache toolkit.prepareImage(image2,.., imageObserver2); The image2 has image1 as the base image so it rescale its coordinates/dimension and passes the base instead of self to the imageObserver2 which does not look correct. The resolution variant can have a reference to the base image. Loading the base image it still should compare its result with the resolution variant loading. So the image loading status is just moved from the SunToolkit to the ToolkitImage. If you are referring to comparing the new and old dimensions then the newInfo method would have access to that information if it could ask a resolution variant what its base image is. That should be a very simple change to ToolkitImage (add a set/getBaseImage() method). Then it can do the associated comparisons and parameter adjustments right before calling back to the observer. Perhaps I haven't explained my vision of modifying newInfo very well. I'll look into sketching it out in another email since I think the cache/wrappers are OK for now. I think that the current fix for the checkImage/prepareImage in the SunToolkit is the simplest for the current implementation. The separate fix for the ToolkitImage/ImageRepresentation can get all things right. I didn't really follow you here, but I said above that the cache should be fine for now. The cache is definitely needed if you are wrapping observers as I said above in the first couple of paragraphs. Also, why does ObserverCache exist? Couldn't the cache just be a static field on MRToolkitImage? MRToolkitImage can be used in drawImage(Image,..,ImageObserver) method always with null observer. So the is no need to create the observer cache or use a synchronization during the cache initialization. Maybe I'm missing something about your answer here, but I think you may have misunderstood my question. You placed the field that holds the reference to the cache inside an inner class. I didn't see why the reference could not be stored in the base class. Why is there an empty inner class to wrap a single field? In other words, why w
Re: [OpenJDK 2D-Dev] [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays
Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8011059/webrev.12/ - Image observers are not cached now for the resolution variants - base image width and height are checked in the wrapped image observer On 11/30/2013 3:27 AM, Jim Graham wrote: Hi Alexander, I suppose the wrapping solution works for ImageObservers, but I think the suggestion I gave in recent emails was to simply modify the newInfo method (and a couple of other methods) to deliver the same information with no caches, no hashmaps/lookups, no wrapping the observer, and no wrapping with soft references. Keep in mind that observers are typically references to Component objects so any delay in processing the soft references could keep relatively large component hierarchies in play (if they are parents). It should work well for a first draft, though. It seems that just updating the newInfo method is not enough. The resolution variant can have a reference to the base image. Loading the base image it still should compare its result with the resolution variant loading. So the image loading status is just moved from the SunToolkit to the ToolkitImage. I think that the current fix for the checkImage/prepareImage in the SunToolkit is the simplest for the current implementation. The separate fix for the ToolkitImage/ImageRepresentation can get all things right. Also, why does ObserverCache exist? Couldn't the cache just be a static field on MRToolkitImage? MRToolkitImage can be used in drawImage(Image,..,ImageObserver) method always with null observer. So the is no need to create the observer cache or use a synchronization during the cache initialization. The current way of baking in the image dimensions assumes that they are known. If the image is loaded asynchronously, then the calls to getWidth/Height(null) may return -1 and the cached observer wrapper has no way to get them later. I would think this would fail in the typical default scenario where the user gets an image and the first even that triggers loading it is to render to the screen which would bake the -1's into the observer wrapper in all of those cases. This should probably be addressed sooner rather than later. I added the base image width/height check to the observer wrapper. However, there is no way to rescale image representation width and height in case if the base image width and height are not known. If an @2x image gets an error we silently start using just the regular version of the image. In addition MediaTracker should not reflect that error in its answers, but I think it currently does since you add it as an implicit extra media with the same ID. I think this is OK for the first pass, but we need to track it as an issue. I saw that you fixed the toolkit versions of check/prepare image. Component also has the same operations (via its peers). The Component versions do back off to the toolkit versions, but only if they fail to find an actual peer to delegate to. Note that MediaTracker uses the Component versions so it is affected by this, but I don't think it will cause functional problems that I can think of. Eventually we'd want MT to only load the version that is appropriate for the indicated Component - and at that point then this might be an issue, but I don't think it is an issue for this first draft if it tracks both variations. I checked all peers like W/LW/XComponentPeer and toolkits. They all delegate check/prepareImage calls to the default toolkit. Thanks, Alexandr. I think it is OK to send multiple notifications to an observer since we've always been loose on the exact sequencing and the operations are all asynchronous. There could potentially be several notifications in flight at the time that the observer indicates a lack of interest and there is no way to stop them. This could be considered "another case of that". Eventually we could consider addressing this with a tighter integration between the wrapper mechanism and the code that calls imageUpdate() and receives the answer that the observer is no longer interested, but I think this is all OK for now. The only thing I think we should worry about now for this first draft is the issue of getting the right dimensions for the observer wrappers. They need to be more asynchronous about that. It already has a handle on the original image anyway, so I think it just needs to get the dimensions from there instead of passing them into the constructor, with appropriate checks for -1's... ...jim On 11/28/13 6:45 AM, Alexander Scherbatiy wrote: Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8011059/webrev.11/ I have moved new API supporting to the separate issue: JDK-8029339 Custom MultiResolution image support on HiDPI displays https://bugs.openjdk.java.net/br
Re: [OpenJDK 2D-Dev] [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays
kes is adding "ToolkitImage.set/getBaseImage()" to ToolkitImage, defaulting to "this", but the Multi-Res image will set the base image on both of its variants to the Multi-Res instance. Then we get full notices of all resolution variants. It would be best to scale the xywh supplied there as well. That should be fairly easy, but if it is a big problem then that is lower priority than getting the "ALLBITS" notification right, as that is the main trigger for so many uses. (In the future we can add ImageObserver2 (or MultiResObserver?) which has support for receiving notifications of variants without translation.) ...jim On 11/27/13 6:28 AM, Sergey Bylokhov wrote: On 27.11.2013 15:12, Sergey Bylokhov wrote: Hello. Probably we are in a point when it is necessary to stop and move out all extensions to the separate bugs. We should give a time to our sqe to test these changes. Actually current version looks good to me. Small additional notes, It would be good: - to wrap initial observer in the drawImage and replace img/x,y, in the observer. - don't draw hidpi image, if it was loaded with errors. - sun.com package should be used? - If MediaTracker was requested to load MultiResolutionImage,it should load all versions? On 26.11.2013 2:31, Jim Graham wrote: On 11/25/13 5:51 AM, Alexander Scherbatiy wrote: On 11/23/2013 5:11 AM, Jim Graham wrote: Hi Alexander, If we are going to be advertising it as something that developers use in production code then we would need to file this via CCC. With the current implementation, any user that has their own ImageObservers must use this API and so it becomes not only public, at a time when there is a lockdown on new public API in 8.0, but it also means that we are tied to its implementation and we can't rely on "it was experimental and so we can change it at any point" - you can't say that about an API that is necessary to correctly use a touted feature. This issue has its own history. It has been already fixed for the JDK 7u for the Java2D demo. It was decided to not bring new API for the HiDPI support in the JDK 7. It was suggested to using code like below to create a ScalableImage. -- /** * Class that loads and returns images according to * scale factor of graphic device * * {@code *Image image = new ScalableImage() { * *public Image createScaledImage(float scaleFactor) { *return (scaleFactor <= 1) ? loadImage("image.png") *: loadImage("im...@2x.png"); *} * *Image loadImage(String name){ *// load image *} *};} */ -- It was based on the display scale factor. The scale factor should be manually retrieved from the Graphics2D and was not a part of the public API: g2d.drawImage(scalbleImage.getScaledImage(scaleFactor),..). From that time it was clear that there should be 2 basic operations for the HiDPI images support: - retrieve an image with necessary resolution - retrieve images with all resolutions The second one is useful when it is necessary to create one set of images using the given one (for example, to draw a shape on all images). For now, these are just ToolkitImages and you can't draw on them, so this is moot for the case of @2x support in getImage(). The JDK 7 solution has been revisited for the JDK 8 and the neccesary CCC request 8011059 has been created and approved. Both the JDK-8011059 issue description and the approved CCC request says: --- A user should have a unified way to provide high resolution images that can be drawn on HIDPI displays according to the user provided algorithm. --- We've implemented the Hint part of that solution, but the mechanism for creating custom multi-res images was not workable. I no longer sit as the client rep on the CCC or I would have pointed that out, my apologies. The 8011059 fix consists of two parts: - Provide a way for a custom image creation that holds images with different resolutions - Load @2x images on Mac OS X The first one of the fix can be used without the second. it is not difficult to crate just a class which loads @2x images using the given file path/url. If we support @2x transparently behind the scenes as we are doing now, who is the requester for the way to create a custom set of resolution variants? I think the most important customer-driven request was to support @2x for the Mac developers, wasn't it? Now it is suggested to implement the given MultiResolutionImage interface and override two methods: - Image getResolutionVariant(int width, int height) - List getResolutionVariants() An image with necessary resolution should be drawn automatically in SunGraphics2D
Re: [OpenJDK 2D-Dev] [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays
Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8011059/webrev.10/ - FLIP and TRANSLATE bit masks are also included for the SCALE transform checking - LWCToolkit checks an image in cache before requesting multi-resolution image creation to prevent excessive string manipulation. It needs to make imgCache accessible from the LWCToolkit or move image2x staff calculation to the SunToolkit to use the right synchronization. imgCache is not created per Application context so there can be issues to make it protected. The image2x staff does not relate to SunToolkit. That is why all staff related to ToolkitImage creation used to move into ToolkitImageUtil in some previous fix. I just skipped the synchronization for this particular case. - The containsResolutionVariant method is removed from the MultiResolutionImage - The image observer is nullified in drawImage(Image,..ImageObserver) method for the resolution variant. The original image is always loaded because it needs to know the image width and height for the resolution variant size calculation. Only the original image sends notifications that an image is loaded. The resolution variant is drawn silently so it does not break existed image observer implementation. On 11/23/2013 5:11 AM, Jim Graham wrote: Hi Alexander, If we are going to be advertising it as something that developers use in production code then we would need to file this via CCC. With the current implementation, any user that has their own ImageObservers must use this API and so it becomes not only public, at a time when there is a lockdown on new public API in 8.0, but it also means that we are tied to its implementation and we can't rely on "it was experimental and so we can change it at any point" - you can't say that about an API that is necessary to correctly use a touted feature. This issue has its own history. It has been already fixed for the JDK 7u for the Java2D demo. It was decided to not bring new API for the HiDPI support in the JDK 7. It was suggested to using code like below to create a ScalableImage. -- /** * Class that loads and returns images according to * scale factor of graphic device * * {@code *Image image = new ScalableImage() { * *public Image createScaledImage(float scaleFactor) { *return (scaleFactor <= 1) ? loadImage("image.png") *: loadImage("im...@2x.png"); *} * *Image loadImage(String name){ *// load image *} *};} */ -- It was based on the display scale factor. The scale factor should be manually retrieved from the Graphics2D and was not a part of the public API: g2d.drawImage(scalbleImage.getScaledImage(scaleFactor),..). From that time it was clear that there should be 2 basic operations for the HiDPI images support: - retrieve an image with necessary resolution - retrieve images with all resolutions The second one is useful when it is necessary to create one set of images using the given one (for example, to draw a shape on all images). The JDK 7 solution has been revisited for the JDK 8 and the neccesary CCC request 8011059 has been created and approved. Both the JDK-8011059 issue description and the approved CCC request says: --- A user should have a unified way to provide high resolution images that can be drawn on HIDPI displays according to the user provided algorithm. --- The 8011059 fix consists of two parts: - Provide a way for a custom image creation that holds images with different resolutions - Load @2x images on Mac OS X The first one of the fix can be used without the second. it is not difficult to crate just a class which loads @2x images using the given file path/url. Now it is suggested to implement the given MultiResolutionImage interface and override two methods: - Image getResolutionVariant(int width, int height) - List getResolutionVariants() An image with necessary resolution should be drawn automatically in SunGraphics2D. What we need is to focus on the first part of the fix. From this point of view it up to the user' code to load all or some resolution variants by MediaTracker and using Component.prepareImage/checkImage methods. Thanks, Alexandr. I do not see any compatibility risks with the current fix. If a user does not provide @2x images or explicitly overrides the MultiResolutionImage nothing should be changed in his code. If a developer uses a stock Java module in their app and they hand it some @2x-enabled images then that code could fail. If a developer creates an app and then later a graphic artist replaces its media with a new set of media that includes @2x images then the app could fail. If an applet is deployed that uses stock imagery from its own we
Re: [OpenJDK 2D-Dev] [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays
On 11/21/2013 8:13 PM, Jim Graham wrote: Hi Alexander, I just noticed that the new interface was created in com.sun. I also note that you discuss a number of issues below that relate to a developer creating one of their own Multi-Resolution images. We should not be exporting an interface at this point for a developer to do any of that. Any use of these interfaces beyond our own internal use to support @2x images will be unsupported in JDK8 as we do not have time to properly test and expose an interface with the remaining time in the JDK8 release schedule. This interface should be moved to the internal sun.* hierarchy until we have time to vet it. I see that NSImage on Mac OS X has API to work with representations and has methods like addRepresentation/removeRepresentation/bestRepresentationForRect. Does it mean that it is possible to use not only @2x images in an application but create NSImage with different images resolutions as well? Should we support the same thing in Java? For example, a user wants to provide images with resolutions 0.5, 1, 2, and 3. According to the current transform the best resolution variant should be chosen. The MultiResolutionImage is in com.sun* package and it still means that it can be changed or removed. It can require the CCC request, though. Putting it to sun.com prevents using this feature in applets. I do not see any compatibility risks with the current fix. If a user does not provide @2x images or explicitly overrides the MultiResolutionImage nothing should be changed in his code. The time testing of this API is the same as testing TollkitImages that can hold @2x images. We also will have the feedback about these API earlier. It is better than introducing a public API in next release that will be difficult to change. All of the places that call containsResolutionVariant() are pointing out a bug with this implementation. The resolution variants are internal implementation details and should never be leaked through any current interfaces. It looks like most of the cases involve imageUpdate() methods that should be receiving a reference to the original image, not the resolution variant. These pieces of could should not be changing and the fact that you had to change them points out that you've created a huge compatibility issue that blocks this solution. If someone uses the API with multi-resolution images he needs also update his code according to this image usage. It should be up to a user to preload all resolutions or only part of them using the MediaTracker. The only place where an image is replaced to a resolution variant and the image observer is invoked is the drawImage(Image,..,ImageObserver). There are 3 solutions how it can be handled: - Preload an image with all resolution variants. The image observer is not invoked in this case. - Using the original image in the image observer for the resolution variant (x,y,w,h should be rescaled). - Using the resolution variant in the observer The first one is not good solution for the ToolkitImage because it is designed to load asynchronously. The second one still can be surprising for a user because he has notification that the original image has been already preloaded. Note that a user can do something with this image so his actions will be based on the image with another resolution. The third one provides the actual information to the user. However, it requires that the user update his code in the image observer. There are no compatibility problems. If multi-resolution images are not used nothing should be changed. If they used, image observers should be updated accordingly. Thanks, Alexandr. On 11/21/2013 6:15 AM, Alexander Scherbatiy wrote: To check if it is "identity-ish", I'd use: ((type & ~(TYPE_TRANSLATION | TYPE_FLIP)) == 0) To check for scale, use: ((type & ~(TYPE_TRANSLATION | TYPE_FLIP | TYPE_SCALE_MASK)) == 0) and then the distance/hypot equations are in the final else as they are now. Updated. The test at line 3152 is sub-optimal. If the transform includes both a SCALE and a TRANSLATE then this optimization will fail to trigger and the code will fall through to the default case (which produces the correct output, but misses the optimization). Note above that I include the FLIP and TRANSLATE flags in with the SCALE mask to take care of this in my pseudo-code above. - Some related to the ToolkitImage staff is moved from SunToolkit to ToolkitImageUtil I'm not sure why this was done - it added a bunch of busy-work for approving the webrev that I don't have time for. Hopefully someone else can verify the cut/paste and refactoring. I have reverted it back. Thanks, it is much easier to verify what was modified now. I'm still not sure why it isn't just left as DEFAULT on all p
Re: [OpenJDK 2D-Dev] [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays
On 11/20/2013 3:17 PM, Jim Graham wrote: In looking through the ToolkitImage code some more it occurs to me that it was already designed (moreso in a previous life) to hold onto multiple representations of the image anyway. In a prior life in 1.0 and 1.1 it held separate ImageRepresentation objects for each size it was scaled to (via drawImage(w,h)), but that was simplified to a single ImageRep when we converted to Java2D and started doing scaling on the fly for all drawImage() operations. Still, the concept of an alternate resolution version of the image is probably more in line with the ImageRepresentation object within the ToolkitImage than in having multiple ToolkitImage objects and a wrapper for them. This would also deal more naturally with the ImageObserver issue since there really would be only one Image and SG2D would not deal with the sub-representations, it would be dealt with in the DrawImage pipeline code when it queries the ToolkitImage for the ImageRepresentation object. In the end, I think that design would be simpler overall, would it be possible to shift those gears for this fix? If not, we should consider it for a near-term future cleanup task perhaps... I think that it would be better to postpone this cleanup to a near-term future task. Thanks, Alexandr. ...jim On 11/19/2013 7:58 PM, Jim Graham wrote: Based on the information below, I have the following suggestions... We should probably allow asynchronous loading of the scaled resolution variants. This is a minor variation of what the ImageObserver was originally designed for, but technically might violate some developer's expectations if they have only dealt with a post-Java2D version of Java. Some education might be helpful here. The wording on drawImage(dxy12, sxy12) should probably be reworded to indicate that asynchronous scaling would not happen, but alternate versions of the image may be accessed. In all cases, if the version of the image that we would ideally want to show hasn't been loaded, but the standard version has (or if @2x was loaded, but we want the regular version too?) then we should probably go with the version that was loaded, but still trigger the loading of the alternate version and notify their Observer as it is loaded. I also examined the places in the code where we notify the ImageObserver. A search for the observer method should show all places we call it, but the primary ones look like they are fairly few places. If we tag the resolution variant images with the composite image from which they came, then we can have those few places do something like: Image obsimg = (img instanceof SunToolkitImage) ? (() img).getObservedImage() : img; I think in most cases the img is already known to be our internal SunToolkit image and so we don't even need to check instanceof... ...jim On 11/19/2013 7:01 PM, Jim Graham wrote: I did some more reading about the various ways in which the ImageObserver is used and noticed the following points, some of which may be to our advantage. Many of the drawImage() calls mention that they will notify the observer as the image is "scaled or converted for the output device". I believe in the world before 2D, we would often have to load or convert the image asynchronously for various changes in the output. When we created the 2D interface and added some on-the-fly image scaling code, we mentioned that scaling was no longer asynchronous, but technically the API is still designed for asynchronous operations when the rendering parameters change. However, the drawImage() call that takes 8 parameters dxy12, sxy12 - specifically includes the words: * This method always uses the unscaled version of the image * to render the scaled rectangle and performs the required * scaling on the fly. It does not use a cached, scaled version * of the image for this operation. Scaling of the image from source * to destination is performed such that the first coordinate * of the source rectangle is mapped to the first coordinate of * the destination rectangle, and the second source coordinate is * mapped to the second destination coordinate. The subimage is * scaled and flipped as needed to preserve those mappings. public abstract boolean drawImage(Image img, int dx1, int dy1, int dx2, int dy2, int sx1, int sy1, int sx2, int sy2, ImageObserver observer); which basically, at the time it was created in 1.1, was an opt "out" of the asynchronous operations that might happen when you scaled an image with drawImage(xywh). As worded, this would suggest that this method does not use the @2x version and always uses the default resolution version, but those words were not put there for this particular intent, they were p
Re: [OpenJDK 2D-Dev] [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays
Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8011059/webrev.09/ On 11/20/2013 3:55 AM, Jim Graham wrote: Hi Alexander: SunHints: ON - Always use resolution-specific variants of images OFF - Use only the standard resolution of an image DEFAULT - Choose image resolutions based on a default heuristic (perhaps add "(when available)" to ON?) Note that I think you are still using the descriptions I asked for in the first quotation which I then revised because they were confusing. Please use the latter descriptions (ON, OFF, DEFAULT above). Updated. SG2D: - If you rewrite getTransformedDestImageSize() to return the resolution variant instead of a Dimension then you avoid having to create a Dimension object that gets immediately tested for null again and/or consumed by the caller. Just move the call to getResolutionVariant() inside the helper method and rename it. Updated. - In getTransformedDestImageSize - dx,dy,sx,sy - should be named dw,dh,sw,sh? Updated. - Also in getTransformedDestImageSize - the array of Point objects is problematic because they are used for the return values and that means early truncation/rounding, the methods that take arrays of doubles are generally less problematic in terms of garbage generation. Also, you can use deltaTransform() because you don't need to worry about translation components since you only want the distance. Finally, why not use the optimization I gave to just grab the matrix elements directly and do the calculations on them instead of 3 separate point transforms which have predictable outcomes? Here is the recommendation again: And even for the case of GENERAL transform, the above can be simplified. The tx.deltaTransform method in that case simply loads the array with: { m00, m10, m01, m11 } and so the hypot methods are simply: xScale = Math.hypot(tx.getScaleX(), tx.getShearY()); yScale = Math.hypot(tx.getShearX(), tx.getScaleY()); Multiply those values with dw,dh and you have your answer. Another way to look at this is that your 3 transformed points were basically: { dx1 * m00 + dy1 * m01 + tx, dx1 * m10 + dy1 * m11 + ty } { dx2 * m00 + dy1 * m01 + tx, dx2 * m10 + dy1 * m11 + ty } { dx1 * m00 + dy2 * m01 + tx, dx1 * m10 + dy2 * m11 + ty } Subtracting [1] - [0] for the distance calculation eliminates a lot of terms and gives you: Math.hypot((dx2 - dx1) * m00, (dx2 - dx1) * m10) == (dx2 - dx1) * Math.hypot(m00, m10); == (dx2 - dx1) * Math.hypot(tx.getScaleX(), tx.getShearY()); Subtracting [2] - [0] also reduces: Math.hypot((dy2 - dy1) * m01, (dy2 - dy1) * m11); == (dy2 - dy1) * Math.hypot(m01, m11); == (dy2 - dy1) * Math.hypot(tx.getShearX(), tx.getScaleY()); - Also in getTransformeDestImageSize - the usage of the transform.getType() is off, some of the values are bitmasks so a <= comparison isn't the right comparison to use. I'd use the following structure: Updated. To check if it is "identity-ish", I'd use: ((type & ~(TYPE_TRANSLATION | TYPE_FLIP)) == 0) To check for scale, use: ((type & ~(TYPE_TRANSLATION | TYPE_FLIP | TYPE_SCALE_MASK)) == 0) and then the distance/hypot equations are in the final else as they are now. Updated. - In calculating scaledWidth/width and the cast to float - even though I think the precedence order is on your side, I usually add extra parentheses to highlight the order so that people reading the code and/or modifying it don't mess it up. My cutoff on what is obvious to a casual reader is usually "additive vs. multiplicative" and anything more subtle than that I use parentheses just in case someone comes along who is less familiar with the order and decides to interpose some additional operations that get in the way of the proper calculation. I would have written "((float) sW) / w" Updated. just to make it obvious. In LWCToolkit.java: - there is a lot of string manipulation going on for every image fetch. I guess this would only hurt those who use the Toolkit as their image storage (getting the image every time they need it), so I'm not going to worry about it, but it would be good to eventually get this integrated into the caching mechanism better so that a quick lookup of a previously loaded image could return faster. One thing I did not examine very closely was the transfer of hash lookup and security code to the new utility class. You should get a double-check from another engineer on that code. Some more inline comments: On 11/19/2013 5:06 AM, Alexander Scherbatiy wrote: Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8011059/webrev.08/ - Some related to the ToolkitImage staff is moved from SunToolkit to ToolkitImageUtil I'm not sure why this was done - it added a bunch of busy-work for approving the webrev that I
Re: [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays
ultiResolutionImage for that. Thanks, Alexandr. ...jim On 11/13/13 8:11 AM, Alexander Scherbatiy wrote: Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8011059/webrev.07/ - The default sun hint is added. However, it looks the same as the resolution variant ON hint right now. It would better to leave the behavior on the non HiDPI displays the same as it was before. So the resolution variant hint is disabled by default for non HiDPI displays. - Resolution variant hints description is updated. - The logic operator in the isHiDPIImage() method is formatted. The @2x images are not preloaded in the LWCToolkit. It really can cause image reloading after moving a window from a display from one resolution to another. However, it is not clear during the MultiResolutionImage creation will the images be used on HiDPI displays or not. May be there should be a flag that enables the high resolution images preloading. The original image can be replaced by the high resolution one in the paint() method. It causes that the observer could get an image which is different from the original one. May be there is no any issue? If a MultiResolutionImage is not used then all works as before. If a user implements MultiResolutionImage may be he needs to have an information about the actual drawn image in the observer even it is different from the original. Thanks, Alexandr. On 11/12/2013 11:43 PM, Jim Graham wrote: Hi Alexander, Some minor issues with this fix: - All RenderingHints have a default setting where the system can choose for you. The primary thing that the default settings allow is for the answer to be based off of another hint. Often the QUALITY hint provides the swing vote if an individual hint is left "DEFAULT". That should probably also be the setting used for SG2D, and would hinge off of the devScale, for example, as the deciding factor. - Descriptions for "on" and "off" should be something like "Use resolution variants of images" and "Use only default resolution variants of images" (and "Default resolution variant usage"). Most of the other descriptions on hints are statements of what is going to happen or what is going to be used rather than a simple 'the X state is Y'. - It looks like the transform is used in SG2D to decide if the hiDPI image should be used. I'm not familiar with the Mac's native use of @2x, but I thought that they hinged the decision off of the "retina scale" of the display, not off of the current transform. That way, if you scale down an icon it doesn't change its look when it reaches .5x scale (or .75x depending on the cutoff). Personally, I think it is better to not use the transform to keep animations smooth, but I'm not sure what Apple did. - The logic in using the transform is also a bit murky. I think if you set the scale on a retina display to exactly 1/2 it would use the HiDPI version even though the scale was 1:1. Since I support not examining the transform there, I'm going to present this as another reason why we should just base it on devScale, but the logic could be fixed if we really plan to use the transform here. - The new logic in "isHiDPIImage()" is confusing because you line up logic operations from 2 different levels of parentheses. I believe that one version of our style guidelines included a rule that allowed "indenting to parentheses level" and I would think that would be a good rule to apply here. Or do something else to make the logic flow there less tricky to read. - Eventually we are going to need this support in more pipelines. I believe that Win8 already has parameters that affect choices of images, but they are only currently deployed on the Surface tablets (i.e. there are no supported high DPI displays for desktop that I'm aware of, but some of the Surface tablets ship with high DPI screens). What would the impact be if we moved this into a more general class in src/share? I suppose we might spend extra time looking for variants of images that we don't need? - Has any thought been given to the issues that someone raised with cursors? - Has any thought been given to my comments about MediaTracker and image observer states for multi-res images? I don't see any attempt here to preload the @2x image. The problem would probably only be seen on multi-headed systems where one display is retina and one is not - you would find the images suddenly reloading when you move the window to the other screen and the application might not expect that to happen. Which image is the Observer registered on? Since the image is handed to the Observer, will an application be surprised when their observer gets a handle to an image they've never seen? Is it an issue if one of the "alternate resolution variant
Re: [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays
Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8011059/webrev.07/ - The default sun hint is added. However, it looks the same as the resolution variant ON hint right now. It would better to leave the behavior on the non HiDPI displays the same as it was before. So the resolution variant hint is disabled by default for non HiDPI displays. - Resolution variant hints description is updated. - The logic operator in the isHiDPIImage() method is formatted. The @2x images are not preloaded in the LWCToolkit. It really can cause image reloading after moving a window from a display from one resolution to another. However, it is not clear during the MultiResolutionImage creation will the images be used on HiDPI displays or not. May be there should be a flag that enables the high resolution images preloading. The original image can be replaced by the high resolution one in the paint() method. It causes that the observer could get an image which is different from the original one. May be there is no any issue? If a MultiResolutionImage is not used then all works as before. If a user implements MultiResolutionImage may be he needs to have an information about the actual drawn image in the observer even it is different from the original. Thanks, Alexandr. On 11/12/2013 11:43 PM, Jim Graham wrote: Hi Alexander, Some minor issues with this fix: - All RenderingHints have a default setting where the system can choose for you. The primary thing that the default settings allow is for the answer to be based off of another hint. Often the QUALITY hint provides the swing vote if an individual hint is left "DEFAULT". That should probably also be the setting used for SG2D, and would hinge off of the devScale, for example, as the deciding factor. - Descriptions for "on" and "off" should be something like "Use resolution variants of images" and "Use only default resolution variants of images" (and "Default resolution variant usage"). Most of the other descriptions on hints are statements of what is going to happen or what is going to be used rather than a simple 'the X state is Y'. - It looks like the transform is used in SG2D to decide if the hiDPI image should be used. I'm not familiar with the Mac's native use of @2x, but I thought that they hinged the decision off of the "retina scale" of the display, not off of the current transform. That way, if you scale down an icon it doesn't change its look when it reaches .5x scale (or .75x depending on the cutoff). Personally, I think it is better to not use the transform to keep animations smooth, but I'm not sure what Apple did. - The logic in using the transform is also a bit murky. I think if you set the scale on a retina display to exactly 1/2 it would use the HiDPI version even though the scale was 1:1. Since I support not examining the transform there, I'm going to present this as another reason why we should just base it on devScale, but the logic could be fixed if we really plan to use the transform here. - The new logic in "isHiDPIImage()" is confusing because you line up logic operations from 2 different levels of parentheses. I believe that one version of our style guidelines included a rule that allowed "indenting to parentheses level" and I would think that would be a good rule to apply here. Or do something else to make the logic flow there less tricky to read. - Eventually we are going to need this support in more pipelines. I believe that Win8 already has parameters that affect choices of images, but they are only currently deployed on the Surface tablets (i.e. there are no supported high DPI displays for desktop that I'm aware of, but some of the Surface tablets ship with high DPI screens). What would the impact be if we moved this into a more general class in src/share? I suppose we might spend extra time looking for variants of images that we don't need? - Has any thought been given to the issues that someone raised with cursors? - Has any thought been given to my comments about MediaTracker and image observer states for multi-res images? I don't see any attempt here to preload the @2x image. The problem would probably only be seen on multi-headed systems where one display is retina and one is not - you would find the images suddenly reloading when you move the window to the other screen and the application might not expect that to happen. Which image is the Observer registered on? Since the image is handed to the Observer, will an application be surprised when their observer gets a handle to an image they've never seen? Is it an issue if one of the "alternate resolution variant" images leaks into an application's "hands" via the
Re: [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays
On 11/7/2013 8:23 PM, Eirik Bakke wrote: Just passing by... Will retina-enabled Image objects work with Toolkit.createCustomCursor [1] under this patch? This patch allows to create an image with different resolutions that can be used for the custom cursor as well. To make it work with custom cursor it needs to properly pass these images to the native system. I have created an issue to track this: JDK-8028212 Custom cursor HiDPI support https://bugs.openjdk.java.net/browse/JDK-8028212 Thanks, Alexandr. (I have a spreadsheet application that requires an Excel-style "plus" cursor when the user hovers over it, and I'd like to supply a retina-enabled cursor image as well.) -- Eirik [1] http://docs.oracle.com/javase/6/docs/api/java/awt/Toolkit.html#createCustom Cursor(java.awt.Image, java.awt.Point, java.lang.String) On 11/5/13, 6:16 AM, "Alexander Scherbatiy" wrote: Thank you for the review. Could you look at the updated fix: http://cr.openjdk.java.net/~alexsch/8011059/webrev.05/ - URL is parsed to protocol, host, port, and path parts in the LWCToolkit class. I checked that URL(protocol, host, port, file) constructor correctly handles -1 port value. - Only last file name after last '/' in the URL path is converted to @2x name - Tests that check correct URL and path translation to @2x names are added to the ScalableImageTest Thanks, Alexandr. On 11/1/2013 12:46 AM, Peter Levart wrote: On 10/29/2013 05:45 PM, Alexander Scherbatiy wrote: 2. I'm not sure that the proposed getScaledImageName() implementation in ScalableToolkitImage works perfectly for URLs like this: http://www.exampmle.com/dir/image In this case it will try to find 2x image here: http://www.exam...@2x.com/dir/image which doesn't look correct. Fixed. Only path part of a URL is converted to path2x. Hi Alexander, URLs like this: http://www.example.com/dir.ext/image will still be translated to: http://www.example.com/d...@2x.ext/image I think you need to search for last '.' after the last '/' in the getScaledImageName(); Also the following code has some additional bugs: 853 static Image toScalableImage(Image image, URL url) { 854 855 if (url != null && !url.toString().contains("@2x") 856 && !(image instanceof ScalableToolkitImage)) { 857 String protocol = url.getProtocol(); 858 String host = url.getHost(); 859 String file = url.getPath(); 860 String file2x =*host +*getScaledImageName(file); 861 try { 862 URL url2x = new URL(protocol, host, file2x); 863 url2x.openStream(); 864 return new ScalableToolkitImage(image, getDefaultToolkit().getImage(url2x)); 865 } catch (Exception ignore) { 866 } 867 } 868 return image; 869 } Why are you prepending *host* to getScaledImageName(file) in line 860? Let's take the following URL for example: http://www.example.com/dir/image.jpg protocol = "http" host = "www.example.com" file = "/dir/image.jpg" file2x = "*www.example.com*/dir/im...@2x.jpg" url2x = URL("http://www.example.com*www.example.com*/dir/im...@2x.jpg";) You are missing a part in URL (de)construction - the optional port! For example in the following URL: http://www.example.com:8080/dir/image.jpg You should extract the port from original URL and use it in new URL construction if present (!= -1). I would also close the stream explicitly after probing for existence of resource rather than delegating to GC which might not be promptly and can cause resource exhaustion (think of MAX. # of open file descriptors): try (InputStream probe = url.openStream()) {} Regards, Peter
[8] Review request for 8025126 [macosx] Invalid calls to setValueAt() within JTable in Java 7 on Mac OS X
Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8025126 webrev: http://cr.openjdk.java.net/~alexsch/8025126/webrev.00 The extended key code is set to the key code if the key char is undefined. Thanks, Alexandr.
Re: [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays
Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8011059/webrev.06/ Only internal API is exposed: - MultiResolutionImage interface with method "getResolutionVariant(int width, int height)" is added to the com.sun.awt package - Hints to switch on/off the resolution variant usage are added to SunHints class - Test is updated to use the MultiResolutionImage interface Thanks, Alexandr. On 11/5/2013 3:16 PM, Alexander Scherbatiy wrote: Thank you for the review. Could you look at the updated fix: http://cr.openjdk.java.net/~alexsch/8011059/webrev.05/ - URL is parsed to protocol, host, port, and path parts in the LWCToolkit class. I checked that URL(protocol, host, port, file) constructor correctly handles -1 port value. - Only last file name after last '/' in the URL path is converted to @2x name - Tests that check correct URL and path translation to @2x names are added to the ScalableImageTest Thanks, Alexandr. On 11/1/2013 12:46 AM, Peter Levart wrote: On 10/29/2013 05:45 PM, Alexander Scherbatiy wrote: 2. I'm not sure that the proposed getScaledImageName() implementation in ScalableToolkitImage works perfectly for URLs like this: http://www.exampmle.com/dir/image In this case it will try to find 2x image here: http://www.exam...@2x.com/dir/image which doesn't look correct. Fixed. Only path part of a URL is converted to path2x. Hi Alexander, URLs like this: http://www.example.com/dir.ext/image will still be translated to: http://www.example.com/d...@2x.ext/image I think you need to search for last '.' after the last '/' in the getScaledImageName(); Also the following code has some additional bugs: 853 static Image toScalableImage(Image image, URL url) { 854 855 if (url != null && !url.toString().contains("@2x") 856 && !(image instanceof ScalableToolkitImage)) { 857 String protocol = url.getProtocol(); 858 String host = url.getHost(); 859 String file = url.getPath(); 860 String file2x =*host +*getScaledImageName(file); 861 try { 862 URL url2x = new URL(protocol, host, file2x); 863 url2x.openStream(); 864 return new ScalableToolkitImage(image, getDefaultToolkit().getImage(url2x)); 865 } catch (Exception ignore) { 866 } 867 } 868 return image; 869 } Why are you prepending *host* to getScaledImageName(file) in line 860? Let's take the following URL for example: http://www.example.com/dir/image.jpg protocol = "http" host = "www.example.com" file = "/dir/image.jpg" file2x = "*www.example.com*/dir/im...@2x.jpg" url2x = URL("http://www.example.com*www.example.com*/dir/im...@2x.jpg";) You are missing a part in URL (de)construction - the optional port! For example in the following URL: http://www.example.com:8080/dir/image.jpg You should extract the port from original URL and use it in new URL construction if present (!= -1). I would also close the stream explicitly after probing for existence of resource rather than delegating to GC which might not be promptly and can cause resource exhaustion (think of MAX. # of open file descriptors): try (InputStream probe = url.openStream()) {} Regards, Peter
Re: [OpenJDK 2D-Dev] [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays
On 11/6/2013 5:39 AM, Jim Graham wrote: Why is getScaledInstance() being consulted here? It seems a misuse of that method. We need to introduce a new API that allows developers to return scaled images for HiDPI displays. Scaled images are just new images N times bigger than the original one and created by designers. The good way is to allow a user to override a method like getScaledImage() which has either scaledFactor or width and height parameters and returns necessary scaled images. It could look like: - final Image image1 = ImageIO.read("image1.png"); final Image image2 = ImageIO.read("image2.png"); final Image image3 = ImageIO.read("image3.png"); final int WIDTH = image1.getWidth(null); final int HEIGHT = image1.getHeight(null); Image image = new BufferedImage(WIDTH, HEIGHT, BufferedImage.TYPE_INT_RGB) { @Override public Image getScaledImage(int width, int height) { if (width < WIDTH && height < HEIGHT) { return this; } else if (width < 2 * WIDTH && height < 2 * HEIGHT) { return image2; } else if (width < 3 * WIDTH && height < 3 * HEIGHT) { return image3; } } }; image.getGraphics().drawImage(image1, 0, 0, null); - But the getScaledImage(width, height) method looks similar to the getScaledInstance(width, height, hints). It also can confuse a user which method should be overridden to return the scaled version of an image. The current fix is based on the CCC request 8011059 where it has been approved to use the getScaledInstance(width, height, hints) method for this purposes. The method was designed to return a rescaled version of the same pixels that you would get if you examined the raw pixels. You are overriding it to return a different image. That does not fit in with the original intent of that method as I created it. It is also causing implementation headaches (read reflection) in the SG2D code to try to use it that way. The Image.SCALE_DEFAULT hint is used to retrieve user scaled images in the CCC request 8011059. That was a wrong idea. There are intersections with the original meaning with the SCALE_DEFAULT hint. Also there are performance regression and exceptions like OOM or IOBE if the getScaledInstance(width, height, hints) is not overridden and it is invoked for each image. The better way could be to use a new Image hint designed only to retrieve scaled images provided by a user. I have created the CCC request 8027655 for this. Once the request will be approved it will be possible to get rid of the reflection usage. Thanks, Alexandr. The @2x mechanism should be based on different API. I guess it would have to be internal-only for 8.0 and could be exposed to allow developers to call it and possibly to be a provider for it in JDK9... ...jim On 10/31/13 9:19 AM, Alexander Scherbatiy wrote: Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8011059/webrev.04/ The reflection is used to skip the Image.getScaledInstance() method call if it is not overridden by a user. On 10/29/2013 11:08 PM, Sergey Bylokhov wrote: Hi, Alexander. The fix looks fine to me in general. But there is at least one issue. I build you fix and test it: - Consuming of cpu increased by 500 times Java2Demo on images tab. - FPS is dropped from 220(jdk8)/35(jdk7u40) to 15 in guimark2. Note that jdk6 has the same FPS(15) on my system. The main problem is that the Image.SCALE_DEFAULT hint is used to retrieve a scaled image from Image.getScaledInstance() method. It always uses the ReplicateScaleFilter for images which getScaledInstance() method has not been overridden. The ReplicateScaleFilter creates a lot of arrays and consumes the CPU during the image parsing. The better fix would be to introduce the new Image.SCALE_CUSTOM hint which could be used to get a scaled image and does not use filters by default. But it should be a separated bug with a new CCC request. Thanks, Alexandr. On 29.10.2013 20:45, Alexander Scherbatiy wrote: Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8011059/webrev.03 On 10/28/2013 2:33 PM, Artem Ananiev wrote: Hi, Alexander, a few comments: 1. SunGraphics2D.java:3076 - should isHiDPIImage() be used here? The isHiDPIImage() method is used to check that the drawHiDPIImage should be called like: if (isHiDPIImage(img)) { return drawHiDPIImage(...); } 2. I'm not sure that the proposed getScaledImageName() implementation in ScalableToolkitImage works perfectly for URLs like this: http://www.exampmle.com/dir/image In this
Re: [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays
Thank you for the review. Could you look at the updated fix: http://cr.openjdk.java.net/~alexsch/8011059/webrev.05/ - URL is parsed to protocol, host, port, and path parts in the LWCToolkit class. I checked that URL(protocol, host, port, file) constructor correctly handles -1 port value. - Only last file name after last '/' in the URL path is converted to @2x name - Tests that check correct URL and path translation to @2x names are added to the ScalableImageTest Thanks, Alexandr. On 11/1/2013 12:46 AM, Peter Levart wrote: On 10/29/2013 05:45 PM, Alexander Scherbatiy wrote: 2. I'm not sure that the proposed getScaledImageName() implementation in ScalableToolkitImage works perfectly for URLs like this: http://www.exampmle.com/dir/image In this case it will try to find 2x image here: http://www.exam...@2x.com/dir/image which doesn't look correct. Fixed. Only path part of a URL is converted to path2x. Hi Alexander, URLs like this: http://www.example.com/dir.ext/image will still be translated to: http://www.example.com/d...@2x.ext/image I think you need to search for last '.' after the last '/' in the getScaledImageName(); Also the following code has some additional bugs: 853 static Image toScalableImage(Image image, URL url) { 854 855 if (url != null && !url.toString().contains("@2x") 856 && !(image instanceof ScalableToolkitImage)) { 857 String protocol = url.getProtocol(); 858 String host = url.getHost(); 859 String file = url.getPath(); 860 String file2x =*host +*getScaledImageName(file); 861 try { 862 URL url2x = new URL(protocol, host, file2x); 863 url2x.openStream(); 864 return new ScalableToolkitImage(image, getDefaultToolkit().getImage(url2x)); 865 } catch (Exception ignore) { 866 } 867 } 868 return image; 869 } Why are you prepending *host* to getScaledImageName(file) in line 860? Let's take the following URL for example: http://www.example.com/dir/image.jpg protocol = "http" host = "www.example.com" file = "/dir/image.jpg" file2x = "*www.example.com*/dir/im...@2x.jpg" url2x = URL("http://www.example.com*www.example.com*/dir/im...@2x.jpg";) You are missing a part in URL (de)construction - the optional port! For example in the following URL: http://www.example.com:8080/dir/image.jpg You should extract the port from original URL and use it in new URL construction if present (!= -1). I would also close the stream explicitly after probing for existence of resource rather than delegating to GC which might not be promptly and can cause resource exhaustion (think of MAX. # of open file descriptors): try (InputStream probe = url.openStream()) {} Regards, Peter
Re: [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays
Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8011059/webrev.04/ The reflection is used to skip the Image.getScaledInstance() method call if it is not overridden by a user. On 10/29/2013 11:08 PM, Sergey Bylokhov wrote: Hi, Alexander. The fix looks fine to me in general. But there is at least one issue. I build you fix and test it: - Consuming of cpu increased by 500 times Java2Demo on images tab. - FPS is dropped from 220(jdk8)/35(jdk7u40) to 15 in guimark2. Note that jdk6 has the same FPS(15) on my system. The main problem is that the Image.SCALE_DEFAULT hint is used to retrieve a scaled image from Image.getScaledInstance() method. It always uses the ReplicateScaleFilter for images which getScaledInstance() method has not been overridden. The ReplicateScaleFilter creates a lot of arrays and consumes the CPU during the image parsing. The better fix would be to introduce the new Image.SCALE_CUSTOM hint which could be used to get a scaled image and does not use filters by default. But it should be a separated bug with a new CCC request. Thanks, Alexandr. On 29.10.2013 20:45, Alexander Scherbatiy wrote: Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8011059/webrev.03 On 10/28/2013 2:33 PM, Artem Ananiev wrote: Hi, Alexander, a few comments: 1. SunGraphics2D.java:3076 - should isHiDPIImage() be used here? The isHiDPIImage() method is used to check that the drawHiDPIImage should be called like: if (isHiDPIImage(img)) { return drawHiDPIImage(...); } 2. I'm not sure that the proposed getScaledImageName() implementation in ScalableToolkitImage works perfectly for URLs like this: http://www.exampmle.com/dir/image In this case it will try to find 2x image here: http://www.exam...@2x.com/dir/image which doesn't look correct. Fixed. Only path part of a URL is converted to path2x. 3. RenderingHints spec references Retina or non-Retina displays, which should be removed. Fixed. - devScale is used instead of transform parsing in the drawHiDPIImage() method just to not have performance regression more than 2 times on HiDPI displays - LWCToolkit.ScalableToolkitImage is made public for the fix 8024926 [macosx] AquaIcon HiDPI support. Thanks, Alexandr. Thanks, Artem On 10/25/2013 5:18 PM, Alexander Scherbatiy wrote: Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8011059/webrev.02/ - Scaled image width and height are transformed according to the AffineTransform type. - ToolkitImage subclass is used to hold @2x image instance. Thanks, Alexandr. On 10/23/2013 7:24 PM, Alexander Scherbatiy wrote: Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8011059/webrev.01/ The JCK failures has been resolved: - Some tests tries to draw an image with Integer.MAX_VALUE width or height. Passing large values to image.getScaledImage(width, height, hints). leads that an Image filter is not able to create necessary arrays. The fix uses the original image if width or height are equal to Integer.MAX_VALUE. - Using Image.SCALE_DEFAULT hint for the getScaledImage(width, height, hints) method to get the high resolution image interferes with JCK tests that expect that the scaled image by certain algorithm is returned. This is fixed by invoking the super.getScaledImage(width, height, hints) method in ToolkitImage in case if a high resolution image is not set. Thanks, Alexandr. On 10/22/2013 1:31 PM, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8011059 webrev: http://cr.openjdk.java.net/~alexsch/8011059/webrev.00 The IMAGE_SCALING rendering hint is added to the RenderingHints class. Enabling the image scaling rendering hint forces the SunGraphics2D to use getScaledInstance(width, height, hints) method from Image class with SCALE_DEFAULT hint. By default the image scaling rendering hint is enabled on HiDPI display and disabled for standard displays. User can override the getScaledInstance(width, height, hints) method and return necessary high resolution image according to the given image width and height. For example: - final Image highResolutionImage = new BufferedImage(2 * WIDTH, 2 * HEIGHT, BufferedImage.TYPE_INT_RGB); Image image = new BufferedImage(WIDTH, HEIGHT, BufferedImage.TYPE_INT_RGB) { @Override public Image getScaledInstance(int width, int height, int hints) { if ((hints & Image.SCALE_DEFAULT) != 0) { return (width <= WIDTH && height <= HEIGHT) ? this : highResolutionImage; } return super.getScaledIn
[8] Review request for 8024926 [macosx] AquaIcon HiDPI support
Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8024926 webrev: http://cr.openjdk.java.net/~alexsch/8024926/webrev.00 The fix returns a high resolution system icon in the overridden getScaledInstance(width, height, hints) method. The fix relies on the fix for the issue JDK-8011059 [macosx] Make JDK demos look perfect on retina displays: http://mail.openjdk.java.net/pipermail/awt-dev/2013-October/006133.html - getScaledInstance(width, height, hints) method is used for the image drawing when IMAGE_SCALING hints are enabled - LWCToolkit.ScalableToolkitImage class is public Thanks, Alexandr.
Re: [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays
Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8011059/webrev.03 On 10/28/2013 2:33 PM, Artem Ananiev wrote: Hi, Alexander, a few comments: 1. SunGraphics2D.java:3076 - should isHiDPIImage() be used here? The isHiDPIImage() method is used to check that the drawHiDPIImage should be called like: if (isHiDPIImage(img)) { return drawHiDPIImage(...); } 2. I'm not sure that the proposed getScaledImageName() implementation in ScalableToolkitImage works perfectly for URLs like this: http://www.exampmle.com/dir/image In this case it will try to find 2x image here: http://www.exam...@2x.com/dir/image which doesn't look correct. Fixed. Only path part of a URL is converted to path2x. 3. RenderingHints spec references Retina or non-Retina displays, which should be removed. Fixed. - devScale is used instead of transform parsing in the drawHiDPIImage() method just to not have performance regression more than 2 times on HiDPI displays - LWCToolkit.ScalableToolkitImage is made public for the fix 8024926 [macosx] AquaIcon HiDPI support. Thanks, Alexandr. Thanks, Artem On 10/25/2013 5:18 PM, Alexander Scherbatiy wrote: Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8011059/webrev.02/ - Scaled image width and height are transformed according to the AffineTransform type. - ToolkitImage subclass is used to hold @2x image instance. Thanks, Alexandr. On 10/23/2013 7:24 PM, Alexander Scherbatiy wrote: Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8011059/webrev.01/ The JCK failures has been resolved: - Some tests tries to draw an image with Integer.MAX_VALUE width or height. Passing large values to image.getScaledImage(width, height, hints). leads that an Image filter is not able to create necessary arrays. The fix uses the original image if width or height are equal to Integer.MAX_VALUE. - Using Image.SCALE_DEFAULT hint for the getScaledImage(width, height, hints) method to get the high resolution image interferes with JCK tests that expect that the scaled image by certain algorithm is returned. This is fixed by invoking the super.getScaledImage(width, height, hints) method in ToolkitImage in case if a high resolution image is not set. Thanks, Alexandr. On 10/22/2013 1:31 PM, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8011059 webrev: http://cr.openjdk.java.net/~alexsch/8011059/webrev.00 The IMAGE_SCALING rendering hint is added to the RenderingHints class. Enabling the image scaling rendering hint forces the SunGraphics2D to use getScaledInstance(width, height, hints) method from Image class with SCALE_DEFAULT hint. By default the image scaling rendering hint is enabled on HiDPI display and disabled for standard displays. User can override the getScaledInstance(width, height, hints) method and return necessary high resolution image according to the given image width and height. For example: - final Image highResolutionImage = new BufferedImage(2 * WIDTH, 2 * HEIGHT, BufferedImage.TYPE_INT_RGB); Image image = new BufferedImage(WIDTH, HEIGHT, BufferedImage.TYPE_INT_RGB) { @Override public Image getScaledInstance(int width, int height, int hints) { if ((hints & Image.SCALE_DEFAULT) != 0) { return (width <= WIDTH && height <= HEIGHT) ? this : highResolutionImage; } return super.getScaledInstance(width, height, hints); } }; - The LWCToolkit and ToolkitImage classes are patched to automatically get provided im...@2x.ext images on MacOSX. There are no significant changes in the Java2D demo to make it look perfect on Retina displays. It needs only to put necessary images with the @2x postfix and they will be automatically drawn. Thanks, Alexandr.
Re: [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays
Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8011059/webrev.02/ - Scaled image width and height are transformed according to the AffineTransform type. - ToolkitImage subclass is used to hold @2x image instance. Thanks, Alexandr. On 10/23/2013 7:24 PM, Alexander Scherbatiy wrote: Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8011059/webrev.01/ The JCK failures has been resolved: - Some tests tries to draw an image with Integer.MAX_VALUE width or height. Passing large values to image.getScaledImage(width, height, hints). leads that an Image filter is not able to create necessary arrays. The fix uses the original image if width or height are equal to Integer.MAX_VALUE. - Using Image.SCALE_DEFAULT hint for the getScaledImage(width, height, hints) method to get the high resolution image interferes with JCK tests that expect that the scaled image by certain algorithm is returned. This is fixed by invoking the super.getScaledImage(width, height, hints) method in ToolkitImage in case if a high resolution image is not set. Thanks, Alexandr. On 10/22/2013 1:31 PM, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8011059 webrev: http://cr.openjdk.java.net/~alexsch/8011059/webrev.00 The IMAGE_SCALING rendering hint is added to the RenderingHints class. Enabling the image scaling rendering hint forces the SunGraphics2D to use getScaledInstance(width, height, hints) method from Image class with SCALE_DEFAULT hint. By default the image scaling rendering hint is enabled on HiDPI display and disabled for standard displays. User can override the getScaledInstance(width, height, hints) method and return necessary high resolution image according to the given image width and height. For example: - final Image highResolutionImage = new BufferedImage(2 * WIDTH, 2 * HEIGHT, BufferedImage.TYPE_INT_RGB); Image image = new BufferedImage(WIDTH, HEIGHT, BufferedImage.TYPE_INT_RGB) { @Override public Image getScaledInstance(int width, int height, int hints) { if ((hints & Image.SCALE_DEFAULT) != 0) { return (width <= WIDTH && height <= HEIGHT) ? this : highResolutionImage; } return super.getScaledInstance(width, height, hints); } }; - The LWCToolkit and ToolkitImage classes are patched to automatically get provided im...@2x.ext images on MacOSX. There are no significant changes in the Java2D demo to make it look perfect on Retina displays. It needs only to put necessary images with the @2x postfix and they will be automatically drawn. Thanks, Alexandr.
Re: Aqua Icons support on HiDPI displays.
On 10/19/2013 1:21 PM, Hendrik Schreiber wrote: On Oct 19, 2013, at 12:45 AM, Mike Swingler wrote: On Oct 18, 2013, at 4:49 AM, Alexander Scherbatiy wrote: The GetIconRef method is used in the CImage.m now to get the system icons on MacOSX. The NSImage + (id)imageNamed:(NSString *)name method can be used directly instead. There are NSImageNameFolder, NSImageNameFolderBurnable, and NSImageNameFolderSmart constants but I can't find the constant for the open folder icon. https://developer.apple.com/library/mac/documentation/cocoa/reference/applicationkit/classes/NSImage_Class/Reference/Reference.html#//apple_ref/doc/constant_group/Toolbar_Named_Images The same is for the alert stop system icon. There are NSImageNameInfo and NSImageNameCaution icons but I can't find the constant for the alert stop icon. Alexander, may I ask through which API you intend to make the icons available? There is a way to load system images and icons on MacOS X in Java by using: Toolkit.getDefaultToolkit().getImage("NSImage://NSImageName") method: https://developer.apple.com/library/mac/documentation/java/conceptual/java14development/04-JavaUIToolkits/JavaUIToolkits.html This method calls [NSImage imageNamed: "NSImageName"] and we are going to reuse it for the system icons loading. Also, will there be a way to create custom HiDPI images/icons? If so, how? There is the fix that has been recently sent to review: http://mail.openjdk.java.net/pipermail/awt-dev/2013-October/006133.html A user needs to override the getScaledInstance(width, height, hints) method in Image class and returns images with necessary resolution according to the given width and height. Images with @2x postfix should be automatically loaded into the ToolkitImage on MacOS X. Thanks, Alexandr. Thanks, -hendrik
Re: [8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays
Could you review the updated fix: http://cr.openjdk.java.net/~alexsch/8011059/webrev.01/ The JCK failures has been resolved: - Some tests tries to draw an image with Integer.MAX_VALUE width or height. Passing large values to image.getScaledImage(width, height, hints). leads that an Image filter is not able to create necessary arrays. The fix uses the original image if width or height are equal to Integer.MAX_VALUE. - Using Image.SCALE_DEFAULT hint for the getScaledImage(width, height, hints) method to get the high resolution image interferes with JCK tests that expect that the scaled image by certain algorithm is returned. This is fixed by invoking the super.getScaledImage(width, height, hints) method in ToolkitImage in case if a high resolution image is not set. Thanks, Alexandr. On 10/22/2013 1:31 PM, Alexander Scherbatiy wrote: Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8011059 webrev: http://cr.openjdk.java.net/~alexsch/8011059/webrev.00 The IMAGE_SCALING rendering hint is added to the RenderingHints class. Enabling the image scaling rendering hint forces the SunGraphics2D to use getScaledInstance(width, height, hints) method from Image class with SCALE_DEFAULT hint. By default the image scaling rendering hint is enabled on HiDPI display and disabled for standard displays. User can override the getScaledInstance(width, height, hints) method and return necessary high resolution image according to the given image width and height. For example: - final Image highResolutionImage = new BufferedImage(2 * WIDTH, 2 * HEIGHT, BufferedImage.TYPE_INT_RGB); Image image = new BufferedImage(WIDTH, HEIGHT, BufferedImage.TYPE_INT_RGB) { @Override public Image getScaledInstance(int width, int height, int hints) { if ((hints & Image.SCALE_DEFAULT) != 0) { return (width <= WIDTH && height <= HEIGHT) ? this : highResolutionImage; } return super.getScaledInstance(width, height, hints); } }; - The LWCToolkit and ToolkitImage classes are patched to automatically get provided im...@2x.ext images on MacOSX. There are no significant changes in the Java2D demo to make it look perfect on Retina displays. It needs only to put necessary images with the @2x postfix and they will be automatically drawn. Thanks, Alexandr.
[8] Review request for 8011059 [macosx] Make JDK demos look perfect on retina displays
Hello, Could you review the fix: bug: https://bugs.openjdk.java.net/browse/JDK-8011059 webrev: http://cr.openjdk.java.net/~alexsch/8011059/webrev.00 The IMAGE_SCALING rendering hint is added to the RenderingHints class. Enabling the image scaling rendering hint forces the SunGraphics2D to use getScaledInstance(width, height, hints) method from Image class with SCALE_DEFAULT hint. By default the image scaling rendering hint is enabled on HiDPI display and disabled for standard displays. User can override the getScaledInstance(width, height, hints) method and return necessary high resolution image according to the given image width and height. For example: - final Image highResolutionImage = new BufferedImage(2 * WIDTH, 2 * HEIGHT, BufferedImage.TYPE_INT_RGB); Image image = new BufferedImage(WIDTH, HEIGHT, BufferedImage.TYPE_INT_RGB) { @Override public Image getScaledInstance(int width, int height, int hints) { if ((hints & Image.SCALE_DEFAULT) != 0) { return (width <= WIDTH && height <= HEIGHT) ? this : highResolutionImage; } return super.getScaledInstance(width, height, hints); } }; - The LWCToolkit and ToolkitImage classes are patched to automatically get provided im...@2x.ext images on MacOSX. There are no significant changes in the Java2D demo to make it look perfect on Retina displays. It needs only to put necessary images with the @2x postfix and they will be automatically drawn. Thanks, Alexandr.
Aqua Icons support on HiDPI displays.
Hello, The GetIconRef method is used in the CImage.m now to get the system icons on MacOSX. The NSImage + (id)imageNamed:(NSString *)name method can be used directly instead. There are NSImageNameFolder, NSImageNameFolderBurnable, and NSImageNameFolderSmart constants but I can't find the constant for the open folder icon. https://developer.apple.com/library/mac/documentation/cocoa/reference/applicationkit/classes/NSImage_Class/Reference/Reference.html#//apple_ref/doc/constant_group/Toolbar_Named_Images The same is for the alert stop system icon. There are NSImageNameInfo and NSImageNameCaution icons but I can't find the constant for the alert stop icon. Where is it possible to get the open folder and stop icons that are defined as kOpenFolderIcon and kAlertStopIcon for the GetIconRef method? Thanks, Alexandr.