There's a lot to think about here but I have some requests to first
clean up the proposed code to conform to coding standards.
I see lots of lines > 80 chars. Please fix
I see if(foo){ and else{ which should be "if (foo) {" and "else {"
Basically always have a space before { and after ) and around "=" and "=="
One line that WAS
51 hb_codepoint_t u = (variation_selector==0) ? unicode :
variation_selector;
has no spaces around "==" almost certainly to avoid going over 80 chars.
Now you've broken the line you can fix that.
Eliminate all wild card imports and import exactly what is needed.
Maybe this is only about the test.
remove what looks like SCCS style comments on the @test line.
Make the test simply print a warning if the person didn't install fonts
where you expected.
Why? Because @ignore defaults to .. not being ignored.
For the JNI methods calls read the spec and see if calling them may
throw an exception
I'm looking at sequences like
env->SetIntArrayRegion(unicodes, 0, 2, tmp);
71 env->CallVoidMethod(font2D, sunFontIDs.f2dCharsToGlyphsMID, 2,
unicodes, results);
72 env->GetIntArrayRegion(results, 0, 2, tmp);
73 *glyph = tmp[0];
74 cleanup:
75 if (unicodes != NULL) env->DeleteLocalRef(unicodes);
76 if (results != NULL) env->DeleteLocalRef(results);
If call GetIntArrayRegion may leave a pending exception it needs to be
checked and cleared
before you make another call.
Look at the performance implications of things like the extra
work done now in FontUtilities.isSimpleChar() and see how to minimise
it since it could affect all text rendering in a way that is measurable
in at least some way.
I idly wonder if
public static boolean isBaseChar(int charCode){ ...
might be more cleanly or efficiently implemented with a switch ?
Not sure.
-phil.
On 5/14/18, 1:28 AM, Toshio 5 Nakamura wrote:
Could someone review our proposal to support Unicode Variation Selectors?
> Bug: https://bugs.openjdk.java.net/browse/JDK-8187100
> Webrev: http://cr.openjdk.java.net/~srl/8187100/webrev.00/
<http://cr.openjdk.java.net/%7Esrl/8187100/webrev.00/>
Toshio Nakamura
> From: "Steven R. Loomis" <srl...@gmail.com>
> To: 2d-dev <2d-dev@openjdk.java.net>
> Date: 2018/05/03 03:27
> Subject: Re: [OpenJDK 2D-Dev] RFR: JDK-8187100: support Variation
> Selectors (Resend)
> Sent by: "2d-dev" <2d-dev-boun...@openjdk.java.net>
>
> I added a screenshot to https://bugs.openjdk.java.net/browse/JDK-8187100
> if anyone wants to see what the impact of this fix is
>
> On Wed, Apr 25, 2018 at 8:39 AM, Steven R. Loomis <srl...@gmail.com>
wrote:
> (Retrying as actual text)
>
> Support Unicode Variation Selectors.
>
> Code by my colleague Toshio Nakamura, I added a simple test, and
> include a test that was part of JDK 8187100. (Both tests are run
manually.)
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8187100
> Webrev: http://cr.openjdk.java.net/~srl/8187100/webrev.00/
<http://cr.openjdk.java.net/%7Esrl/8187100/webrev.00/>
>
> On 04/08/2018 11:46 PM, Toshio 5 Nakamura wrote:
> >
> > Hello
> >
> > IBM would like to propose Unicode Variation Selector[1] capability
to AWT
> > and Swing components.
> > (This proposal was posted to i18n-dev first, and I got a
suggestion to
> > discuss
> > in 2d-dev.)
> >
> > This proposal changed the following files:
> > src/java.desktop/share/classes/sun/font/CMap.java
> > src/java.desktop/share/classes/sun/font/CharToGlyphMapper.java
> > src/java.desktop/share/classes/sun/font/CompositeGlyphMapper.java
> > src/java.desktop/share/classes/sun/font/Font2D.java
> > src/java.desktop/share/classes/sun/font/FontRunIterator.java
> > src/java.desktop/share/classes/sun/font/FontUtilities.java
> > src/java.desktop/share/classes/sun/font/TrueTypeGlyphMapper.java
> > src/java.desktop/share/native/common/font/sunfontids.h
> > src/java.desktop/share/native/libfontmanager/hb-jdk-font.cc
> > src/java.desktop/share/native/libfontmanager/sunFont.c
> > src/java.desktop/share/classes/javax/swing/text/DefaultEditorKit.java
> > 542 lines will be changed.
> >
> > There are three parts.
> > 1) Adding CMap format 14 support
> > 2) Adding CharsToGlyphs functions support Variation Selector Sequences
> > 3) Swing text component's DEL and BS key operations change
> >
> >
> > How would I go about obtaining a sponsor?
> >
> > [1] _http://www.unicode.org/versions/Unicode10.0.0/ch23.pdf_
> > Chapter 23.4 Variation Selectors
> >
> > Best regards,
> >
> > Toshio Nakamura
> > IBM Japan
>
