Re: [OpenJDK 2D-Dev] [12] RFR JDK-8211795: ArrayIndexOutOfBoundsException in PNGImageReader after JDK-6788458
Looks fine. On 16/11/2018 02:16, Jayathirth D V wrote: Hi Sergey, As discussed offline I did more analysis on whether we can use common variable to determine number of bands. Since we have "outputSampleSize.length - 1" and "inputBands + 1" kind of things. Actually scale array will be used on input data(ps[]), so we should use input bands value to create and use scale array. Before JDK-6788458 there was no difference between input bands and output bands so we didn't see any issue. But after JDK-6788458 number of bands data is different between input and output data for PNG_COLOR_RGB/GRAY having tRNS chunk. So I have made change to use inputBands data for creation and use of scale array. Ran complete imageio jtreg and JCK tests there are no failures. Please find updated webrev for review: http://cr.openjdk.java.net/~jdv/8211795/webrev.01/ Thanks, Jay -Original Message- From: Jayathirth D V Sent: Wednesday, November 14, 2018 1:09 PM To: Sergey Bylokhov; 2d-dev Subject: RE: [OpenJDK 2D-Dev] [12] RFR JDK-8211795: ArrayIndexOutOfBoundsException in PNGImageReader after JDK-6788458 Hi Sergey, Thanks for the review. As you pointed out, yes the AIOOB is happening for ps[b]. I have updated the analysis in JBS bug also. Basically the calculation of numBands is not proper because we take numBands value from destination image. This destination image will have extra alpha channel for Gray or RGB input data(ps[]) which will throw AIOOB. So we need to update the logic of how we calculate numBands different for PNG Gray/RGB image havng tRNS chunk. Fortunately, webrev.00 is actually doing this job. Regarding whether we need to change scale array logic : We expect first 3 channel to be RGB and first channel to be Gray for PNG_COLOR_RGB and PNG_COLOR_GRAY respectively. So just updating numBands information will create proper scale array. So there is no need to change the scale array logic. History JDK-6788458 : Toolkit was able to show transparent color information for RGB/Gray PNG image when it has tRNS chunk, but ImageIO didn't support it. To use tRNS data and show transparent color in output image we needed to add extra alpha channel for PNG RGB/Gray image with tRNS chunk. But fix present in JDK-6788458 didn't handle the case where bitDepth adjustment is needed and we are using band information from output image(having extra alpha channel) on input image which has no alpha channel. Change in numBands logic for this bug fixes that issue. Thanks, Jay -Original Message- From: Sergey Bylokhov Sent: Wednesday, November 14, 2018 4:07 AM To: Jayathirth D V; 2d-dev Subject: Re: [OpenJDK 2D-Dev] [12] RFR JDK-8211795: ArrayIndexOutOfBoundsException in PNGImageReader after JDK-6788458 Hi, Jay. Can you please provide some more detail about this bug. Root cause : In JDK-6788458 we are adding extra alpha channel for destination whenever we have tRNS chunk. But the number of bands in bitDepth scale array was not changed when we have tRNS chunk. This is causing ArrayIndexOutOfBoundsException for scale array. As far as I understand the AIOOB is occurred when we access ps[b] at line 1308 not when we access the scale array, because the scale array is created as "scale = new int[numBands][]". So maybe numBands should depends on the passRow? or the creation of scale[][xxx] should be updated? BTW this code uses +1/-1 in a lot of places already, and it is not always clear why. -- Best regards, Sergey.
Re: [OpenJDK 2D-Dev] RFR: 8130264 : change the mechanism by which JDK loads the platform-specific PrinterJob implementation
Looks fine. On 15/11/2018 13:41, Phil Race wrote: bug: http://cr.openjdk.java.net/~prr/8130264/ webrev: http://cr.openjdk.java.net/~prr/8130264/ Currently java launcher code embeds the name of the java.desktop module's PrinterJob implementation class for each platform in a system property which is later read by the java.desktop code to use to reflectively locate the class and instantiate it. This fix removes that entirely from the launcher code and the desktop module now looks it up internally via a simple platform proxy class. This builds on all platforms and we rely on existing printing tests to verify that we can still locate the implementation class. The new regression test just verifies the system property name space is no longer polluted. I didn't find any test (apart from this new one) that references it. Since that system property has been around for a long time I am thinking I should file a CSR to document its removal .. unless there is a concensus it is not necessary. -phil. -- Best regards, Sergey.
Re: [OpenJDK 2D-Dev] [12] RFR JDK-8139178: Wrong fontMetrics when printing in Landscape
Looks good to me. Just some nits. Can we split the long lines like this ? :- #define contextAwareMetricsX(vx, vy) \ (FTFixedToFloat(context->transform.xx) * (vx) - \ FTFixedToFloat(context->transform.xy) * (vy)) and why "vx" and "vy", not just x and y ? -phil. On 11/10/18 8:53 AM, Andrew Brygin wrote: Hello, could you please review a fix for JDK-8139178. Bug: https://bugs.openjdk.java.net/browse/JDK-8139178 Webrev: http://cr.openjdk.java.net/~bae/8139178/webrev.00/ The freetype scaler calculates font metrics with a transform applied to the scaler context. We have to revert this transform in order to get correct font metrics. Thanks, Andrew
Re: [OpenJDK 2D-Dev] RFR: 8214002 Cannot use italic font style if the font has embeded bitmap
Hi,
Thanks for spotting this. But I'm not sure the conditions are right.
Don't we need to unconditionally remove FT_LOAD_RENDER if synthetic styles
are requested, regardless of the value of that flag ?
Perhaps we should not set FT_LOAD_RENDER upfront in which case we'll just call
it once
we are done, making the logic simpler and avoid all of the error prone "in this
case we disable it again" logic.
So instead of your fix just change the initialisation to :
int renderFlags = FT_LOAD_DEFAULT;
-phil.
On 11/16/18 9:37 AM, Ichiroh Takiguchi wrote:
Hello.
Could you review the fix ?
Issue:
Cannot use italic font style if the font has embeded bitmap.
Bug: https://bugs.openjdk.java.net/browse/JDK-8214002
Change: https://cr.openjdk.java.net/~itakiguchi/8214002/webrev.00/
It seems it's side-effect for:
8204929: Fonts with embedded bitmaps are not always rotated
Test instruction and screen shot are in JDK-8214002.
Thanks,
Ichiroh Takiguchi
IBM Japan, Ltd.
On 2018-07-27 20:22, Ichiroh Takiguchi wrote:
Hello.
According to my investigation, FT_Render_Glyph() was not called
even if FT_GlyphSlot_Oblique() was called.
=
if (ftglyph->format == FT_GLYPH_FORMAT_OUTLINE) {
FT_Render_Glyph(ftglyph, FT_LOAD_TARGET_MODE(target)); <<===
}
=
It seemed FT_Load_Glyph() and renderFlags affected this issue.
On my Windows,
For "MS Mincho" with italic, renderFlags was "FT_LOAD_TARGET_MONO |
FT_LOAD_NO_BITMAP | FT_LOAD_RENDER".
I also tested "Meiryo" font (it could handle italic style)
For "Meiryo" with italic, renderFlags was "FT_LOAD_TARGET_MONO |
FT_LOAD_RENDER".
I think, after FT_LOAD_NO_BITMAP is turned on, FT_LOAD_RENDER should
be turned off.
So how about following fix ?
=
diff -r 1edcf36fe15f
src/java.desktop/share/native/libfontmanager/freetypeScaler.c
---
a/src/java.desktop/share/native/libfontmanager/freetypeScaler.c Wed
Jul 18 11:57:51 2018 -0400
+++
b/src/java.desktop/share/native/libfontmanager/freetypeScaler.c Fri
Jul 27 19:44:12 2018 +0900
@@ -700,6 +700,9 @@
if (!context->useSbits) {
renderFlags |= FT_LOAD_NO_BITMAP;
+ if (context->doBold || context->doItalize) {
+ renderFlags &= ~FT_LOAD_RENDER;
+ }
}
/* NB: in case of non identity transform
=
On 2018-07-25 19:29, Ichiroh Takiguchi wrote:
Hello.
I'm using jdk-11+23 build on Japanese Windows 7.
I ran Font2DTest Demo, then select like:
Font: MS Mincho
Range: Basic Latin
Method: drawString
Size: 24
Style: Italic
But style was not changed to Italic.
Antialiasing and Fractional Metrics did not affect this issue.
I assume it's side-effect for:
8204929: Fonts with embedded bitmaps are not always rotated
Could you check it ?
Thanks,
Ichiroh Takiguchi
IBM Japan, Ltd.
Re: [OpenJDK 2D-Dev] RFR: 8130264 : change the mechanism by which JDK loads the platform-specific PrinterJob implementation
Hi Phil, Looks fine from the core-libs perspective. Thanks, thanks for removing this cross module dependency. Roger On 11/15/2018 04:41 PM, Phil Race wrote: bug: http://cr.openjdk.java.net/~prr/8130264/ webrev: http://cr.openjdk.java.net/~prr/8130264/ Currently java launcher code embeds the name of the java.desktop module's PrinterJob implementation class for each platform in a system property which is later read by the java.desktop code to use to reflectively locate the class and instantiate it. This fix removes that entirely from the launcher code and the desktop module now looks it up internally via a simple platform proxy class. This builds on all platforms and we rely on existing printing tests to verify that we can still locate the implementation class. The new regression test just verifies the system property name space is no longer polluted. I didn't find any test (apart from this new one) that references it. Since that system property has been around for a long time I am thinking I should file a CSR to document its removal .. unless there is a concensus it is not necessary. -phil.
[OpenJDK 2D-Dev] RFR: 8214002 Cannot use italic font style if the font has embeded bitmap
Hello.
Could you review the fix ?
Issue:
Cannot use italic font style if the font has embeded bitmap.
Bug:https://bugs.openjdk.java.net/browse/JDK-8214002
Change: https://cr.openjdk.java.net/~itakiguchi/8214002/webrev.00/
It seems it's side-effect for:
8204929: Fonts with embedded bitmaps are not always rotated
Test instruction and screen shot are in JDK-8214002.
Thanks,
Ichiroh Takiguchi
IBM Japan, Ltd.
On 2018-07-27 20:22, Ichiroh Takiguchi wrote:
Hello.
According to my investigation, FT_Render_Glyph() was not called
even if FT_GlyphSlot_Oblique() was called.
=
if (ftglyph->format == FT_GLYPH_FORMAT_OUTLINE) {
FT_Render_Glyph(ftglyph, FT_LOAD_TARGET_MODE(target)); <<===
}
=
It seemed FT_Load_Glyph() and renderFlags affected this issue.
On my Windows,
For "MS Mincho" with italic, renderFlags was "FT_LOAD_TARGET_MONO |
FT_LOAD_NO_BITMAP | FT_LOAD_RENDER".
I also tested "Meiryo" font (it could handle italic style)
For "Meiryo" with italic, renderFlags was "FT_LOAD_TARGET_MONO |
FT_LOAD_RENDER".
I think, after FT_LOAD_NO_BITMAP is turned on, FT_LOAD_RENDER should
be turned off.
So how about following fix ?
=
diff -r 1edcf36fe15f
src/java.desktop/share/native/libfontmanager/freetypeScaler.c
---
a/src/java.desktop/share/native/libfontmanager/freetypeScaler.c Wed
Jul 18 11:57:51 2018 -0400
+++
b/src/java.desktop/share/native/libfontmanager/freetypeScaler.c Fri
Jul 27 19:44:12 2018 +0900
@@ -700,6 +700,9 @@
if (!context->useSbits) {
renderFlags |= FT_LOAD_NO_BITMAP;
+if (context->doBold || context->doItalize) {
+renderFlags &= ~FT_LOAD_RENDER;
+}
}
/* NB: in case of non identity transform
=
On 2018-07-25 19:29, Ichiroh Takiguchi wrote:
Hello.
I'm using jdk-11+23 build on Japanese Windows 7.
I ran Font2DTest Demo, then select like:
Font: MS Mincho
Range: Basic Latin
Method: drawString
Size: 24
Style: Italic
But style was not changed to Italic.
Antialiasing and Fractional Metrics did not affect this issue.
I assume it's side-effect for:
8204929: Fonts with embedded bitmaps are not always rotated
Could you check it ?
Thanks,
Ichiroh Takiguchi
IBM Japan, Ltd.
Re: [OpenJDK 2D-Dev] Segmentation fault caused by Java2D Disposer, crashing application running on JBoss
Whenever I have seen this before it is due to removing files from the /tmp directory that are still in use. That's an error on your end. This mailing list is for developing the OpenJDK. Not support. And the binary distribution you are using is one provided by Redhat. -phil. On 11/9/18 7:12 PM, Simon DEVINEAU wrote: Hello the Java 2d API Team, I am writing to you because since several days in production, my jboss server crashes without any notice in the log. We don't succeed to generate an appropriate dump but the error file indicates that the JAVA2D disposer thread is responsible of the crash. # A fatal error has been detected by the Java Runtime Environment: # # SIGSEGV (0xb) at pc=0x003be4a75f05, pid=39931, tid=140659847448320 # Problematic frame: # C [libc.so.6+0x75f05] Current thread (0x7fed5c0e5000): JavaThread "Java2D Disposer" daemon [_thread_in_native, id=40219, stack(0x7fedec2dc000,0x7fedec3dd000)] siginfo:si_signo=SIGSEGV: si_errno=0, si_code=128 (), si_addr=0x Our environment * jboss : JBOSS EAP 6.2 * jvm : 1.7.0.121-2.6.8.1.el6_8 * OS:Red Hat Enterprise Linux Server release 6.9 (Santiago) * uname:Linux 2.6.32-696.30.1.el6.x86_64 * libc:glibc 2.12 We have no idea how to reproduce this bug I have to admit after reading this file and some searches on the internet, I don't really understand what happens. It would be a bug when the Java 2D thread try to dispose some native objects generated by the use of java.awt library ? If I well research, it might be linked to those bugs * https://bugs.openjdk.java.net/browse/JDK-7103530 * https://bugs.openjdk.java.net/browse/JDK-6962367 which references 6953445I don't have access to. I attach to this email our error file with some ip and path anonymization. Would it be possible to have your help on this ? Thank you in advance for your answer Simon Ce message et toutes les pieces jointes sont confidentiels et etablis a l'attention exclusive de leurs destinataires. Si vous n’etes pas destinataire de ce message, merci de le detruire et d’en avertir immediatement l’expediteur. Toute lecture non autorisee, utilisation ou diffusion, totale ou partielle, est interdite. L’Internet ne permettant pas de garantir l’integrite des messages, leur contenu ne constitue pas un engagement de la part de Malakoff Mederic. Malakoff Mederic decline toute responsabilite au titre de ce message s’il a ete modifie ou falsifie ainsi qu’en cas de virus recu par ce biais. SI2M, groupement d'interet economique, inscrit au repertoire SIRENE sous le numero SIREN 478 042 831, dont le siege social est situe 21 rue Laffitte, 75009 Paris.
Re: [OpenJDK 2D-Dev] RFR(XS): 8213944: Fix AIX build after the removal of Xrandr.h and add a configure check for it
On 11/15/2018 05:06 PM, Volker Simonis wrote:
> can I please have a review for the following small change:
>
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8213944/
*) I tested it on platform without libxrandr-dev installed, and configure
reported the failure
appopriately.
*) Indent looks off at L2207 here:
2205 static char *get_output_screen_name(JNIEnv *env, int screen) {
2206 #ifdef _AIX
2207 return NULL;
2208 #else
2209 if (!awt_XRRGetScreenResources || !awt_XRRGetOutputInfo) {
Thanks,
-Aleksey
signature.asc
Description: OpenPGP digital signature
[OpenJDK 2D-Dev] [12] RFR(XS) JDK-8212875: ftp: links for tiff/TTN2.draft.txt do not respond
Hello All, Please review the following fix in JDK12: Bug : https://bugs.openjdk.java.net/browse/JDK-8212875 Webrev : http://cr.openjdk.java.net/~jdv/8212875/webrev.00/ TIFF Technical Note #2 link is broken at many places in our code. I tried finding replacement for broken link, unfortunately there are no reliable source. So I have removed broken links. Thanks, Jay
Re: [OpenJDK 2D-Dev] RFR(XS): 8213944: Fix AIX build after the removal of Xrandr.h and add a configure check for it
On Thu, Nov 15, 2018 at 6:01 PM Philip Race wrote: > > PS I am not sure why xrandr headers would not be available for AIX. > They are a standard part of the xdistribution. > I'm not an X11 guru, but as far as I understand, xrandr is an extension and as such it doesn't have to be supported by every implementation. To the best of my knowledge (I've just started another poll among some experts) AIX doesn't support Xrandr and does not have the corresponding headers. > If true, think what you are going to have to do is add a > --with-xrandr-include option > and provide it that way. > What if there are no standard Xrandr headers on a platform? Do you really want to force users to get them from some dubious sources just for building the OpenJDK? Sorry, but I don't think that's a good solution. Than I'd rather prefer the ugly ifdefs (or I check the headers back in again in an AIX-specific directory :) Thank you and best regards, Volker > -phil. > > On 11/15/18, 8:55 AM, Philip Race wrote: > > Hmm. I don't like the ifdefs. > > > > Xrandr is a requirement for the build. If its not there at runtime > > that's OK. > > > > -phil. > > > > On 11/15/18, 8:06 AM, Volker Simonis wrote: > >> Hi, > >> > >> can I please have a review for the following small change: > >> > >> http://cr.openjdk.java.net/~simonis/webrevs/2018/8213944/ > >> https://bugs.openjdk.java.net/browse/JDK-8213944 > >> > >> Change JDK-8210863 removed the Xrandr.h/randr.h headers from the > >> OpenJDK sources but forgot to add a configure check for the Xrandr > >> extension which is now a build dependency. > >> > >> The change also broke the AIX build. AIX never supported Xrandr, but > >> that was only detected at runtime, when the JDK was unable to > >> dynamically load libXrand.so. Now, without Xrandr.h/randr.h in the > >> source tree any more, we have to conditionally compile some parts of > >> src/java.desktop/unix/native/libawt_xawt/awt/awt_GraphicsEnv.c such > >> that it doesn't require the definitions and declarations from > >> Xrandr.h/randr.h any more. > >> > >> Thank you and best regards, > >> Volker
Re: [OpenJDK 2D-Dev] [12] RFR JDK-8211795: ArrayIndexOutOfBoundsException in PNGImageReader after JDK-6788458
Hi Sergey, As discussed offline I did more analysis on whether we can use common variable to determine number of bands. Since we have "outputSampleSize.length - 1" and "inputBands + 1" kind of things. Actually scale array will be used on input data(ps[]), so we should use input bands value to create and use scale array. Before JDK-6788458 there was no difference between input bands and output bands so we didn't see any issue. But after JDK-6788458 number of bands data is different between input and output data for PNG_COLOR_RGB/GRAY having tRNS chunk. So I have made change to use inputBands data for creation and use of scale array. Ran complete imageio jtreg and JCK tests there are no failures. Please find updated webrev for review: http://cr.openjdk.java.net/~jdv/8211795/webrev.01/ Thanks, Jay -Original Message- From: Jayathirth D V Sent: Wednesday, November 14, 2018 1:09 PM To: Sergey Bylokhov; 2d-dev Subject: RE: [OpenJDK 2D-Dev] [12] RFR JDK-8211795: ArrayIndexOutOfBoundsException in PNGImageReader after JDK-6788458 Hi Sergey, Thanks for the review. As you pointed out, yes the AIOOB is happening for ps[b]. I have updated the analysis in JBS bug also. Basically the calculation of numBands is not proper because we take numBands value from destination image. This destination image will have extra alpha channel for Gray or RGB input data(ps[]) which will throw AIOOB. So we need to update the logic of how we calculate numBands different for PNG Gray/RGB image havng tRNS chunk. Fortunately, webrev.00 is actually doing this job. Regarding whether we need to change scale array logic : We expect first 3 channel to be RGB and first channel to be Gray for PNG_COLOR_RGB and PNG_COLOR_GRAY respectively. So just updating numBands information will create proper scale array. So there is no need to change the scale array logic. History JDK-6788458 : Toolkit was able to show transparent color information for RGB/Gray PNG image when it has tRNS chunk, but ImageIO didn't support it. To use tRNS data and show transparent color in output image we needed to add extra alpha channel for PNG RGB/Gray image with tRNS chunk. But fix present in JDK-6788458 didn't handle the case where bitDepth adjustment is needed and we are using band information from output image(having extra alpha channel) on input image which has no alpha channel. Change in numBands logic for this bug fixes that issue. Thanks, Jay -Original Message- From: Sergey Bylokhov Sent: Wednesday, November 14, 2018 4:07 AM To: Jayathirth D V; 2d-dev Subject: Re: [OpenJDK 2D-Dev] [12] RFR JDK-8211795: ArrayIndexOutOfBoundsException in PNGImageReader after JDK-6788458 Hi, Jay. Can you please provide some more detail about this bug. > Root cause : In JDK-6788458 we are adding extra alpha channel for destination > whenever we have tRNS chunk. But the number of bands in bitDepth scale array > was not changed when we have tRNS chunk. This is causing > ArrayIndexOutOfBoundsException for scale array. As far as I understand the AIOOB is occurred when we access ps[b] at line 1308 not when we access the scale array, because the scale array is created as "scale = new int[numBands][]". So maybe numBands should depends on the passRow? or the creation of scale[][xxx] should be updated? BTW this code uses +1/-1 in a lot of places already, and it is not always clear why. -- Best regards, Sergey.
Re: [OpenJDK 2D-Dev] RFR(XS): 8213944: Fix AIX build after the removal of Xrandr.h and add a configure check for it
On Thu, Nov 15, 2018 at 5:31 PM Aleksey Shipilev wrote:
>
> On 11/15/2018 05:06 PM, Volker Simonis wrote:
> > can I please have a review for the following small change:
> >
> > http://cr.openjdk.java.net/~simonis/webrevs/2018/8213944/
>
> *) I tested it on platform without libxrandr-dev installed, and configure
> reported the failure
> appopriately.
>
Thanks for testing!
> *) Indent looks off at L2207 here:
>
> 2205 static char *get_output_screen_name(JNIEnv *env, int screen) {
> 2206 #ifdef _AIX
> 2207 return NULL;
> 2208 #else
> 2209 if (!awt_XRRGetScreenResources || !awt_XRRGetOutputInfo) {
>
You're right. Fixed it.
Thank you and best regards,
Volker
>
> Thanks,
> -Aleksey
>
Re: [OpenJDK 2D-Dev] RFR(XS): 8213944: Fix AIX build after the removal of Xrandr.h and add a configure check for it
On Thu, Nov 15, 2018 at 5:55 PM Philip Race wrote: > > Hmm. I don't like the ifdefs. > Mee too, but what else can we do, if there are no Xrandr headers available on a platform? Please see my answer to your other mail... > Xrandr is a requirement for the build. If its not there at runtime > that's OK. > > -phil. > > On 11/15/18, 8:06 AM, Volker Simonis wrote: > > Hi, > > > > can I please have a review for the following small change: > > > > http://cr.openjdk.java.net/~simonis/webrevs/2018/8213944/ > > https://bugs.openjdk.java.net/browse/JDK-8213944 > > > > Change JDK-8210863 removed the Xrandr.h/randr.h headers from the > > OpenJDK sources but forgot to add a configure check for the Xrandr > > extension which is now a build dependency. > > > > The change also broke the AIX build. AIX never supported Xrandr, but > > that was only detected at runtime, when the JDK was unable to > > dynamically load libXrand.so. Now, without Xrandr.h/randr.h in the > > source tree any more, we have to conditionally compile some parts of > > src/java.desktop/unix/native/libawt_xawt/awt/awt_GraphicsEnv.c such > > that it doesn't require the definitions and declarations from > > Xrandr.h/randr.h any more. > > > > Thank you and best regards, > > Volker
