fccm / OCamlSDL2

OCaml interface to SDL 2.0 (for Linux, Windows, MacOS, and ChromeBook)
Other
51 stars 10 forks source link

fix color masks #23

Closed cacilhas closed 3 years ago

fccm commented 3 years ago

Hi, In my work in progress local version, I'm using:

#if SDL_BYTEORDER == SDL_BIG_ENDIAN
    Uint32 Rmask = 0xff000000;
    Uint32 Gmask = 0x00ff0000;
    Uint32 Bmask = 0x0000ff00;
    Uint32 Amask = 0x000000ff;
#else
    Uint32 Rmask = 0x000000ff;
    Uint32 Gmask = 0x0000ff00;
    Uint32 Bmask = 0x00ff0000;
    Uint32 Amask = 0xff000000;
#endif

Did you check the source code of SDL_CreateRGBSurface() to check what's happening when we feed with zeros?

cd /tmp
wget https://www.libsdl.org/release/SDL2-2.0.14.tar.gz
$EDITOR SDL2-2.0.14/src/video/SDL_surface.c
$EDITOR SDL2-2.0.14/src/video/SDL_pixels.c

For what I understand SDL_CreateRGBSurface() calls SDL_MasksToPixelFormatEnum() which resides in video/SDL_pixels.c.

What do you think about these default values?

cacilhas commented 3 years ago

Sorry, the mark order was really inverted.

cacilhas commented 3 years ago

Did you check the source code of SDL_CreateRGBSurface() to check what's happening when we feed with zeros?

I didn’t check the source… but when I tried to create a transparent surface to blit over the window surface, the background became black instead of transparent.

What do you think about these default values?

I think they’re perfect. It just need a way to supply to the OCaml code which byte order the system uses.

OCamlSDL does it by supplying a function to convert from RGB int * int * int to int32 according to the surface masks; but that way you’re obliged to inform a surface every time you need to calculate an int32 color – it’s overkill.

I think it’d be enough a function doing that conversion based on the SDL byte order (SDL_BYTEORDER).

Besides, the input could be int * int * int -> int (RGB → A).

fccm commented 3 years ago

Maybe we should just expose these parameters to the interface? With a default value set to zero?

fccm commented 3 years ago

Does this branch provide a solution for what you need? https://github.com/fccm/OCamlSDL2/tree/surface-rgba-mask

cacilhas commented 3 years ago

Mmmm… I can’t see a solution in that… I’d need to run some tests to be sure.

On order hand, I can work on a simple solution according to my proposal and open a new PR, if you agree.

I just need a couple of days, ’cause I’m little busy this weekend, and I can’t get into it right now.

cacilhas commented 3 years ago

@fccm I’ve coded this, but I’m not familiar to the caml/*.h interface, so I’m doing something wrong, and the prompt keeps crashing every time I run Sdl.Surface.rgba_color ~rgb:(0xff, 0xff, 0xff) ~a:0xff.

Can you help me to see what’s the problem?

cacilhas commented 3 years ago

@fccm I’ve coded this, but I’m not familiar to the caml/*.h interface, so I’m doing something wrong, and the prompt keeps crashing every time I run Sdl.Surface.rgba_color ~rgb:(0xff, 0xff, 0xff) ~a:0xff.

Can you help me to see what’s the problem?

@fccm I think I’ve solved it: I was using Val_long, but it should be caml_copy_int32.

cacilhas commented 3 years ago

@fccm It’s done, I think this PR solves the problem in a simple fashion.

Please review this.

fccm commented 3 years ago

when I tried to create a transparent surface to blit over the window surface, the background became black instead of transparent.

Do you have the same issue with this example: https://github.com/fccm/OCamlSDL2/blob/master/examples/ex_sprite.ml

cacilhas commented 3 years ago

I’ve gotten this:

File "ex_sprite.ml", line 4, characters 18-41:
4 |   | Event.KeyDown { keycode = Keycode.Q }
                      ^^^^^^^^^^^^^^^^^^^^^^^
Error (warning 9 [missing-record-field-pattern]): the following labels are not bound in this record pattern:
ke_timestamp, ke_window_id, ke_state, ke_repeat, scancode, keymod
Either bind these labels explicitly or add '; _' to the pattern.

And a lotta other error messages, the same or related.

fccm commented 3 years ago

I'm still using OCaml version 4.11.1.

I think you're using a different version.

If you're an opam user, you could install 4.11.* inside an opam switch.

If you don't want to switch, I think the errors you're reporting are not difficult for you to fix. When I read your code, you seem quite good enough for fixing these kind of simple errors. The error message you're copy pasting even tells you what you have to do:

bind these labels explicitly or add '; _' to the pattern

The following also works with 4.11.1:

  | Event.KeyDown { keycode = Keycode.Q; _ }
  | Event.KeyDown { keycode = Keycode.Escape; _ }
fccm commented 3 years ago

This commit 931ffff should fix this error.

You said there are a lot other error messages, the same or related. Could you copy-past?

fccm commented 3 years ago

@fccm I’ve coded this, but I’m not familiar to the caml/*.h interface,

This is something you could write in pure ocaml, if you're not familiar with C.