Fix negative coordinates in tiled image sampling

src/pixie/images.nim

@@ -367,8 +367,8 @@ proc blur*(
 proc getRgbaSmooth*(
   image: Image, x, y: float32, wrapped = false
 ): ColorRGBX {.raises: [].} =
-  ## Gets a interpolated color with float point coordinates.
-  ## Pixels outside the image are transparent.
+  ## Gets an interpolated color with floating point coordinates.
+  ## Pixels outside the image are transparent unless wrapped is true.
   let
     x0 = x.floor.int
     y0 = y.floor.int
@@ -379,10 +379,26 @@ proc getRgbaSmooth*(
 
   var x0y0, x1y0, x0y1, x1y1: ColorRGBX
   if wrapped:
-    x0y0 = image.unsafe[x0 mod image.width, y0 mod image.height]
-    x1y0 = image.unsafe[x1 mod image.width, y0 mod image.height]
-    x0y1 = image.unsafe[x0 mod image.width, y1 mod image.height]
-    x1y1 = image.unsafe[x1 mod image.width, y1 mod image.height]
+    template wrapCoordinate(value, size: int): int =
+      # Image dimensions are positive. Expand here to avoid a function call
+      # for every coordinate in the sampling loop.
+      block:
+        let extent = size
+        var coordinate = value mod extent
+        if coordinate < 0:
+          coordinate += extent
+        coordinate
+
+    # Adjacent samples need only a boundary check, not another division.
+    let
+      x0 = wrapCoordinate(x0, image.width)
+      x1 = if x0 + 1 < image.width: x0 + 1 else: 0
+      y0 = wrapCoordinate(y0, image.height)
+      y1 = if y0 + 1 < image.height: y0 + 1 else: 0
+    x0y0 = image.unsafe[x0, y0]
+    x1y0 = image.unsafe[x1, y0]
+    x0y1 = image.unsafe[x0, y1]
+    x1y1 = image.unsafe[x1, y1]
   else:
     x0y0 = image[x0, y0]
     x1y0 = image[x1, y0]

tests/bench_images_tiled.nim

@@ -0,0 +1,95 @@
+## Run with: nim c -r -d:release tests/bench_images_tiled.nim
+## Append "negative" to also benchmark the inputs fixed by issue #597.
+## On Windows, -d:benchAffinity pins this process to logical CPU 2.
+## CSV timings exclude setup and checksums. Compare multiple interleaved runs.
+import std/[monotimes, os, strformat, strutils]
+import pixie
+
+when defined(windows) and defined(benchAffinity):
+  proc GetCurrentProcess(): int {.stdcall, dynlib: "kernel32", importc.}
+  proc SetProcessAffinityMask(handle: int, mask: uint64): cint
+    {.stdcall, dynlib: "kernel32", importc.}
+  doAssert SetProcessAffinityMask(GetCurrentProcess(), 4) != 0
+
+const buildLabel {.strdefine.} = "working_tree"
+var checksum: uint64
+
+proc imageChecksum(image: Image): uint64 =
+  for c in image.data:
+    result = (result xor cast[uint32](c).uint64) * 1099511628211'u64
+
+template measure(name: string, iterations: int, body: untyped) =
+  block:
+    for warmup in 0 ..< 3:
+      body
+    let start = getMonoTime().ticks
+    for iteration in 0 ..< iterations:
+      body
+    let milliseconds = (getMonoTime().ticks - start).float64 / 1e6 / iterations.float64
+    echo buildLabel, ",", name, ",", iterations, ",",
+      formatFloat(milliseconds, ffDecimal, 6)
+
+proc makeTile(width, height: int, stripes = false): Image =
+  result = newImage(width, height)
+  for y in 0 ..< height:
+    for x in 0 ..< width:
+      result[x, y] = if stripes:
+        (if y == 0: rgba(35, 115, 210, 255) else: rgba(255, 255, 255, 255))
+      else:
+        rgba(((x * 37 + y * 13) and 255).uint8,
+          ((x * 11 + y * 43) and 255).uint8,
+          ((x * 23 + y * 17) and 255).uint8, 255)
+
+proc runFill(name: string, width, height, tileWidth, tileHeight, iterations: int,
+    negative = false) =
+  let
+    tile = makeTile(tileWidth, tileHeight, tileWidth == 4)
+    image = newImage(width, height)
+    paint = newPaint(TiledImagePaint)
+  paint.image = tile
+  paint.imageMat = rotate(-7.float32 * PI.float32 / 180)
+  if not negative:
+    # Keep all source coordinates positive so the unfixed build is valid.
+    paint.imageMat = paint.imageMat *
+      translate(vec2(-tileWidth.float32 * 512, -tileHeight.float32 * 512))
+  measure(name, iterations):
+    image.fill(paint)
+  echo &"checksum,{name},{imageChecksum(image)}"
+
+runFill("fill_4x4_256x160_positive", 256, 160, 4, 4, 160)
+runFill("fill_4x4_1024_positive", 1024, 1024, 4, 4, 8)
+runFill("fill_63x47_1024_positive", 1024, 1024, 63, 47, 8)
+
+block:
+  let
+    tile = makeTile(64, 64)
+    image = newImage(1024, 1024)
+  measure("drawTiled_64x64_identity", 10):
+    image.drawTiled(tile, mat3())
+  echo &"checksum,drawTiled_64x64_identity,{imageChecksum(image)}"
+
+block:
+  let
+    source = makeTile(512, 512)
+    image = newImage(1024, 1024)
+    transform = translate(vec2(256.25, 256.75)) * rotate(-7.float32 * PI.float32 / 180)
+  measure("draw_smooth_nonwrapped", 20):
+    image.draw(source, transform)
+  echo &"checksum,draw_smooth_nonwrapped,{imageChecksum(image)}"
+
+block:
+  let tile = makeTile(63, 47)
+  measure("sampler_positive_262144", 20):
+    var local: uint64
+    for y in 0 ..< 512:
+      for x in 0 ..< 512:
+        let sample = tile.getRgbaSmooth(
+          x.float32 + 0.25, y.float32 + 0.75, true
+        )
+        local += cast[uint32](sample).uint64
+    checksum = local
+  echo &"checksum,sampler_positive_262144,{checksum}"
+
+if paramCount() > 0 and paramStr(1) == "negative":
+  runFill("fill_4x4_256x160_negative", 256, 160, 4, 4, 160, true)
+  runFill("fill_63x47_1024_negative", 1024, 1024, 63, 47, 8, true)

tests/test_images_tiled.nim

@@ -0,0 +1,65 @@
+import pixie
+
+proc repeatImage(tile: Image, columns, rows: int): Image =
+  result = newImage(tile.width * columns, tile.height * rows)
+  for row in 0 ..< rows:
+    for column in 0 ..< columns:
+      for y in 0 ..< tile.height:
+        for x in 0 ..< tile.width:
+          result[column * tile.width + x, row * tile.height + y] = tile[x, y]
+
+block:
+  # Wrapped sampling must match an explicitly repeated image, including at
+  # negative coordinates and across horizontal, vertical and corner seams.
+  for size in [ivec2(3, 2), ivec2(1, 3), ivec2(3, 1), ivec2(1, 1)]:
+    let tile = newImage(size.x, size.y)
+    for y in 0 ..< tile.height:
+      for x in 0 ..< tile.width:
+        tile[x, y] = rgbx(
+          (x * 37 + y * 13).uint8,
+          (x * 11 + y * 43).uint8,
+          (x * 23 + y * 17).uint8,
+          (128 + x * 19 + y * 29).uint8
+        )
+    let repeated = repeatImage(tile, 7, 7)
+    for y in -2 * tile.height .. 2 * tile.height:
+      for x in -2 * tile.width .. 2 * tile.width:
+        for fy in [0.float32, 0.25, 0.5, 0.75]:
+          for fx in [0.float32, 0.25, 0.5, 0.75]:
+            doAssert tile.getRgbaSmooth(x.float32 + fx, y.float32 + fy, true) ==
+              repeated.getRgbaSmooth(
+                (x + 3 * tile.width).float32 + fx,
+                (y + 3 * tile.height).float32 + fy
+              )
+    doAssert tile.getRgbaSmooth(-2, -2) == rgbx(0, 0, 0, 0)
+
+block:
+  # https://github.com/treeform/pixie/issues/597
+  let tile = newImage(4, 4)
+  tile.fill(rgba(255, 255, 255, 255))
+  for x in 0 ..< tile.width:
+    tile[x, 0] = rgba(35, 115, 210, 255)
+
+  let repeated = repeatImage(tile, 192, 192)
+  for degrees in [-7.float32, 7, -90, 90, 180]:
+    let paint = newPaint(TiledImagePaint)
+    paint.image = tile
+    paint.imageMat = rotate(degrees * PI.float32 / 180)
+    let
+      image = newImage(256, 160)
+      inverse = paint.imageMat.inverse()
+    image.fill(paint)
+    for y in 0 ..< image.height:
+      for x in 0 ..< image.width:
+        let
+          sample = inverse * vec2(x.float32 + 0.5, y.float32 + 0.5)
+          expected = repeated.getRgbaSmooth(
+            sample.x - 0.5 + 256, sample.y - 0.5 + 256
+          )
+          actual = image[x, y]
+        # Moving the reference coordinates can change float32 rounding by
+        # one channel value, but cannot change the stripe placement or alpha.
+        doAssert abs(actual.r.int - expected.r.int) <= 1
+        doAssert abs(actual.g.int - expected.g.int) <= 1
+        doAssert abs(actual.b.int - expected.b.int) <= 1
+        doAssert actual.a == 255

tests/tests.nim

@@ -6,6 +6,7 @@ import
   test_gif,
   test_images,
   test_images_draw,
+  test_images_tiled,
   test_jpeg,
   test_paints,
   test_paths,