Passing `image[x, y]` to var parameter is compile error

[demotomohiro]

Compiling following code is compile error:

import pixie

var image = newImage(10, 10)
image[0, 0] = rgba(0, 255, 255, 255)
image[0, 1] = rgba(255, 255, 0, 255)
swap(image[0, 0], image[0, 1])
doAssert image[0, 0] == rgba(255, 255, 0, 255)
doAssert image[0, 1] == rgba(0, 255, 255, 255)

Error message:

/tmp/tmp/testpixie/test.nim(11, 5) Error: type mismatch: got <ColorRGBX, ColorRGBX>
but expected one of:
proc swap[T](a, b: var T)
  first type mismatch at position: 1
  required type for a: var T
  but expression 'image[0, 0]' is immutable, not 'var'

expression: swap(image[0, 0], image[0, 1])

Adding proc `[]`(image: var Image, x, y: int): var ColorRGBX allows passing image[x, y] to var parameters. Following code compiles and runs:

import pixie

proc `[]`(image: var Image, x, y: int): var ColorRGBX {.inline, raises: [].} =
  assert image.inside(x, y)
  image.unsafe[x, y]

var image = newImage(10, 10)
image[0, 0] = rgba(0, 255, 255, 255)
image[0, 1] = rgba(255, 255, 0, 255)
swap(image[0, 0], image[0, 1])
doAssert image[0, 0] == rgba(255, 255, 0, 255)
doAssert image[0, 1] == rgba(0, 255, 255, 255)

Another workaround is copy image[x, y] to temporal variable, pass it to var parameter and copy back to image[x, y].

Can adding proc `[]`(image: var Image, x, y: int): var ColorRGBX to pixie/images.nim cause any problems?

[guzba]

Yes that does cause some trouble sadly. I tries this out myself and here is what I ran into:

The image[x, y] accessor is a safe way to access image data that returns rgbx(0, 0, 0, 0) if you attempt to read outside the bounds of the image.

We must check if image.inside(x, y) first, not just assert as shown in your example. Further, asserts are not run in release so we'd need doAssert. But the point is to not crash and just get transparent black, so asserts are not an option.

Since we need to branch and cannot guarantee we have an addressable heap memory thing to return a pointer to (var type), we cannot return a var ColorRGBX.

If you want to swap I suggest using swap image.data[image.dataIndex(x, y)], image.data[image.dataIndex(x, y)] and accept your program will crash if you attempt to swap out of bounds.

[demotomohiro]

Thank you for your reply.

So image[x, y] works differently from seq[T] when it got out of bound coordinate. myseq[idx] always raise exception when idx is out of bound unless -d:danger is set. But image[x, y] returns transparent black, and image[x, y] = v does nothing instead of raising an exception. And there is no way to image[x, y] returns var ColorRGBX safely in that case.

Adding new procedure that raises exception in the same way as myseq[idx] allows returning var ColorRGBX safely. And I can add it to my code.

import std/strformat
import pixie

proc `{}`(image: var Image, x, y: int): var ColorRGBX {.inline.} =
  ## Accessing any pixels outside the bounds of the image is error
  when not defined(danger):
    if not image.inside(x, y):
      raise newException(IndexDefect, &"({x}, {y}) not in range ({image.width - 1}, {image.height - 1})")

  image.unsafe[x, y]

var image = newImage(10, 10)
image[0, 0] = rgba(0, 255, 255, 255)
image[0, 1] = rgba(255, 255, 0, 255)
swap(image{0, 0}, image{0, 1})
doAssert image[0, 0] == rgba(255, 255, 0, 255)
doAssert image[0, 1] == rgba(0, 255, 255, 255)
doAssertRaises(IndexDefect):
  echo image{11, 0}

[guzba]

This is sort of true. You should be careful about the exception from seq[] though / IndexDefect. They are not considered catchable.

IndexDefect is a Defect and Defects are "Abstract base class for all exceptions that Nim's runtime raises but that are strictly uncatchable as they can also be mapped to a quit / trap / exit operation."

So counting on seq[] access exceptions to manage your control flow (using try catch) could be troublesome.

[guzba]

I have another suggestion and change we can make here:

It should work to use:

swap image.unsafe[x, y], image.unsafe[x + 1,y]

To enable this we can make image.unsafe[x, y] return a var ColorRGBX since it will segfault if you ask for something out of bounds.

[guzba]

I guess as templates they already work / the var or not part of the return doesnt matter.

[demotomohiro]

When we use seq[T], we avoid accessing it with out of bounds index. But when we accidentally access it with out of bound index, it is a bug need to be fixed. If seq[T] returns default(T) instead of raising IndexDefect, it is hard to find the bug.

There are cases you want image[x, y] returns transparent black when x and/or y are outside the bounds of the image. But when you write code so that x and y are always inside the bound of the image, bound check helps to find the bug.

swap(image.unsafe[0, 0], image.unsafe[0, 1]) works. But there is only bound check in Image.data. It doesn't check for x >= image.width.

import std/strformat
import pixie

var image = newImage(10, 10)
image[0, 0] = rgba(0, 255, 255, 255)
image[0, 1] = rgba(255, 255, 0, 255)
swap(image.unsafe[0, 0], image.unsafe[0, 1])
doAssert image[0, 0] == rgba(255, 255, 0, 255)
doAssert image[0, 1] == rgba(0, 255, 255, 255)
doAssertRaises(IndexDefect):
  echo image.unsafe[10, 10]
# This assert fails because IndexDefect is not raised.
# doAssertRaises(IndexDefect):
#   echo image.unsafe[11, 0]