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

a marqué ce sujet comme résolu.

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 :

SDL C Wrapper
SDL_Color sdl::Color
SDL_Point sdl::Point
SDL_Rect sdl::Rect
SDL_Surface* sdl::Surface
SDL_Texture* sdl::Texture
SDL_Window* sdl::Window

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

+0 -0

Lu’!

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.

vincentp

Je ne suis pas convaincu de la pertinence de ce choix. Je vois deux cas de figure :

  1. la fonctionnalité utilisée appartient à SDL,
  2. c’est un code écrit en langage C qui fait des manipulations à travers ce contexte.

Dans le cas (1), la fonction doit avoir un équivalent utilisant du passage par référence et c’est marre. Ce sera moins casse-gueule et on obtient vraiment un wrapper C++ qui permet de développer dans un style purement C++.

Dans le cas (2), la fonction est très fortement susceptible de faire de la manipulation à la C, y compris d’éventuelles libérations et manipulation sur les pointeurs qui risquent de venir foutre la merde dans tes structures internes, ce que tu veux éviter par dessus tout.

Interdire la copie de surface me semble être une limitation rude. Interdire la copie de texture, ok, celle de surface c’est un peu dommage à l’usage.

Concernant le code ne duplique pas les données connues (taille de surface, de rectangle, etc …). Pour construire tes exceptions, utilise SDL_error. Ensuite :

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.

Finalement, le plus gros problème que j’ai vu se présente avec loadBMP. Elle casse ton wrapping. Exemple concret, le même que unique_ptr + new :

1
2
3
4
5
void foo(Surface s1, Surface s2);

void bar(){
  foo(Surface{ loadBMP("truc") }, Surface{ loadBMP("muche") });
}

Le compilateur est autorisé à traduire bar en :

1
2
3
4
5
void bar(){
  auto p = loadBMP("truc");
  auto q = loadBMP("muche");
  foo(Surface{ p }, Surface{ q });
}

qui n’est pas exception-safe. Il te faudrait probablement quelque chose du genre :

1
2
3
4
template<class ...Args>
Surface make_surface(auto f, Args... args){
  return Surface(f(args...));
}

Où tu rendrais ton constructeur par pointeur de surface privé et ta classe Surface amie de make_surface.

Une remarque que je me suis faite récemment est que l’on a bien envie également d’avoir un moyen de construire des unique_ptr et shared_ptr de types SDL sans créer deux niveaux d’indirections et tout en permettant l’usage des fonctions membres de nos types. Cela induit qu’on a besoin d’écrire des moyens de conversions implicites, mais je n’ai pas encore commencé à regarder comment faire.

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.

+0 -0

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.

vincentp

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_;
}
+0 -0

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

vincentp

Oui mais bind n’est pas plus pratique que les lambdas et a de bonnes chances d’être aussi déprécié à l’avenir. En plus tu te débarrasses de la dépendances à <functional> dans un source à moins de t’en servir pour autre chose dans le fichier.

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

vincentp

J’adore le C … Sinon :

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);
};

refactorisable en :

1
2
3
auto const load_bmp = [](std::string const& filename) {
  return make_surface([](auto const& f) { return SDL_LoadBMP(f.c_str());}, 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

vincentp

Pour la copie, je m’étais contenté de créer une nouvelle surface de la bonne taille avec la fonction appropriée puis de blitter l’ancienne dessus. Et pour l’affectation, copy-and-swap idiom.

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 :

Et pour l’affectation, copy-and-swap idiom.

Ksass`Peuk

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..?

+0 -0

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)

+0 -0

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.

+0 -0

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

J’aimerais juste revenir sur un point :

Concernant le code ne duplique pas les données connues (taille de surface, de rectangle, etc …).

Ksass`Peuk

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 ;) )

+0 -0

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 :

il n’y a pas d’invariant à maintenir, c’est juste des données, donc tu peux donner l’accès plein.

Ksass`Peuk

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.

+0 -0
Connectez-vous pour pouvoir poster un message.
Connexion

Pas encore membre ?

Créez un compte en une minute pour profiter pleinement de toutes les fonctionnalités de Zeste de Savoir. Ici, tout est gratuit et sans publicité.
Créer un compte