Re: Review Request JDK:-8162959 [HiDPI] screenshot artifacts using AWT Robot
Hi Alexander, Please review update patch as per review comments. http://cr.openjdk.java.net/~pkbalakr/8162959/webrev.02/ With this patch, Robot.createScreenCapture(Rectangle) method returns a scaled down image on the HiDPI display. Regards, Prem From: Alexandr Scherbatiy Sent: Thursday, November 03, 2016 4:05 PM To: Prem Balakrishnan; Sergey Bylokhov; awt-dev@openjdk.java.net Subject: Re: Review Request JDK:-8162959 [HiDPI] screenshot artifacts using AWT Robot On 11/2/2016 1:57 PM, Prem Balakrishnan wrote: Hi Alexander, Please review updated patch. HYPERLINK "http://cr.openjdk.java.net/%7Epkbalakr/8162959/webrev.01/"http://cr.openjdk.java.net/~pkbalakr/8162959/webrev.01/ Added a new public API "Image createHiDPIScreenCapture(Rectangle screenRect)". Returns an ordinary screenshot(BufferedImage) if the UI scale is 1. Returns MultiResolutionImage for HiDPI display with two resolution variants, 1. Low Resolution/base image with user input width and height, I have used interpolation algorithm for scaling , adapted from JavaFX Robot (http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/89a5de54b7dc), 2. High Resolution image with scaled width and height . - Please, check that the Robot.createScreenCapture(Rectangle) method returns a scaled down image on the HiDPI display - Probably existing Java methods can be used for an image scaling. For example, there is the Image.getScaledInstance(int width, int height, int hints) method. Or may be it is better just to create an image with required size and draw the high-resolution image into it using a specific scale and rendering hints. Thanks, Alexandr. Regards, Prem From: Alexander Scherbatiy Sent: Thursday, October 13, 2016 3:21 PM To: Prem Balakrishnan; Sergey Bylokhov; HYPERLINK "mailto:awt-dev@openjdk.java.net"awt-dev@openjdk.java.net Subject: Re: Review Request JDK:-8162959 [HiDPI] screenshot artifacts using AWT Robot On 06/10/16 15:28, Prem Balakrishnan wrote: Hi, Please review fix for JDK 9, Bug: https://bugs.openjdk.java.net/browse/JDK-8162959 Webrev: HYPERLINK "http://cr.openjdk.java.net/%7Epkbalakr/8162959/webrev.00/"http://cr.openjdk.java.net/~pkbalakr/8162959/webrev.00/ I have adapted the same fix as used for JavaFX Robot Bug: HYPERLINK "https://bugs.openjdk.java.net/browse/JDK-8162783"JDK-8162783. Patch: http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/89a5de54b7dc New Public API " BufferedImage createScreenCapture(Rectangle screenRect, boolean isHiDPI)" is added, Which will have a parameter to specify if HiDPI. It is better to a add public method which returns MultiResolution image on HiDPI display and consists of two resoltion variants - base image which has size requested by a user - high-resolution image which creates an original screen capture The proposed by your algorithm can be applied to the base image. For more details see JDK-8020618 [macosx] java.awt.Robot makes blurry screen captures on Retina Thanks, Alexandr. Regards, Prem
Re: [9] Review request for 8151189: Possible getAppContext() NPE in java.awt.Desktop and java.awt.Taskbar
A "main" appcontext will be created if you are a standalone app, but
not if running in webstart (although the means of determining that is
somewhat hokey) :-
if (numAppContexts.get() == 0) {
if
(System.getProperty("javaplugin.version") == null &&
System.getProperty("javawebstart.version") == null) {
initMainAppContext();
So if that is not initialised it appears to rely solely on an appcontext
being
associated with the current threadgroup - or a parent threadgroup.
If for some reason this does not return an appcontext you'll get the NPE.
This doesn't have to mean it is the Toolkit thread.
This example may be a little contrived but it illustrates the problem :-
If the webstart system property is set you will never see "got desktop"
printed
because the finalizer thread gets an exception.
===
import java.awt.Desktop;
public class GD {
public void finalize() {
System.out.println("get desktop");
System.out.println(Desktop.getDesktop());
System.out.println("got desktop");
}
public static void main(String[] args) {
if (args.length == 0) {
System.out.println("Setting webstart version");
System.setProperty("javawebstart.version", "8");
}
while (true) {
new GD();
System.gc();
}
}
}
-phil.
On 11/15/2016 10:48 AM, Phil Race wrote:
So are you saying we will never call this from the Toolkit thread, so
provably
there will never be an NPE ?
Seems we have had a ton of NPE bugs from getAppContext() returning null
so I am not so confident about that.
-phil.
On 11/15/2016 10:44 AM, Sergey Bylokhov wrote:
I guess this should be closed as not a defect. getAppContext() can
return null if it will be called from the toolkit thread. If this
method is called by the user then appcontext should not be null,
additionally we should not cache this value in the static, so all
other code will use this cached static value.
On 15.11.16 20:38, Phil Race wrote:
+1
-phil.
On 11/15/2016 08:24 AM, Alexander Zvegintsev wrote:
Hello,
please review the fix
http://cr.openjdk.java.net/~azvegint/jdk/9/8151189/00/
for the issue
https://bugs.openjdk.java.net/browse/JDK-8151189
Re: [9] Review request for 8151189: Possible getAppContext() NPE in java.awt.Desktop and java.awt.Taskbar
So are you saying we will never call this from the Toolkit thread, so provably there will never be an NPE ? Seems we have had a ton of NPE bugs from getAppContext() returning null so I am not so confident about that. -phil. On 11/15/2016 10:44 AM, Sergey Bylokhov wrote: I guess this should be closed as not a defect. getAppContext() can return null if it will be called from the toolkit thread. If this method is called by the user then appcontext should not be null, additionally we should not cache this value in the static, so all other code will use this cached static value. On 15.11.16 20:38, Phil Race wrote: +1 -phil. On 11/15/2016 08:24 AM, Alexander Zvegintsev wrote: Hello, please review the fix http://cr.openjdk.java.net/~azvegint/jdk/9/8151189/00/ for the issue https://bugs.openjdk.java.net/browse/JDK-8151189
Re: [9] Review request for 8151189: Possible getAppContext() NPE in java.awt.Desktop and java.awt.Taskbar
I guess this should be closed as not a defect. getAppContext() can return null if it will be called from the toolkit thread. If this method is called by the user then appcontext should not be null, additionally we should not cache this value in the static, so all other code will use this cached static value. On 15.11.16 20:38, Phil Race wrote: +1 -phil. On 11/15/2016 08:24 AM, Alexander Zvegintsev wrote: Hello, please review the fix http://cr.openjdk.java.net/~azvegint/jdk/9/8151189/00/ for the issue https://bugs.openjdk.java.net/browse/JDK-8151189 -- Best regards, Sergey.
Re: [9] Review request for 8151189: Possible getAppContext() NPE in java.awt.Desktop and java.awt.Taskbar
+1 --Semyon On 15.11.2016 20:38, Phil Race wrote: +1 -phil. On 11/15/2016 08:24 AM, Alexander Zvegintsev wrote: Hello, please review the fix http://cr.openjdk.java.net/~azvegint/jdk/9/8151189/00/ for the issue https://bugs.openjdk.java.net/browse/JDK-8151189
Re: [9] Review request for 8151189: Possible getAppContext() NPE in java.awt.Desktop and java.awt.Taskbar
+1 -phil. On 11/15/2016 08:24 AM, Alexander Zvegintsev wrote: Hello, please review the fix http://cr.openjdk.java.net/~azvegint/jdk/9/8151189/00/ for the issue https://bugs.openjdk.java.net/browse/JDK-8151189
[9] Review request for 8151189: Possible getAppContext() NPE in java.awt.Desktop and java.awt.Taskbar
Hello, please review the fix http://cr.openjdk.java.net/~azvegint/jdk/9/8151189/00/ for the issue https://bugs.openjdk.java.net/browse/JDK-8151189 -- Thanks, Alexander.
Re: [9] Review request for 8166683: On macOS (Mac OS X) getting a ScreenMenuBar when not running "com.apple.laf.AquaLookAndFeel"
Hi Sergey, I've not found casting issues, but I've found the issue when previous fix does not treat dynamically changed "apple.laf.useScreenMenuBar" property correctly. (e.g. ScreenMenuBarInputTwice test fails). So please see the updated changeset: http://cr.openjdk.java.net/~azvegint/jdk/9/8166683/01/ Thanks, Alexander. On 11/11/16 2:14 PM, Sergey Bylokhov wrote: Hi, Alexander. Did you run the tests on non-Aqua l? I assume that we can have a places in other l where we try to cast the MenuBarUI to some specific UI delegate. On 09.11.16 16:58, Alexander Zvegintsev wrote: Hello, please review the fix http://cr.openjdk.java.net/~azvegint/jdk/9/8166683/00/ for the issue https://bugs.openjdk.java.net/browse/JDK-8166683 This fix adds support for ScreenMenuBar for L's other than Aqua. With this fix it is enabled by default if apple.laf.useScreenMenuBar property is true. This behavior can be disabled by setting apple.laf.disableForcedScreenMenuBar property to true.
Re: [9] Review request for 8169133: This time, on Windows: java/awt/Robot/SpuriousMouseEvents/SpuriousMouseEvents.java
What test did you use to verify the bug? It seems that the test in closed ws was reworked by Manajit and moved to the open ws. But closed version was existed till today. (Note probably the fix for JDK-8145784 should be reworked because it return behavior which was before JDK-8013116). On 11.11.16 17:06, Semyon Sadetsky wrote: Hello, Please review fix for JDK9: bug: https://bugs.openjdk.java.net/browse/JDK-8169133 webrev: http://cr.openjdk.java.net/~ssadetsky/8169133/webrev.00/ Screen specific AWT robot was not implemented on Windows platform. The fix proposes the missed implementation. --Semyon -- Best regards, Sergey.
