Juicy.Pixels
Juicy.Pixels copied to clipboard
Check image width on writePixel
writePixel
on a MutableImage
seems to be intended as an abstraction of the underlying color (Pixel) implementation, as well as the coordinate mapping from (x, y) to a single integer index.
In my opinion, it seems unintuitive that writePixel
does not check whether the supplied x
coordinate exceeds the image's width. This usually does not even cause an error, but simply (due to the way mutablePixelBaseIndex
is implemented) writes the pixel in a later row. Supplying a too large y
always causes an out-of-bounds error (as expected).
This snippet reproduces the "x overflow":
img <- createMutableImage 50 100 (PixelRGB8 255 0 0)
writePixel img 25 25 (PixelRGB8 0 255 0)
writePixel img 75 25 (PixelRGB8 0 255 255)
freezeImage img
>>= writePng "test.png"
Obviously, checking the boundaries on every single writePixel
call could have some serious performance implications, which is why I suggest to add a new function, such as writePixelChecked
or safeWritePixel
(analogous to unsafeWritePixel
) which would check the x
and y
coordinates separately against the image dimensions and either throw an error, or more nicely return a Maybe ()
or Either String ()
.
A suggestion for an implementation:
safeWritePixel :: (PrimMonad m, Pixel a) => MutableImage (PrimState m) a -> Int -> Int -> a -> m (Either String ())
safeWritePixel image@MutableImage{ mutableImageWidth = w, mutableImageHeight = h } x y px
| x >= w = return $ Left "x exceeds the image's width"
| y >= h = return $ Left "y exceeds the image's height"
| otherwise = Right <$> writePixel image x y px
Which could e.g. be used as follows:
safeWritePixel img 50 25 (PixelRGB8 255 0 255)
>>= \case
Left err -> putStrLn err
Right () -> return ()