[Demande de conseils] Wrapper C++ pour la SDL2

Salut à tous !

Cette fois je vais essayer de ne pas me planter en postant Je poste ici afin de présenter un petit projet sur lequel je travaille, en amateur, sur mon temps libre. Il s’agit d’un wrapper C++ (C++14, tant qu’à faire) pour la SDL. A la base je le fais surtout pour moi, mais je me dis que d’une part, ça peut peut-être aider de le partager (sait-on jamais), et que ça me permettra de recueillir quelques avis. N’hésitez donc pas à me faire part de toute remarque. (Je ne bosse pas beaucoup dessus donc les updates seront par contre assez irréguliers)

Je précise que je me remets doucement au C++ (je dirais même au dev en général), et que par conséquent il risque d’y avoir quelques "petites" ( ) imperfections. Je n’ai pas la prétention de faire une lib révolutionnaire ou quoi hein

Bref.

L’idée est avant tout de ne pas avoir à se soucier de la gestion de la mémoire, mais accessoirement de pouvoir garder une certaine compatibilité avec la SDL C. A cet effet, la plupart des entités proposées par le wrapper est implicitement convertible vers son équivalent C. Exemple :

1
2
3
4
5
6

void foo(SDL_Window *window){
  // ...
}
// ...
sdl::Window window{"My Window", sdl::Size{500, 500}};
foo(window); // Ok !

L’approche OO

GUI

Il est tout à fait possible de partir de zéro avec ce wrapper, en faisant ça en orienté objet. Pour la partie affichage, sdl::Window peut tout à fait faire le travail :

1
2
3
4

sdl::Window window{"My Window", sdl::Size{500, 500}};
sdl::Surface surface{sdl::load_bmp("image.bmp")}; // Lèvera une exception si problème
// ...
window.blit(surface, sdl::Point{50, 50});

Evénements

Concernant la gestion des événements, il y a une petite toolbox :

 1
 2
 3
 4
 5
 6
 7
 8
 9
10

// Gestionnaire d'événéments
sdl::EventHandler handler;
auto& mouse_handler = handler.get<sdl::MouseStateHandler>();
auto& keyboard_handler = handler.get<sdl::keyboardStateHandler>();
// Binding d'événements :
handler.on_press(sdl::KeyCode::A, [](){
  std::cout << "Key A pressed" << std::endl;
});
// Et dans la main loop :
handler.update(); // Va appeler sdl::MouseStateHandler::update() et sdl::KeyboardStateHandler::update()

(Derrière tout ça il y a un ECS qui tourne, qu’il est peut-être possible d’utiliser pour d’autres choses, genre gestion de scènes par exemple ?)

Audio

Pour tout ce qui touche à l’audio… je le ferai peut-être un jour

Autres

Il reste encore la gestion du texte et plein d’autres trucs, ça sera fait quand je prendrai le temps nécessaire, je n’oublie pas

L’approche classique

Comme évoqué précédemment, certaines entités SDL ont leur équivalent dans ce wrapper :

Voici les entités disponibles façon SDL C et leur équivalent dans le wrapper :

Il est ainsi possible de les utiliser dans un code SDL classique, car comme évoqué précédemment, chaque classe du type sdl::Color, sdl::Point, … est implicitement convertible vers SDL_Color, SDL_Point, etc.

Conclusion

Voilà, en (très) gros l’idée du projet. C’est actuellement en plein chantier (je n’ai pas énormément de temps à y consacrer), mais je souhaiterais avoir des avis sur le code, et éventuellement (sûrement même ^^) pouvoir revoir certaines parties. N’étant pas un expert (loin de là), je suis sûr qu’il y a beaucoup à dire sur le sujet. Parallèlement à cela, j’ai espoir que ça puisse servir à ceux qui aimeraient utiliser la SDL en C++. Mais bon, je pense qu’en l’état, c’est encore un peu trop limité pour être réellement utile.

En attendant, merci d’avoir eu la patience de lire ce post !

EDIT - Avec un lien vers le repo c’est mieux

Salut !

Merci de ta réponse (La mienne sera assez courte, je n’ai que peu de temps pour le moment, je l’étofferai sûrement en journée)

Je suis globalement d’accord avec tes remarques. Je reviens sur celle-là :

1 2 3

if (!ptr) { throw std::runtime_error{"Null pointer"}; }

Beurk. Soit tu acceptes le pointeur nul et tu ne mets pas cette exception, soit c’est refusé et tu cales une assertion, pas une exception.

(Dans Wrapper::Wrapper) L’exception est là pour ne pas se taper une assertion quand on veut par exemple créer une sdl::Surface ; comme ça dépend de conditions externes (est-ce que le fichier existe ? Est-ce que l’utilisateur a mis un chemin correct ? Etc.) je préfère passer par une exception. Mais avec un peu de recul je pense avoir vu le problème dans le mauvais sens ; ça n’est pas à Wrapper de prévoir les "mauvais" cas d’utilisation de ses classes filles.

En revanche, passer par quelque chose comme make_surface permettrait certainement de régler le soucis et de mettre un assert.

C’est exactement là que je veux en venir. Wrapper est correct avec nul, ce sont les usages que l’on en fait qui sont susceptible de ne pas l’être.

Du coup, pour charger une sdl::Surface, l’idée serait d’avoir un truc du genre :

1
2
3
4
5
6
7
8

template<class ...Args>
Surface make_surface(auto f, Args... args){
  auto ptr = f(args...);
  if (!ptr) {
    // Exception
  }
  return Surface(ptr);
}

Et on pourrait presque imaginer un truc du genre :

1
2

namespace ph = std::placeholders;
const auto load_bmp = std::bind(make_surface, SDL_LoadBMP, ph::_1);

?

(Pas sûr que ça soit viable ou quoi, mais je propose quand même)

Ouais, juste te fais pas chier avec bind, c’est plus lisible avec une lambda. Ce machin a l’air de marcher :

1
2
3
4
5
6
7
8

template<class Function, class ...Args>
Surface make_surface(Function f, Args... args){
  auto ptr = f(args...);
  if (!ptr) throw "erreur";
  return Surface(ptr);
}

auto const load_bmp = [](auto v){ return make_surface(SDL_LoadBMP, v); };

Pour lambda vs std::bind, la différence est juste syntaxique non ?

Sinon, SDL_LoadBMP étant une macro, j’ai dû bidouiller comme ça car je peux pas directement la passer à make_surface :

1
2
3
4
5
6

auto const load_bmp = [](std::string const& filename) {
  const auto load= [](std::string const& f) {
    return SDL_LoadBMP(f.c_str());
  };
  return make_surface(load, filename);
};

Sinon : si l’on veut rendre sdl::Surface copiable, je me demande comment procéder pour le constructeur de copie / l’operator= : est-ce que je garde en membre privé le chemin du fichier pour faire un truc du genre : => Pas viable si on construit la surface autrement que via SDL_LoadBMP

1
2
3
4
5

sdl::Surface::Surface(Surface const& surface) {
  // Avec ptr_ un SDL_Surface*
  ptr_ = SDL_LoadBMP(surface.filename_);
  // Gestion des erreurs
}

Ou est-il préférable d’allouer un SDL_Surface* pour copier celle existante ? (Attention, code crade en approche)

1
2
3
4
5

sdl::Surface::Surface(Surface const& surface) {
  assert(surface.ptr_ && "Null pointer");
  ptr_ = new SDL_Surface;
  *ptr_ = *surface.ptr_;
}

Pas bête l’idée du SDL_BlitSurface. Du coup je suis parti sur quelque chose comme :

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37

// Valeurs pompées depuis la doc
enum Mask : Uint32 {
#if SDL_BYTEORDER == SDL_BIG_ENDIAN
  R = 0xff000000,
  G = 0x00ff0000,
  B = 0x0000ff00,
  A = 0x000000ff
#else
  R = 0x000000ff,
  G = 0x0000ff00,
  B = 0x00ff0000,
  A = 0xff000000
#endif
};

// Puis
sdl::Surface::Surface(Surface const& surface) {
  copy_surface(surface);
}

void sdl::Surface::copy_surface(Surface const& surface) {
  // Vérifier que ptr_ de la surface actuelle == nullptr ?
  assert(surface.ptr_ && "Null pointer");
  assert(surface.ptr_->format && "Null pointer");
  ptr_ = SDL_CreateRGBSurface(0, surface.ptr_->w,
    surface.ptr_->h, surface.ptr_->format->BitsPerPixel, Mask::R, Mask::G, Mask::B, Mask::A);
  if (!ptr_) {
    throw std::runtime_error{"Cannot create surface"};
  }
  SDL_Rect rect;
  rect.x = 0;
  rect.y = 0;
  rect.w = surface.ptr_->w;
  rect.h = surface.ptr_->h;
  SDL_BlitSurface(surface.ptr_, &rect, ptr_, 0);
  // Vérifier ptr_ ?
}

Par contre :

Pourquoi pas un truc du genre :

1
2
3
4

sdl::Surface& sdl::Surface::operator=(Surface const& surface) {
  copy_surface(surface);
  return *this;
}

Je ne comprends pas pourquoi swap..?

Parce que tu as potentiellement l’ancienne surface à libérer. Donc :

1
2
3
4
5

sdl::Surface& sdl::Surface::operator=(Surface const& surface) {
  Surface copy(surface);
  std::swap(*this, copy);
  return *this;
}

Comme ça l’ancienne surface est placée dans "copy" et libérée en fin de bloc.

Effectivement, c’est pas con du tout.

J’aurais simplement tendance à remplacer std::swap(*this, copy); par std::swap(ptr_, copy.ptr_);, avec ptr_ le SDL_Surface* contenu dans sdl::Texture. (Dans certains cas j’ai des asserts qui pètent sinon)

Normalement, c’est sensé être la même chose puisque ta surface est assimilable à un pointeur . Donc si tu as des asserts qui cassent, il faut comprendre la raison. Normalement, les textures ne restent pas liées à la surface dont elles sont originaires (puisque l’on peut les supprimer).

Ah non mais je dis une connerie je crois en fait.

Il me semble que j’avais rajouté un constructeur par défaut pour un test bidon, et qu’après l’appel à operator=, comme ptr_ était nul le premier assert de copy_surface ne passait plus.

(J’ai pas le code sous la main là, je suis au boulot, mais je vérifie ce soir)

EDIT - Effectivement, c’était bien ça.

J’ai commencé les modifs, je finis de tout commiter ce soir. (Notamment virer l’operator T*)

J’aimerais juste revenir sur un point :

Ok pour la taille de la surface.

Toutefois, pour sdl::Rect et sdl::Point, j’aimerais pouvoir accéder aux variables membres (x, y, w, h) sans avoir à passer par des getters/setters (qui de toute façon cassent l’encapsulation en plus de rendre le code verbeux), ce qui m’oblige à les dupliquer.

Je me demande déjà pourquoi tu as fait de nouvelles classes. Un simple renommage et on était bon :

1
2
3
4

namespace sdl {
  using Rect = SDL_Rect;
  using Point = SDL_Point;
}

Et roulez jeunesse. Au pire, si c’est le typage "int" qui te gêne, les getters/setters ne sont pas un problème ici : ces éléments ne sont simplement pas des classes, ce sont des agrégats de données. Il n’y pas d’encapsulation qui tienne, il n’y a pas d’invariant à maintenir, c’est juste des données, donc tu peux donner l’accès plein.

J’y ai déjà pensé, mais il y a deux points qui me gênent :

Point 1. La construction : je veux pouvoir construire sdl::Rect en ne lui passant que deux valeurs : w et h pour avoir un rect qui vaut {0, 0, w, h} (ce qui me fait penser que je me suis planté dans l’implémentation actuelle ^^). De plus, si jamais SDL_Rect change (que pour une raison X ou Y l’ordre des variables membres passe de {x, y, w, h} à {w, h, x, y} par exemple) je ne me fasse pas avoir en voulant construire mon rect :

1
2
3
4
5

auto x = ...;
auto y = ...;
auto width = ...;
auto height = ...;
sdl::Rect rect{x, y, w, h}; // Et paf, je me retrouve avec w=x, h=y, x=w et h=y

Cas très hypothétique qui n’a approximativement aucune chance d’arriver, j’en conviens, mais on ne sait jamais.

EDIT : En revanche, on peut imaginer une fonction make_rect. Ce qui permet de contrebalancer le truc. Du style :

1
2

sdl::Rect make_rect(int x, int y, unsigned w, unsigned h);
sdl::Rect make_rect(unsigned w, unsigned h);

Point 2. L’héritage : Actuellement, sdl::Rect hérite de api::IConvertible<SDL_Rect> (qui va sûrement dégager car ça ne m’est pas utile), et ecs::Component, qui servira lorsque je mettrai en place une notion de "composant dynamique" (juste à l’état d’idée pour l’instant, mais je vais creuser ça), ou plus simplement de scènes.

Je sais, ce n’est pas très clair (désolé :/) et sûrement un peu perché, mais je pense que ça va être bientôt utile. (Et si finalement j’abandonne l’idée 2 et que le point 1 s’avère peu utile, alors je considérerai à nouveau l’idée de faire des alias )

Alors pour le changement de signature, il n’a strictement aucune chance d’arriver. On ne casse pas une API sans raison. Si c’est la construction qui t’inquiète alors part sur quelque chose comme :

 1
 2
 3
 4
 5
 6
 7
 8
 9
10

namespace sdl{
  class Rect : private SDL_Rect{
  public:
    Rect(/*parameters*/);
    using SDL_Rect::x ;
    using SDL_Rect::y ;
    using SDL_Rect::w ;
    using SDL_Rect::h ;
  };
}

Et ça résout aussi ton problème d’héritage.

Je reviens juste là-dessus :

Je suis d’accord sur le fond, mais si l’on doit définir un Rect, on dira que c’est l’agrégation de coordonnées X et Y, et d’une taille. Or, le typage int permet des tailles négative (ex : rect.w = -1), d’où ma question : si l’on accepte que Rect, en tant qu’agrégat de données, puisse avoir une taille négative, comment doit-on considérer/gérer ce cas ? Est-ce à l’appelant de vérifier que w >= 0 && h >= 0, ou doit-on prévoir ce cas d’utilisation dans l’implémentation du wrapper ? L’intérêt du type unsigned étant de présenter la garantie de ne pas avoir à ce soucier de ce genre de comportement.

Après, j’admets que je me prends un peu la tête sur une broutille

Sinon, l’idée de l’héritage privé, j’aime beaucoup, mais j’ai l’impression qu’hériter d’un type C ne plait pas à mon compilo :

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16

'y' : a déjà été initialisé
"h" n'est ni un membre de données non statique ni une classe de base de la classe "sdl::Rect"
"h" n'est ni un membre de données non statique ni une classe de base de la classe "sdl::Rect"
"w" n'est ni un membre de données non statique ni une classe de base de la classe "sdl::Rect"
"w" n'est ni un membre de données non statique ni une classe de base de la classe "sdl::Rect"
"x" n'est ni un membre de données non statique ni une classe de base de la classe "sdl::Rect"
"x" n'est ni un membre de données non statique ni une classe de base de la classe "sdl::Rect"
"y" n'est ni un membre de données non statique ni une classe de base de la classe "sdl::Rect"
"y" n'est ni un membre de données non statique ni une classe de base de la classe "sdl::Rect"
'h' : a déjà été initialisé
'h' : a déjà été initialisé
'w' : a déjà été initialisé
'w' : a déjà été initialisé
'x' : a déjà été initialisé
'x' : a déjà été initialisé
'y' : a déjà été initialisé

C’est au niveau des constructeurs que ça pète.

Pour ton problème de typage. Avec des strong types, tu pourra, d’une part, forcer l’utilisateur à utiliser le type qui te plait et effectuer toutes les vérification nécessaire (comme width > 0) ; d’autre part tu gagnera en expressivité. Ton code sera par contre un peu plus verbeux.

@GaaH : Aurais-tu un exemple d’utilisation applicable dans ce contexte ? J’avoue avoir un peu de mal à l’imaginer :/

C’est quoi ton compilo (incl version) ? Et fais voir ton code.