On 10.11.16 19:11, Philip Race wrote:
Anyway based on the report that you've used this for months in
production I can give this a "+1"
Sergey already reviewed too.
Who is going to push this ?
I'll push it today.
-phil.
On 11/10/16, 7:58 AM, Dmitry Batrak wrote:
Updated webrev (removed wild card import):
http://cr.openjdk.java.net/~avu/JDK-8169202/webrev.03/
<http://cr.openjdk.java.net/%7Eavu/JDK-8169202/webrev.03/>
As for UnicodeScalarValue, I couldn't find its description (or the
description of an equivalent type UTF32Char) on developer.apple.com
<http://developer.apple.com> either. It's defined at MacTypes.h, at
the same place where UniChar is defined, like this:
/* ...
UnicodeScalarValue, A complete Unicode character in UTF-32
format, with
UTF32Char values from 0 through 0x10FFFF
(excluding the surrogate
range 0xD800-0xDFFF and
certain disallowed values)
... */
typedef UInt32 UnicodeScalarValue;
For what it's worth, the proposed change is part of OpenJDK-based
build bundled with JetBrains IDEs (in production since July this
year), and there are no known issues related to it, either reported by
users, found in IDEs testing or in JDK testing (AWT/Swing OpenJDK 8
jtreg tests).
Best regards,
Dmitry Batrak
On Wed, Nov 9, 2016 at 11:55 PM, Phil Race <[email protected]
<mailto:[email protected]>> wrote:
29 import java.awt.*;
We avoid wild card imports - even in tests.
Can you provide a pointer to any Apple docs on UnicodeScalarValue.
I can't find it .. I presume its a typedef of int so it can hold a
composed
unicode code point outside the BMP ..
By already existing support I guess you refer to this :-
592 CGGI_CreateImageForUnicode
593 (CGGI_GlyphCanvas *canvas, const AWTStrike *strike,
594 const CGGI_RenderingMode *mode, const UnicodeScalarValue uniChar)
...
602 if (uniChar > 0xFFFF) {
603 UTF16Char charRef[2];
604 CTS_BreakupUnicodeIntoSurrogatePairs(uniChar, charRef);
...
This code was a bit more work to review than I was expecting because
of having to follow things around to see if numGlyphs != num16bitChars
was a problem or if the "non-negative" cases where we already have
a glyph were handled properly. And I'm still not entirely sure.
What reg. tests and other general testing has been done with this
fix ?
-phil.
On 11/07/2016 06:52 AM, Dmitry Batrak wrote:
Hello,
I'd like to propose a patch for
https://bugs.openjdk.java.net/browse/JDK-8169202
<https://bugs.openjdk.java.net/browse/JDK-8169202>.
I have a Contributor status via agreement signed by JetBrains,
hope someone can sponsor the patch.
This repeats the proposal I've sent earlier to this mailing list,
adding now a reference to the created OpenJDK issue.
Currently, if requested font cannot render a Unicode character
represented by a surrogate pair,
no substitution is performed - Font.canDisplay will return false,
and the character will be
rendered as a 'missing' glyph. This behaviour doesn't violate any
specification, but it looks like
it can be easily improved, as underlying OS framework used under
the hood does support surrogate pairs.
The proposed change consists of two parts. First part is
adjusting the code in CoreTextSupport.m
to handle surrogate pairs while performing char-to-glyph mapping,
by encoding non-displayable
surrogate pairs using negative values of the codepoint, similar
to how non-displayable BMP characters
are encoded. Second part is fixing the rendering code (in
CGGlyphImages.m), where wrong type was used
to pass character values around, so that code for surrogate pairs
handling, already present there,
could work.
Webrev for the fix is available at
http://cr.openjdk.java.net/~avu/JDK-8169202/webrev.02/
<http://cr.openjdk.java.net/%7Eavu/JDK-8169202/webrev.02/>
(kindly posted by my colleague, having access to
cr.openjdk.java.net <http://cr.openjdk.java.net>).
Best regards,
Dmitry Batrak
--
Best regards, Sergey.