[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] RFR: JDK-8187100: support Variation Selectors(Resend)
From: "Toshio 5 Nakamura" <TOSHIONA () jp ! ibm ! com>
Date: 2018-06-23 0:08:17
Message-ID: OFD3BB5948.07CE98BB-ON002582B4.00824552-492582B5.0000C22B () notes ! na ! collabserv ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
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 <philip.race@oracle.com>
To: "Steven R. Loomis" <srl@icu-project.org>
Cc: Toshio 5 Nakamura <TOSHIONA@jp.ibm.com>, 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 <philip.race@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
- 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 <
srl@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@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> wrote on 2018/05/18
13:39:35:
> From: Philip Race <philip.race@oracle.com>
> To: Toshio 5 Nakamura <TOSHIONA@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" <srl295@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-bounces@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@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
> >
[Attachment #5 (text/html)]
<html><body bgcolor="#FFFFFF"><p><font size="2">Hi Phil, Steven,</font><br><br><font \
size="2">Thank you so much for your support, and sorry for taking your \
time.</font><br><br><font size="2">As Akira-san suggested, Harfbuzz has VS \
capability.</font><br><font size="2">It seems difficult to use its glyph picking up \
mechanism in this case,</font><br><font size="2">but its layout mechanism we can use. \
And, I tried to use it.</font><br><br><font size="2">Regarding default UVS table, if \
composite fonts keep font slot,</font><br><font size="2">we just need to check \
whether Base+VS is defined in Non-default UVS table or not. </font><br><font \
size="2">When Non-default UVS doesn't have the entry, we always use its default \
glyph.</font><br><font size="2"><br>Best regards,<br><br>Toshio \
Nakamura<br></font><br><img width="16" height="16" \
src="cid:1__=8FBB0827DF11C3C28f9e8a93df938690918c8FB@" border="0" alt="Inactive hide \
details for Phil Race ---2018/06/23 07:34:06---Oh .. so this is doing what I \
suggested a couple of emails back. I"><font size="2" color="#424282">Phil Race \
---2018/06/23 07:34:06---Oh .. so this is doing what I suggested a couple of emails \
back. If we see a VS we always delegate t</font><br><br><font size="2" \
color="#5F5F5F">From: </font><font size="2">Phil Race \
<philip.race@oracle.com></font><br><font size="2" color="#5F5F5F">To: \
</font><font size="2">"Steven R. Loomis" \
<srl@icu-project.org></font><br><font size="2" color="#5F5F5F">Cc: \
</font><font size="2">Toshio 5 Nakamura <TOSHIONA@jp.ibm.com>, 2d-dev \
<2d-dev@openjdk.java.net></font><br><font size="2" color="#5F5F5F">Date: \
</font><font size="2">2018/06/23 07:34</font><br><font size="2" \
color="#5F5F5F">Subject: </font><font size="2">Re: [OpenJDK 2D-Dev] RFR: \
JDK-8187100: support Variation Selectors(Resend)</font><br><hr width="100%" size="2" \
align="left" noshade style="color:#8091A5; "><br><br><br>Oh .. so this is doing what \
I suggested a couple of emails back.<br>If we see a VS we always delegate to \
TextLayout ?<br>Yes, it does simplify the patch a lot and I am perfectly OK \
with<br>this approach but I am surprised as, but Toshio was very keen on<br>having it \
there without TextLayout being needed .. why the change of heart ?<br><br>But I don't \
get the removal (skipping) of the Default UVS table ?<br><br>-phil.<br><br>On \
06/22/2018 03:26 PM, Steven R. Loomis wrote: <ul><ul>Phil, does this help: <br><br> \
<a href="https://gist.github.com/srl295/a81ec3a8495d53b85f368a7872138e86#file-webrev-02-vs-03-diff"><u><font \
color="#0000FF">https://gist.github.com/srl295/a81ec3a8495d53b85f368a7872138e86#file-webrev-02-vs-03-diff</font></u></a><br><br>it \
is a diff between the 02 and 03 versions. <br><br>On Fri, Jun 22, 2018 at 2:59 PM \
Phil Race <<a href="mailto:philip.race@oracle.com"><u><font \
color="#0000FF">philip.race@oracle.com</font></u></a>> wrote: <ul>Please save me \
time and tell me where I will find the changes from the .02 version ?<br>In \
particular I mean where are the changes associated with<br>"Use TextLayout \
(Harfbuzz) if VS appears." ?<br><br>-phil.<br><br>On 06/22/2018 12:59 PM, Steven \
R. Loomis wrote: <ul><ul>Updated webrev: <br><br><a \
href="http://cr.openjdk.java.net/%7Esrl/8187100/webrev.03" target="_blank"><u><font \
color="#0000FF">http://cr.openjdk.java.net/~srl/8187100/webrev.03</font></u></a><br><br>- \
Use TextLayout (Harfbuzz) if VS appears.<br>- Composite font's behavior was changed \
to not change the font by VS.<br>These changes made the patch so simplified.<br>- add \
comment about suggested DejaVu version<br><br><br><br>On Thu, May 31, 2018 at 3:19 PM \
Steven R. Loomis <<a href="mailto:srl@icu-project.org" target="_blank"><u><font \
color="#0000FF">srl@icu-project.org</font></u></a>> wrote:<br>Updated webrev: \
<br><br><a href="http://cr.openjdk.java.net/%7Esrl/8187100/webrev.01/" \
target="_blank"><u><font \
color="#0000FF">http://cr.openjdk.java.net/~srl/8187100/webrev.01/</font></u></a><br><br>On \
Fri, May 18, 2018 at 9:16 AM, Toshio 5 Nakamura <<a \
href="mailto:TOSHIONA@jp.ibm.com" target="_blank"><u><font \
color="#0000FF">TOSHIONA@jp.ibm.com</font></u></a>> wrote: <ul><font \
size="2">Thank you for your review, Phil.<br>I'm working to handle your \
points.<br><br>Toshio Nakamura</font><br><tt><font size="2"><br>Philip Race \
<</font></tt><a href="mailto:philip.race@oracle.com" target="_blank"><tt><u><font \
size="2" color="#0000FF">philip.race@oracle.com</font></u></tt></a><tt><font \
size="2">> wrote on 2018/05/18 13:39:35:<br><br>> From: Philip Race \
<</font></tt><a href="mailto:philip.race@oracle.com" target="_blank"><tt><u><font \
size="2" color="#0000FF">philip.race@oracle.com</font></u></tt></a><tt><font \
size="2">><br>> To: Toshio 5 Nakamura <</font></tt><a \
href="mailto:TOSHIONA@jp.ibm.com" target="_blank"><tt><u><font size="2" \
color="#0000FF">TOSHIONA@jp.ibm.com</font></u></tt></a><tt><font \
size="2">><br>> Cc: 2d-dev <</font></tt><a \
href="mailto:2d-dev@openjdk.java.net" target="_blank"><tt><u><font size="2" \
color="#0000FF">2d-dev@openjdk.java.net</font></u></tt></a><tt><font \
size="2">><br>> Date: 2018/05/18 13:39</font></tt><p><tt><font \
size="2"><br>> Subject: Re: [OpenJDK 2D-Dev] RFR: JDK-8187100: support Variation \
<br>> Selectors(Resend)<br>> <br>> There's a lot to think about here but I \
have some requests to first<br>> clean up the proposed code to conform to coding \
standards.<br>> <br>> I see lots of lines > 80 chars. Please fix<br>> \
<br>> I see if(foo){ and else{ which should be "if (foo) {" and \
"else {"<br>> <br>> Basically always have a space before { and after \
) and around "=" and "=="<br>> <br>> One line that \
WAS<br>> 51 hb_codepoint_t u = (variation_selector==0) ? unicode : <br>> \
variation_selector;<br>> <br>> has no spaces around "==" almost \
certainly to avoid going over 80 chars.<br>> Now you've broken the line you can \
fix that.</font></tt><br><tt><font size="2"><br>> <br>> Eliminate all wild card \
imports and import exactly what is needed.<br>> Maybe this is only about the \
test.<br>> <br>> remove what looks like SCCS style comments on the @test \
line.<br>> Make the test simply print a warning if the person didn't install \
<br>> fonts where you expected.<br>> Why? Because @ignore defaults to .. not \
being ignored.<br>> <br>> For the JNI methods calls read the spec and see if \
calling them may <br>> throw an exception<br>> <br>> I'm looking at \
sequences like<br>> env->SetIntArrayRegion(unicodes, 0, 2, tmp);<br>> 71 \
env->CallVoidMethod(font2D, <br>> sunFontIDs.f2dCharsToGlyphsMID, 2, unicodes, \
results);<br>> 72 env->GetIntArrayRegion(results, 0, 2, tmp);<br>> \
73 *glyph = tmp[0];<br>> 74 cleanup:<br>> 75 if (unicodes \
!= NULL) env->DeleteLocalRef(unicodes);<br>> 76 if (results != NULL) \
env->DeleteLocalRef(results);<br>> </font></tt><br><tt><font size="2"><br>> \
If call GetIntArrayRegion may leave a pending exception it needs to <br>> be \
checked and cleared<br>> before you make another call.<br>> <br>> Look at \
the performance implications of things like the extra<br>> work done now in \
FontUtilities.isSimpleChar() and see how to minimise<br>> it since it could affect \
all text rendering in a way that is measurable<br>> in at least some way.<br>> \
<br>> I idly wonder if<br>> <br>> <br>> public static boolean \
isBaseChar(int charCode){ ...<br>> <br>> might be more cleanly or efficiently \
implemented with a switch ?<br>> <br>> Not sure.<br>> <br>> \
-phil.<br>> <br>> On 5/14/18, 1:28 AM, Toshio 5 Nakamura wrote: <br>> \
Could someone review our proposal to support Unicode Variation Selectors?<br>> \
> Bug: </font></tt><a href="https://bugs.openjdk.java.net/browse/JDK-8187100" \
target="_blank"><tt><u><font size="2" \
color="#0000FF">https://bugs.openjdk.java.net/browse/JDK-8187100</font></u></tt></a><tt><font \
size="2"><br>> > Webrev: </font></tt><a \
href="http://cr.openjdk.java.net/%7Esrl/8187100/webrev.00/" \
target="_blank"><tt><u><font size="2" \
color="#0000FF">http://cr.openjdk.java.net/~srl/8187100/webrev.00/</font></u></tt></a><tt><font \
size="2"><br>> <br>> Toshio Nakamura<br>> <br>> > From: "Steven \
R. Loomis" <</font></tt><a href="mailto:srl295@gmail.com" \
target="_blank"><tt><u><font size="2" \
color="#0000FF">srl295@gmail.com</font></u></tt></a><tt><font size="2">><br>> \
> To: 2d-dev <</font></tt><a href="mailto:2d-dev@openjdk.java.net" \
target="_blank"><tt><u><font size="2" \
color="#0000FF">2d-dev@openjdk.java.net</font></u></tt></a><tt><font \
size="2">><br>> > Date: 2018/05/03 03:27<br>> > Subject: Re: [OpenJDK \
2D-Dev] RFR: JDK-8187100: support Variation <br>> > Selectors (Resend)<br>> \
> Sent by: "2d-dev" <</font></tt><a \
href="mailto:2d-dev-bounces@openjdk.java.net" target="_blank"><tt><u><font size="2" \
color="#0000FF">2d-dev-bounces@openjdk.java.net</font></u></tt></a><tt><font \
size="2">><br>> > <br>> > I added a screenshot to </font></tt><a \
href="https://bugs.openjdk.java.net/browse/JDK-8187100" target="_blank"><tt><u><font \
size="2" color="#0000FF">https://bugs.openjdk.java.net/browse/JDK-8187100</font></u></tt></a><tt><font \
size="2"><br>> > if anyone wants to see what the impact of this fix is<br>> \
> <br>> > On Wed, Apr 25, 2018 at 8:39 AM, Steven R. Loomis \
<</font></tt><a href="mailto:srl295@gmail.com" target="_blank"><tt><u><font \
size="2" color="#0000FF">srl295@gmail.com</font></u></tt></a><tt><font size="2">> \
wrote:<br>> > (Retrying as actual text)<br>> > <br>> > Support \
Unicode Variation Selectors.<br>> > <br>> > Code by my colleague Toshio \
Nakamura, I added a simple test, and <br>> > include a test that was part of \
JDK 8187100. (Both tests are run manually.) <br>> > <br>> > Bug: \
</font></tt><a href="https://bugs.openjdk.java.net/browse/JDK-8187100" \
target="_blank"><tt><u><font size="2" \
color="#0000FF">https://bugs.openjdk.java.net/browse/JDK-8187100</font></u></tt></a><tt><font \
size="2"><br>> > Webrev: </font></tt><a \
href="http://cr.openjdk.java.net/%7Esrl/8187100/webrev.00/" \
target="_blank"><tt><u><font size="2" \
color="#0000FF">http://cr.openjdk.java.net/~srl/8187100/webrev.00/</font></u></tt></a><tt><font \
size="2"><br>> > <br>> > On 04/08/2018 11:46 PM, Toshio 5 Nakamura \
wrote:<br>> > ><br>> > > Hello<br>> > ><br>> > > \
IBM would like to propose Unicode Variation Selector[1] capability to AWT<br>> \
> > and Swing components.<br>> > > (This proposal was posted to \
i18n-dev first, and I got a suggestion to <br>> > > discuss<br>> > \
> in 2d-dev.)<br>> > ><br>> > > This proposal changed the \
following files:<br>> > > \
src/java.desktop/share/classes/sun/font/CMap.java<br>> > > \
src/java.desktop/share/classes/sun/font/CharToGlyphMapper.java<br>> > > \
src/java.desktop/share/classes/sun/font/CompositeGlyphMapper.java<br>> > > \
src/java.desktop/share/classes/sun/font/Font2D.java<br>> > > \
src/java.desktop/share/classes/sun/font/FontRunIterator.java<br>> > > \
src/java.desktop/share/classes/sun/font/FontUtilities.java<br>> > > \
src/java.desktop/share/classes/sun/font/TrueTypeGlyphMapper.java<br>> > > \
src/java.desktop/share/native/common/font/sunfontids.h<br>> > > \
src/java.desktop/share/native/libfontmanager/hb-jdk-font.cc<br>> > > \
src/java.desktop/share/native/libfontmanager/sunFont.c<br>> > > \
src/java.desktop/share/classes/javax/swing/text/DefaultEditorKit.java<br>> > \
> 542 lines will be changed.<br>> > ><br>> > > There are three \
parts.<br>> > > 1) Adding CMap format 14 support<br>> > > 2) Adding \
CharsToGlyphs functions support Variation Selector Sequences<br>> > > 3) \
Swing text component's DEL and BS key operations change<br>> > ><br>> \
> ><br>> > > How would I go about obtaining a sponsor?<br>> > \
><br>> > > [1] _</font></tt><a \
href="http://www.unicode.org/versions/Unicode10.0.0/ch23.pdf_" \
target="_blank"><tt><u><font size="2" \
color="#0000FF">http://www.unicode.org/versions/Unicode10.0.0/ch23.pdf_</font></u></tt></a><tt><font \
size="2"><br>> > > Chapter 23.4 Variation Selectors<br>> > \
><br>> > > Best regards,<br>> > ><br>> > > Toshio \
Nakamura<br>> > > IBM Japan<br>> > \
</font></tt></ul></ul></ul></ul></ul></ul><br><br><BR> </body></html>
["graycol.gif" (image/gif)]
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic