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

Kevin Rushforth kevin.rushforth at oracle.com
Tue Jul 24 11:44:55 UTC 2018


As an FYI, this is filed in JBS as:

https://bugs.openjdk.java.net/browse/JDK-8207839

and is now being reviewed.

-- Kevin

On 7/5/2018 8:10 PM, Nakajima Akira wrote:
> 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