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 commits@source.squeak.org
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)!
2013/12/13 Nicolas Cellier nicolas.cellier.aka.nice@gmail.com
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 commits@source.squeak.org
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)!
On 20.12.2013, at 00:55, Nicolas Cellier nicolas.cellier.aka.nice@gmail.com wrote:
2013/12/13 Nicolas Cellier nicolas.cellier.aka.nice@gmail.com 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 commits@source.squeak.org
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)!
vm-dev@lists.squeakfoundation.org