Re: [squeak-dev] The Trunk: MorphicExtras-nice.137.mcz

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [squeak-dev] The Trunk: MorphicExtras-nice.137.mcz

Nicolas Cellier
 
I feel like it would be better to change pixelValueAt: so as to always unhibernate, but I did not dare if ever someone had the idea of sending it in tight loops (one shouldn't, there is BitBlt for that purpose, but who knows...).

Maybe we could fail primitivePixelValueAt if receiver is Byte like, and unhibernate in fallback code.
We would let every other BitBlt primitives accept a ByteArray to allow BitBlt tricks.

Thoughts?


2013/12/13 <[hidden email]>
Nicolas Cellier uploaded a new version of MorphicExtras to project The Trunk:
http://source.squeak.org/trunk/MorphicExtras-nice.137.mcz

==================== Summary ====================

Name: MorphicExtras-nice.137
Author: nice
Time: 13 December 2013, 10:07:57.316 pm
UUID: 127ce68e-b210-42a3-b0b3-b40c69941d02
Ancestors: MorphicExtras-nice.136

Correct an awfull bug in PaintBoxMorph>>showColor
One should NEVER send pixelValueAt: nor colorAt: to an hibernated Form (or ColorForm).

=============== Diff against MorphicExtras-nice.136 ===============

Item was changed:
  ----- Method: PaintBoxMorph>>showColor (in category 'actions') -----
  showColor
        "Display the current color in all brushes, both on and off."

        | offIndex onIndex center |
        currentColor ifNil: [^self].
        "colorPatch color: currentColor.        May delete later"
        (brushes isNil or: [brushes first owner ~~ self])
                ifTrue:
                        [brushes := OrderedCollection new.
                        #(#brush1: #brush2: #brush3: #brush4: #brush5: #brush6:)
                                do: [:sel | brushes addLast: (self submorphNamed: sel)]].
+       brushes last offImage unhibernate.
+       brushes last onImage unhibernate.
+       brushes last pressedImage unhibernate.
+       center := brushes last offImage extent // 2.
+       offIndex := brushes last offImage pixelValueAt: center.
+       onIndex := brushes last onImage pixelValueAt: center.
-       center := (brushes sixth) offImage extent // 2.
-       offIndex := (brushes sixth) offImage pixelValueAt: center.
-       onIndex := (brushes sixth) onImage pixelValueAt: center.
        brushes do:
                        [:bb |
                        bb offImage colors at: offIndex + 1 put: currentColor.
                        bb offImage clearColormapCache.
                        bb onImage colors at: onIndex + 1 put: currentColor.
                        bb onImage clearColormapCache.
                        bb invalidRect: bb bounds].
        self invalidRect: (brushes first topLeft rect: brushes last bottomRight)!



Reply | Threaded
Open this post in threaded view
|

Re: [squeak-dev] The Trunk: MorphicExtras-nice.137.mcz

Nicolas Cellier
 
2013/12/13 Nicolas Cellier <[hidden email]>
I feel like it would be better to change pixelValueAt: so as to always unhibernate, but I did not dare if ever someone had the idea of sending it in tight loops (one shouldn't, there is BitBlt for that purpose, but who knows...).

Maybe we could fail primitivePixelValueAt if receiver is Byte like, and unhibernate in fallback code.
We would let every other BitBlt primitives accept a ByteArray to allow BitBlt tricks.

Thoughts?


No one raised a comment so far.
Isn't it a good idea to fail the primitive?
If yes, then it's very simple to implement, just add:

    (interpreterProxy isWords: bitmap)    ifFalse:[interpreterProxy primitiveFail].

somewhere in the preamble of  BitBltSimulation>>primitivePixelValueAtX: xVal y: yVal

I check the senders of pixelValueAt:, and some of them would loop over each pixel.
So sending unhibernate at each pixel does not seem a good idea anyway.
Leaking unhibernate sends out of the loop neither is, because then 3rd party classes must be aware of an implementation detail of Form.

 
2013/12/13 <[hidden email]>

Nicolas Cellier uploaded a new version of MorphicExtras to project The Trunk:
http://source.squeak.org/trunk/MorphicExtras-nice.137.mcz

==================== Summary ====================

Name: MorphicExtras-nice.137
Author: nice
Time: 13 December 2013, 10:07:57.316 pm
UUID: 127ce68e-b210-42a3-b0b3-b40c69941d02
Ancestors: MorphicExtras-nice.136

Correct an awfull bug in PaintBoxMorph>>showColor
One should NEVER send pixelValueAt: nor colorAt: to an hibernated Form (or ColorForm).

=============== Diff against MorphicExtras-nice.136 ===============

Item was changed:
  ----- Method: PaintBoxMorph>>showColor (in category 'actions') -----
  showColor
        "Display the current color in all brushes, both on and off."

        | offIndex onIndex center |
        currentColor ifNil: [^self].
        "colorPatch color: currentColor.        May delete later"
        (brushes isNil or: [brushes first owner ~~ self])
                ifTrue:
                        [brushes := OrderedCollection new.
                        #(#brush1: #brush2: #brush3: #brush4: #brush5: #brush6:)
                                do: [:sel | brushes addLast: (self submorphNamed: sel)]].
+       brushes last offImage unhibernate.
+       brushes last onImage unhibernate.
+       brushes last pressedImage unhibernate.
+       center := brushes last offImage extent // 2.
+       offIndex := brushes last offImage pixelValueAt: center.
+       onIndex := brushes last onImage pixelValueAt: center.
-       center := (brushes sixth) offImage extent // 2.
-       offIndex := (brushes sixth) offImage pixelValueAt: center.
-       onIndex := (brushes sixth) onImage pixelValueAt: center.
        brushes do:
                        [:bb |
                        bb offImage colors at: offIndex + 1 put: currentColor.
                        bb offImage clearColormapCache.
                        bb onImage colors at: onIndex + 1 put: currentColor.
                        bb onImage clearColormapCache.
                        bb invalidRect: bb bounds].
        self invalidRect: (brushes first topLeft rect: brushes last bottomRight)!




Reply | Threaded
Open this post in threaded view
|

Re: [squeak-dev] The Trunk: MorphicExtras-nice.137.mcz

Bert Freudenberg
 

On 20.12.2013, at 00:55, Nicolas Cellier <[hidden email]> wrote:

2013/12/13 Nicolas Cellier <[hidden email]>
I feel like it would be better to change pixelValueAt: so as to always unhibernate, but I did not dare if ever someone had the idea of sending it in tight loops (one shouldn't, there is BitBlt for that purpose, but who knows...).

Maybe we could fail primitivePixelValueAt if receiver is Byte like, and unhibernate in fallback code.
We would let every other BitBlt primitives accept a ByteArray to allow BitBlt tricks.

Thoughts?


No one raised a comment so far.
Isn't it a good idea to fail the primitive?

The primitive must fail, yes. That is simply a bug. Before that primitive existed, pixelValueAt: used BitBlt which did the unhibernate. 

If yes, then it's very simple to implement, just add:

    (interpreterProxy isWords: bitmap)    ifFalse:[interpreterProxy primitiveFail].

IMHO the primitive should do the same check as BitBlt: verify that the size of the bits is exactly right given width, height, and depth.

- Bert -


somewhere in the preamble of  BitBltSimulation>>primitivePixelValueAtX: xVal y: yVal

I check the senders of pixelValueAt:, and some of them would loop over each pixel.
So sending unhibernate at each pixel does not seem a good idea anyway.

Leaking unhibernate sends out of the loop neither is, because then 3rd party classes must be aware of an implementation detail of Form.

 
2013/12/13 <[hidden email]>

Nicolas Cellier uploaded a new version of MorphicExtras to project The Trunk:
http://source.squeak.org/trunk/MorphicExtras-nice.137.mcz

==================== Summary ====================

Name: MorphicExtras-nice.137
Author: nice
Time: 13 December 2013, 10:07:57.316 pm
UUID: 127ce68e-b210-42a3-b0b3-b40c69941d02
Ancestors: MorphicExtras-nice.136

Correct an awfull bug in PaintBoxMorph>>showColor
One should NEVER send pixelValueAt: nor colorAt: to an hibernated Form (or ColorForm).

=============== Diff against MorphicExtras-nice.136 ===============

Item was changed:
  ----- Method: PaintBoxMorph>>showColor (in category 'actions') -----
  showColor
        "Display the current color in all brushes, both on and off."

        | offIndex onIndex center |
        currentColor ifNil: [^self].
        "colorPatch color: currentColor.        May delete later"
        (brushes isNil or: [brushes first owner ~~ self])
                ifTrue:
                        [brushes := OrderedCollection new.
                        #(#brush1: #brush2: #brush3: #brush4: #brush5: #brush6:)
                                do: [:sel | brushes addLast: (self submorphNamed: sel)]].
+       brushes last offImage unhibernate.
+       brushes last onImage unhibernate.
+       brushes last pressedImage unhibernate.
+       center := brushes last offImage extent // 2.
+       offIndex := brushes last offImage pixelValueAt: center.
+       onIndex := brushes last onImage pixelValueAt: center.
-       center := (brushes sixth) offImage extent // 2.
-       offIndex := (brushes sixth) offImage pixelValueAt: center.
-       onIndex := (brushes sixth) onImage pixelValueAt: center.
        brushes do:
                        [:bb |
                        bb offImage colors at: offIndex + 1 put: currentColor.
                        bb offImage clearColormapCache.
                        bb onImage colors at: onIndex + 1 put: currentColor.
                        bb onImage clearColormapCache.
                        bb invalidRect: bb bounds].
        self invalidRect: (brushes first topLeft rect: brushes last bottomRight)!