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 <[email protected]
<mailto:[email protected]>> 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
<[email protected] <mailto:[email protected]>> 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
<[email protected] <mailto:[email protected]>> wrote:
Thank you for your review, Phil.
I'm working to handle your points.
Toshio Nakamura
Philip Race <[email protected]
<mailto:[email protected]>> wrote on 2018/05/18
13:39:35:
> From: Philip Race <[email protected]
<mailto:[email protected]>>
> To: Toshio 5 Nakamura <[email protected]
<mailto:[email protected]>>
> Cc: 2d-dev <[email protected]
<mailto:[email protected]>>
> 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" <[email protected]
<mailto:[email protected]>>
> > To: 2d-dev <[email protected]
<mailto:[email protected]>>
> > Date: 2018/05/03 03:27
> > Subject: Re: [OpenJDK 2D-Dev] RFR: JDK-8187100:
support Variation
> > Selectors (Resend)
> > Sent by: "2d-dev" <[email protected]
<mailto:[email protected]>>
> >
> > 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
<[email protected] <mailto:[email protected]>> 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
> >
