Huge API change

ben.campbell at cyberlife.co.uk wrote:

Personally I’m for changing the API if it makes it more robust/useful
(Although passing an extra pointer into every function does make me cringe a
little - I’d rather have a SDL_GetLastError() interface :-).

Yes, would some sort of ErrorQueue with funtionaltiy similar to the
EventQueue be possible, it would have a member SDL_Thread *thread that
could be checked to see if it was relevant to us.

Stuart

As someone mentioned earlier, I still think this can be done without changing
the API or using macro wrappers (which can introduce problems of their own), by
having a thread-specific error structure. Unless you want the dubious
flexibility of a separate thread structure for each function call.

What’s wrong with something like:

/* Code to create new thread /
/
/
{
SDL_Error errbuf;
SDL_SetErrorPtr(&errbuf);
if ( SDL_BlitSurface(src, &srcrect, dst, &dstrect) < 0 ) {
/
You could use the old SDL_GetError() here if you hadn’t used
SDL_SetErrorPtr() – SDL initialization would set the errbuf
to a global location, and if you use SDL_GetError(), it
would refer to that location.
Alternately, SDL_GetError() would always refer to the error
buffer of the main thread, which SDL would allocate and set
on initialization.
/
fprintf(stderr, “Blit failed: %s\n”, SDL_GetErrorEx(&errbuf));
/
This is actually a possible error */
if ( errbuf.code == SDL_ERR_LOSTDIRECTXVIDEO ) {
ReloadSurfaces();
}
}
}

With a method like the above, you don’t have to make any API changes, yet you
still have thread-independent error buffers. And, if you want a different
error buffer for each function call in the same thread (unlikely though
that may be), you can always call SDL_SetErrorPtr() with another location,
before calling the function. For example:

SDL_SetErrorPtr(&err1);
SDL_BlitSurface(…);
/* Check returned error in err1 /
SDL_SetErrorPtr(&err2);
/
Some other SDL function /
/
Check returned error in err2 */

The SDL_SetErrorPtr() function would internally get the current thread ID and
use that to insert the error buffer pointer on a list of error buffers for
each thread (the first time it’s called for a given thread). If the current
thread ID is already on the list, then it simply modifies the pointer, rather
than adding a new node to the list.

Then, all the functions which need to return an error, get the current thread
ID, and look it up on the list, to determine which error buffer to write the
error to.

If you’re concerned about the overhead of looking up the ID’s on the list, you
can make it an array of error buffers, sorted by thread ID, and use a binary
search to find it. Add another SDL_ function to set the maximum number of
threads to keep errors for, if you decide to do this, so the array can be
dynamically allocated at that time.

It seems to me that this would give all the benefits, with none of the
disadvantages. And, if no error buffer has been set for a given thread ID, no
error processing need be done, except to return the normal error return values.

Warren E. Downs
______________________________ Reply Separator _________________________________Subject: Re: [SDL] Huge API change
Author: at internet-mail
Date: 7/28/99 12:54 AM

I don’t see where the global variable comes in. I assume Sam’s
thead library maintains some structure per thread. Add an error pointer
to that structure, and initialize it to zero. It is meant to point
to an instance of an error structure, but on thread launch will
contain NULL. A thread will then allocate an error structure on
its own, and call the new SDL_Thread_Error function (or whatever it
is called) which updates that thread’s error structure pointer. On
error a thread will look for that pointer, and if it is non zero
the error structure it points to will be filled in. If the programmer
doesn’t call SDL_Thread_Error with a unique error structure, he
deserves the resultant bug(s).

That’s the current implementation. Part of the trickiness is the fact
that per-thread variables aren’t easy to implement in a safe way.

The talk of lock/unlock/mutex…It leaves me thinking of race conditions,
lockups where both threads are waiting for the other to unlock something
(I won’t release mine until he releases his :^). In my mind all this
is theory anyway. In my bomber game where I replaced my sound fork()
code with SDL’s sound callback mechanism, I had the main program
pass sound commands in a circular buffer with a put pointer that
wraps around. The callback function, in another thread (sdl audio
is done with threads I assume), looks into the same buffer but with
a different pointer (“take”). When take!=put, the audio callback
function will keep fetching commands until the pointers are the
same again. There is no locking, no conflicts, and no problems.

I would imagine most thread stuff that requires locking could be
implemented in some other way so as not to require locking. But it is
just a feeling, not from actual experience. I’ve been programming for
23 years and my instincts have usually served me well :^).

What you have in bomberman is busy waiting. If you wanted to wait
on a mutex when the buffer was full, it would get a little more complex. :slight_smile:
Just an observation.

All I really care about is that the SDL library function calls don’t
change. Now, if they must change because they weren’t set up correctly
with threads in mind, then I’d say make the minimum changes necessary to
fix the problem. But speaking of changing every SDL function call is
a scary prospect. I picture a nightmare of tech support emails, “My
program won’t compile with the new version but it works with the old,”
“I’m getting core dumps on programs that worked before I upgraded to a
new version of SDL,” etc, etc.

I am ambivalent about the change. It would be the “Right Thing To Do”,
but the API gets more complicated, and the benefit only touches a few
people:

How many people actually care about the precise return values of functions?
Part of this is lack of planning on the design of the API, i.e.
I didn’t have a set way of returning error codes and a set of meanings
for those error codes. Adding an error pointer parameter adds an official
way of representing both the error code and the raw information needed to
display an error message in a thread-safe way. However, it adds complexity
to the API which isn’t necessarily desireable.

So. The question is, what are the benefits v.s. the pains?

Pro:
Thread-safe error handling
Internationalized error messages
Function-specific errors (i.e. no global errno value)
Better defined error conditions

Con:
Adds complexity to the API

So, what is the actual effect of the proposed change on the API?

Let’s take for example SDL_BlitSurface()

Old method:
if ( SDL_BlitSurface(src, &srcrect, dst, &dstrect) < 0 ) {
fprintf(stderr, “Blit failed: %s\n”, SDL_GetError());
}

New method:
if ( SDL_BlitSurface(&error, src, &srcrect, dst, &dstrect) < 0 ) {
fprintf(stderr, “Blit failed: %s\n”, SDL_GetError(&error));
/* This is actually a possible error */
if ( error.code == SDL_ERR_LOSTDIRECTXVIDEO ) {
ReloadSurfaces();
}
}
or if you don’t care about thread-safety (using a global error location):
if ( SDL_BlitSurface(NULL, src, &srcrect, dst, &dstrect) < 0 ) {
fprintf(stderr, “Blit failed: %s\n”, SDL_GetError(NULL));
}
or if you don’t care about errors at all:
SDL_BlitSurface(0, src, &srcrect, dst, &dstrect);

By the way, to put things in perspective, the new POSIX thread-safe way
of returning errors is to return -errno and have errno be an index into
a static table of error strings. I want SDL to be a little more flexible
in that you can have informative error messages like:
“Couldn’t set video mode: Must be less than 2048x2048”

On the pro side of the complexity, all SDL functions will take exactly
the same error parameter, so you won’t have to think about it once you
get used to it, and you’ll no longer need to check the return value of
functions, just check the error code at certain points in your code:

(Notice the changing of error to result)

    SDL_ClearResult(&result);
    screen = SDL_SetVideoMode(&result, 640, 480, 8, SDL_SWSURFACE); 
    if ( result.error ) {
            SDL_PrintError(&result, "Couldn't set video mode"); 
            exit(0);
    }
    loaded = SDL_LoadBMP(&result, "image.bmp"); 
    image = SDL_DisplayFormat(&result, loaded); 
    SDL_FreeSurface(NULL, loaded);
    if ( result.error ) {
            SDL_PrintError(&result, "Couldn't load and convert image.bmp"); 
            exit(0);
    }
    for ( i=0; i<screen->w/2; ++i ) {
            SDL_Rect rect;
 
            rect.x = i;
            rect.y = 0;
            rect.w = image->w;
            rect.h = image->h;
            SDL_BlitSurface(&result, image, NULL, screen, &rect); 
            SDL_UpdateRects(&result, screen, 1, &rect);
    }
    if ( result.error ) {
            SDL_PrintError(&result, "Couldn't move image across screen"); 
            exit(0);
    }

SDL_PrintError() could be defined as:
SDL_PrintError(SDL_result *result, const char *m)
{
char message[BUFSIZ];
fprintf(stderr, “%s: %s\n”, m, SDL_GetResult(&result, message, BUFSIZ));
}
It could also pop up a message box on Windows and/or MacOS.

I will decide whether or not to implement this based on user response,
since you all are the reason this library exists in the first place,
but either way this is decided, it must be done before 1.0.

I’ve noticed things in the SDL-demos that don’t even match the latest
versions of the library…Speaking of Sam or whoever making sweeping
changes to the library makes me nervous. How many bugs are going to be
introduced? Will the examples be updated correctly? Lots of details
haven’t been dealt with before as regards minor changes…

That’s because the development versions are places where new features
are tested, documentation isn’t updated, things may or may not work, etc.
The stable releases get all the bug fixes, and the API stays stable.

What happened with 0.9 is that it stayed out way too long and became
a stable version even though the documentation and examples hadn’t been
updated. This was because I didn’t want to go to version 1.0 without
addressing problems such as thread-safety, and internationalization
support which are important in production level code. However, they
were not so important that I took time off of Loki work to work on them.

I’m thinking specifically of the audio stereo stuff, where stereo
used to be a flag in the init audio function, but then switched to
a member in the structure (channels). And the audio example program
on the latest docs said to use the old method, not the new one!

I still need to update the docs. Once this issue is resolved, I update
the docs, and release the next stable version of SDL, hopefully 1.0. :slight_smile:

The stable version of SDL will stay, with no API changes, just bug-fixes,
as the version of SDL which can be used for production code. The next
development version will start having official MacOS support and I’ll
start on multi-monitor support.

BTW, I’m also planning a tutorial on making a game with SDL. :slight_smile:

So… what do you all think?
Cliff, you might want to join the SDL mailing list temporarily to
join the discussion. Lurkers, I want to hear from you too. :slight_smile:

    -Sam Lantinga                                (slouken at devolution.com)

Lead Programmer, Loki Entertainment Software

“Any sufficiently advanced bug is indistinguishable from a feature”
– Rich Kulawiec

Okay, the official word on the API change:

I will clean up the current SDL release and make it into SDL 0.10.0,
the next stable SDL release.

Currently SDL uses a per-thread error variable, which I will explain later
is a potential problem. Over time I will make the internal SDL functions
reentrant as far as error reporting goes, and sometime in the future I will
expose those APIs in a way that does not break existing code.

Whew! :slight_smile:

I suppose that at some point SDL will have international error messages
and completely thread-safe, useful, error reporting, but at the moment
it’s fine for the uses to which it is put daily.

Just to clarify, useful error reporting, internationalized error message,
and reentrant error returns are separate issues, which can be independently
solved. – I just lump them together since I’ll probably implement them
at roughly the same time.

:slight_smile:
Sorry for making such a big deal about it.

-Sam Lantinga				(slouken at devolution.com)

Lead Programmer, Loki Entertainment Software–
“Any sufficiently advanced bug is indistinguishable from a feature”
– Rich Kulawiec