Review Request Re: [PATCH] Fix bug to handle minus value glyph_id on Windows

Nakajima Akira nakajima.akira at nttcom.co.jp
Fri Jul 6 03:10:34 UTC 2018


Hi All.

I created github account today
  and re-posted this patch to github.
Please review the following fix:

patch: https://github.com/javafxports/openjdk-jfx/pull/125

Regards,
Akira Nakajima

--------------------------------------
Company: NTT Comware Corporation
Name: Akira Nakajima
E-Mail: nakajima.akira at nttcom.co.jp
OLA : http://www.oracle.com/technetwork/community/oca-486395.html#n
--------------------------------------

On 2018/06/27 15:02, Nakajima Akira wrote:
> This bug is happened on Windows.
>
> Sorry, I should use SHORTMASK instead of intMask.
> I re-send patch.
> Difference is name only.
> intMask -> SHORTMASK
>
>
>
> diff --git
> a/modules/javafx.graphics/src/main/java/com/sun/javafx/font/directwrite/DWGlyph.java
> b/modules/javafx.graphics/src/main/java/com/sun/javafx/font/directwrite/DWGlyph.java
>
> ---
> a/modules/javafx.graphics/src/main/java/com/sun/javafx/font/directwrite/DWGlyph.java
>
> +++
> b/modules/javafx.graphics/src/main/java/com/sun/javafx/font/directwrite/DWGlyph.java
>
> @@ -52,6 +52,7 @@
>      private static D2D1_COLOR_F WHITE = new D2D1_COLOR_F(1f, 1f, 1f, 1f);
>      private static D2D1_MATRIX_3X2_F D2D2_MATRIX_IDENTITY = new
> D2D1_MATRIX_3X2_F(1,0, 0,1, 0,0);
>
> +    static final int SHORTMASK = 0x0000ffff;
>
>      DWGlyph(DWFontStrike strike, int glyphCode, boolean drawShapes) {
>          this.strike = strike;
> @@ -303,12 +304,12 @@
>
>      @Override
>      public int getGlyphCode() {
> -        return run.glyphIndices;
> +        return ((int)run.glyphIndices & SHORTMASK);
>      }
>
>      @Override
>      public RectBounds getBBox() {
> -        return strike.getBBox(run.glyphIndices);
> +        return strike.getBBox((int)run.glyphIndices & SHORTMASK);
>      }
>
>      @Override
> @@ -321,7 +322,7 @@
>
>      @Override
>      public Shape getShape() {
> -        return strike.createGlyphOutline(run.glyphIndices);
> +        return strike.createGlyphOutline((int)run.glyphIndices &
> SHORTMASK);
>      }
>
>      @Override
> diff --git
> a/modules/javafx.graphics/src/main/java/com/sun/javafx/font/directwrite/DWGlyphLayout.java
> b/modules/javafx.graphics/src/main/java/com/sun/javafx/font/directwrite/DWGlyphLayout.java
>
> ---
> a/modules/javafx.graphics/src/main/java/com/sun/javafx/font/directwrite/DWGlyphLayout.java
>
> +++
> b/modules/javafx.graphics/src/main/java/com/sun/javafx/font/directwrite/DWGlyphLayout.java
>
> @@ -138,6 +138,7 @@
>          int i, j;
>          int[] iglyphs = new int[glyphCount];
>          int slotMask = slot << 24;
> +        final int SHORTMASK = 0x0000ffff;
>          boolean missingGlyph = false;
>          i = 0; j = rtl ? glyphCount - 1 : 0;
>          while (i < glyphCount) {
> @@ -145,7 +146,7 @@
>                  missingGlyph = true;
>                  if (composite) break;
>              }
> -            iglyphs[i] = glyphs[j] | slotMask;
> +            iglyphs[i] = ((int)glyphs[j] & SHORTMASK) | slotMask;
>              i++;
>              j+=step;
>          }
>
>
> Thanks.
> Akira Nakajima
>
>
> On 2018/06/27 14:52, Nakajima Akira wrote:
>> # This patch is separated from
>> http://mail.openjdk.java.net/pipermail/openjfx-dev/2018-June/022005.html
>>
>>
>> Java issue error of ArrayIndexOutOfBoundsException by run following Test
>> program on Windows.
>> This bug is that Java handles big glyph_id as minus value.
>> For Example,
>> Inside JavaFX, glyph_id 49496(Uint16) is now handled as -16040(short).
>>
>>
>> ScreenShots of before/after applying this patch.
>> Windows10 x64 (openjfx11 and Oracle JDK 10.0.1)
>> https://drive.google.com/drive/folders/1UHPfCbQF4X_SSvjqgNGGKfEm-8GiNP80
>>
>>
>>
>> # NOTICE
>> Now Windows API provide glyphIndices as UINT16.
>> Exsample)
>> https://msdn.microsoft.com/en-us/library/windows/desktop/dd316625%28v=vs.85%29.aspx
>>
>>
>>
>> But originally, Windows API should provide as UINT32.
>> Because CMAP Format 8,12,13 have UINT32.
>> https://docs.microsoft.com/en-us/typography/opentype/spec/cmap
>> https://developer.apple.com/fonts/TrueType-Reference-Manual/RM06/Chap6cmap.html
>>
>>
>>
>> I have not reported to Microsoft, since I guess Microsoft knows this
>> problem.
>>
>> CMap.java provide char (UINT16).
>> Freetype2 provide UINT32.
>>
>>
>>
>> Thanks.
>> Akira Nakajima
>>
>>
>> [PATCH]
>> javafx.graphics/src/main/java/com/sun/javafx/font/directwrite/DWGlyph.java
>>
>> javafx.graphics/src/main/java/com/sun/javafx/font/directwrite/DWGlyphLayout.java
>>
>>
>>
>> =========================
>> PATCH
>> =========================
>> diff --git
>> a/modules/javafx.graphics/src/main/java/com/sun/javafx/font/directwrite/DWGlyph.java
>>
>> b/modules/javafx.graphics/src/main/java/com/sun/javafx/font/directwrite/DWGlyph.java
>>
>>
>> ---
>> a/modules/javafx.graphics/src/main/java/com/sun/javafx/font/directwrite/DWGlyph.java
>>
>>
>> +++
>> b/modules/javafx.graphics/src/main/java/com/sun/javafx/font/directwrite/DWGlyph.java
>>
>>
>> @@ -52,6 +52,7 @@
>>      private static D2D1_COLOR_F WHITE = new D2D1_COLOR_F(1f, 1f, 1f,
>> 1f);
>>      private static D2D1_MATRIX_3X2_F D2D2_MATRIX_IDENTITY = new
>> D2D1_MATRIX_3X2_F(1,0, 0,1, 0,0);
>>
>> +    static final int intMask = 0x0000ffff;
>>
>>      DWGlyph(DWFontStrike strike, int glyphCode, boolean drawShapes) {
>>          this.strike = strike;
>> @@ -303,12 +304,12 @@
>>
>>      @Override
>>      public int getGlyphCode() {
>> -        return run.glyphIndices;
>> +        return ((int)run.glyphIndices & intMask);
>>      }
>>
>>      @Override
>>      public RectBounds getBBox() {
>> -        return strike.getBBox(run.glyphIndices);
>> +        return strike.getBBox((int)run.glyphIndices & intMask);
>>      }
>>
>>      @Override
>> @@ -321,7 +322,7 @@
>>
>>      @Override
>>      public Shape getShape() {
>> -        return strike.createGlyphOutline(run.glyphIndices);
>> +        return strike.createGlyphOutline((int)run.glyphIndices &
>> intMask);
>>      }
>>
>>      @Override
>> diff --git
>> a/modules/javafx.graphics/src/main/java/com/sun/javafx/font/directwrite/DWGlyphLayout.java
>>
>> b/modules/javafx.graphics/src/main/java/com/sun/javafx/font/directwrite/DWGlyphLayout.java
>>
>>
>> ---
>> a/modules/javafx.graphics/src/main/java/com/sun/javafx/font/directwrite/DWGlyphLayout.java
>>
>>
>> +++
>> b/modules/javafx.graphics/src/main/java/com/sun/javafx/font/directwrite/DWGlyphLayout.java
>>
>>
>> @@ -138,6 +138,7 @@
>>          int i, j;
>>          int[] iglyphs = new int[glyphCount];
>>          int slotMask = slot << 24;
>> +        final int intMask = 0x0000ffff;
>>          boolean missingGlyph = false;
>>          i = 0; j = rtl ? glyphCount - 1 : 0;
>>          while (i < glyphCount) {
>> @@ -145,7 +146,7 @@
>>                  missingGlyph = true;
>>                  if (composite) break;
>>              }
>> -            iglyphs[i] = glyphs[j] | slotMask;
>> +            iglyphs[i] = ((int)glyphs[j] & intMask) | slotMask;
>>              i++;
>>              j+=step;
>>          }
>>
>>
>>
>> =========================
>> Test for Win10
>> =========================
>> import javafx.application.Application;
>> import static javafx.application.Application.launch;
>> import javafx.scene.Group;
>> import javafx.scene.Scene;
>> import javafx.scene.paint.Color;
>> import javafx.scene.text.Font;
>> import javafx.scene.text.Text;
>> import javafx.scene.text.TextFlow;
>> import javafx.stage.Stage;
>>
>> public class TestHandlingBigGlyphID_Win10 extends Application {
>>     @Override
>>     public void start(Stage stage) throws Exception {
>>         final String fontName = "ARIALUNI.TTF"; // Arial Unicode MS
>>         // download from
>> https://www.wfonts.com/download/data/2015/05/16/arial-unicode-ms/ARIALUNI.TTF
>>
>>
>>         //  and place in $(user.home)/fonts/
>>         //    e.g. C:/Users/username/fonts/arialuni.ttf
>>
>>         Font ourFont =
>> Font.loadFont("file://"+System.getProperty("user.home")+"/fonts/"+fontName.toString(),
>>
>> 48);
>>         TextFlow textFlow = new TextFlow();
>>                                                 // Unicode(GlyphID)
>>         Text text = new Text("\uD7A3\u0E3F");    // U+D7A3(49496) +
>> U+0E3F(1262)
>>         /* Inside JavaFX, 49496(Uint16) is handled as -16040(short).
>>          * By ScriptMapper.isComplexCharCode(), U+0E3F is handled as
>> complex.
>>          * When in condition with minus glyphID value and complex
>>          * , JavaFX is forcibly terminated.
>>          *     (java.lang.ArrayIndexOutOfBoundsException)
>>          */
>>
>>         text.setFill(Color.GREEN);
>>         text.setFont(ourFont);
>>         textFlow.getChildren().addAll(text);
>>
>>         Group group = new Group(textFlow);
>>         Scene scene = new Scene(group, 100, 60, Color.WHITE);
>>         stage.setScene(scene);
>>         stage.show();
>>     }
>> }
>>
>> =========================
>> Test for Win7 and Win10
>> =========================
>> import javafx.application.Application;
>> import static javafx.application.Application.launch;
>> import javafx.scene.Group;
>> import javafx.scene.Scene;
>> import javafx.scene.paint.Color;
>> import javafx.scene.text.Font;
>> import javafx.scene.text.Text;
>> import javafx.scene.text.TextFlow;
>> import javafx.stage.Stage;
>>
>> public class TestHandlingBigGlyphID_Win7_and_Win10 extends Application {
>>     @Override
>>     public void start(Stage stage) throws Exception {
>>         final String family = "Arial Unicode MS"; // ARIALUNI.TTF
>>         // download from
>> https://www.wfonts.com/download/data/2015/05/16/arial-unicode-ms/ARIALUNI.TTF
>>
>>
>>         //  and You NEED install font.
>>
>>         Font ourFont = Font.font(family, 48);
>>         TextFlow textFlow = new TextFlow();
>>                                                 // Unicode(GlyphID)
>>         Text text = new Text("\uD7A3\u0E3F");    // U+D7A3(49496) +
>> U+0E3F(1262)
>>         /* Inside JavaFX, 49496(Uint16) is handled as -16040(short).
>>          * By ScriptMapper.isComplexCharCode(), U+0E3F is handled as
>> complex.
>>          * When in condition with minus glyphID value and complex
>>          * , JavaFX is forcibly terminated.
>>          *     (java.lang.ArrayIndexOutOfBoundsException)
>>          */
>>
>>         text.setFill(Color.GREEN);
>>         text.setFont(ourFont);
>>         textFlow.getChildren().addAll(text);
>>
>>         Group group = new Group(textFlow);
>>         Scene scene = new Scene(group, 100, 60, Color.WHITE);
>>         stage.setScene(scene);
>>         stage.show();
>>     }
>> }
>>
>>
>> --------------------------------------
>> Company: NTT Comware Corporation
>> Name: Akira Nakajima
>> E-Mail: nakajima.akira at nttcom.co.jp
>> --------------------------------------


More information about the openjfx-dev mailing list