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

Toshio 5 Nakamura TOSHIONA at jp.ibm.com
Mon Jun 18 17:28:35 UTC 2018 

Hi Phil,

Thank you very much for your great comments.
I tried to handle your comment as much as I could. Could you kindly
rereview it?
(So far, it contains only our contribution.)

> Updated webrev 02 http://cr.openjdk.java.net/~srl/8187100/webrev.02/

The following points were not directly applicable. I'd like to explain
them.

- Performance concern of fast path
We'd like to handle direct drawing such as Graphics2D.drawString, and
layout is not used in this case.
I tired to separate the original methods and VS capable ones to minimize a
impact.
Only if VS character appears, VS capable method will be called.
(except FontRunIterator.java:next, which uses a member variable and
couldn't use that way.)

- Complex code of CMap
Composite fonts need to search a glyph by two steps.
1 - Search VS specific glyph in each composed font.
2 - If it's not available in all fonts, search a glyph without VS.
I changed getGlyph method with allowFallback parameter,
hope it clears our purpose.

- isVariationSelector for two blocks
VS characters are U+FE00-U+FE0F and U+E0100-U+E01FF.
The latter is represented by Surrogate pair in a char array.
My previous code named the same for them, and it may cause a confusion.
I updated the names to isVariationSelectorBMP and isVariationSelectorExt.


Steven,
Thank you for your kind support.

Best regards,
Toshio Nakamura, IBM Japan



From:	"Steven R. Loomis" <srl at icu-project.org>
To:	Toshio 5 Nakamura <TOSHIONA at jp.ibm.com>
Cc:	Philip Race <philip.race at oracle.com>, 2d-dev
            <2d-dev at openjdk.java.net>
Date:	2018/06/19 02:02
Subject:	Re: [OpenJDK 2D-Dev] RFR: JDK-8187100: support Variation
            Selectors(Resend)
Sent by:	srl295 at gmail.com



Updated webrev 02 http://cr.openjdk.java.net/~srl/8187100/webrev.02/

On Thu, May 31, 2018 at 3:19 PM, Steven R. Loomis <srl at icu-project.org>
wrote:
  Updated webrev:

  http://cr.openjdk.java.net/~srl/8187100/webrev.01/

  On Fri, May 18, 2018 at 9:16 AM, Toshio 5 Nakamura <TOSHIONA at jp.ibm.com>
  wrote:
   Thank you for your review, Phil.
   I'm working to handle your points.

   Toshio Nakamura

   Philip Race <philip.race at oracle.com> wrote on 2018/05/18 13:39:35:

   > From: Philip Race <philip.race at oracle.com>
   > To: Toshio 5 Nakamura <TOSHIONA at jp.ibm.com>
   > Cc: 2d-dev <2d-dev at openjdk.java.net>
   > Date: 2018/05/18 13:39



   > Subject: Re: [OpenJDK 2D-Dev] RFR: JDK-8187100: support Variation
   > Selectors(Resend)
   >
   > 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/
   >
   > 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/
   > >
   > > 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/20180619/4dac8b5a/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: graycol.gif
Type: image/gif
Size: 105 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20180619/4dac8b5a/graycol.gif>

More information about the 2d-dev mailing list