[Vm-dev] primitiveDisplayString fails on empty Strings

[Eliot Miranda]

Hi Fabio,

On Wed, May 23, 2018 at 1:22 PM, Fabio Niephaus <lists at fniephaus.com> wrote:

>
> Hi Eliot,
>
>
> On Wed, May 23, 2018 at 9:48 PM Eliot Miranda <eliot.miranda at gmail.com>
> wrote:
>
>>
>> Hi Fabio,
>>
>>
>>
>> On Wed, May 23, 2018 at 12:11 PM, Fabio Niephaus <lists at fniephaus.com>
>> wrote:
>>
>>>
>>> Hi all,
>>>
>>> I noticed that BitBlt's primitiveDisplayString fails relatively often
>>> and then found out that it's sometimes called to draw empty strings.
>>>
>>> `(ByteString allInstances select: [:ea | ea isEmpty]) size` revealed
>>> that there are 2245 empty strings in my image and I assume some of them
>>> must be either in the FrameRateMorph morph or in the Clock morph.
>>>
>>> Anyway...looking at BitBlt>>#primDisplayString:from:to:map:xTable:kern:,
>>> the fallback code basically does nothing if stopIndex is zero (`startIndex
>>> to: stopIndex do: ...`) which makes sense.
>>>
>>> What I think doesn't make sense is that the primitive fails in the first
>>> place. Instead of failing if stopIndex is not greater 0, I would suggest to
>>> use: `stopIndex < 1 ifTrue: [^ interpreterProxy pop: 6].` which cleans up
>>> the stack, leaves the receiver, and returns early.
>>>
>>> Let me know what you think!
>>>
>>
>> I don't object.  So you suggest the following?
>>
>> ((interpreterProxy isArray: xTable)
>> and: [(interpreterProxy isArray: glyphMap)
>> and: [(interpreterProxy slotSizeOf: glyphMap) = 256
>> and: [(interpreterProxy isBytes: sourceString)
>> and: [startIndex > 0
>> and: [stopIndex > 0
>> and: [stopIndex <= (interpreterProxy byteSizeOf: sourceString)
>> and: [(self loadBitBltFrom: bbObj)
>> and: [combinationRule ~= 30 "these two need extra source alpha"
>> and: [combinationRule ~= 31]]]]]]]]]) ifFalse:
>> [^interpreterProxy primitiveFail].
>>
>> =>
>>
>> ((interpreterProxy isArray: xTable)
>> and: [(interpreterProxy isArray: glyphMap)
>> and: [(interpreterProxy slotSizeOf: glyphMap) = 256
>> and: [(interpreterProxy isBytes: sourceString)
>> and: [startIndex > 0
>> *and: [stopIndex >= 0 "to avoid failing for empty strings..."*
>> and: [stopIndex <= (interpreterProxy byteSizeOf: sourceString)
>> and: [(self loadBitBltFrom: bbObj)
>> and: [combinationRule ~= 30 "these two need extra source alpha"
>> and: [combinationRule ~= 31]]]]]]]]]) ifFalse:
>> [^interpreterProxy primitiveFail].
>>
>> stopIndex = 0 ifTrue:
>> [^interpreterProxy pop: 6. "the string is empty; pop args, return rcvr"].
>>
>
>> Any other opinions either way?
>>
>
> Why not do the stopIndex = 0 check early and move it all the way up? The
> primitive won't fail if any of the other checks fail which I don't think is
> bad if you don't want to draw anything (stopIndex = 0). On the other hand,
> these other checks are not executed at all (e.g. loadBitBltFrom:) and you
> don't have to change the stopIndex > 0 check. :)
>

Because it feels hack to me to not fail the primitive if one supplied an
empty string but an otherwise corrupted bitblt obj.  IMO, the primitive
should succeed only if all its input arguments are correct.  Pushing the
stopIndex check earlier circumvents these checks.  I'll commit the above
and if you feel strongly you can commit an updated version (that documents
your preference).


> Fabio
>
>
>>
>> _,,,^..^,,,_
>> best, Eliot
>>
>

_,,,^..^,,,_
best, Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20180524/47e17cb1/attachment.html>