8054203: add regression tests for JDK vs ICU layout

[Steven R. Loomis]

On 08/15/2014 02:32 PM, Doug Felt wrote:
> On Fri, Aug 15, 2014 at 1:33 PM, Steven R. Loomis <steven.loomis at oracle.com>
> wrote:
>
>> On 08/15/2014 11:41 AM, Doug Felt wrote:
>>> Thanks for getting the ball rolling, Steven!
>> Welcome!
>>
>>> I'm not sure what the format is for code reviews, so I'll just send
>>> this email with general comments and maybe someone can tell me the
>>> right way to do it.  These comments are a bit more high level and most
>>> don't focus on this code in particular, anyway.
>>>
>>> 1) it would be nice if we used some more structured web-based tool to
>>> review the code. But I don't know how hard it is to set one up.
>> There's always rietveld :)
>> I was just trying to follow the openjdk playbook.
>>
> Yeah, rietveld was what I thought of, though I couldn't remember the name,
> and there might be other open-source alternatives available these days.  I'm
> not sure if what android uses is open sourced, but that could be too
> involved
> for what we need. I've just spent so much time doing code reviews with
> these kinds of tools, I'm kind of spoiled for anything else.
Not sure. Maybe others more current on OpenJDK processes can chime in
here. I don't know what the options are for openjdk.

> [snip some great discussion]
>>> 4) Harfbuzz uses FreeType to get kerning values, while ICU uses
>>> kerning values directly from the kerning table in the font.  Freetype
>>> applies heuristics to adjust the kerning values for smaller point
>>> sizes (like, under 25 pt), and rounds the scaled kerning values to
>>> design units (I think, might be an option). This means ICU and
>>> HarfBuzz kern differently, and this changes the advances. This makes
>>> it difficult to use images as a regression tool.
>>> I think it will be difficult to get full fidelity to the glyph
>>> positions. I expect, since most clients (on Linux) use FreeType
>>> kerning values directly, that we might be better off just going with
>>> FreeType's kerning values. But we probably want to see what other
>>> platforms do.
>> Yes, and as you of all people should know :), one of the places where
>> ICU/ICUJDK diverges is in the kerning table management.
>>
>> Perhaps it is a good case for turning kerning *off* for some types of
>> tests, and using it with a lot of fuzzing when it is on?
>>
> My gut feeling is that fuzzing should be particular to what's being measured
> and what we expect, rather than a generic 'loosening of the acceptable
> bounds'.
> It's pretty easy to loosen the check to a point where it's not really
> testing
> enough to catch real errors.  Something more tailored to the expected output
> is more likely to catch a real issue.  On the other hand, if one runs
> 1000's of
> tests than anything to limit what needs direct investigation helps.
Right. Changes in how calculation is done might lead to rounding
differences/etc.

Behdad, what have you been doing for comparisons w/ Uniscribe?


>
>
>>> 5) HarfBuzz does its computations in integer device units, with
>>> rounding to 16.16 or 24.8 or 26.6 values (though iculehb does some in
>>> floating point). ICU makes more use of native float units.  I've not
>>> been able to track down what exactly happens, but it does seem that
>>> advances might differ between ICU and HB even if kerning is not
>>> applied. The main place I've seen suggestions of this is with scaling
>>> based on common fractions (e.g. 1/10, etc.), native float units can
>>> represent common fractions much better than fixed point power-of-two
>>> units can, and small differences can accumulate over the course of a
>>> line of text. Occasionally this trips over a pixel and glyph images
>>> change.
>>>
>>> So I guess I think we need to first figure out what degree of
>>> compatibility is achievable, and what we want, and then design our
>>> regression/metrics tests around that.
>> OK.
>>
>> Maybe I should rephrase this particular ticket - it is for very basic
>> compatibility, to first verify if  embedded-ICU vs external-ICU is
>> compatible, and then secondly to compare embedded-ICU with
>> external-ICU-really-HarfBuzz.
>>
> Sure, for different ways to access ICU this is of course fine, and that's
> a perfectly good place to start.  I was just focusing on the switch to HB.
I think you are looking at perhaps a more agressive internal change than
I was trying to do at first.  Plus I am over optimistic about the kinds
of changes needed.

OK.  So I'll open another ticket for follow on conformance testing.  

Will you sign off on this test as approved for now then? :)

-s

-- 

IBMer but all opinions are mine.
https://www.ohloh.net/accounts/srl295 // fingerprint @ https://ssl.icu-project.org/trac/wiki/Srl