Re: [OpenJDK 2D-Dev] RFR(M): 8170525: Fix minor issues in awt coding
Hi Phil, does that mean I can push the change, or will this happen through jprt? Best regards, Goetz. > -Original Message- > From: Philip Race [mailto:philip.r...@oracle.com] > Sent: Tuesday, December 06, 2016 4:21 AM > To: Lindenmaier, Goetz > Cc: Sergey Bylokhov ; awt- > d...@openjdk.java.net; 2d-dev <2d-...@openjdk.java.net> > Subject: Re: [OpenJDK 2D-Dev] RFR(M): 8170525: Fix minor > issues in awt coding > > I didn't eyeball what you changed but JPRT is now happy. > The build passes on all platforms... > > -phil. > > On 12/5/16, 2:47 PM, Lindenmaier, Goetz wrote: > > Hi Phil, > > > > sorry for that! I fixed it, there is an other occurrence, too. > > I double checked all the casts, there was also a mp_flags<-> mp_sign > wrong in mpi.c > > http://cr.openjdk.java.net/~goetz/wr16/8170525-awt-dev/webrev.05/ > > (Also rebased the change) > > > > Best regards, > >Goetz > > > >> -Original Message- > >> From: Phil Race [mailto:philip.r...@oracle.com] > >> Sent: Monday, December 05, 2016 7:26 PM > >> To: Lindenmaier, Goetz; Sergey Bylokhov > >> > >> Cc: awt-dev@openjdk.java.net; 2d-dev<2d-...@openjdk.java.net> > >> Subject: Re: [OpenJDK 2D-Dev] RFR(M): 8170525: Fix minor > >> issues in awt coding > >> > >> I tried it .. and just as well I did. It fails in the crypto code on Mac. > >> > >> jdk/src/jdk.crypto.ec/share/native/libsunec/impl/ec.c:261:12: error: > >> expression which evaluates to zero treated as a null pointer constant of > type > >> 'mp_digit *' (aka 'unsigned long *') > >> [-Werror,-Wnon-literal-null-conversion] > >> k.dp = (mp_digit)0; > >> ^~~ > >> 1 error generated. > >> > >> -phil. > >> > >> > >> > >> On 12/3/2016 9:53 AM, Lindenmaier, Goetz wrote: > >>> Hi Phil, > >>> > >>> that would be great, unfortunately I don't have access to JPRT. > >>> > >>> Thanks, > >>> Goetz. > >>> > -Original Message- > From: Phil Race [mailto:philip.r...@oracle.com] > Sent: Friday, December 02, 2016 8:46 PM > To: Lindenmaier, Goetz; Sergey > Bylokhov > > Cc: awt-dev@openjdk.java.net; 2d-dev<2d-...@openjdk.java.net> > Subject: Re: [OpenJDK 2D-Dev] RFR(M): 8170525: Fix minor > issues in awt coding > > I had no other comments, except that it would be good to be sure > that this builds on all relevant platforms .. using the 'blessed' > compilers. > If you like I can submit a JPRT job for you based on this patch. > > -phil. > > On 12/01/2016 11:58 PM, Lindenmaier, Goetz wrote: > > Hi Phil, > > > > I added the initialization to the other function. > > http://cr.openjdk.java.net/~goetz/wr16/8170525-awt- > dev/webrev.04/ > > > > I missed that because Coverity didn't complain. > > It's not a perfect tool, but it finds sufficient issues > > to make it worthwhile. Is the other awt code fine? > > > > Best regards, > > Goetz. > > > > > > > >> -Original Message- > >> From: Phil Race [mailto:philip.r...@oracle.com] > >> Sent: Donnerstag, 1. Dezember 2016 22:31 > >> To: Lindenmaier, Goetz; Sergey > >> Bylokhov > >> ; Vincent Ryan > > >> Cc: awt-dev@openjdk.java.net; 2d-dev<2d-...@openjdk.java.net>; > security- > >> d...@openjdk.java.net > >> Subject: Re: [OpenJDK 2D-Dev] RFR(M): 8170525: Fix > minor > issues > >> in awt coding > >> > >> Sorry. it is > >> ops->GetRasInfo(env, ops, lockInfo); > >> that initialises it .. > >> > >> > >> That is still before the dereference > >> Anyway, what was the reason for updating one function, but not the > other. > >> I don't mind the change, but the inconsistency looks odd. > >> > >> -phil. > >> > >> On 12/01/2016 03:03 AM, Lindenmaier, Goetz wrote: > >>> Hi Phil, > >>> > Did you actually compile this ? The variable is called "rasBase", not > "resBase". > >>> Yes, I compiled it, and I fixed the error, but that was another repo > >>> I use for the coverity checks. Somehow it did not find its way into > >>> the webrev. > >>> > >>> I don't see where ops->Lock() initializes this field. > >>> ops->GetRasInfo(env, ops, lockInfo); does so if it resolves > >>> to BufImg_GetRasInfo(). I can't look in our code scan results > >>> because the issue is gone after fixing it, that lists the path > >>> of execution that leads to the issue. > >>> If you are sure this is correct I will remove the initialization, > >>> else I will also put it into the other method. > >>> > >>> DBN_GetPixelPointer checks lockInfo->rasBase for NULL and > >>> does what looks like the error case if it's NULL. Therefore NULL > >>> seemed a good initialization to me. > >>> > >>> Best regards, > >>> Goetz. > >>> > >>> > >>> > -Original Message- > From:
Re: Review Request JDK:-8162959 [HiDPI] screenshot artifacts using AWT Robot
Hi Alexander, Please review updated patch as per review comments. http://cr.openjdk.java.net/~pkbalakr/8162959/webrev.03/ Regards, Prem From: Alexandr Scherbatiy Sent: Monday, November 21, 2016 8:10 PM To: Prem Balakrishnan; Sergey Bylokhov; awt-dev@openjdk.java.net; Phil Race Subject: Re: Review Request JDK:-8162959 [HiDPI] screenshot artifacts using AWT Robot On 11/16/2016 8:46 AM, Prem Balakrishnan wrote: Hi Alexander, Please review update patch as per review comments. HYPERLINK "http://cr.openjdk.java.net/%7Epkbalakr/8162959/webrev.02/"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. - The native part on Mac OS X and Linux should be updated as well. - 416 * Returns BufferedImage for Non-HiDPI display and MultiResolutionImage 417 * for HiDPI display with two resolution variants. There should be added an explanation what is the content of the first and the second resolution variant. Thanks, Alexandr. Regards, Prem From: Alexandr Scherbatiy Sent: Thursday, November 03, 2016 4:05 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 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 8165428: Security Warning dialog should be always on the top when multiple applets with APPLICATION_MODAL dialog launched in a browser
This logic looks better by it is unclear why you increase the toolkit’s counter? [AWTToolkit eventCountPlusPlus]; This counter should be increased in the native callbacks and should indicate that there are some activity on the toolkit thread. But it seems it is unnecessary in the new isBlocked() method? > 2 дек. 2016 г., в 3:16, dmitry markov написал(а): > > Hi Sergey, > > According to the current implementation we disable a window only when we are > going to show a modal dialog. However I agree it is not a good idea to use > isEnabled flag for testing whether the window is blocked or not, since such > logic is not clear and might be accidentally broken. So I have updated the > fix; new webrev is located at > http://cr.openjdk.java.net/~dmarkov/8165428/webrev.01/ > Summary of changes: > - Added a new function isBlocked() to CPlatformWindow class > - In AWTWindow.m use isBlocked() instead of isEnabled in the cases where we > have to decide whether the ordering operation is required or not. > > Thanks, > Dmitry > On 01/12/2016 03:29, Sergey Bylokhov wrote: >> Hi, Dmitry. >> Is it true that the window is disable only if blocked by some other window? >> Is it possible a situation when it can be disabled by application and in the >> same moment can have an enabled child which should be moved upfront? >> >
Re: Fwd: [PATCH]: few memory errors fixes
Hi, the fix to imageDecompressor.cpp is already reported in https://bugs.openjdk.java.net/browse/JDK-8170663 I'll send RFR for that today. I'll add you to contributors. Best regards, Goetz. > -Original Message- > From: awt-dev [mailto:awt-dev-boun...@openjdk.java.net] On Behalf Of > Volker Simonis > Sent: Dienstag, 6. Dezember 2016 10:59 > To: Java Core Libs ; awt- > d...@openjdk.java.net > Subject: Fwd: [PATCH]: few memory errors fixes > > Hi David, > > thanks for your contribution. Your fixes look reasonable. > I'm forwarding your mail to to core-libs-dev and awt-dev for reviewing. > > Regards, > Volker > > > -- Forwarded message -- > From: David CARLIER > Date: Mon, Dec 5, 2016 at 10:10 PM > Subject: [PATCH]: few memory errors fixes > To: jdk9-...@openjdk.java.net > > > Hi, > > this is my first patch sent to the mailing list. One corrects the wrong > delete operator used and a potential memory leak. > > Hope it is useful. > > Kind regards.
Fwd: [PATCH]: few memory errors fixes
Hi David,
thanks for your contribution. Your fixes look reasonable.
I'm forwarding your mail to to core-libs-dev and awt-dev for reviewing.
Regards,
Volker
-- Forwarded message --
From: David CARLIER
Date: Mon, Dec 5, 2016 at 10:10 PM
Subject: [PATCH]: few memory errors fixes
To: jdk9-...@openjdk.java.net
Hi,
this is my first patch sent to the mailing list. One corrects the wrong
delete operator used and a potential memory leak.
Hope it is useful.
Kind regards.
diff --git a/src/java.base/share/native/libjimage/imageDecompressor.cpp
b/src/java.base/share/native/libjimage/imageDecompressor.cpp
--- a/src/java.base/share/native/libjimage/imageDecompressor.cpp
+++ b/src/java.base/share/native/libjimage/imageDecompressor.cpp
@@ -181,7 +181,7 @@
}
} while (has_header);
memcpy(uncompressed, decompressed_resource, (size_t) uncompressed_size);
-delete decompressed_resource;
+delete [] decompressed_resource;
}
// Zip decompressor
diff --git a/src/java.desktop/unix/native/common/awt/fontpath.c
b/src/java.desktop/unix/native/common/awt/fontpath.c
--- a/src/java.desktop/unix/native/common/awt/fontpath.c
+++ b/src/java.desktop/unix/native/common/awt/fontpath.c
@@ -289,6 +289,7 @@
onePath = SAFE_SIZE_ARRAY_ALLOC(malloc, strlen (fDirP->name[index]) +
2, sizeof( char ) );
if (onePath == NULL) {
free ( ( void *) appendDirList );
+free ( ( void *) newFontPath );
XFreeFontPath ( origFontPath );
return;
}
