[squeak-dev] Re: VMMaker loading uncovers a bug in MC

Nicolas Cellier nicolas.cellier.aka.nice at gmail.com
Mon Mar 8 13:59:06 UTC 2010


2010/3/8 Levente Uzonyi <leves at elte.hu>:
> On Mon, 8 Mar 2010, Nicolas Cellier wrote:
>
>> OK, I re-analyzed the stop conditions to sort the mess out.
>> Take for example
>> MultiCharacterScanner >> scanMultiCharactersFrom: startIndex to:
>> stopIndex in: sourceString rightX: rightX stopConditions: stops kern:
>> kernDelta
>>
>> Once upon a time, the test was:
>>                (encoding = 0 and: [ascii < stopConditions size and:
>> [(stopConditions at: ascii + 1) ~~ nil]]) ifTrue: [^ stops at: ascii +
>> 1].
>> There was a conflict with endOfRun/CrossedX in case of ascii=256 or
>> ascii=257 (A macron...)
>> But this did not happen with Unicode leadingChar = 15 (Unicode).
>>
>> This code was the starting point for reifying a TextStopConditions object.
>>
>> Then Andreas tested with 256 instead of stopConditions size:
>>                (encoding = 0 and: [ascii < 256 and: [(stops at: ascii + 1)
>> notNil]])
>>                        ifTrue: [^ stops at: ascii + 1].
>> This was a more lightweight solution, no TextStopConditions object
>> required anymore.
>> The A macron potential bug was out.
>>
>> Then FreeType was merged, reintroducing
>>                (encoding = 0 and: [ascii < stopConditions size and:
>> [(stopConditions at: ascii + 1) ~~ nil]]) ifTrue: [^ stops at: ascii +
>> 1].
>> The A macron potential bug was back in.
>>
>> Then I corrected, because the instance variable stopConditions was
>> shadowing the argument stops:
>>                (encoding = 0 and: [ascii < stops size and: [(stops at:
>> ascii + 1)
>> ~~ nil]]) ifTrue: [^ stops at: ascii + 1].
>> The A macron potential bug was still in.
>>
>> So I wonder if I will not revert to the simple stopConditions
>> class=Array solution with Andreas test ascii < 256.
>> The main reason is for efficiency, this is a critical internal
>> CPU-consuming loop.
>
> I think we should keep TextStopCondition, but it should be a variable
> subclass. This gives us fast accessing, it can have 256 or 258 slots (for
> backwards compatiblity, if necessary) and have the instance variables and
> accessors for endOfRun and crossedX.
>
>
> Levente
>

Excellent idea! we can then keep my lengthy comments :)

About these comments, I jwas just wondering if putting them in the
most internal class was not a mistake...
After thought, I think not. These are implementation details, and thus
belongs to most internal classes.
Providing hooks in bottom classes to upper level classes/selector thus
constitutes a kind of bottom up documentation and enable faster
discovery of features by browsing. This is complementary to top down
documentation that a help system would provide.

Of course, if one reuses TextStopConditions for a completely different
purpose (after all, it's just a kind of Dictionary), then the comment
becomes a bit out of date and misleading. But hey, it would be
hacking. The class name speaks for itself.

Nicolas

>> Unless we want to handle control characters with codePoint >= 256 ?
>>
>> What do you think ?
>>
>> Nicolas
>>
>> 2010/3/8 Nicolas Cellier <nicolas.cellier.aka.nice at gmail.com>:
>>>
>>> 2010/3/8 Andreas Raab <andreas.raab at gmx.de>:
>>>>
>>>> On 3/7/2010 8:06 PM, Levente Uzonyi wrote:
>>>>>
>>>>> I uploaded two packages to the Inbox which (seem to be) fixing the
>>>>> issue. After evaluating this in an updated Trunk image (9634) FreeType
>>>>> should load normally:
>>>>>
>>>>> (Installer squeak project: 'inbox')
>>>>> install: 'Graphics-ul.117.mcz';
>>>>> install: 'Multilingual-ul.100.mcz'.
>>>>> CharacterScanner initialize.
>>>>>
>>>>>
>>>>> Levente
>>>>>
>>>>> P.S. I uploaded these to the Inbox because I'm not familiar with this
>>>>> part of Squeak at all, and there's a bit cleaner way to implement these
>>>>> fixes if they are correct.
>>>>
>>>> Nicolas should look at the changes. I think this is a side effect of the
>>>> Unicode leading char updates he posted.
>>>>
>>>> Cheers,
>>>>  - Andreas
>>>>
>>>>
>>>
>>> Yes,we cross-posted.
>>> Take Levente workaround, it's just perfect.
>>>
>>> Nicolas
>>>
>>
>
>
>
>



More information about the Squeak-dev mailing list