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

2018-06-22 Thread Toshio 5 Nakamura 

Hi Phil, Steven,

Thank you so much for your support, and sorry for taking your time.

As Akira-san suggested, Harfbuzz has VS capability.
It seems difficult to use its glyph picking up mechanism in this case,
but its layout mechanism we can use. And, I tried to use it.

Regarding default UVS table, if composite fonts keep font slot,
we just need to check whether Base+VS is defined in Non-default UVS table
or not.
When Non-default UVS doesn't have the entry, we always use its default
glyph.

Best regards,

Toshio Nakamura



From:   Phil Race 
To: "Steven R. Loomis" 
Cc: Toshio 5 Nakamura , 2d-dev
<2d-dev@openjdk.java.net>
Date:   2018/06/23 07:34
Subject:    Re: [OpenJDK 2D-Dev] RFR: JDK-8187100: support Variation
    Selectors(Resend)



Oh .. so this is doing what I suggested a couple of emails back.
If we see a VS we always delegate to TextLayout  ?
Yes, it does simplify the patch a lot and I am perfectly OK with
this approach but I am surprised as, but Toshio was very keen on
having it there without TextLayout being needed .. why the change of
heart ?

But I don't get the removal (skipping) of the Default UVS table ?

-phil.

On 06/22/2018 03:26 PM, Steven R. Loomis wrote:
  Phil, does this help:


  
https://gist.github.com/srl295/a81ec3a8495d53b85f368a7872138e86#file-webrev-02-vs-03-diff

  it is a diff between the 02 and 03 versions.

  On Fri, Jun 22, 2018 at 2:59 PM Phil Race 
  wrote:
Please save me time and tell me where I will find the changes from
the .02 version ?
In particular I mean where are the changes associated with
"Use TextLayout (Harfbuzz) if VS appears." ?

-phil.

On 06/22/2018 12:59 PM, Steven R. Loomis wrote:
  Updated webrev:

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

  - Use TextLayout (Harfbuzz) if VS appears.
  - Composite font's behavior was changed to not change the
  font by VS.
  These changes made the patch so simplified.
  - add comment about suggested DejaVu version



  On Thu, May 31, 2018 at 3:19 PM Steven R. Loomis <
  s...@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 <
   toshi...@jp.ibm.com> wrote:
 Thank you for your review, Phil.
 I'm working to handle your points.

 Toshio Nakamura

 Philip Race  wrote on 2018/05/18
 13:39:35:

 > From: Philip Race 
 > To: Toshio 5 Nakamura 
 > Cc: 2d-dev <2d-dev@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->CallV

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

2018-06-22 Thread Phil Race 

Oh .. so this is doing what I suggested a couple of emails back.
If we see a VS we always delegate to TextLayout  ?
Yes, it does simplify the patch a lot and I am perfectly OK with
this approach but I am surprised as, but Toshio was very keen on
having it there without TextLayout being needed .. why the change of heart ?

But I don't get the removal (skipping) of the Default UVS table ?

-phil.

On 06/22/2018 03:26 PM, Steven R. Loomis wrote:

Phil, does this help:

https://gist.github.com/srl295/a81ec3a8495d53b85f368a7872138e86#file-webrev-02-vs-03-diff

it is a diff between the 02 and 03 versions.

On Fri, Jun 22, 2018 at 2:59 PM Phil Race <mailto:philip.r...@oracle.com>> wrote:


Please save me time and tell me where I will find the changes from
the .02 version ?
In particular I mean where are the changes associated with
"Use TextLayout (Harfbuzz) if VS appears." ?

-phil.

On 06/22/2018 12:59 PM, Steven R. Loomis wrote:

Updated webrev:

http://cr.openjdk.java.net/~srl/8187100/webrev.03
<http://cr.openjdk.java.net/%7Esrl/8187100/webrev.03>

- Use TextLayout (Harfbuzz) if VS appears.
- Composite font's behavior was changed to not change the font by VS.
These changes made the patch so simplified.
- add comment about suggested DejaVu version



On Thu, May 31, 2018 at 3:19 PM Steven R. Loomis
mailto:s...@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
mailto:toshi...@jp.ibm.com>> wrote:

Thank you for your review, Phil.
I'm working to handle your points.

Toshio Nakamura

Philip Race mailto:philip.r...@oracle.com>> wrote on 2018/05/18
13:39:35:

> From: Philip Race mailto:philip.r...@oracle.com>>
> To: Toshio 5 Nakamura mailto:toshi...@jp.ibm.com>>
> Cc: 2d-dev <2d-dev@openjdk.java.net
<mailto:2d-dev@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

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

2018-06-22 Thread Steven R. Loomis 
Phil, does this help:


https://gist.github.com/srl295/a81ec3a8495d53b85f368a7872138e86#file-webrev-02-vs-03-diff

it is a diff between the 02 and 03 versions.

On Fri, Jun 22, 2018 at 2:59 PM Phil Race  wrote:

> Please save me time and tell me where I will find the changes from the .02
> version ?
> In particular I mean where are the changes associated with
> "Use TextLayout (Harfbuzz) if VS appears." ?
>
> -phil.
>
> On 06/22/2018 12:59 PM, Steven R. Loomis wrote:
>
> Updated webrev:
>
> http://cr.openjdk.java.net/~srl/8187100/webrev.03
>
> - Use TextLayout (Harfbuzz) if VS appears.
> - Composite font's behavior was changed to not change the font by VS.
> These changes made the patch so simplified.
> - add comment about suggested DejaVu version
>
>
>
> On Thu, May 31, 2018 at 3:19 PM Steven R. Loomis 
> wrote:
>
>> Updated webrev:
>>
>> http://cr.openjdk.java.net/~srl/8187100/webrev.01/
>>
>> On Fri, May 18, 2018 at 9:16 AM, Toshio 5 Nakamura 
>> wrote:
>>
>>> Thank you for your review, Phil.
>>> I'm working to handle your points.
>>>
>>> Toshio Nakamura
>>>
>>> Philip Race  wrote on 2018/05/18 13:39:35:
>>>
>>> > From: Philip Race 
>>> > To: Toshio 5 Nakamura 
>>> > Cc: 2d-dev <2d-dev@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

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

2018-06-22 Thread Steven R. Loomis 
I'll generate a delta

El El vie, jun. 22, 2018 a las 2:59 PM, Phil Race 
escribió:

> Please save me time and tell me where I will find the changes from the .02
> version ?
> In particular I mean where are the changes associated with
> "Use TextLayout (Harfbuzz) if VS appears." ?
>
>
> -phil.
>
>
> On 06/22/2018 12:59 PM, Steven R. Loomis wrote:
>
> Updated webrev:
>
> http://cr.openjdk.java.net/~srl/8187100/webrev.03
>
> - Use TextLayout (Harfbuzz) if VS appears.
> - Composite font's behavior was changed to not change the font by VS.
> These changes made the patch so simplified.
> - add comment about suggested DejaVu version
>
>
>
> On Thu, May 31, 2018 at 3:19 PM Steven R. Loomis 
> wrote:
>
>> Updated webrev:
>>
>> http://cr.openjdk.java.net/~srl/8187100/webrev.01/
>>
>> On Fri, May 18, 2018 at 9:16 AM, Toshio 5 Nakamura 
>> wrote:
>>
>>> Thank you for your review, Phil.
>>> I'm working to handle your points.
>>>
>>> Toshio Nakamura
>>>
>>> Philip Race  wrote on 2018/05/18 13:39:35:
>>>
>>> > From: Philip Race 
>>> > To: Toshio 5 Nakamura 
>>> > Cc: 2d-dev <2d-dev@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" 
>>

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

2018-06-22 Thread Phil Race 
Please save me time and tell me where I will find the changes from the 
.02 version ?

In particular I mean where are the changes associated with
"Use TextLayout (Harfbuzz) if VS appears." ?

-phil.

On 06/22/2018 12:59 PM, Steven R. Loomis wrote:

Updated webrev:

http://cr.openjdk.java.net/~srl/8187100/webrev.03 
<http://cr.openjdk.java.net/%7Esrl/8187100/webrev.03>


- Use TextLayout (Harfbuzz) if VS appears.
- Composite font's behavior was changed to not change the font by VS.
These changes made the patch so simplified.
- add comment about suggested DejaVu version



On Thu, May 31, 2018 at 3:19 PM Steven R. Loomis <mailto:s...@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
mailto:toshi...@jp.ibm.com>> wrote:

Thank you for your review, Phil.
I'm working to handle your points.

Toshio Nakamura

Philip Race mailto:philip.r...@oracle.com>> wrote on 2018/05/18 13:39:35:

> From: Philip Race mailto:philip.r...@oracle.com>>
> To: Toshio 5 Nakamura mailto:toshi...@jp.ibm.com>>
> Cc: 2d-dev <2d-dev@openjdk.java.net
<mailto:2d-dev@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" mailto:srl...@gmail.com>>
> > To: 2d-dev <2d-dev@openjdk.java.net
<mailto:2d-dev@openjdk.java.net>>
> > Date: 2018/05/03 03:27
> > Subject: Re: [OpenJDK 2D-Dev] RFR: JDK-8187100: su

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

2018-06-22 Thread Steven R. Loomis 
Updated webrev:

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

- Use TextLayout (Harfbuzz) if VS appears.
- Composite font's behavior was changed to not change the font by VS.
These changes made the patch so simplified.
- add comment about suggested DejaVu version



On Thu, May 31, 2018 at 3:19 PM Steven R. Loomis 
wrote:

> Updated webrev:
>
> http://cr.openjdk.java.net/~srl/8187100/webrev.01/
>
> On Fri, May 18, 2018 at 9:16 AM, Toshio 5 Nakamura 
> wrote:
>
>> Thank you for your review, Phil.
>> I'm working to handle your points.
>>
>> Toshio Nakamura
>>
>> Philip Race  wrote on 2018/05/18 13:39:35:
>>
>> > From: Philip Race 
>> > To: Toshio 5 Nakamura 
>> > Cc: 2d-dev <2d-dev@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" 
>> > > 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 
>> wrote:
>> > > (Retrying as actual text)
>> > >
>> > > Support Unicode Variation Selectors.
>> > >
>> > > Code by my colleague Toshio Nakamura,  I added a simple test, and
>> > > in

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

2018-06-20 Thread Philip Race 

Composite fonts are imperfect. Hence exclusion ranges
which create other problems. And monospaced fonts, well, aren't ..

But I think opentype layout likes to take as much as it can from the
same font, and other cases should be the same. If VS were common
you might say otherwise.. or make sure the VS enabled font were the
preferred one. But we can leave it as is until we see a problem in practice.

-phil.

On 6/20/18, 4:47 PM, Toshio 5 Nakamura wrote:


Hi Phil,

Thank you for your comments.

That situation (mixing fonts by characters) sometimes
occurs in Japanese environment, and I naturally thought
displaying specified glyph has priority.

For example, a composite font has
FontA (contains Japanese Kanji Level 1/2 (basic)) and
FontB (contains Kanji Level 1/2/3/4 (basic+extension)).
Then, if using a Kanji Level 3 character among Kanji
Level 1/2 characters, FontB appears among FontA.

The behavior of composite font with VS is not described
in Unicode Standard. I'm not sure which behavior should
be chosen.

Best regards,
Toshio Nakamura

Inactive hide details for Phil Race ---2018/06/20 05:38:24---On 
06/18/2018 10:28 AM, Toshio 5 Nakamura wrote: >Phil Race ---2018/06/20 
05:38:24---On 06/18/2018 10:28 AM, Toshio 5 Nakamura wrote: >


From: Phil Race 
To: Toshio 5 Nakamura , "Steven R. Loomis" 


Cc: 2d-dev <2d-dev@openjdk.java.net>, srl...@gmail.com
Date: 2018/06/20 05:38
Subject: Re: [OpenJDK 2D-Dev] RFR: JDK-8187100: support Variation 
Selectors(Resend)








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+ 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_ 

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




Updated webrev 02
_http://cr.openj

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

2018-06-20 Thread Toshio 5 Nakamura 

Hi Phil,

Thank you for your comments.

That situation (mixing fonts by characters) sometimes
occurs in Japanese environment, and I naturally thought
displaying specified glyph has priority.

For example, a composite font has
FontA (contains Japanese Kanji Level 1/2 (basic)) and
FontB (contains Kanji Level 1/2/3/4 (basic+extension)).
Then, if using a Kanji Level 3 character among Kanji
Level 1/2 characters, FontB appears among FontA.

The behavior of composite font with VS is not described
in Unicode Standard. I'm not sure which behavior should
be chosen.

Best regards,
Toshio Nakamura



From:   Phil Race 
To: Toshio 5 Nakamura , "Steven R. Loomis"

Cc: 2d-dev <2d-dev@openjdk.java.net>, srl...@gmail.com
Date:   2018/06/20 05:38
Subject:    Re: [OpenJDK 2D-Dev] RFR: JDK-8187100: support Variation
    Selectors(Resend)





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/

  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+ 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

  From: "Steven R. Loomis" 
  To: Toshio 5 Nakamura 
  Cc: Philip Race , 2d-dev
  <2d-dev@openjdk.java.net>
      Date: 2018/06/19 02:02
  Subject: Re: [OpenJDK 2D-Dev] RFR: JDK-8187100: support Variation
  Selectors(Resend)
  Sent by: srl...@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 <
  s...@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 <
toshi...@jp.ibm.com> wrote:
Thank you for your review, Phil.
I'm working to handle your points.

Toshio Nakamura

Philip Race  wrote on 2018/05/18
13:39:35:

> From: Philip Race 
> To: Toshio 5 Nakamura 
> Cc: 2d-dev <2d-dev@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{ whi

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

2018-06-19 Thread Nakajima Akira 

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+ 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


From: "Steven R. Loomis" 
To: Toshio 5 Nakamura 
Cc: Philip Race , 2d-dev <2d-dev@openjdk.java.net>
Date: 2018/06/19 02:02
Subject: Re: [OpenJDK 2D-Dev] RFR: JDK-8187100: support Variation
Selectors(Resend)
Sent by: srl...@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@icu-project.org_ <mailto:s...@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
<_toshi...@jp.ibm.com_ <mailto:toshi...@jp.ibm.com>> wrote:
Thank you for your review, Phil.
I'm working to handle your points.

Toshio Nakamura

Philip Race <_philip.race@oracle.com_
<mailto:philip.r...@oracle.com>> wrote on 2018/05/18 13:39:35:

> From: Philip Race <_philip.race@oracle.com_
<mailto:philip.r...@oracle.com>>
> To: Toshio 5 Nakamura <_toshi...@jp.ibm.com_
<mailto:toshi...@jp.ibm.com>>
    > Cc: 2d-dev <_2d-...@openjdk.java.net_
    <mailto:2d-dev@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.

>

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

2018-06-19 Thread Phil Race 



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+ 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 



From: "Steven R. Loomis" 
To: Toshio 5 Nakamura 
Cc: Philip Race , 2d-dev <2d-dev@openjdk.java.net>
Date: 2018/06/19 02:02
Subject: Re: [OpenJDK 2D-Dev] RFR: JDK-8187100: support Variation 
Selectors(Resend)

Sent by: srl...@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@icu-project.org_ <mailto:s...@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
<_toshi...@jp.ibm.com_ <mailto:toshi...@jp.ibm.com>> wrote:
Thank you for your review, Phil.
I'm working to handle your points.

Toshio Nakamura

Philip Race <_philip.race@oracle.com_
<mailto:philip.r...@oracle.com>> wrote on 2018/05/18 13:39:35:

> From: Philip Race <_philip.race@oracle.com_
<mailto:philip.r...@oracle.com>>
> To: Toshio 5 Nakamura <_toshi...@jp.ibm.com_
<mailto:toshi...@jp.ibm.com>>
> Cc: 2d-dev <_2d-...@openjdk.java.net_
    <mailto:2d-dev@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

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

2018-06-18 Thread Steven R. Loomis 
Thank you. I did not add the note about DejaVu — it seems at least version
2.20 from 2007 has the Variation Selector support.


On Mon, Jun 18, 2018 at 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/~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
>
> [image: 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
>
> From: "Steven R. Loomis" 
> To: Toshio 5 Nakamura 
> Cc: Philip Race , 2d-dev <2d-dev@openjdk.java.net>
> Date: 2018/06/19 02:02
> Subject: Re: [OpenJDK 2D-Dev] RFR: JDK-8187100: support Variation
> Selectors(Resend)
> Sent by: srl...@gmail.com
> --
>
>
>
> Updated webrev 02 *http://cr.openjdk.java.net/~srl/8187100/webrev.02/*
> <http://cr.openjdk.java.net/~srl/8187100/webrev.02/>
>
> On Thu, May 31, 2018 at 3:19 PM, Steven R. Loomis <*s...@icu-project.org*
> > wrote:
>
>Updated webrev:
>
>*http://cr.openjdk.java.net/~srl/8187100/webrev.01/*
><http://cr.openjdk.java.net/~srl/8187100/webrev.01/>
>
>On Fri, May 18, 2018 at 9:16 AM, Toshio 5 Nakamura <
>*toshi...@jp.ibm.com* > wrote:
>Thank you for your review, Phil.
>I'm working to handle your points.
>
>Toshio Nakamura
>
>Philip Race <*philip.r...@oracle.com* > wrote
>on 2018/05/18 13:39:35:
>
>> From: Philip Race <*philip.r...@oracle.com* >
>> To: Toshio 5 Nakamura <*toshi...@jp.ibm.com* >
>> Cc: 2d-dev <*2d-dev@openjdk.java.net* <2d-dev@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,

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

2018-06-18 Thread Toshio 5 Nakamura 

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" 
To: Toshio 5 Nakamura 
Cc: Philip Race , 2d-dev
<2d-dev@openjdk.java.net>
Date:   2018/06/19 02:02
Subject:        Re: [OpenJDK 2D-Dev] RFR: JDK-8187100: support Variation
        Selectors(Resend)
Sent by:srl...@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 
wrote:
  Updated webrev:

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

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

   Toshio Nakamura

   Philip Race  wrote on 2018/05/18 13:39:35:

   > From: Philip Race 
   > To: Toshio 5 Nakamura 
   > Cc: 2d-dev <2d-dev@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/
   >

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

2018-06-18 Thread Steven R. Loomis 
Updated webrev 02 http://cr.openjdk.java.net/~srl/8187100/webrev.02/

On Thu, May 31, 2018 at 3:19 PM, Steven R. Loomis 
wrote:

> Updated webrev:
>
> http://cr.openjdk.java.net/~srl/8187100/webrev.01/
>
> On Fri, May 18, 2018 at 9:16 AM, Toshio 5 Nakamura 
> wrote:
>
>> Thank you for your review, Phil.
>> I'm working to handle your points.
>>
>> Toshio Nakamura
>>
>> Philip Race  wrote on 2018/05/18 13:39:35:
>>
>> > From: Philip Race 
>> > To: Toshio 5 Nakamura 
>> > Cc: 2d-dev <2d-dev@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" 
>> > > 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 
>> 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
>> > > Web

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

2018-06-11 Thread Phil Race 
l chars to glyph used even by drawGlyphVector If this can't be made 
more efficient, then maybe it should be just part of the TextLayout path 
as mentioned earlier. can we have a boolean maybeVariationSelector(char 
c) { return c >= HI_SURROGATE_START;} 99.99% of the time that one test will return false and then you know 
you will not see a variation selector, or a surrogate encoded variation 
selector
supplement code point since all possibilities are excluded. So you can 
write boolean maybe = maybeVariationSelector(unicodes[i+step]);

if (maybe) {
... only then do the detailed checking
}



56 env->ExceptionDescribe();
This is for debugging. Don't use it in production code.

Since we are trying to be careful about clearing exceptions, I am fairly 
sure

the allocation calls like this :

65 unicodes = env->NewIntArray(2); and 69 results = env->NewIntArray(2); 
may throw OutOfMemoryError as well as returning NULL. So cleanup: should 
start with if (env->ExceptionOccurred()) { env->ExceptionClear();
} This : 74 tmp = new jint[2]; would be more efficient as a stack 
allocation and then you don't need to free it either -phil.


On 05/31/2018 03:19 PM, Steven R. Loomis 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 
mailto:toshi...@jp.ibm.com>> wrote:


Thank you for your review, Phil.
I'm working to handle your points.

Toshio Nakamura

Philip Race mailto:philip.r...@oracle.com>> wrote on 2018/05/18 13:39:35:

> From: Philip Race mailto:philip.r...@oracle.com>>
> To: Toshio 5 Nakamura mailto:toshi...@jp.ibm.com>>
    > Cc: 2d-dev <2d-dev@openjdk.java.net
<mailto:2d-dev@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
    <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" mailto:srl...@gmail.com>>
> > To: 2d-dev <2d-dev@op

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

2018-05-31 Thread Steven R. Loomis 
Updated webrev:

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

On Fri, May 18, 2018 at 9:16 AM, Toshio 5 Nakamura 
wrote:

> Thank you for your review, Phil.
> I'm working to handle your points.
>
> Toshio Nakamura
>
> Philip Race  wrote on 2018/05/18 13:39:35:
>
> > From: Philip Race 
> > To: Toshio 5 Nakamura 
> > Cc: 2d-dev <2d-dev@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" 
> > > 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 
> 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:
> > > &g

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

2018-05-18 Thread Toshio 5 Nakamura 
Thank you for your review, Phil.
I'm working to handle your points.

Toshio Nakamura

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

> From: Philip Race <philip.r...@oracle.com>
> To: Toshio 5 Nakamura <toshi...@jp.ibm.com>
> Cc: 2d-dev <2d-dev@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" <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/
> >
> > 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
>

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

2018-05-17 Thread Philip Race 

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
>

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

2018-05-02 Thread Steven R. Loomis 
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  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
>
>
>

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

2018-04-25 Thread Toshio 5 Nakamura 

Hi Steven,

Thank you for becoming the sponsor of my proposal and creating an official
Webrev and tests.
Of course, they are fine for me.

Toshio Nakamura



From:   "Steven R. Loomis" 
To: 2d-dev <2d-dev@openjdk.java.net>
Date:   2018/04/26 00:41
Subject:[OpenJDK 2D-Dev] RFR: JDK-8187100: support Variation Selectors
(Resend)
Sent by:"2d-dev" <2d-dev-boun...@openjdk.java.net>



(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