Re: [Silgraphite-devel] Announce graphite engine 1.3.1

[Jonathan K. <jfk...@gm...>]

On 31/8/15 14:17, Jonathan Kew wrote:
> On 31/8/15 11:59, Martin Hosken wrote:
>> Dear All,
>>
>> Oh blast, there's a bug in the engine and it's pointless running with
>> the current release. So there will be a 1.3.2 coming out real soon.
>> The 1.3.1 is perfectly good for testing and integration, but will
>> fail on some compressed fonts. But since nobody can make compressed
>> fonts yet, that's not a problem.
>>
>
> Dear Martin,
>
> I'm afraid I think there may be another bug in the engine, probably as a
> result of the bidi changes...... I tried a test run with 1.3.1 in Gecko,
> and hit a crash in graphite2::Segment::reverseSlots(). See
>
> https://treeherder.mozilla.org/logviewer.html#?job_id=10910728&repo=try
>
> for more detail. Judging by the crash address (0x8), I assume it's
> trying to call the method on a NULL Segment, but I haven't tried to
> debug and see exactly where that's coming from.
>
> (For the testcase that triggers the crash, see
> http://mxr.mozilla.org/mozilla-central/source/layout/reftests/text/wordbreak-9.html?force=1.)
>

....and in particular, note that this testcase involves shaping segments 
that contain ONLY an Arabic diacritic. Looking at 
Segment::reverseSlots(), it appears that if all slots in the segment 
have bidiClass 16, the |out| pointer never gets set by the loop, so it's 
still NULL at the end.

AFAICS, just wrapping the tail of that method in an |if (out)| condition 
should fix things:

     if (out)
     {
         out->prev(0);
         m_last = tlast;
         m_first = out;
     }

so that a diacritics-only segment is left unchanged; though I haven't 
thought carefully about whether edge cases such as segments with leading 
diacritics will actually work properly.

JK