Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ambiguous recomendation for SDL_LockSurface #887

Open
mikrosk opened this issue Oct 24, 2024 · 0 comments
Open

Ambiguous recomendation for SDL_LockSurface #887

mikrosk opened this issue Oct 24, 2024 · 0 comments

Comments

@mikrosk
Copy link
Collaborator

mikrosk commented Oct 24, 2024

Yes, I know that I'm like 10 years late :) but I have found out an interesting problem while porting an older game (still using SDL 1.2). The game explicitly asks for SDL_SWSURFACE in SDL_SetVideoMode. And since its wish is granted, its author didn't need to worry about surface locking:

SDL-1.2/include/SDL_video.h

Lines 572 to 574 in 2a0b742

* pixel format of the surface will not change. In particular, if the
* SDL_HWSURFACE flag is not given when calling SDL_SetVideoMode(), you
* will not need to lock the display surface before accessing it.

Therefore the game doesn't lock surfaces neither checks SDL_MUSTLOCK. Now, this all works nice if the returned surface is the same dimension as requested. However what if isn't?

Then there's a big problem: SDL_UpdateRects tries to do the right thing and take the offset into consideration:

SDL-1.2/src/video/SDL_video.c

Lines 1096 to 1106 in 2a0b742

if ( screen->offset ) {
for ( i=0; i<numrects; ++i ) {
rects[i].x += video->offset_x;
rects[i].y += video->offset_y;
}
video->UpdateRects(this, numrects, rects);
for ( i=0; i<numrects; ++i ) {
rects[i].x -= video->offset_x;
rects[i].y -= video->offset_y;
}
} else {

However the game author doesn't care / know anything about offsets, he just accesses pixels and rightfully so as documentation explicitly discourages him to use MUSTLOCK / SDL_LockSurface. Outcome is clear, the game fills first 200 lines of the software surface while SDL_UpdateRects offers pixels to backends from an offset, leading to a shifted display.

So my question is ... if we were in 2010, what would have been your recommendation as fix?

  • documentation update, i.e. encourage developers to always call SDL_MUSTLOCK and tell the author to suck it up and fix his code?
  • when requesting software surfaces, always return the requested dimensions and do the offsetting in the backend?
    • this would imply that SDL_LockSurface and SDL_UpdateRects wouldn't need to worry about offsets for software surfaces in general
  • use the internal SDL_ShadowSurface if dimensions do not match ? (this looks like the cleanest solution to me as it is very similar to the situation when we request a software surface and get a hardware one however currently we take dimensions from SDL_VideoSurface which is the one with "bad" dimensions)
  • ?

Of course, I'm at the liberty to choose any approach as both the game and SDL are open source but still, I'd appreciate some input and reasoning behind the current state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant