[OpenJDK 2D-Dev] RFR: JDK-8187100: support Variation Selectors(Resend)

Philip Race philip.race at oracle.com
Fri May 18 04:39:35 UTC 2018


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" <srl295 at gmail.com>
> > To: 2d-dev <2d-dev at 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-bounces at 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 <srl295 at 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
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20180517/5e9436b0/attachment.html>


More information about the 2d-dev mailing list