[OpenJDK 2D-Dev] RFR: JDK-8187100: support Variation Selectors(Resend)
Nakajima Akira
nakajima.akira at nttcom.co.jp
Wed Jun 20 05:10:16 UTC 2018
Hi Phil.
>> > Updated webrev
>> 02 _http://cr.openjdk.java.net/~srl/8187100/webrev.02/_
>> <http://cr.openjdk.java.net/%7Esrl/8187100/webrev.02/>
I think that this patch is better than my patch.
So I drop my patch.
Thanks.
Akira Nakajima
On 2018/06/20 5:38, Phil Race wrote:
>
>
> On 06/18/2018 10:28 AM, Toshio 5 Nakamura wrote:
>>
>> 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/_
>> <http://cr.openjdk.java.net/%7Esrl/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.
>>
>
> right and I was saying in my email of 11th June, that your way of
> detecting if a VS char is there could be more efficient
> It is somewhat improved in this update ..
>
>> (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.
>>
>
> I believe that if you are looking for a glyph for (say) U+NNNN then it
> should always come
> from the same slot of a composite font whether a variation selector is
> specified or not.
> Otherwise you may get glyphs with variations from a different font than
> the glyphs from
> the same script without variations and that may look very wrong.
> So I am not sure about that and maybe we'll change it in the future.
>
> I am running some general font tests to make sure this does not break
> anything,
>
> If that passes OK then we should be OK.
>
> -phil.
>
>>
>> - 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
>>
>> Inactive hide details for "Steven R. Loomis" ---2018/06/19
>> 02:02:35---Updated webrev 02
>> https://urldefense.proofpoint.com/v2/ur"Steven R. Loomis"
>> ---2018/06/19 02:02:35---Updated webrev 02 INVALID URI REMOVED
>> <INVALID%20URI%20REMOVED>
>>
>> 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/_
>> <http://cr.openjdk.java.net/%7Esrl/8187100/webrev.02/>
>>
>> On Thu, May 31, 2018 at 3:19 PM, Steven R. Loomis
>> <_srl at icu-project.org_ <mailto:srl at icu-project.org>> wrote:
>>
>> Updated webrev:
>>
>> _http://cr.openjdk.java.net/~srl/8187100/webrev.01/_
>> <http://cr.openjdk.java.net/%7Esrl/8187100/webrev.01/>
>>
>> On Fri, May 18, 2018 at 9:16 AM, Toshio 5 Nakamura
>> <_TOSHIONA at jp.ibm.com_ <mailto: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_
>> <mailto:philip.race at oracle.com>> wrote on 2018/05/18 13:39:35:
>>
>> > From: Philip Race <_philip.race at oracle.com_
>> <mailto:philip.race at oracle.com>>
>> > To: Toshio 5 Nakamura <_TOSHIONA at jp.ibm.com_
>> <mailto:TOSHIONA at jp.ibm.com>>
>> > Cc: 2d-dev <_2d-dev at openjdk.java.net_
>> <mailto: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/_
>> <http://cr.openjdk.java.net/%7Esrl/8187100/webrev.00/>
>> >
>> > Toshio Nakamura
>> >
>> > > From: "Steven R. Loomis" <_srl295 at gmail.com_
>> <mailto:srl295 at gmail.com>>
>> > > To: 2d-dev <_2d-dev at openjdk.java.net_
>> <mailto: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_
>> <mailto: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_ <mailto: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
>> > >
>>
>>
>>
>>
>
--
--------------------------------------
NTTコムウェア株式会社
技術企画部 技術SE部門 OSSセンタ
中島輝 Tel: 03-6713-3033（内線 +25-6730）
E-Mail: nakajima.akira at nttcom.co.jp
〒140-0002
東京都品川区東品川4-12-1
品川シーサイドサウスタワー5F東
--------------------------------------
More information about the 2d-dev
mailing list