primitiveDisplayString fails on empty Strings

 

Hi Fabio,

On Wed, May 23, 2018 at 12:11 PM, Fabio Niephaus <[hidden email]> 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?

_,,,^..^,,,_

best, Eliot

 

Hi Fabio,

On Wed, May 23, 2018 at 1:22 PM, Fabio Niephaus <[hidden email]> wrote:

 

Hi Eliot,

On Wed, May 23, 2018 at 9:48 PM Eliot Miranda <[hidden email]> wrote:

 

Hi Fabio,

On Wed, May 23, 2018 at 12:11 PM, Fabio Niephaus <[hidden email]> 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