Smalltalk › Squeak › Squeak - Dev

Questionable Form>>setExtent:depth:bits:

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

2 messages Options

Nicolas Cellier

Questionable Form>>setExtent:depth:bits:

Here is the code:

setExtent: extent depth: bitsPerPixel bits: bitmap
    "Create a virtual bit map with the given extent and bitsPerPixel."

    width := extent x asInteger.
    width < 0 ifTrue: [width := 0].
    height := extent y asInteger.
    height < 0 ifTrue: [height := 0].
    depth := bitsPerPixel.
    (bits isNil or:[self bitsSize = bitmap size]) ifFalse:[^self error:'Bad dimensions'].
    bits := bitmap

If we look at the guard for bitsSize, there are two things weird:

- the test bits isNil disengage the guard the first time this initialization is sent, that is virtually every time (did we ever tried to recycle a Form this way?)

- the test could be bitmap isNil or: [...

but in this case we can no longer initialize a Form read from png for example (see examples in PaintBoxMorph class 'resources'), because such initialization occurs with a ByteArray...

So the test could be
    (bitmap isNil
        or: [(bitmap class isWords and: [self bitsSize = bitmap size])
        or: [bitmap class isBytes and: [self bitsSize * 4 = bitmap size]]])

(or the same with

but then this would forbid initialization with compressed (hibernated) ByteArray format (no idea if it is used...)

The alternative is to completely remove the guard I don't know if the BitBlt primitives are protected enough from bitmap overflow, but anyway as demonstrated above the guard is not active.

It's always questionable to touch those parts sealed by famous predecessors (di, ar, ...).

It ain't really broken, and even Juan did not change it in Cuis.

But I feel like those Form have accumulated some dust and could shine a bit brighter.

Bert Freudenberg

Re: Questionable Form>>setExtent:depth:bits:

On 19.12.2013, at 17:38, Nicolas Cellier <[hidden email]> wrote:

Here is the code:

setExtent: extent depth: bitsPerPixel bits: bitmap
    "Create a virtual bit map with the given extent and bitsPerPixel."

    width := extent x asInteger.
    width < 0 ifTrue: [width := 0].
    height := extent y asInteger.
    height < 0 ifTrue: [height := 0].
    depth := bitsPerPixel.
    (bits isNil or:[self bitsSize = bitmap size]) ifFalse:[^self error:'Bad dimensions'].
    bits := bitmap

If we look at the guard for bitsSize, there are two things weird:

- the test bits isNil disengage the guard the first time this initialization is sent, that is virtually every time (did we ever tried to recycle a Form this way?)

- the test could be bitmap isNil or: [...

Yes. Looks like a simple typo to me.

but in this case we can no longer initialize a Form read from png for example (see examples in PaintBoxMorph class 'resources'), because such initialization occurs with a ByteArray...

So the test could be
    (bitmap isNil
        or: [(bitmap class isWords and: [self bitsSize = bitmap size])
        or: [bitmap class isBytes and: [self bitsSize * 4 = bitmap size]]])

That looks much more like the intention.

(or the same with
but then this would forbid initialization with compressed (hibernated) ByteArray format (no idea if it is used...)

The hibernation is pretty much private to the Form, don't think anyone would set that from the outside.

The alternative is to completely remove the guard I don't know if the BitBlt primitives are protected enough from bitmap overflow, but anyway as demonstrated above the guard is not active.

It's always questionable to touch those parts sealed by famous predecessors (di, ar, ...).

Not at all. They would readily admit to having done stupid stuff occasionally ;)

It ain't really broken, and even Juan did not change it in Cuis.
But I feel like those Form have accumulated some dust and could shine a bit brighter.

- Bert -