Demande d'avis : Horloge, SDL/C

Invité · 12/08/2006, 09h26

Bonjour, débutant avec la SDL, je viens de terminer une simple petite horloge graphique (au design kitsch

). J'aimerai votre avis sur le code, merci.

Les sources
l'executable (windows)

Pour ceux qui n'ont pas 7zip ou ne veulent pas télécharger les archives, je met le code ici :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

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
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
 
 
#include <stdio.h>
#include <stdlib.h>
#include <math.h>
#include <time.h>
#include <locale.h>
#include <SDL/SDL.h>
#include <SDL/SDL_Image.h>
 
 
#define RAYON 80
#define rectifHEU 30
#define rectifMIN 15
 
void programme(void);
void allume_pixel(int x, int y,Uint32 couleur);
void dessineHorloge(void);
void recupTemps(void);
void afficheSecondes(int seconde);
void afficheMinutes(int minute);
void afficheHeures(int heure);
void tracerSegment(int xi,int yi,int xf,int yf, Uint32 couleur);
 
SDL_Surface *ecran;
int init = 0;
 
int main (int argc, char *argv[])
{
 
    atexit(SDL_Quit);
 
    if (SDL_Init(SDL_INIT_VIDEO) == -1)
    {
        fprintf(stderr, "Erreur lors de l'initialisation de la SDL : %s\n", SDL_GetError());
        exit (EXIT_FAILURE);
    }
 
    SDL_Surface *icone = IMG_Load("horloge.gif");
    if (!icone)
    {
        fprintf(stderr, "Impossible de charger l'image : %s.\n",SDL_GetError);
        exit(EXIT_FAILURE);
    }
 
    SDL_WM_SetIcon(icone, NULL);
    ecran = SDL_SetVideoMode(200, 200, 32, SDL_HWSURFACE);
    if ( ecran == NULL )
    {
        fprintf(stderr, "Impossible de mettre la fenêtre en 200x200x8 : %s\n", SDL_GetError());
        exit(EXIT_FAILURE);
    }
 
    SDL_WM_SetCaption("Horloge","horloge.ico");
 
 
    dessineHorloge();
 
    programme();
 
    return EXIT_SUCCESS;
}
 
void programme(void)
{
 
    SDL_Event event;
    int done = 0;
    /* On ignore certains évenements qui font scintiller l'horloge */
    SDL_EventState (SDL_MOUSEMOTION , SDL_IGNORE);
    SDL_EventState (SDL_ACTIVEEVENT, SDL_IGNORE);
    SDL_EventState (SDL_KEYDOWN, SDL_IGNORE);
    SDL_EventState (SDL_KEYUP, SDL_IGNORE);
    while(!done)
    {
        while(SDL_PollEvent(&event))
        {
            switch(event.type)
            {
                case SDL_QUIT:
                done = 1;
                break;
 
                default:
                break;
            }
        }
 
        recupTemps();
 
    }
}
 
 
 
void dessineHorloge(void)
{
 
    SDL_Rect positionPixel;
    SDL_Surface *points = IMG_Load("points.bmp");
    if (!points)
    {
        fprintf(stderr, "Impossible de charger l'image : %s.\n", SDL_GetError());
        exit(EXIT_FAILURE);
    }
    int rayon = 80;
    int compteur;
    int angle = 30;
    int decalage = 10;
 
    positionPixel.x = positionPixel.y = 0;
    SDL_FillRect(ecran, NULL, SDL_MapRGB(ecran->format,0,0,0));
    SDL_BlitSurface(ecran, NULL, ecran, &positionPixel);
 
 
 
 
    for (compteur = 12; compteur > 0; compteur --)
    {
        positionPixel.x = rayon * cos((M_PI * (angle) / 180) * compteur) +rayon + points->w + decalage;
        positionPixel.y = rayon * sin((M_PI * (angle) / 180) * compteur) +rayon + points->h + decalage;
        SDL_BlitSurface(points, NULL, ecran, &positionPixel);
 
 
    }
 
    SDL_Flip(ecran);
    SDL_FreeSurface(points);
 
}
void recupTemps(void)
{
 
    time_t timePresent;
    struct tm *tPre, *tAnc;
 
    timePresent = time (NULL);
    tPre = localtime(&timePresent);
 
    if (init == 0)
    {
        tAnc->tm_sec = 0;
        tAnc->tm_min = 0;
        tAnc->tm_hour = 0;
        afficheSecondes(tPre->tm_sec);
        afficheMinutes(tPre->tm_min);
        afficheHeures(tPre->tm_hour);
        init = 1;
    }
    else
    {
 
 
 
 
        if (tAnc->tm_sec != tPre->tm_sec)
        {
            dessineHorloge();
 
            afficheMinutes(tPre->tm_min);
            afficheHeures(tPre->tm_hour);
            afficheSecondes(tPre->tm_sec);
            tAnc->tm_sec = tPre->tm_sec;
        }
 
 
 
 
    }
 
 
}
void afficheSecondes(int seconde)
{
    SDL_Rect positionCentre;
    positionCentre.x = ecran->w/2;
    positionCentre.y = ecran->h/2;
    SDL_Rect positionFin;
 
    Uint32 rose = SDL_MapRGB(ecran->format, 255, 4, 255);
 
    double R = RAYON;
    double A;
 
 
 
    A = (M_PI_2)-seconde*(M_PI)/30;
    positionFin.x = (int)(cos(A)*R + positionCentre.x);
    positionFin.y = (int)(-sin(A)*R + positionCentre.y);
 
    tracerSegment(positionCentre.x, positionCentre.y, positionFin.x, positionFin.y, rose);
 
 
 
}
 
void afficheMinutes(int minute)
{
    SDL_Rect positionCentre;
    positionCentre.x = ecran->w/2;
    positionCentre.y = ecran->h/2;
    SDL_Rect positionFin;
 
    Uint32 bleu = SDL_MapRGB(ecran->format, 4, 4, 255);
 
    double R = RAYON - rectifMIN;
    double A;
 
 
 
    A = (M_PI_2)-minute*(M_PI)/30;
    positionFin.x = (int)(cos(A)*R + positionCentre.x);
    positionFin.y = (int)(-sin(A)*R + positionCentre.y);
 
    tracerSegment(positionCentre.x, positionCentre.y, positionFin.x, positionFin.y, bleu);
 
 
 
}
void afficheHeures(int heure)
{
    SDL_Rect positionCentre;
    positionCentre.x = ecran->w/2;
    positionCentre.y = ecran->h/2;
    SDL_Rect positionFin;
 
    Uint32 blanc = SDL_MapRGB(ecran->format, 255, 255, 255);
 
    double R = RAYON-rectifHEU;
    double A;
 
 
 
    A =(M_PI_2)-heure*(M_PI)/6;
    positionFin.x = (int)(cos(A)*R + positionCentre.x);
    positionFin.y = (int)(-sin(A)*R + positionCentre.y);
 
    tracerSegment(positionCentre.x, positionCentre.y, positionFin.x, positionFin.y, blanc);
 
 
 
}
 
/* Algo de Bresenham */
/* http://raphaello.univ-fcomte.fr/Ig/Algorithme/ */
void tracerSegment(int xi,int yi,int xf,int yf, Uint32 couleur)
{
 
    int dx,dy,i,xinc,yinc,cumul,x,y ;
    x = xi ;
    y = yi ;
    dx = xf - xi ;
    dy = yf - yi ;
    xinc = ( dx > 0 ) ? 1 : -1 ;
    yinc = ( dy > 0 ) ? 1 : -1 ;
    dx = abs(dx) ;
    dy = abs(dy) ;
    allume_pixel(x,y, couleur) ;
    if ( dx > dy )
    {
        cumul = dx / 2 ;
        for ( i = 1 ; i <= dx ; i++ )
        {
            x += xinc ;
            cumul += dy ;
            if (cumul >= dx)
            {
                cumul -= dx ;
                y += yinc ;
            }
            allume_pixel(x,y, couleur) ;
        }
    }
    else
    {
        cumul = dy / 2 ;
        for ( i = 1 ; i <= dy ; i++ )
        {
            y += yinc ;
            cumul += dx ;
            if ( cumul >= dy )
            {
                cumul -= dy ;
                x += xinc ;
            }
            allume_pixel(x,y,couleur) ;
        }
    }
 
    SDL_Flip(ecran);
}
 
 
void allume_pixel(int x, int y,Uint32 couleur)
{
    SDL_Rect position;
    position.x = x;
    position.y = y;
    SDL_Surface *surface;
 
    surface = SDL_CreateRGBSurface(SDL_HWSURFACE, 2,2,32,0,0,0,0);
    SDL_FillRect(surface, NULL, couleur);
    SDL_BlitSurface(surface, NULL, ecran, &position);
 
    SDL_FreeSurface(surface);
}

**fearyourself** · 12/08/2006, 12h52

J'aurais quelques remarques sur le programme (mais pas le temps aujourd'hui, cela devra attendre demain) mais ce serait bien de mettre ton code en .zip ou .tar.gz... Car le .7z est tout de même plus rare.

De plus, il serait bien de joindre tes images parce que juste avec le code, le programme ne tournera pas...

Jc

Invité · 12/08/2006, 13h13

Je viens de vérifier et les images (il n'y a que : points.bmp pour les points du cadran, plus les deux icones) y sont.

Pour ce qui est du format, c'est une preference, mais voici une autre archive (zip) au cas où.

**fearyourself** · 13/08/2006, 16h39

1) Utilise plutôt :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

#include <SDL/SDL_image.h>

que

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

#include <SDL/SDL_Image.h>

Sous linux, la casse des include est importante...

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
 
void programme(void);
void allume_pixel(int x, int y,Uint32 couleur);
void dessineHorloge(void);
void recupTemps(void);
void afficheSecondes(int seconde);
void afficheMinutes(int minute);
void afficheHeures(int heure);
void tracerSegment(int xi,int yi,int xf,int yf, Uint32 couleur);

Toutes ces fonctions ne sont valables que dans le fichier main.c, elles devraient être nommées sous forme de static...

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
 
SDL_Surface *ecran;
int init = 0;

En ce qui concerne les variables globales, ceci est à limiter au maximum ou une bonne habitude est de les regrouper dans une structure globale...

Maintenant nous arrivons au code, lorsque je compile, j'obtiens :

~/tmp/$ gcc main.c `sdl-config --libs --cflags` -O2 -Wall -Wextra -lSDL_image

main.c: In function ‘main’:
main.c:59: warning: format ‘%s’ expects type ‘char *’, but argument 3 has type ‘char * (*)(void)’
main.c:45: warning: unused parameter ‘argc’
main.c:45: warning: unused parameter ‘argv’
main.c: In function ‘recupTemps’:
main.c:155: warning: ‘tAnc’ is used uninitialized in this function

Ligne 59 :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

fprintf(stderr, "Impossible de charger l'image : %s.\n",SDL_GetError);

Oublie des parenthèses pour l'appel de la fonction SDL_GetError :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

fprintf(stderr, "Impossible de charger l'image : %s.\n",SDL_GetError());

Pour les variables qui ne sont pas utilisées, une solution est de mettre à la fin de la fonction main :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
 
(void) argv;
(void) argc;

Ensuite :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

SDL_WM_SetCaption("Horloge","horloge.ico");

Je ne comprends pas pourquoi tu fais ceci, or tu viens de charger l'icone avec l'image "horloge.gif" juste au-dessus...

Enfin,

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
 
    programme();

Je ne suis pas pour mettre une fonction en plus pour contenir la boucle événementielle du programme, je trouve que cela rend le code plus lourd mais c'est sûrement une remarque très subjective...

Par contre, pour ta fonction programme, je n'ai rien à dire... Sauf que normalement, les événements ne devraient pas faire scintiller ton horloge... Elle scintille parce que tu n'utilise pas un double tampon : voir http://jeux.developpez.com/faq/sdl/?...uble_buffering (si t'as besoin d'aide pour le mettre en place, pose une question)

Regardons maintenant la fonction dessineHorloge :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
 
SDL_Surface *points = IMG_Load("points.bmp");

Tu charges une image ici. Ceci peut se faire si cette fonction n'est que rarement appelée (et encore) ou si l'image chargée varie... Or dans ton cas, cette fonction va être souvent appelée et l'image ne change pas. Il faudrait mieux la chargée une fois pour toute au début du programme et ne jamais le refaire par la suite...

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

int rayon = 80;

Pourquoi ne pas utiliser le define que tu as fait ?

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

SDL_BlitSurface(ecran, NULL, ecran, &positionPixel);

Tu copies la surface ecran sur la surface ecran ? Cela te semble logique ?

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

SDL_Flip(ecran);

Tu fais un Flip sans avoir demandé un double tampon pour l'affichage ?

Nous arrivons maintenant à la fonction recupTemps :

Je rappelle qu'il y avait un warning :

main.c: In function ‘recupTemps’:
main.c:155: warning: ‘tAnc’ is used uninitialized in this function

Ce warning est très grave. Cela veut dire que tu utilises la variable tAnc sans qu'elle soit initialisée ce qui peut avoir des conséquences imprévisibles, genre sur mon ordinateur ton programme me fait un joli :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

Fatal signal: Segmentation Fault (SDL Parachute Deployed)

Le warning et l'erreur de segmentation viennent bien sûr de cette ligne :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

tAnc->tm_sec = 0;

En effet, la variable tAnc est déclarée comme ceci :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

struct tm *tAnc;

Mais n'est jamais initialisée et aucune mémoire n'est allouée pour elle... Tu pourrais à la limite faire ceci :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

static struct tm tAnc;

Pour avoir une variable statique qui gère le temps...

Dans la fonction allume_pixel :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
 
    surface = SDL_CreateRGBSurface(SDL_HWSURFACE, 2,2,32,0,0,0,0);
    SDL_FillRect(surface, NULL, couleur);
    SDL_BlitSurface(surface, NULL, param.ecran, &position);

Cela est un peu compliqué pour juste mettre un petit carré d'une certaine couleur...

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
 
void allume_pixel(int x, int y,Uint32 couleur)
{
    SDL_Rect position;
    position.x = x;
    position.y = y;
    position.w = 2;
    position.h = 2;
 
    SDL_FillRect(param.ecran, &position, couleur);
}

Me semble plus simple, non ?

Ensuite, nous devons avoir une discussion de l'utilisation de la fonction SDL_Flip, déjà, elle est nécessaire que si tu utilises SDL_DOUBLEBUF (de nouveau voir http://jeux.developpez.com/faq/sdl/?...ble_buffering)...

Mais plus important : cette fonction s'appelle seulement une fois par code de rendu... Tu en as mis 2 mais à des endroits qui font qu'il y a 4 appels par rendu...

Voilà les remarques sur ce code... J'ajoute une version corrigée plus propre en gardant ton algorithme et ta facon de faire (j'ai tout de même fusionner le main et la fonction programme...)

Jc

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

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
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
 
#include <stdio.h>
#include <stdlib.h>
#include <math.h>
#include <time.h>
#include <locale.h>
#include <SDL/SDL.h>
#include <SDL/SDL_image.h>
 
/* Definition de certaines constantes */
#define RAYON 80
#define rectifHEU 30
#define rectifMIN 15
 
/* Fonctions statiques */
static void allume_pixel(int x, int y,Uint32 couleur);
static void dessineHorloge(void);
static void recupTemps(void);
static void afficheSecondes(int seconde);
static void afficheMinutes(int minute);
static void afficheHeures(int heure);
static void tracerSegment(int xi,int yi,int xf,int yf, Uint32 couleur);
 
/* Structure pour les variables globales */
typedef struct sVarGlob
{
	SDL_Surface *ecran, *points;
	int init;
}SVarGlob;
 
/* Variable globale */
SVarGlob param = {NULL, NULL, 0 };
 
/* Fonction main */
int main (int argc, char *argv[])
{
    SDL_Event event;
    int done = 0;
 
    atexit(SDL_Quit);
 
    if (SDL_Init(SDL_INIT_VIDEO) == -1)
    {
        fprintf(stderr, "Erreur lors de l'initialisation de la SDL : %s\n", SDL_GetError());
        exit (EXIT_FAILURE);
    }
 
    param.ecran = SDL_SetVideoMode(200, 200, 32, SDL_HWSURFACE|SDL_DOUBLEBUF);
    if ( param.ecran == NULL )
    {
        fprintf(stderr, "Impossible de mettre la fenêtre en 200x200x8 : %s\n", SDL_GetError());
        exit(EXIT_FAILURE);
    }
 
    param.points = IMG_Load("points.bmp");
    if (!param.points)
    {
        fprintf(stderr, "Erreur chargement image : %s.\n", SDL_GetError());
        exit(EXIT_FAILURE);
    }
 
    SDL_WM_SetCaption("Horloge","horloge.ico");
 
    while(!done)
    {
        while(SDL_PollEvent(&event))
        {
            switch(event.type)
            {
                case SDL_QUIT:
                	done = 1;
                break;
 
                default:
                break;
            }
        }
 
        recupTemps();
    }
    (void) argc;
    (void) argv;
 
    return EXIT_SUCCESS;
}
 
void dessineHorloge(void)
{
 
    SDL_Rect positionPixel;
 
    int compteur;
    int angle = 30;
    int decalage = 10;
 
    SDL_FillRect(param.ecran, NULL, SDL_MapRGB(param.ecran->format,0,0,0));
 
    for (compteur = 12; compteur > 0; compteur --)
    {
        positionPixel.x = RAYON * cos((M_PI * (angle) / 180) * compteur) +RAYON 
		+ param.points->w + decalage;
        positionPixel.y = RAYON * sin((M_PI * (angle) / 180) * compteur) +RAYON 
		+ param.points->h + decalage;
        SDL_BlitSurface(param.points, NULL, param.ecran, &positionPixel);
    }
}
 
void recupTemps(void)
{
 
    time_t timePresent;
    struct tm *tPre;
    static struct tm tAnc;
 
    timePresent = time (NULL);
    tPre = localtime(&timePresent);
 
    if (param.init == 0)
    {
        tAnc.tm_sec = 0;
        tAnc.tm_min = 0;
        tAnc.tm_hour = 0;
        afficheSecondes(tPre->tm_sec);
        afficheMinutes(tPre->tm_min);
        afficheHeures(tPre->tm_hour);
        param.init = 1;
    }
    else
    {
        if (tAnc.tm_sec != tPre->tm_sec)
        {
            dessineHorloge();
            afficheMinutes(tPre->tm_min);
            afficheHeures(tPre->tm_hour);
            afficheSecondes(tPre->tm_sec);
            tAnc.tm_sec = tPre->tm_sec;
	    SDL_Flip(param.ecran);
        }
    }
 
 
}
void afficheSecondes(int seconde)
{
    SDL_Rect positionCentre;
    positionCentre.x = param.ecran->w/2;
    positionCentre.y = param.ecran->h/2;
    SDL_Rect positionFin;
 
    Uint32 rose = SDL_MapRGB(param.ecran->format, 255, 4, 255);
 
    double R = RAYON;
    double A;
 
    A = (M_PI_2)-seconde*(M_PI)/30;
    positionFin.x = (int)(cos(A)*R + positionCentre.x);
    positionFin.y = (int)(-sin(A)*R + positionCentre.y);
 
    tracerSegment(positionCentre.x, positionCentre.y, positionFin.x, positionFin.y, rose);
}
 
void afficheMinutes(int minute)
{
    SDL_Rect positionCentre;
    positionCentre.x = param.ecran->w/2;
    positionCentre.y = param.ecran->h/2;
    SDL_Rect positionFin;
 
    Uint32 bleu = SDL_MapRGB(param.ecran->format, 4, 4, 255);
 
    double R = RAYON - rectifMIN;
    double A;
 
    A = (M_PI_2)-minute*(M_PI)/30;
    positionFin.x = (int)(cos(A)*R + positionCentre.x);
    positionFin.y = (int)(-sin(A)*R + positionCentre.y);
 
    tracerSegment(positionCentre.x, positionCentre.y, positionFin.x, positionFin.y, bleu);
}
 
void afficheHeures(int heure)
{
    SDL_Rect positionCentre;
    positionCentre.x = param.ecran->w/2;
    positionCentre.y = param.ecran->h/2;
    SDL_Rect positionFin;
 
    Uint32 blanc = SDL_MapRGB(param.ecran->format, 255, 255, 255);
 
    double R = RAYON-rectifHEU;
    double A;
 
    A =(M_PI_2)-heure*(M_PI)/6;
    positionFin.x = (int)(cos(A)*R + positionCentre.x);
    positionFin.y = (int)(-sin(A)*R + positionCentre.y);
 
    tracerSegment(positionCentre.x, positionCentre.y, positionFin.x, positionFin.y, blanc);
}
 
/* Algo de Bresenham */
/* http://raphaello.univ-fcomte.fr/Ig/Algorithme/ */
void tracerSegment(int xi,int yi,int xf,int yf, Uint32 couleur)
{
 
    int dx,dy,i,xinc,yinc,cumul,x,y ;
    x = xi ;
    y = yi ;
    dx = xf - xi ;
    dy = yf - yi ;
    xinc = ( dx > 0 ) ? 1 : -1 ;
    yinc = ( dy > 0 ) ? 1 : -1 ;
    dx = abs(dx) ;
    dy = abs(dy) ;
    allume_pixel(x,y, couleur) ;
    if ( dx > dy )
    {
        cumul = dx / 2 ;
        for ( i = 1 ; i <= dx ; i++ )
        {
            x += xinc ;
            cumul += dy ;
            if (cumul >= dx)
            {
                cumul -= dx ;
                y += yinc ;
            }
            allume_pixel(x,y, couleur) ;
        }
    }
    else
    {
        cumul = dy / 2 ;
        for ( i = 1 ; i <= dy ; i++ )
        {
            y += yinc ;
            cumul += dx ;
            if ( cumul >= dy )
            {
                cumul -= dy ;
                x += xinc ;
            }
            allume_pixel(x,y,couleur) ;
        }
    }
 
}
 
 
void allume_pixel(int x, int y,Uint32 couleur)
{
    SDL_Rect position;
    position.x = x;
    position.y = y;
    position.w = 2;
    position.h = 2;
 
    SDL_FillRect(param.ecran, &position, couleur);
}

Bonne lecture,
Jc

Invité · 13/08/2006, 18h20

Et bien il y en a des choses à corriger

Pour ce qui est des :

Envoyé par fearyourself

- fonctions Static

Je ne met pas Static car je n'ai qu'un seul fichier .c ici. C'est une habitude que j'ai prise de ne m'occuper de "static" et autres que lors des programmes avec plusieurs fichiers, est-ce une herreur de ma part ?

Envoyé par fearyourself

- variables globales

D'habitude j'évite, mais là pour la Surface *ecran, je trouvais ça plus simple de la mettre globale que de l'envoyer à chaque fonctions qui en a besoin.
Même simplicité pour init (bien que le code qui s'intéresse à init puisse être amélioré à mon avis).
Qu'entends tu par les

Envoyé par fearyourself

regrouper dans une structure globale

?

Envoyé par fearyourself

- warning

J'utilise Codeblock (avec gcc) sous Win XP, aucun warning, même avec -Wall. Par contre je n'ai pas mis les autres options. Mais je vois mes erreurs là.

Envoyé par fearyourself

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
 
SDL_WM_SetCaption("Horloge","horloge.ico");

Je ne comprends pas pourquoi tu fais ceci, or tu viens de charger l'icone avec l'image "horloge.gif" juste au-dessus...

J'ai du mal comprendre le deuxième paramètre de SetCaption. D'autant plus qu'il y a du avoir une erreur de copie/colle (le .ico a été rajouté dans un fichier .rc pour windows et j'en ai profité pour mettre l'icones dans le programme).
Que faut il mettre ? Corriger en horloge.gif ou NULL ? Dans le second cas, pourquoi ce deuxième paramètre alors ?

Envoyé par fearyourself

Je ne suis pas pour mettre une fonction en plus pour contenir la boucle événementielle du programme, je trouve que cela rend le code plus lourd mais c'est sûrement une remarque très subjective...

C'est une preference de ma part, qui, pour moi, est plus lisible, mais c'est subjective évidemment.

Envoyé par fearyourself

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
 
SDL_Surface *points = IMG_Load("points.bmp");

Tu charges une image ici. Ceci peut se faire si cette fonction n'est que rarement appelée (et encore) ou si l'image chargée varie... Or dans ton cas, cette fonction va être souvent appelée et l'image ne change pas. Il faudrait mieux la chargée une fois pour toute au début du programme et ne jamais le refaire par la suite...

D'accord, je croyais le contraire, que justement, la garder en mémoire pouvait augmenter les ressources nécessaires au programme, mais dans ce cas, je note.

Envoyé par fearyourself

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
 
int rayon = 80;

Pourquoi ne pas utiliser le define que tu as fait ?

Erreur de copie/colle, lors de test, j'ai du oublier que je l'avais define, et quand ça a fonctionné pour les secondes, j'ai du bêtement copie/colle+modife pour les autres.

Envoyé par fearyourself

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

SDL_BlitSurface(ecran, NULL, ecran, &positionPixel);

Tu copies la surface ecran sur la surface ecran ? Cela te semble logique ?

Erg quelle horreur ! Je ne sais pas pourquoi j'ai commis ça ! Après le FillRect... ignoble

Envoyé par fearyourself

SDL_Flip(ecran);

Tu fais un Flip sans avoir demandé un double tampon pour l'affichage ?

J'utilise toujours le double tampon, d'où le fait que je ne me sois même pas posé la question du Flip et des scintillements donc...

Envoyé par fearyourself

Nous arrivons maintenant à la fonction recupTemps :

Je rappelle qu'il y avait un warning :

main.c: In function ‘recupTemps’:
main.c:155: warning: ‘tAnc’ is used uninitialized in this function

Ce warning est très grave. Cela veut dire que tu utilises la variable tAnc sans qu'elle soit initialisée ce qui peut avoir des conséquences imprévisibles, genre sur mon ordinateur ton programme me fait un joli :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

Fatal signal: Segmentation Fault (SDL Parachute Deployed)

Le warning et l'erreur de segmentation viennent bien sûr de cette ligne :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

tAnc->tm_sec = 0;

En effet, la variable tAnc est déclarée comme ceci :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

struct tm *tAnc;

Mais n'est jamais initialisée et aucune mémoire n'est allouée pour elle... Tu pourrais à la limite faire ceci :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
 
static struct tm tAnc;

Pour avoir une variable statique qui gère le temps...

Je pense qu'il faut que j'aille revoir mes base en C, car je ne vois pas, même avec tes explications la faute, enfin surtout pour quoi le "static" eviterai la faute. En faisant tAnc->tm_sec = 0; je ne l'initialise pas ? (Enfin, une partie seulement, il faudrait que je le fasse pour tous les éléments de la structure peut-être?)

Envoyé par fearyourself

Dans la fonction allume_pixel :

(snip...)

C'est vrai que c'est plus simple quand même

Le BlitSurface est donc inutile lors d'un appel de FillRect, je viens de voir.

Envoyé par fearyourself

l'utilisation de la fonction SDL_Flip,

Je suis bien conscient de ces choses, c'est une erreur de codage.

Merci, pour ces critiques et ces remarques que je note précieusement, c'est très agréable de voir des corrections comme celle là, ça donne envie d'appronfondir le sujet

**fearyourself** · 13/08/2006, 19h24

Envoyé par _-Slash-_

Je ne met pas Static car je n'ai qu'un seul fichier .c ici. C'est une habitude que j'ai prise de ne m'occuper de "static" et autres que lors des programmes avec plusieurs fichiers, est-ce une erreur de ma part ?

Je trouve que même avec un fichier c'est important parce qu'on a vite fait d'ajouter un fichier et donc devoir mettre tous les static....

D'habitude j'évite, mais là pour la Surface *ecran, je trouvais ça plus simple de la mettre globale que de l'envoyer à chaque fonctions qui en a besoin.
Même simplicité pour init (bien que le code qui s'intéresse à init puisse être amélioré à mon avis).
Qu'entends tu par les

regrouper dans une structure globale

?

J'ai mis dans le code que je donnais :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
/* Structure pour les variables globales */
typedef struct sVarGlob
{
	SDL_Surface *ecran, *points;
	int init;
}SVarGlob;

Ceci permet d'avoir toutes les variables globales au même endroit... Question de lisibilité et goût...

J'utilise Codeblock (avec gcc) sous Win XP, aucun warning, même avec -Wall. Par contre je n'ai pas mis les autres options. Mais je vois mes erreurs là.

Effectivement, mets au moins -Wall -Wextra -O2..

J'ai du mal comprendre le deuxième paramètre de SetCaption. D'autant plus qu'il y a du avoir une erreur de copie/colle (le .ico a été rajouté dans un fichier .rc pour windows et j'en ai profité pour mettre l'icones dans le programme).
Que faut il mettre ? Corriger en horloge.gif ou NULL ? Dans le second cas, pourquoi ce deuxième paramètre alors ?

Comme dans le code que j'ai donné, enlève le code :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
    SDL_Surface *icone = IMG_Load("horloge.gif");
    if (!icone)
    {
        fprintf(stderr, "Impossible de charger l'image : %s.\n",SDL_GetError);
        exit(EXIT_FAILURE);
    }
 
    SDL_WM_SetIcon(icone, NULL);

et garde l'autre appel...

D'accord, je croyais le contraire, que justement, la garder en mémoire pouvait augmenter les ressources nécessaires au programme, mais dans ce cas, je note.

Il y a deux choses à savoir :

- Utiliser trop de mémoire va ralentir le programme
- Faire trop de choses va ralentir le programme

Il faut donc trouver le juste milieu. A savoir que de nos jours et pour des petits projets comme ce programme, la priorité est :

- Limiter au maximum le code qui gére le rendu pour accélérer le nombre d'affichage par seconde...

J'utilise toujours le double tampon, d'où le fait que je ne me sois même pas posé la question du Flip et des scintillements donc...

Comme je l'ai montré dans mon code et par le lien, il faut dire à SDL que tu veux un double tampon... Donc faire ceci :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

ecran = SDL_SetVideoMode(200, 200, 32, SDL_HWSURFACE|SDL_DOUBLEBUF);

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
 
    ecran = SDL_SetVideoMode(200, 200, 32, SDL_HWSURFACE);

Je pense qu'il faut que j'aille revoir mes base en C, car je ne vois pas, même avec tes explications la faute, enfin surtout pour quoi le "static" eviterai la faute. En faisant tAnc->tm_sec = 0; je ne l'initialise pas ? (Enfin, une partie seulement, il faudrait que je le fasse pour tous les éléments de la structure peut-être?)

Tu déclares un pointeur vers une structure. Mais un pointeur doit pointer vers une zone mémoire. Le langage C ne le fait pas gentillement pour toi, tu dois le faire à l'aide d'un appel à la fonction malloc (allocation dynamique).

Le problème : sans cet appel, ton pointeur pointe vers une zone non initialisée donc provoque un comportement indéfini.

Aussi : d'un appel à un autre, tu perds cette information (les variables locales sont perdues d'un appel à un autre SAUF si elles sont déclarées static...)

C'est vrai que c'est plus simple quand même

Le BlitSurface est donc inutile lors d'un appel de FillRect, je viens de voir.

Si les zones du blit et du fill sont les mêmes, oui...

Merci, pour ces critiques et ces remarques que je note précieusement, c'est très agréable de voir des corrections comme celle là, ça donne envie d'appronfondir le sujet

De rien, c'était un plaisir

Jc

**Bakura** · 13/08/2006, 20h00

Salut fearyoursel,

Désolé d'envahir ce sujet, mais j'ai lu les remarques que tu as fait et j'ai appris quelques trucs d'intéressants, notamment d'utiliser les structures pour éviter d'avoir trop de variables globales (sur un ancien programme, j'avais plein de variables globales, une pour un bitmap, une autre pour un autre bitmap, une autre encore, une autre pour un sprite, une autre pour un sprite,...). C'était la seule technique que j'avais en tête pour éviter de tout envoyer à chaque fonction, et puis ca simplifiait quand même pas mal si je voulais pas me retrouver avec des prototypes de fonction à rallonge.

Bref, sur ces structures, j'aimerai savoir concrètement la différence qu'il y a à utiliser une structure avec toutes les variables globales à l'intérieur plutôt que simplement énumerer les variables globales. Ou peut-être existe d'autres techniques plus élégantes (je code en C++).

Mais j'ai essayé un petit exemple tout simple :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
#include <iostream>
using namespace std;
 
typedef struct SvarGlobales
{
   int * nombre1;
   int * nombre2;
} SVarGlobales;
 
SVarGlobales var1 = {NULL, NULL};
 
int main()
{
   *(var1.nombre1) = 1;
 
   cout << *(var1.nombre1);
   return 0;
}

Je créer juste une structure avec deux pointeurs sur des entiers que j'initialise à NULL, puis je veux en initialiser une à un et l'afficher.

Tout d'abord première question, quelle différence y a t'il entre :

struct SVarGlobales
{
int * nombre1;
int * nombre2;
};

ce que j'utilisais avant, et ça, que tu as utilisé plus haut :

typedef SvarGlobales
{
int * nombre1;
int * nombre2;
} SVarGlobales;

Ensuite, quand je veux changer la valeur de l'entier, il faut normalement que je le déréference comme je le fais avec un pointeur normal,... Mais là ça ne marche pas, j'ai une erreur à l'execution,...

Enfin, j'ai remarqué dans le code corrigé que tu as mis plus haut qu'à aucun moment tu ne fais des delete sur les pointeurs de la structure, il n'y a donc pas de fuites de mémoire ? (Enfin perso, j'aurai utiliser boost::shared_ptr <int> nombre1, comme ça, j'ai pas à me casser la tête

)

Invité · 13/08/2006, 21h03

Envoyé par fearyourself

(Snip... le haut que j'ai noté)
Tu déclares un pointeur vers une structure. Mais un pointeur doit pointer vers une zone mémoire. Le langage C ne le fait pas gentillement pour toi, tu dois le faire à l'aide d'un appel à la fonction malloc (allocation dynamique).

Le problème : sans cet appel, ton pointeur pointe vers une zone non initialisée donc provoque un comportement indéfini.

Aussi : d'un appel à un autre, tu perds cette information (les variables locales sont perdues d'un appel à un autre SAUF si elles sont déclarées static...)

Dis comme ça, je comprends tout de suite beaucoup mieux

Je ne suis pas assez avancé en C et donc je commet encore des "erreurs de jeunesse" à vite corriger.

**fearyourself** · 13/08/2006, 22h23

Envoyé par TicTac

Bref, sur ces structures, j'aimerai savoir concrètement la différence qu'il y a à utiliser une structure avec toutes les variables globales à l'intérieur plutôt que simplement énumerer les variables globales. Ou peut-être existe d'autres techniques plus élégantes (je code en C++).

Non, cela change juste que le code me semble plus ordonné...

Mais j'ai essayé un petit exemple tout simple :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
#include <iostream>
using namespace std;
 
typedef struct SvarGlobales
{
   int * nombre1;
   int * nombre2;
} SVarGlobales;
 
SVarGlobales var1 = {NULL, NULL};
 
int main()
{
   *(var1.nombre1) = 1;
 
   cout << *(var1.nombre1);
   return 0;
}

Encore une fois... Le pointeur nombre1 n'est pas initialisé... Du coup, tu est en train de demander de mettre la valeur 1 quelque part en mémoire qui n'est pas alloué par la fonction malloc. Ceci provoque un comportement indéterminé.

Je créer juste une structure avec deux pointeurs sur des entiers que j'initialise à NULL, puis je veux en initialiser une à un et l'afficher.

Il faudra d'abord allouer de la mémoire avant...

Tout d'abord première question, quelle différence y a t'il entre :

struct SVarGlobales
{
int * nombre1;
int * nombre2;
};

ce que j'utilisais avant, et ça, que tu as utilisé plus haut :

typedef struct SvarGlobales
{
int * nombre1;
int * nombre2;
} SVarGlobales;

Aucune différence sauf qu'après je peux déclarer une variable de type SVarGlobales tandis que toi tu dois garder le type struct SVarGlobales.

Ensuite, quand je veux changer la valeur de l'entier, il faut normalement que je le déréference comme je le fais avec un pointeur normal,... Mais là ça ne marche pas, j'ai une erreur à l'execution,...

Aucune zone mémoire n'a été allouée et pointeur NULL -> Segmentation fault assuré

Enfin, j'ai remarqué dans le code corrigé que tu as mis plus haut qu'à aucun moment tu ne fais des delete sur les pointeurs de la structure, il n'y a donc pas de fuites de mémoire ?

Oui et non, une fuite mémoire est causée lorsqu'on perd l'adresse d'une zone allouée. Ici ce n'est pas le cas. SDL_Quit va désallouer les surfaces créées mais c'était un oubli, c'est toujours mieux de le faire avant la fin du programme.

(Enfin perso, j'aurai utiliser boost::shared_ptr <int> nombre1, comme ça, j'ai pas à me casser la tête

)

Oui mais on est en C dans ce cas précis.

Jc

**Bakura** · 14/08/2006, 10h57

Ah oui en effet j'avais oublié d'allouer la mémoire, alors que généralement je fais pas cette erreur, la structure m'a troublée. ^^

Merci, ça marche bien

.

**Eusebe** · 14/08/2006, 12h16

Si tu codes en C++, et pas en C, plutôt qu'utiliser malloc, pourquoi ne pas plutôt faire un new ?

Et quel est l'intérêt de pointeurs sur des int ? Ne pourrais-tu pas stocker directement les int à la place ?

**fearyourself** · 14/08/2006, 18h17

Envoyé par Eusebe

Si tu codes en C++, et pas en C, plutôt qu'utiliser malloc, pourquoi ne pas plutôt faire un new ?

Et quel est l'intérêt de pointeurs sur des int ? Ne pourrais-tu pas stocker directement les int à la place ?

Si c'est en C++, c'est clair c'est new qu'il faut utiliser...

Ensuite, peut-être voulait-il un tableau à la place ? Ou c'était un exemple...

Jc

**Bakura** · 14/08/2006, 19h00

Oui oui bien sûr que j'utilise des new, je pensais que c'était pareil en C ^^. Sinon pour les pointeurs sur les int, c'était juste pour le test bien sûr.

Demande d'avis : Horloge, SDL/C

SDL

Discussions similaires

Partager

Partager