[Vm-dev] primitiveDisplayString fails on empty Strings

Fabio Niephaus lists at fniephaus.com
Thu May 24 23:18:31 UTC 2018


Hi Eliot,

On Thu, May 24, 2018 at 9:51 PM Eliot Miranda <eliot.miranda at gmail.com>
wrote:

>
> 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).
>

All good and thanks! This is the first time I'm proposing changes to the
VMMaker code base, so I'm just learning best practices from you here :)

Fabio


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


More information about the Vm-dev mailing list