Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
Hi, Shashi.
The big problem was that I was not able to reproduce this problem neither at my
end nor at the systems used for PIT testing. Only Sergey had produced this NPE
till now.
If you cannot reproduce this exception on the system w/o
printers, then it is interesting what will return this code:
259 ::EnumPrinters(PRINTER_ENUM_LOCAL | PRINTER_ENUM_CONNECTIONS,
260NULL, 4, pPrinterEnum, cbNeeded, ,
261);
262
263 if (cReturned > 0) {}
what will happen if you try to print something to the printer
which is returned in the case above?
--
Best regards, Sergey.
Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732
Hi Phil, There were 2 problems earlier. One is that, from the native it is possible to have no printers being associated with the system(causing to return null reference) and other problem in the implementation was that there may be no network printers and still return a valid array reference containing a null string. The first problem is fixed by handling null references returned from this function and other problem was that earlier there were different base conditions, for updating the reference and use it to create the printer name array which could produce a valid reference pointing to null string. Here is the updated Webrev which fixes these problems: http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.02/ The big problem was that I was not able to reproduce this problem neither at my end nor at the systems used for PIT testing. Only Sergey had produced this NPE till now. Thanks and regards, Shashi From: Phil Race Sent: Wednesday, November 28, 2018 11:19 PM To: Shashidhara Veerabhadraiah ; Prasanta Sadhukhan ; 2d-dev@openjdk.java.net Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732 I am not understanding you. I thought the problem to be we got an array of (say) 3 values (ie printer names) returned from native where some or all of the *values* were NULL. And I am saying we should in such a case in the native code, before returning, remove from the returned array all values that are NULL. That could result in an empty (zero length) array being returned from native but no null names .. -phil. On 11/26/18 10:23 PM, Shashidhara Veerabhadraiah wrote: The windows function EnumPrinters() returns a value that could be zero or greater. If it is zero we have no other option but to return null from the function. I do not think there is a way where we can always make sure we get a non-zero value considering the various system scenarios. So we should handle the null return values as well in the caller of this function I think. Here is the updated Webrev for test fix: http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.01/ Thanks and regards, Shashi From: Phil Race Sent: Tuesday, November 27, 2018 1:52 AM To: Prasanta Sadhukhan HYPERLINK "mailto:prasanta.sadhuk...@oracle.com;; Shashidhara Veerabhadraiah HYPERLINK "mailto:shashidhara.veerabhadra...@oracle.com;; HYPERLINK "mailto:2d-dev@openjdk.java.net"2d-dev@openjdk.java.net Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732 [Removed swing-dev as this as nothing to do with swing]. You mention in the bug eval that you don't need a new test because this is already covered by the test for 8153732. If that is the case then this bugid should be added to that test. Although it also looks like there are plenty of tests that provoke this .. if all you need is a system without any printers then this is a serious regression. I am not sure I am following why doCompare() is the place to fix this. If getRemotePrinterNames() is returning NULL strings, then maybe it should not ! I suggest to fix it there. -phil. On 11/26/18 7:51 AM, Prasanta Sadhukhan wrote: I am not against doCompare() changes. I am just saying run() changes are not needed. Regards Prasanta On 26-Nov-18 9:15 PM, Shashidhara Veerabhadraiah wrote: There is a possible case and that is the reason for this fix. The possible states for prevRemotePinters and currentRemotePrinters are null and valid values and they can reach those states at any time either because of network disconnect or any other OS related changes. With that in mind, this fix tries to eliminate the possible NPE cases. Thanks and regards, Shashi From: Prasanta Sadhukhan Sent: Monday, November 26, 2018 8:01 PM To: Shashidhara Veerabhadraiah HYPERLINK "mailto:shashidhara.veerabhadra...@oracle.com;; HYPERLINK "mailto:swing-...@openjdk.java.net"swing-...@openjdk.java.net; HYPERLINK "mailto:2d-dev@openjdk.java.net"2d-dev@openjdk.java.net Subject: Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732 On 26-Nov-18 6:51 PM, HYPERLINK "mailto:shashidhara.veerabhadra...@oracle.com"shashidhara.veerabhadra...@oracle.com wrote: Hi Prasanta, I think we should not create a behavior across the functions. doCompare() does only the comparison and it may be used for other purposes and is complete with respect to the comparison functionality. run() function has a different behavior as it needs to populate the prevRemotePrinters and then the currentRemotePrinters and then use the comparison functionality. I think this is a good way to do. Even without the if-else check, it does populates the prevRemotePrinters, no? if "prevRemotePrinters" is null and currentRemotePrinters is null, then no need to update. If currentRemotePrinters is null, then what is the point of using getRemotePrintersNames() to update prevRemotePrinters as
Re: [OpenJDK 2D-Dev] RFR: 8214002 Cannot use italic font style if the font has embeded bitmap
Hello Phil.
Test case is available.
Could you review the fix again ?
Bug:https://bugs.openjdk.java.net/browse/JDK-8214002
Change: https://cr.openjdk.java.net/~itakiguchi/8214002/webrev.02/
Thanks,
Ichiroh Takiguchi
On 2018-11-28 10:13, Philip Race wrote:
Oh .. there's no regression test. If you can't find one then I think
you
can write one relatively easily by selecting the known font - MS
Mincho,
making sure we are on Windows (don't try it on Mac even if the font
exists), draw the text to a BufferedImage in plain / regular style.
Repeat to a different BufferedImage in italic
Compare the images, and pass only if they differ.
-phil.
On 11/27/18, 5:08 PM, Ichiroh Takiguchi wrote:
Hello Phil.
Do you need me to push this ?
Yes, if possible.
Currently, no sponsor is assigned for this issue.
Ichiroh Takiguchi
On 2018-11-28 05:43, Phil Race wrote:
On 11/27/18 9:36 AM, Ichiroh Takiguchi wrote:
Hello Phil.
I don't have any concern about this fix.
I'm thinking why initial programmer used FT_LOAD_RENDER instead of
FT_LOAD_DEFAULT.
Probably that this was what we wanted to do in almost all cases and
it has
now turned out to be untrue ..
On my testing, this fix was fine for me.
Do you need me to push this ?
-phil.
Ichiroh Takiguchi
On 2018-11-27 03:59, Philip Race wrote:
It seems fine to me. What is your concern when you say :
But it may change font rendering behavior...
-phil
On 11/18/18, 9:35 PM, Ichiroh Takiguchi wrote:
Hello Phill.
I tested and checked your suggested code. [1]
It worked fine.
But it may change font rendering behavior...
Please review the fix ?
Bug:https://bugs.openjdk.java.net/browse/JDK-8214002
Change: https://cr.openjdk.java.net/~itakiguchi/8214002/webrev.01/
Thanks,
Ichiroh Takiguchi
IBM Japan, Ltd.
[1]
https://www.freetype.org/freetype2/docs/reference/ft2-base_interface.html#FT_LOAD_DEFAULT
On 2018-11-17 06:04, Phil Race wrote:
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.
