[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] RFR: 8233097: Fontmetrics for large Fonts has zero width
From: Prasanta Sadhukhan <prasanta.sadhukhan () oracle ! com>
Date: 2019-11-04 6:16:22
Message-ID: 4da7b43e-8e8e-4fb8-ec58-58ece092ef18 () oracle ! com
[Download RAW message or body]
I believe
668 /* See the comments in getGlyphMetricsNative. They apply here too.
*/ should be
getGlyphAdvanceNative
Other than that, looks good to me.
Regards
Prasanta
On 03-Nov-19 7:31 PM, Jayathirth Rao wrote:
> Changes are fine.
>
> Thanks,
> Jay
>
>> On 01-Nov-2019, at 1:29 AM, Sergey Bylokhov <Sergey.Bylokhov@oracle.com> wrote:
>>
>> Looks fine.
>>
>> On 10/30/19 11:37 am, Phil Race wrote:
>>> PS .. a positive from this is that with this fix the regression test
>>> runs about 10X faster than it did with say JDK 13 GA .. due to
>>> skipping the redundant images
>>> -phil.
>>> On 10/30/19 11:05 AM, Phil Race wrote:
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8233097
>>>> Webrev: http://cr.openjdk.java.net/~prr/8233097/
>>>>
>>>> This bug is a regression from a fix in JDK 13.0.1 / 11.0.5
>>>>
>>>> To work around a bug in old versions of freetype we capped the size
>>>> of the glyph image but overlooked that we may use that code to get
>>>> metrics even when the glyph image isn't needed.
>>>>
>>>> The fix is to still get the metrics the same way but in such a case
>>>> skip getting the image.
>>>>
>>>> When the new "renderImage" variable is true the changes in the
>>>> code should be a no-op, and this is used for metrics and images
>>>> for typical sizes. So theoretically at least, safe there.
>>>>
>>>> For renderImage == false, as used from the advance/metrics functions
>>>> it guards against rendering the image or accessing bitmap fields.
>>>>
>>>> The advance/metrics cases discard the image so it should also be safe there.
>>>>
>>>> I'm running regression tests as well as Font2DTest and all passes so far.
>>>>
>>>> This will need to be backported to 11u, so does still need careful review.
>>>>
>>>> -phil.
>>>>
>>>>
>>>>
>>>>
>>>>
>>
>> --
>> Best regards, Sergey.
[Attachment #3 (text/html)]
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p>I believe <br>
</p>
<pre style="color: rgb(0, 0, 0); font-style: normal; font-variant-ligatures: \
normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: \
2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; \
word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration-style: initial; \
text-decoration-color: initial;"><span class="changed" style="color: blue;">668 \
/* See the comments in getGlyphMetricsNative. They apply here too. */ should be \
</span> getGlyphAdvanceNative<span class="changed" style="color: blue;"></span></pre>
<div class="moz-cite-prefix">Other than that, looks good to me.</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix">Regards</div>
<div class="moz-cite-prefix">Prasanta<br>
</div>
<div class="moz-cite-prefix">On 03-Nov-19 7:31 PM, Jayathirth Rao
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:BFA6B5E9-C85E-42CF-B7CA-092022DCF425@oracle.com">
<pre class="moz-quote-pre" wrap="">Changes are fine.
Thanks,
Jay
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">On 01-Nov-2019, at 1:29 AM, Sergey \
Bylokhov <a class="moz-txt-link-rfc2396E" \
href="mailto:Sergey.Bylokhov@oracle.com"><Sergey.Bylokhov@oracle.com></a> \
wrote:
Looks fine.
On 10/30/19 11:37 am, Phil Race wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">PS .. a positive from this is that with \
this fix the regression test runs about 10X faster than it did with say JDK 13 GA .. \
due to skipping the redundant images
-phil.
On 10/30/19 11:05 AM, Phil Race wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">Bug: <a class="moz-txt-link-freetext" \
href="https://bugs.openjdk.java.net/browse/JDK-8233097">https://bugs.openjdk.java.net/browse/JDK-8233097</a>
Webrev: <a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~prr/8233097/">http://cr.openjdk.java.net/~prr/8233097/</a>
This bug is a regression from a fix in JDK 13.0.1 / 11.0.5
To work around a bug in old versions of freetype we capped the size
of the glyph image but overlooked that we may use that code to get
metrics even when the glyph image isn't needed.
The fix is to still get the metrics the same way but in such a case
skip getting the image.
When the new "renderImage" variable is true the changes in the
code should be a no-op, and this is used for metrics and images
for typical sizes. So theoretically at least, safe there.
For renderImage == false, as used from the advance/metrics functions
it guards against rendering the image or accessing bitmap fields.
The advance/metrics cases discard the image so it should also be safe there.
I'm running regression tests as well as Font2DTest and all passes so far.
This will need to be backported to 11u, so does still need careful review.
-phil.
</pre>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">
--
Best regards, Sergey.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
</pre>
</blockquote>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic