createCursor is broken, should pack Bools in Word8, sample working implementation provided

[Mikolaj]

https://hackage.haskell.org/package/sdl2-2.5.3.0/docs/SDL-Input-Mouse.html#v:createCursor

is broken, shows every one of four vertical lines. It should probably use

https://hackage.haskell.org/package/base-4.12.0.0/docs/Foreign-C-Types.html#t:CBool

instead of Bool, as per

https://stackoverflow.com/questions/41777675/large-size-of-bool-in-foreign-storable/46784931

However, due to old Ubuntu and so old C sdl2, the latest I can use is sdl2-2.4.1.0, so perhaps this was fixed (no related issue seems to exist, but I didn't search commits). Should I unpack old sdl2, fix it and see if that helps?

[Mikolaj]

I've looked at the implementation in my old version:

createCursor dta msk (V2 w h) (P (V2 hx hy)) =
    liftIO . fmap Cursor $
        throwIfNull "SDL.Input.Mouse.createCursor" "SDL_createCursor" $
            V.unsafeWith (V.map (bool 0 1) dta) $ \unsafeDta ->
            V.unsafeWith (V.map (bool 0 1) msk) $ \unsafeMsk ->
                Raw.createCursor unsafeDta unsafeMsk w h hx hy

and CBool would probably not help, because the booleans are converted to Word8. Also, this code is unchanged on master.

[Mikolaj]

Oh, the C version says something the Haskell one doesn't: 'The cursor width (w) must be a multiple of 8 bits.' https://wiki.libsdl.org/SDL_CreateCursor

[Mikolaj]

Oh, and the data the C function receives clearly needs to be packed, not one bool per Word8.

[Mikolaj]

I've fixed this locally: LambdaHack/LambdaHack@b39c31b

[Mikolaj]

I guess the decision is what the input type for the correct operation should be. The one I opted for in the implementation is vector of Word8, which gives the most power to the user. However, poweruser can use the Raw versions, so a list of Bools is an option, as well, and I provide a conversion from that to the vector of Word8. Basically, the fix is ready, but requires maintainer's decision.

[dpwiz]

Using bool lists to define cursors produces quite a show for something even remotely large.
On the other hand, base-2 literals make it almost nice.

box8x8 :: Vector Word8
box8x8 =
  [ 0b11111111
  , 0b10000001
  , 0b10000001
  , 0b10000001
  , 0b10000001
  , 0b10000001
  , 0b10000001
  , 0b11111111
  ]

Perhaps even better UX would be allowing larger Word sizes:

  [ 0b1111111111111111
  , 0b1000000100000001
  , 0b1000000100000001
  , 0b1000000100000001
  , 0b1000000100000001
  , 0b1000000100000001
  , 0b1000000100000001
  , 0b1111111111111111
  , 0b1000000100000001
  , 0b1000000100000001
  , 0b1000000100000001
  , 0b1000000100000001
  , 0b1000000100000001
  , 0b1000000100000001
  , 0b1111111111111111
  ]

I'm postponing fixing this one in order not to stall next Hackage release. We can bikeshed the options for a while (:

[Mikolaj]

Using bool lists to define cursors produces quite a show for something even remotely large.
On the other hand, base-2 literals make it almost nice.

Right. That's not far from the format I'm using in the link above. Newest version: https://github.com/LambdaHack/LambdaHack/blob/7ed94b4b6c75fc46afeef01d8ac476c4598fb822/engine-src/Game/LambdaHack/Client/UI/Frontend/Sdl.hs#L421-L455

[dpwiz]

How about replacing Bool with a FiniteBits a, Storable a polymorphic type? You can then use Word8 .. Word64 to paint cursors in different chunks.

Alternatively, stick to storage type (Word8) and maybe upstream that crosshair-painting function.

[Mikolaj]

Both make sense.