After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 778052 - SDL.PixelFormat class cannot be instantiated
SDL.PixelFormat class cannot be instantiated
Status: RESOLVED FIXED
Product: vala
Classification: Core
Component: Bindings
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Vala maintainers
Vala maintainers
Depends on:
Blocks:
 
 
Reported: 2017-02-01 23:08 UTC by Nick Schrader
Modified: 2017-02-14 08:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Suggested Patch (1.60 KB, patch)
2017-02-01 23:08 UTC, Nick Schrader
rejected Details | Review
sdl: Make Palette and PixelFormat a struct (2.06 KB, patch)
2017-02-04 12:46 UTC, Rico Tzschichholz
committed Details | Review

Description Nick Schrader 2017-02-01 23:08:25 UTC
Created attachment 344750 [details] [review]
Suggested Patch

Basically the same problem as with #669279 just with another class. PixelFormat and Palette are simple structs that cannot be mapped to classes, see suggested patch.

At the moment the component is not usable!
Comment 1 Rico Tzschichholz 2017-02-02 09:44:05 UTC
sdl.vapi is not SDL2

What you want to test and use is this:
https://git.gnome.org/browse/vala-extra-vapis/tree/sdl2.vapi#n1701
Comment 2 Nick Schrader 2017-02-02 11:20:19 UTC
I'm totally talking about SDL1: https://www.libsdl.org/release/SDL-1.2.15/docs/html/sdlpixelformat.html

Just try to compile the following program using vanilla Vala:

using SDL;

public class SDLTest {
  static int main(string[] args) {
    PixelFormat pf = new PixelFormat();
    return 0;
  }
}

It won't work because PixelFormat has no constructor (and is not even a compact class).
Comment 3 Rico Tzschichholz 2017-02-02 12:08:33 UTC
Sorry, I see, your "patch" didn't show much experience which is why I made this conclusion.

Afaics creating a PixelFormat that way is not supported at all? The structure simply serves the purpose to access this information of the corresponding Surface. So defining it as compact class seems appropriate.

Of course the PixelFormat.*mask fields are wrongly typed.

I personally have no experience with the SDL API though.
Comment 4 Nick Schrader 2017-02-02 16:50:10 UTC
I actually noticed that there is a problem when trying to use Surface.convert(). You need to instantiate a PixelFormat to pass it as a parameter which is not possible at the moment.

Regarding my suggested solution, I just did the same as you did with the AudioSpec structure in #669279, because it is exactly the same situation: The structure is not only needed to access information but also to pass it to some functions. I don't know how to avoid properly the pointer to the Palette class/struct in PixelFormat class/struct.

Concerning my experience writing vapi files you are right, it is rather tinkering than really knowing
Comment 5 Rico Tzschichholz 2017-02-04 12:46:49 UTC
Created attachment 344924 [details] [review]
sdl: Make Palette and PixelFormat a struct
Comment 6 Rico Tzschichholz 2017-02-04 12:47:05 UTC
Could you provide a more complex/real-life test-case of the API in question here?
Comment 7 Nick Schrader 2017-02-04 17:33:00 UTC
Okay, something like that shout be quite close to real-life usage:

using SDL;

public class SDLTest {
  static int main(string[] args) {
    Surface s = new Surface.RGB(0, 10, 10, 32, 0, 0xff, 0xff00, 0xff0000);
    PixelFormat pf = PixelFormat();
    s.convert(pf, 0);
    stdout.printf("%d", s.format.palette.colors[0].r);
    return 0;
  }
}

It uses Surface.convert() (where I noticed the problem) and then it tries to access the PixelFormat structure as it worked before.
Comment 8 Rico Tzschichholz 2017-02-14 08:39:30 UTC
Attachment 344924 [details] pushed as 343bafe - sdl: Make Palette and PixelFormat a struct