[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    [OpenJDK 2D-Dev] [PATCH] JDK-8152680: Regression in GlyphVector.getGlyphCharIndex behaviour
From:       Dmitry Batrak <dmitry.batrak () jetbrains ! com>
Date:       2016-04-20 13:14:11
Message-ID: CAET5FPsCXxPxryU2iEt6RBZa+5ALadq-mWMbEtGBT5YJo2RLiQ () mail ! gmail ! com
[Download RAW message or body]

Hello,

I'd like to propose a patch for JDK-8152680 - an issue I've raised
via bugreport.java.com earlier, hope someone can sponsor it.
I have a Contributor status via agreement signed by Jetbrains.

The issue is related to the extraction of glyph-to-character mapping
from results of text layout, performed by Harfbuzz,
when layout is requested for a specified substring of text string.

For LTR case, existing code assumes that cluster values (which are
later interpreted as character indices) are assigned by Harfbuzz
starting from the beginning of substring, but actually it's done
starting from the beginning of whole string (as mentioned by existing
comment in HBShaper.c). For reference, this logic can be found
at hb-buffer.cc:1470 (function hb_buffer_add_utf). The proposed fix
is to take into account this numbering scheme by shifting cluster value
correspondingly. GlyphCharIndicesTest test case is included to cover this
fix.

RTL case is not affected by the problem mentioned above, but there's
another issue with it - cluster value generated by Harfbuzz is ignored
completely, instead character index is taken to be equal to glyph index
(in visual order). This will not work, e.g. in case when there are more
glyphs
than characters - some glyphs will reference non-existing characters.
The proposed fix is just to use the same shifted cluster value,
as for LTR case - I believe it works just as well in RTL case.
GlyphCharIndicesRtlTest test case is included to cover RTL case, but
I'm not sure it should be definitely committed, as it depends on a specific
Windows font, which doesn't seem to be available by default in Windows 10
(even though it must be present in Windows Vista, 7 and 8). I couldn't find
a better font on Windows, demonstrating the issue.

Webrev for the fix is available at
http://cr.openjdk.java.net/~avu/DmitryBatrak/JDK-8152680
(kindly posted by my colleague, having access to cr.openjdk.java.net).

Best regards,
Dmitry Batrak

[Attachment #3 (text/html)]

<div dir="ltr">Hello,<br><br>I&#39;d like to propose a patch for JDK-8152680 - an \
issue I&#39;ve raised <br>via <a \
href="http://bugreport.java.com">bugreport.java.com</a> earlier, hope someone can \
sponsor it.<br>I have a Contributor status via agreement signed by \
Jetbrains.<br><br>The issue is related to the extraction of glyph-to-character \
mapping <br>from results of text layout, performed by Harfbuzz,<br>when layout is \
requested for a specified substring of text string.<br><br>For LTR case, existing \
code assumes that cluster values (which are <br>later interpreted as character \
indices) are assigned by Harfbuzz <br>starting from the beginning of substring, but \
actually it&#39;s done <br>starting from the beginning of whole string (as mentioned \
by existing <br>comment in HBShaper.c). For reference, this logic can be found <br>at \
hb-buffer.cc:1470 (function hb_buffer_add_utf). The proposed fix <br>is to take into \
account this numbering scheme by shifting cluster value<br>correspondingly. \
GlyphCharIndicesTest test case is included to cover this fix.<br><br>RTL case is not \
affected by the problem mentioned above, but there&#39;s <br>another issue with it - \
cluster value generated by Harfbuzz is ignored <br>completely, instead character \
index is taken to be equal to glyph index <br>(in visual order). This will not work, \
e.g. in case when there are more glyphs <br>than characters - some glyphs will \
reference non-existing characters. <br>The proposed fix is just to use the same \
shifted cluster value, <br>as for LTR case - I believe it works just as well in RTL \
case. <br>GlyphCharIndicesRtlTest test case is included to cover RTL case, but \
<br>I&#39;m not sure it should be definitely committed, as it depends on a specific \
<br>Windows font, which doesn&#39;t seem to be available by default in Windows 10 \
<br>(even though it must be present in Windows Vista, 7 and 8). I couldn&#39;t find \
<br>a better font on Windows, demonstrating the issue.<br><br>Webrev for the fix is \
available at <br><a href="http://cr.openjdk.java.net/~avu/DmitryBatrak/JDK-8152680">http://cr.openjdk.java.net/~avu/DmitryBatrak/JDK-8152680</a> \
<br>(kindly posted by my colleague, having access to <a \
href="http://cr.openjdk.java.net">cr.openjdk.java.net</a>).<br><br>Best \
regards,<br>Dmitry Batrak</div>



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic