Hi Folks,
Looking in detail I found two issues here. One is that the addition can silently overflow. The other one is that in the generated C code ALL the variables (arguments and temporaries) are int and not unsigned. (BTW, this is for sure affecting other rules, not only rgbAdd.) If we fixed the int issue, then much simpler (and a bit faster) code would work:
partitionedAdd: word1 to: word2 nBits: nBits nPartitions: nParts "Add word1 to word2 as nParts partitions of nBits each. This is useful for packed pixels, or packed colors" | mask sum result maskedWord1 | mask := maskTable at: nBits. "partition mask starts at the right" result := 0. 1 to: nParts do: [ :i | maskedWord1 _ word1 bitAnd: mask. sum := maskedWord1 + (word2 bitAnd: mask). (sum <= mask "result must not carry out of partition" and: [ sum >= maskedWord1 ]) "**** This is the only change ****" ifTrue: [result := result bitOr: sum] ifFalse: [result := result bitOr: mask]. mask := mask << nBits "slide left to next partition"]. ^ result
I believe we need to add explicit declarations of all variables being unsigned. Or perhaps enhance a bit the code generator, by allowing a plugin to declare its default numeric type. For BitBlt it could be unsigned. For DSP stuff it could be float or double, making the Slang code much nicer by not needing all the explicit type declarations. What do you think?
Cheers, Juan Vuletich
Henrik Sperre Johansen wrote:
On 20.10.2009 15:56, David T. Lewis wrote:
Could this be the cause of issue for which Juan was asking help?
Subject line "[Vm-dev] Bug in BitBlt. Need Help."
http://lists.squeakfoundation.org/pipermail/vm-dev/2009-September/003160.htm...
Yes, that's exactly it, sorry I had not seen it. Attached is a proof-of-concept fix (for partitionedAdd: only), valid as long as nParts > 1 ( I suggest one wouldn't use partitionedAdd anyways if it were :) ) If anyone has a better idea, it'd be appreciated!
Correctness test: | sourceForm destForm blt correctAlphas | correctAlphas := 0. 0 to: 255 do: [:sourceAlpha | sourceForm := Form extent: 1 @ 1 depth: 32. sourceForm bits at: 1 put: sourceAlpha << 24 + (33 << 16) + (25 << 8) + 27. 0 to: 255 do: [:destAlpha | destForm := Form extent: 1 @ 1 depth: 32. destForm bits at: 1 put: destAlpha << 24 + (255 << 16) + (255 << 8) + 255. blt := BitBlt new. blt sourceForm: sourceForm. blt sourceOrigin: 0 @ 0. blt setDestForm: destForm. blt destOrigin: 0 @ 0. blt combinationRule: 20. blt copyBits. correctAlphas := correctAlphas + (((blt destForm bits at: 1) digitAt: 4) = (destAlpha
- sourceAlpha min: 255) ifTrue: [1] ifFalse: [0]) ]]. self assert: 65536 equals: correctAlphas
Performance is not impacted to a significant degree as far as I can tell, the from runtimes of the test: [|sourceForm destForm blt| destForm := Form extent: 99@99 depth: 32. 1 to: destForm bits size do: [:ix | destForm bits at: ix put: ((192 << 24) + (255 << 16) + (255 << 8) + 255).]. sourceForm := Form extent: 99@99 depth: 32. 1 to: sourceForm bits size do: [:ix | sourceForm bits at: ix put: ((192 << 24) + (33 << 16) + (25 << 8) + 27).]. blt := BitBlt new. blt sourceForm: sourceForm. blt sourceOrigin: 0@0. blt setDestForm: destForm. blt destOrigin: 0@0. blt combinationRule: 20. 5000 timesRepeat: [ blt copyBits.]] timeToRun
Cheers, Henry
No virus found in this incoming message. Checked by AVG - www.avg.com Version: 8.5.423 / Virus Database: 270.14.26/2451 - Release Date: 10/22/09 08:51:00