Re: [AD] Timer patch

On Jan 29, 2008 2:35 PM, Peter Wang <novalazy@xxxxxxxxxx> wrote:

On 2008-01-29, Ryan Dickie <goalieca@xxxxxxxxxx> wrote:
> I decided to go ahead and make a timer path for al_current_time() and
> al_rest() to take doubles and return values in seconds. I modified the
> windows timer as well (someone please test it but it should work). The
> examples and demos were also patched. The examples were clean and often
> calculated things like FPS using doubles anyways but i left the demo logic
> using milliseconds and integers so as to not "break" anything.
>
> --Ryan Dickie

Review follows. Just some style things I noticed, so I won't have to go
back and fix them myself :)

> Index: demo/src/UFO.cpp
> ===================================================================
> --- demo/src/UFO.cpp (revision 9954)
> +++ demo/src/UFO.cpp (working copy)
> @@ -19,10 +19,12 @@
> p->die();
> my_play_sample(RES_COLLISION);
> return false;
> - }
> + }
> + unsigned long cur_time = (unsigned long) (al_current_time() * 1000);

Try not to declare variables in the middle of a function. Well, it *used* to
be a rule and I think it's best to stick to it.

Is the cast necessary?

>
> - if (al_current_time() > (unsigned long)nextShot) {
> - nextShot = al_current_time() + SHOT_SPEED;
> +
> + if (cur_time > (unsigned long)nextShot) {

Unnecessary cast?

> + nextShot = cur_time + SHOT_SPEED;
> ResourceManager& rm = ResourceManager::getInstance();
> Player *p = (Player *)rm.getData(RES_PLAYER);
> float px = p->getX();
> @@ -70,9 +72,10 @@
> hp = 8;
> points = 500;
> ufo = true;
> +
> + unsigned long cur_time = (unsigned long) (al_current_time() * 1000);
> + nextShot = cur_time + SHOT_SPEED;

Unnecessary cast?
>
> - nextShot = al_current_time() + SHOT_SPEED;
> -
> ResourceManager& rm = ResourceManager::getInstance();
> bitmaps[0] = (ALLEGRO_BITMAP *)rm.getData(RES_UFO0);
> bitmaps[1] = (ALLEGRO_BITMAP *)rm.getData(RES_UFO1);
> Index: demo/src/Player.cpp
> ===================================================================
> --- demo/src/Player.cpp (revision 9954)
> +++ demo/src/Player.cpp (working copy)
> @@ -63,9 +63,10 @@
> default:
> shotRate = INT_MAX;
> break;
> - }
> - if (((unsigned long)(lastShot+shotRate) < al_current_time()) && input->b1()) {
> - lastShot = al_current_time();
> + }
> + unsigned long cur_time = (unsigned long) (1e3*al_current_time());
> + if (((unsigned long)(lastShot+shotRate) < cur_time) && input->b1()) {
> + lastShot = cur_time;
> float realAngle = -angle;
> float bx = x + radius * cos(realAngle);
> float by = y + radius * sin(realAngle);
> @@ -88,7 +89,7 @@
> }
>
> if (input->cheat()) {
> - al_rest(250);
> + al_rest(0.250);
> std::list<Entity *>::iterator it;
> for (it = entities.begin(); it != entities.end(); it++) {
> Entity *e = *it;

Ditto.

> Index: demo/src/a5teroids.cpp
> ===================================================================
> --- demo/src/a5teroids.cpp (revision 9954)
> +++ demo/src/a5teroids.cpp (working copy)
> @@ -42,7 +42,7 @@
> for (;;) {
> player->load();
>
> - al_rest(500);
> + al_rest(0.500);
>
> play_sample(title_music, 255, 128, 1000, 1);
> int choice = do_menu();
> @@ -53,7 +53,7 @@
> }
> stop_sample(title_music);
>
> - al_rest(250);
> + al_rest(0.250);
> lastUFO = -1;
> canUFO = true;
>
> @@ -61,7 +61,7 @@
> play_sample(game_music, 255, 128, 1000, 1);
>
> int step = 0;
> - long start = al_current_time();
> + long start = (long) (al_current_time()*1e3);

Unnecessary cast?

>
> for (;;) {
> if (entities.size() <= 0) {
> @@ -74,8 +74,8 @@
> if (!logic(step))
> break;
> render(step);
> - al_rest(10);
> - long end = al_current_time();
> + al_rest(0.010);
> + long end = (long) (al_current_time()*1e3);

Unnecessary cast?

> step = end - start;
> start = end;
> }
> Index: demo/src/GUI.cpp
> ===================================================================
> --- demo/src/GUI.cpp (revision 9954)
> +++ demo/src/GUI.cpp (working copy)
> @@ -16,12 +16,12 @@
> if (ud < 0 && selected) {
> selected--;
> my_play_sample(RES_FIRELARGE);
> - al_rest(200);
> + al_rest(.200);
> }
> else if (ud > 0 && selected < (widgets.size()-1)) {
> selected++;
> my_play_sample(RES_FIRELARGE);
> - al_rest(200);
> + al_rest(.200);
> }
> if (input->b1()) {
> if (!widgets[selected]->activate())
> @@ -42,7 +42,7 @@
> widgets[i]->render(i == selected);
> }
> al_flip_display();
> - al_rest(10);
> + al_rest(.010);
> }
> }
>

Might as well write the initial zeros.

> Index: demo/src/logic.cpp
> ===================================================================
> --- demo/src/logic.cpp (revision 9954)
> +++ demo/src/logic.cpp (working copy)
> @@ -9,12 +9,13 @@
> const int MAX_UFO_TIME = 50000;
>
> bool logic(int step)
> -{
> - if (lastUFO < 0) lastUFO = al_current_time();
> +{
> + unsigned long cur_time = (unsigned long) (al_current_time() * 1000);

?

> + if (lastUFO < 0) lastUFO = cur_time;
>
> - if (canUFO && (al_current_time() > (unsigned long)(lastUFO+MIN_UFO_TIME))) {
> + if (canUFO && (cur_time > (unsigned long)(lastUFO+MIN_UFO_TIME))) {
> int r = rand() % (MAX_UFO_TIME-MIN_UFO_TIME);
> - if (r <= step || (al_current_time() > (unsigned long)(lastUFO+MAX_UFO_TIME))) {
> + if (r <= step || (cur_time > (unsigned long)(lastUFO+MAX_UFO_TIME))) {
> canUFO = false;
> UFO *ufo;
> float x, y, dx, dy;

What are those casts for?

> @@ -58,8 +59,9 @@
> for (it = entities.begin(); it != entities.end(); it++) {
> Entity *e = *it;
> if (!e->logic(step)) {
> - if (e->isUFO()) {
> - lastUFO = al_current_time();
> + if (e->isUFO()) {
> + unsigned long cur_time = (unsigned long) (al_current_time()*1e3);
> + lastUFO = cur_time;

Ditto.

> canUFO = true;
> }
> delete e;

> Index: src/win/wtime.c
> ===================================================================
> --- src/win/wtime.c (revision 9954)
> +++ src/win/wtime.c (working copy)
> @@ -36,10 +36,10 @@
>
>
>
> -unsigned long al_current_time(void)
> +double al_current_time(void)
> {
> /* XXX: Maybe use QueryPerformanceCounter? */
> - return timeGetTime() - _al_win_initial_time;
> + return ((double) (timeGetTime() - _al_win_initial_time))*1e-3;
> }

Put spaces around the `*'.

>
>
> @@ -47,9 +47,9 @@
> /* al_rest:
> * Rests the specified amount of milliseconds.
> */
> -void al_rest(long msecs)
> +void al_rest(double seconds)
> {
> - ASSERT(msecs >= 0);
> + ASSERT(seconds >= 0);
>
> - Sleep(msecs);
> + Sleep((DWORD)(seconds*1e3));
> }

Ditto. 1e3 might as well be 1000.0.

> Index: src/unix/utime.c
> ===================================================================
> --- src/unix/utime.c (revision 9954)
> +++ src/unix/utime.c (working copy)
> @@ -17,12 +17,10 @@
>
>
> #include <sys/time.h>
> -#include <sys/select.h>
> -
> #include "allegro5/altime.h"
> +#include <math.h>
>
>
> -
> /* Marks the time Allegro was initialised, for al_current_time(). */
> static struct timeval initial_time;
>

Keep system and internal includes separate.

> @@ -39,17 +37,16 @@
>
>
> /* al_current_time:
> - * Return the current time, in microseconds, since some arbitrary
> + * Return the current time, with microsecond resolution, since some arbitrary
> * point in time.
> */
> -unsigned long al_current_time(void)
> +double al_current_time(void)
> {
> - struct timeval now;
> -
> - gettimeofday(&now, NULL);
> -
> - return ((now.tv_sec - initial_time.tv_sec) * 1000 +
> - (now.tv_usec - initial_time.tv_usec) / 1000);
> + struct timeval tp;
> + gettimeofday(&tp, 0); //null = timezone afaik
> + double time = (double) (tp.tv_sec - initial_time.tv_sec)
> + + (double) (tp.tv_usec - initial_time.tv_usec) * 1.0e-6;
> + return time;
> }
>
>
> @@ -57,14 +54,11 @@
> /* al_rest:
> * Make the caller rest for some time.
> */
> -void al_rest(long msecs)
> +void al_rest(double seconds)
> {
> -#ifdef ALLEGRO_MACOSX
> - usleep(msecs * 1000);
> -#else
> - struct timeval timeout;
> - timeout.tv_sec = 0;
> - timeout.tv_usec = msecs * 1000;
> - select(0, NULL, NULL, NULL, &timeout);
> -#endif
> + struct timespec tp;
> + double fsecs = floor(seconds);
> + tp.tv_sec = (time_t) fsecs;
> + tp.tv_nsec = (suseconds_t) ((seconds - fsecs) * 1e9);
> + nanosleep(&tp,0);
> }

Add a space after the comma.

> Index: include/allegro5/altime.h
> ===================================================================
> --- include/allegro5/altime.h (revision 9954)
> +++ include/allegro5/altime.h (working copy)
> @@ -11,17 +11,18 @@
>
>
> /* Function: al_current_time
> - * Return the number of milliseconds since the Allegro library was
> + * Return the number of seconds since the Allegro library was
> * initialised. The return value is undefined if Allegro is uninitialised.
> + * microsecond resolution.
> */
> -AL_FUNC(unsigned long, al_current_time, (void));
> +AL_FUNC(double, al_current_time, (void));
>
>
>
> /* Function: al_rest
> - * Waits for the specified number of milliseconds.
> + * Waits for the specified number seconds (~1ms accuracy).

number of seconds

~1ms accuracy might be too tight.

> */
> -AL_FUNC(void, al_rest, (long msecs));
> +AL_FUNC(void, al_rest, (double seconds));
>
>
>
> Index: examples/exnew_mouse_events.c
> ===================================================================
> --- examples/exnew_mouse_events.c (revision 9954)
> +++ examples/exnew_mouse_events.c (working copy)
> @@ -50,7 +50,7 @@
> al_clear(al_map_rgb(0, 0, 0));
> al_draw_bitmap(cursor, mx, my, 0);
> al_flip_display();
> - al_rest(5);
> + al_rest(5e-3);
> }
> while(1);
> done:
> Index: examples/exnew_bitmap_target.c
> ===================================================================
> --- examples/exnew_bitmap_target.c (revision 9954)
> +++ examples/exnew_bitmap_target.c (working copy)
> @@ -38,7 +38,7 @@
> static void draw(void)
> {
> double dt = 0;
> - double t = al_current_time() / 1000.0;
> + double t = al_current_time();
> if (last_time > 0) {
> dt = t - last_time;
> }
> @@ -84,7 +84,7 @@
> dy = 63;
>
> int frames = 0;
> - double start = al_current_time() / 1000.0;
> + double start = al_current_time();
> do {
> /* Check for ESC key or close button event and quit in either case. */
> if (!al_event_queue_is_empty(queue)) {
> @@ -105,7 +105,7 @@
> }
> }
> draw();
> - print(0, 0, "FPS: %.1f", frames / (al_current_time() / 1000.0 - start));
> + print(0, 0, "FPS: %.1f", frames / (al_current_time() - start));
> if (al_get_new_bitmap_flags() & ALLEGRO_FORCE_LOCKING) {
> print(0, a5font_text_height(myfont), "using forced bitmap locking");
> }
> Index: examples/exnew_lockscreen.c
> ===================================================================
> --- examples/exnew_lockscreen.c (revision 9954)
> +++ examples/exnew_lockscreen.c (working copy)
> @@ -15,7 +15,7 @@
> ((char *)locked.data)[i] = (i / locked.pitch) * 255 / 99;
> al_unlock_bitmap(bitmap);
> al_flip_display();
> - al_rest(5000);
> + al_rest(5.0);
> return 0;
> }
> END_OF_MAIN()
> Index: examples/exnew_fs_resize.c
> ===================================================================
> --- examples/exnew_fs_resize.c (revision 9954)
> +++ examples/exnew_fs_resize.c (working copy)
> @@ -28,10 +28,10 @@
> }
>
> redraw(picture);
> - al_rest(2500);
> + al_rest(2.5);
> al_resize_display(800, 600);
> redraw(picture);
> - al_rest(2500);
> + al_rest(2.5);
>
> return 0;
> }
> Index: examples/exnew_bitmap.c
> ===================================================================
> --- examples/exnew_bitmap.c (revision 9954)
> +++ examples/exnew_bitmap.c (working copy)
> @@ -15,7 +15,7 @@
> }
> al_draw_bitmap(bitmap, 0, 0, 0);
> al_flip_display();
> - al_rest(1000);
> + al_rest(5.0);
> return 0;
> }
> END_OF_MAIN()
> Index: examples/exnew_opengl.c
> ===================================================================
> --- examples/exnew_opengl.c (revision 9954)
> +++ examples/exnew_opengl.c (working copy)
> @@ -12,7 +12,7 @@
>
> void draw_opengl(void)
> {
> - double ms = al_current_time() / 1000.0;
> + double ms = al_current_time();
>
> glMatrixMode(GL_MODELVIEW);
> glLoadIdentity();
> Index: examples/exnew_icon.c
> ===================================================================
> --- examples/exnew_icon.c (revision 9954)
> +++ examples/exnew_icon.c (working copy)
> @@ -32,7 +32,7 @@
> {
> al_set_display_icon(i & 1 ? icon2 : icon1);
> al_flip_display();
> - al_rest(500);
> + al_rest(1);

1.0

> }
> return 0;
> }
> Index: examples/exnew_mouse.c
> ===================================================================
> --- examples/exnew_mouse.c (revision 9954)
> +++ examples/exnew_mouse.c (working copy)
> @@ -28,7 +28,7 @@
> al_clear(al_map_rgb(0, 0, 0));
> al_draw_bitmap(cursor, msestate.x, msestate.y, 0);
> al_flip_display();
> - al_rest(5);
> + al_rest(5e-3);
> } while (!al_key_down(&kbdstate, ALLEGRO_KEY_ESCAPE)
> && !msestate.buttons);
>
> Index: examples/exnew_resize.c
> ===================================================================
> --- examples/exnew_resize.c (revision 9954)
> +++ examples/exnew_resize.c (working copy)
> @@ -23,7 +23,7 @@
> ALLEGRO_EVENT event;
> ALLEGRO_KEYBOARD *keyboard;
>
> - long last_resize;
> + double last_resize;
> int rs = 100;
>
> /* Initialize Allegro and create an event queue. */
> @@ -59,7 +59,7 @@
> break;
> }
> }
> - if (al_current_time() - last_resize > 100)
> + if (al_current_time() - last_resize > 0.1)
> {
> int s;
> last_resize = al_current_time();
> Index: examples/exnew_blend.c
> ===================================================================
> --- examples/exnew_blend.c (revision 9954)
> +++ examples/exnew_blend.c (working copy)
> @@ -174,7 +174,7 @@
> static void tick(void)
> {
> /* Count frames during the last second. */
> - double t = al_current_time() / 1000.0;
> + double t = al_current_time();
> if (t >= ex.last_second + 1) {
> ex.last_second += 1;
> ex.fps = ex.frames_accum;
> Index: examples/exnewapi.c
> ===================================================================
> --- examples/exnewapi.c (revision 9954)
> +++ examples/exnewapi.c (working copy)
> @@ -17,7 +17,7 @@
> ALLEGRO_EVENT event;
> ALLEGRO_EVENT_QUEUE *events;
> int quit = 0;
> - int ticks = 0, last_rendered = 0, start_ticks;
> + double ticks = 0, last_rendered = 0, start_ticks;

0.0

> int fps_accumulator = 0, fps_time = 0;
> double fps = 0;
> int FPS = 500;
> @@ -110,9 +110,9 @@
> al_draw_line(0, 0, 100, 0, al_map_rgba_f(1, 1, 0, 0.5));
> al_draw_line(0, 0, 0, 100, al_map_rgba_f(1, 1, 0, 0.5));
>
> - long start = al_current_time();
> - long last_move = al_current_time();
> - int frames = 0;
> + double start = al_current_time();
> + double last_move = al_current_time();
> + double frames = 0;

frames as a double?

>
> while (!quit) {
> /* read input */
> @@ -148,7 +148,7 @@
> }
>
> while (last_move < al_current_time()) {
> - last_move++;
> + last_move+=1e-3;

Spaces.

> x += dx;
> if (x == 0)
> dx = 1;
> @@ -186,7 +186,7 @@
> }
> }
>
> - printf("fps=%f\n", (float)(frames * 1000) / (float)(al_current_time()-start));
> + printf("fps=%f\n", frames / (al_current_time()-start));
>
> al_destroy_bitmap(picture);
> al_destroy_bitmap(mask);
> Index: examples/exnew_membmp.c
> ===================================================================
> --- examples/exnew_membmp.c (revision 9954)
> +++ examples/exnew_membmp.c (working copy)
> @@ -36,9 +36,9 @@
>
> static void test(ALLEGRO_BITMAP *bitmap, A5FONT_FONT *font, char *message)
> {
> - long frames = 0;
> - long start_time = al_current_time();
> - int fps = 0;
> + double frames = 0;

Ditto.

> + double start_time = al_current_time();
> + double fps = 0;

0.0

>
> for (;;) {
> if (key_down()) {
> @@ -60,15 +60,13 @@
>
> print(font, message, 0, 0);
> char second_line[100];
> - sprintf(second_line, "%d FPS", fps);
> + sprintf(second_line, "%2f FPS", fps);
> print(font, second_line, 0, a5font_text_height(font)+5);
>
> al_flip_display();
>
> frames++;
> - long now = al_current_time();
> - long mseconds = now - start_time;
> - fps = mseconds > 10 ? 1000 * frames / mseconds : 0;
> + fps = frames / (al_current_time() - start_time);
> }
> }
>
> Index: examples/exnew_lockbitmap.c
> ===================================================================
> --- examples/exnew_lockbitmap.c (revision 9954)
> +++ examples/exnew_lockbitmap.c (working copy)
> @@ -51,7 +51,7 @@
>
> al_draw_bitmap(bitmap, 0, 0, 0);
> al_flip_display();
> - al_rest(5000);
> + al_rest(5.0);
> return 0;
> }
> END_OF_MAIN()

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
--
https://lists.sourceforge.net/lists/listinfo/alleg-developers

I have made those style changes (and found some dead code as well). See attachment

-- Ryan Dickie