Pushed… thanks! On Mon, Jun 25, 2018 at 11:40 AM Phil Race <philip.r...@oracle.com> wrote:
> Ok. Approved. > > -phil. > > On 06/25/2018 10:20 AM, Toshio 5 Nakamura wrote: > > Hi Phil, > > In other words, I just misunderstood your suggestion below. > Recent discussions led me correct understanding, and I took this way. > sorry for your confusion. > > > For example we could explore having isComplexCharCode return true for > these and > > directing all uses down the layout path ? Then we can maybe remove all > this logic from > > case used by the fast path code. > > Toshio Nakamura > ----- Forwarded by Toshio 5 Nakamura/Japan/IBM on 2018/06/26 02:09 ----- > > From: "Toshio 5 Nakamura" <toshi...@jp.ibm.com> <toshi...@jp.ibm.com> > To: Phil Race <philip.r...@oracle.com> <philip.r...@oracle.com> > Cc: 2d-dev <2d-dev@openjdk.java.net> <2d-dev@openjdk.java.net>, "Steven > R. Loomis" <s...@icu-project.org> <s...@icu-project.org> > Date: 2018/06/23 09:08 > Subject: Re: [OpenJDK 2D-Dev] RFR: JDK-8187100: support Variation > Selectors(Resend) > Sent by: "2d-dev" <2d-dev-boun...@openjdk.java.net> > <2d-dev-boun...@openjdk.java.net> > ------------------------------ > > > > 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 > > [image: 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]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 > > From: Phil Race <philip.r...@oracle.com> <philip.r...@oracle.com> > To: "Steven R. Loomis" <s...@icu-project.org> <s...@icu-project.org> > Cc: Toshio 5 Nakamura <toshi...@jp.ibm.com> <toshi...@jp.ibm.com>, 2d-dev > <2d-dev@openjdk.java.net> <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* > > <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.r...@oracle.com* <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 <*s...@icu-project.org* > <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* > <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* > <philip.r...@oracle.com>> wrote on > 2018/05/18 13:39:35: > > > From: Philip Race < > *philip.r...@oracle.com* > <philip.r...@oracle.com>> > > To: Toshio 5 Nakamura < > *toshi...@jp.ibm.com* > <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, 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" < > *srl...@gmail.com* <srl...@gmail.com>> > > > To: 2d-dev < > *2d-dev@openjdk.java.net* > <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* > <2d-dev-boun...@openjdk.java.net>> > > > > > > I added a screenshot to > > *https://bugs.openjdk.java.net/browse/JDK-8187100* > > <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* > <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* > > <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_* > > <http://www.unicode.org/versions/Unicode10.0.0/ch23.pdf_> > > > > Chapter 23.4 Variation > Selectors > > > > > > > > Best regards, > > > > > > > > Toshio Nakamura > > > > IBM Japan > > > > > > > > >
