Looks fine.
On 11/01/16 12:00, prasanta sadhukhan wrote:
Hi Sergey, Phil,
I guess as we discussed offline, it probably is not possible to do the
fix in java as you thought and approved webrev.02 offline.
But as we found the fix, even though it works in osx9 it does not work
for osx10 because CTFontCreatePathForGlyph() cannot accept NaN transform
on osx10 onwards resulting in crash
/
Assertion failed: (transform_is_valid(m)), function CGMutablePathRef
CGPathCreateMutableCopyByTransformingPath(CGPathRef, const
CGAffineTransform *), file Paths/CGPath.cc, line 168. //
//Abort trap: 6//
/
The below webrev takes care of the crash is osx10.
http://cr.openjdk.java.net/~psadhukhan/8023213/webrev.03/
Can you please review and approve the same?
Regards
Prasanta
On 11/19/2015 7:46 PM, Sergey Bylokhov wrote:
I still think that possibly it will be better to block such transforms
on the upper level, somewhere in the java probably in CStrike.java.
On 17.11.15 9:12, prasanta sadhukhan wrote:
Hi Phil,
I have updated webrev to explicitly set glyph image width/height to be 0
in case of NaN transform. Could you please review it?
http://cr.openjdk.java.net/~psadhukhan/8023213/webrev.02/
Regards
Prasanta
On 11/17/2015 3:23 AM, Phil Race wrote:
Would it not be better to be explicit in this so as not to leave it to
chance ?
-phil.
On 11/05/2015 11:49 PM, prasanta sadhukhan wrote:
Hi Phil,
On 11/5/2015 4:54 AM, Philip Race wrote:
This does not look right to me. Who knows what data is on the
canvas ?
it is not clear that it is 'blank'. It could have data from a
previous glyph .. I am not
sure how you know for sure. I can see that canvas is re-used. There
is reference
to a "global shared canvas".
And the actual function you invoke is one of two : one for grey and
one for lcd and
looking at the grey one CGGI_CopyImageFromCanvasToAlphaInfo)
it utilises info->width and info->height which can't be NaN because
they
are uint16 but I don't know if they are valid .. and is the "image"
field
allocated to be 0 length ?
info->width & height is coming out to be 0 in case of NaN transform
therefore "image" field will be of 0 length so "empty" glyph will be
copied when we copy the "glyph image" from canvas (no matter what is
there in canvas) to info->image via this code
http://hg.openjdk.java.net/jdk9/client/jdk/file/298d3fe64572/src/java.desktop/macosx/native/libawt_lwawt/font/CGGlyphImages.m#l294
size_t destRow = y * destRowWidth;
size_t srcRow = y * srcRowWidth;
size_t x;
for (x = 0; x < destRowWidth; x++) {
UInt32 p = src[srcRow + x];
dest[destRow + x] = CGGI_ConvertBWPixelToByteGray(p);
where destRowWidth = destRow = info->width will be 0 so dest =
info->image will be of 0 length
Regards
Prasanta
Could you step through how this is all guaranteed
to be safe/correct ?
-phil.
On 11/2/15, 12:41 AM, prasanta sadhukhan wrote:
Hi Phil,
I have modified as per your review to populate GlyphInfo with
"empty" glyph
and also moved your existing test to "open"
I also added a Infinite Transform test along with your NaN
transform just incase (in fact Sergey informally asked me to check)
http://cr.openjdk.java.net/~psadhukhan/8023213/webrev.01/
Regards
Prasanta
On 10/30/2015 1:03 AM, Phil Race wrote:
Should this new check go before this :
CGGI_ClearCanvas(canvas, info);
since it is using info which is where you get NaN from.
And should we then populate the returned canvas and info to
ensure that we return an "empty" glyph rather than garbage values ?
It is not clear to me that this is occurring.
Why does the bug report not contain the evaluation below ?
Also why is there a new test ? I would expect SQE would
want to run the existing test to verify this fix.
Should we not just open the existing test ?
-phil.
On 10/13/2015 04:49 AM, prasanta sadhukhan wrote:
Gentle reminder for review
Regards
Prasanta
On 10/6/2015 3:25 PM, prasanta sadhukhan wrote:
Hello All,
Please review a fix for jdk9
Bug: https://bugs.openjdk.java.net/browse/JDK-8023213
webrev: http://cr.openjdk.java.net/~psadhukhan/8023213/webrev.00/
drawString takes long time in mac native call
CGContextShowGlyphsAtPoint() if NaN transform is used which
translated to x & y being NaN.
Fix is to prevent calling mac api for such invalid usage.
Regards
Prasanta
--
Best regards, Sergey.
