[bugifx] SprSize panics when range is too high

SprSize sometimes panics if trying to copy range outside the sprite-sheet. The main reason is the algorithm used to clamp the range. It does not take into consideration that width or height can be a non-integer value.

Author: elgopher

internal/testimage/spr_1_at_0_0_2.0_1.0.png

@@ -114,7 +114,6 @@ func Boot() error {
 	numberOfSprites = (ssWidth * ssHeight) / (SpriteWidth * SpriteHeight)
 
 	spritesInLine = ssWidth / SpriteWidth
-	spritesRows = ssHeight / SpriteHeight
 
 	scrWidth = ScreenWidth
 	scrHeight = ScreenHeight

internal/testimage/spr_2_at_0_0_1.0_2.0.png

@@ -204,20 +204,22 @@ func SprSizeFlip(n, x, y int, w, h float64, flipX, flipY bool) {
 
 	screenOffset := y*scrWidth + x
 
-	spriteX := n % spritesInLine
-	spriteY := n / spritesInLine
+	spriteX := (n % spritesInLine) * SpriteWidth
+	spriteY := (n / spritesInLine) * SpriteHeight
 
-	if spriteX+int(w) >= spritesInLine {
-		w = float64(spritesInLine - spriteX)
+	width := int(SpriteWidth * w)
+	height := int(SpriteHeight * h)
+
+	if spriteX+width > ssWidth {
+		width = ssWidth - spriteX
 	}
 
-	if spriteY+int(h) >= spritesRows {
-		h = float64(spritesRows - spriteY)
+	if spriteY+height > ssHeight {
+		height = ssHeight - spriteY
 	}
 
-	spriteSheetOffset := spriteY*ssWidth*SpriteHeight + spriteX*SpriteWidth
+	spriteSheetOffset := spriteY*ssWidth + spriteX
 
-	width := int(SpriteWidth * w)
 	if x < clippingRegion.x {
 		dx := clippingRegion.x - x
 		width -= dx
@@ -231,7 +233,6 @@ func SprSizeFlip(n, x, y int, w, h float64, flipX, flipY bool) {
 		return
 	}
 
-	height := int(SpriteHeight * h)
 	if y < clippingRegion.y {
 		dy := clippingRegion.y - y
 		height -= dy

pi.go

@@ -494,20 +494,25 @@ func testSprSize(t *testing.T, sprSize func(spriteNo int, x, y int, w, h float64
 
 	t.Run("should draw sprite", func(t *testing.T) {
 		tests := map[string]struct {
+			spriteNo           int
 			w, h               float64
 			x, y               int
 			expectedScreenFile string
 		}{
-			"sprite at (0,0,0.5,1.0)": {w: 0.5, h: 1.0, expectedScreenFile: "spr_0_0_0.5_1.0.png"},
-			"sprite at (0,0,1.0,0.5)": {w: 1.0, h: 0.5, expectedScreenFile: "spr_0_0_1.0_0.5.png"},
-			"sprite at (0,0,2.0,1.0)": {w: 2.0, h: 1.0, expectedScreenFile: "spr_0_0_2.0_1.0.png"},
-			"sprite at (0,0,1.0,2.0)": {w: 1.0, h: 2.0, expectedScreenFile: "spr_0_0_1.0_2.0.png"},
-			"sprite at (0,0,2.5,1.0)": {w: 2.5, h: 1.0, expectedScreenFile: "spr_0_0_2.0_1.0.png"},
-			"sprite at (0,0,1.0,2.5)": {w: 1.0, h: 2.5, expectedScreenFile: "spr_0_0_1.0_2.0.png"},
-			"sprite at (8,0,2.0,1.0)": {x: 8, w: 2.0, h: 1.0, expectedScreenFile: "spr_8_0_2.0_1.0.png"},
-			"sprite at (0,8,1.0,2.0)": {y: 8, w: 1.0, h: 2.0, expectedScreenFile: "spr_0_8_1.0_2.0.png"},
-			"sprite at (0,0,1.1,0.5)": {w: 1.1, h: 0.5, expectedScreenFile: "spr_0_0_1.0_0.5.png"}, // should floor(w*8)
-			"sprite at (0,0,0.5,1.1)": {w: 0.5, h: 1.1, expectedScreenFile: "spr_0_0_0.5_1.0.png"}, // should floor(h*8)
+			"sprite at (0,0,0.5,1.0)":   {w: 0.5, h: 1.0, expectedScreenFile: "spr_0_0_0.5_1.0.png"},
+			"sprite at (0,0,1.0,0.5)":   {w: 1.0, h: 0.5, expectedScreenFile: "spr_0_0_1.0_0.5.png"},
+			"sprite at (0,0,2.0,1.0)":   {w: 2.0, h: 1.0, expectedScreenFile: "spr_0_0_2.0_1.0.png"},
+			"sprite at (0,0,1.0,2.0)":   {w: 1.0, h: 2.0, expectedScreenFile: "spr_0_0_1.0_2.0.png"},
+			"sprite at (0,0,2.5,1.0)":   {w: 2.5, h: 1.0, expectedScreenFile: "spr_0_0_2.0_1.0.png"},
+			"sprite at (0,0,1.0,2.5)":   {w: 1.0, h: 2.5, expectedScreenFile: "spr_0_0_1.0_2.0.png"},
+			"sprite at (8,0,2.0,1.0)":   {x: 8, w: 2.0, h: 1.0, expectedScreenFile: "spr_8_0_2.0_1.0.png"},
+			"sprite at (0,8,1.0,2.0)":   {y: 8, w: 1.0, h: 2.0, expectedScreenFile: "spr_0_8_1.0_2.0.png"},
+			"sprite at (0,0,1.1,0.5)":   {w: 1.1, h: 0.5, expectedScreenFile: "spr_0_0_1.0_0.5.png"}, // should floor(w*8)
+			"sprite at (0,0,0.5,1.1)":   {w: 0.5, h: 1.1, expectedScreenFile: "spr_0_0_0.5_1.0.png"}, // should floor(h*8)
+			"sprite 1 at (0,0,2.0,1.0)": {spriteNo: 1, w: 2.0, h: 1.0, expectedScreenFile: "spr_1_at_0_0_2.0_1.0.png"},
+			"sprite 1 at (0,0,1.9,1.0)": {spriteNo: 1, w: 1.9, h: 1.0, expectedScreenFile: "spr_1_at_0_0_2.0_1.0.png"},
+			"sprite 2 at (0,0,1.0,2.0)": {spriteNo: 2, w: 1.0, h: 2.0, expectedScreenFile: "spr_2_at_0_0_1.0_2.0.png"},
+			"sprite 2 at (0,0,1.0,1.9)": {spriteNo: 2, w: 1.0, h: 1.9, expectedScreenFile: "spr_2_at_0_0_1.0_2.0.png"},
 		}
 		for name, test := range tests {
 			t.Run(name, func(t *testing.T) {
@@ -519,7 +524,7 @@ func testSprSize(t *testing.T, sprSize func(spriteNo int, x, y int, w, h float64
 				pi.BootOrPanic()
 				expectedScreen := decodePNG(t, "internal/testimage/"+test.expectedScreenFile)
 				// when
-				sprSize(0, test.x, test.y, test.w, test.h)
+				sprSize(test.spriteNo, test.x, test.y, test.w, test.h)
 				// then
 				assert.Equal(t, expectedScreen.Pixels, pi.ScreenData)
 			})

screen.go

@@ -25,7 +25,6 @@ var (
 	ssWidth, ssHeight int
 	numberOfSprites   int
 	spritesInLine     int
-	spritesRows       int
 )
 
 // Sset sets the pixel color on the sprite sheet. It does not update the global Color.