Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check image width on writePixel #181

Open
Elewyth opened this issue Nov 5, 2019 · 0 comments
Open

Check image width on writePixel #181

Elewyth opened this issue Nov 5, 2019 · 0 comments

Comments

@Elewyth
Copy link

Elewyth commented Nov 5, 2019

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 ()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant