solene@ reported that games/barony has missing textures.[1] Here is a
diff fixing this sourced from a PR by Sylvain Becker.
I used hg bisect to track down the revision in sdl2's repository that
introduced this bug.[2] I contacted Sylvain Becker, the author of the sdl2
commit. Sylvain fixed barony's usage of sdl2.
The bug is with barony's modification of refcount, which is
read-only. The fix uses userdata to stash imgref.
I tried to understand the fix, but more eyes are welcome.
Details
=======
From /usr/local/include/SDL2/SDL_surface.h, refcount is read-only.
/** Application data associated with the surface */
void *userdata; /**< Read-write */
/** Reference count -- used when freeing surface */
int refcount; /**< Read-mostly */
Important variables:
userdata: used to store imgref (the id) of a surface
imgref: global Uint32 used to index into allsurfaces, an array of
GL_Surface pointers[3][4]
refcount: read-only field in SDL_Surface used to count how many
references are made to a GL_Surface. See deleted comments in 1107-1113
in SDL_pixels.c.[2] It is probably used for some other purpose in the
implementation now that is deleted.
Testing
=======
Textures work now with epic game assets downloaded on September 5,
2020. Feedback and tests are welcome. OK?
Footnotes:
[1] https://github.com/TurningWheel/Barony/issues/580
[2] https://github.com/libsdl-org/SDL/commit/ebc12a2fd2bb692908884e7db6cc935941a591f2#diff-98a8d948613c29516e252e8134aee43ba14fb7bcd6457d29be3ba99861fd80bcL1107
[3] Barony-3.3.7/src/main.cpp:344:Uint32 imgref = 1
[4] src/init.cpp:354: allsurfaces = (SDL_Surface**) malloc
Index: Makefile
===================================================================
RCS file: /cvs/ports/games/barony/Makefile,v
retrieving revision 1.7
diff -u -p -u -p -r1.7 Makefile
--- Makefile 4 Oct 2020 11:38:58 -0000 1.7
+++ Makefile 10 Apr 2021 22:39:24 -0000
@@ -4,6 +4,7 @@ V = 3.3.7
COMMENT = 3D, first person roguelike
PKGNAME = ${DISTNAME:L}
CATEGORIES = games x11
+REVISION = 0
GH_ACCOUNT = TurningWheel
GH_PROJECT = Barony
Index: patches/patch-src_draw_cpp
===================================================================
RCS file: patches/patch-src_draw_cpp
diff -N patches/patch-src_draw_cpp
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-src_draw_cpp 10 Apr 2021 22:39:24 -0000
@@ -0,0 +1,109 @@
+$OpenBSD$
+
+Use 'userdata' instead of 'refcount'. 'refcount' is a private SDL_Surface field.
+Use 'imgref' and not 'imgref + 1'
+
+see: https://github.com/TurningWheel/Barony/pull/582
+
+Index: src/draw.cpp
+--- src/draw.cpp.orig
++++ src/draw.cpp
+@@ -443,7 +443,7 @@ void drawImageRotatedAlpha( SDL_Surface* image, SDL_Re
+ }
+
+ // draw a textured quad
+- glBindTexture(GL_TEXTURE_2D, texid[image->refcount]);
++ glBindTexture(GL_TEXTURE_2D, texid[(long int)image->userdata]);
+ glColor4f(1, 1, 1, alpha / 255.1);
+ glBegin(GL_QUADS);
+ glTexCoord2f(1.0 * ((real_t)src->x / image->w), 1.0 * ((real_t)src->y / image->h));
+@@ -492,7 +492,7 @@ void drawImageColor( SDL_Surface* image, SDL_Rect* src
+ }
+
+ // draw a textured quad
+- glBindTexture(GL_TEXTURE_2D, texid[image->refcount]);
++ glBindTexture(GL_TEXTURE_2D, texid[(long int)image->userdata]);
+ real_t r = ((Uint8)(color >> mainsurface->format->Rshift)) / 255.f;
+ real_t g = ((Uint8)(color >> mainsurface->format->Gshift)) / 255.f;
+ real_t b = ((Uint8)(color >> mainsurface->format->Bshift)) / 255.f;
+@@ -546,7 +546,7 @@ void drawImageAlpha( SDL_Surface* image, SDL_Rect* src
+ }
+
+ // draw a textured quad
+- glBindTexture(GL_TEXTURE_2D, texid[image->refcount]);
++ glBindTexture(GL_TEXTURE_2D, texid[(long int)image->userdata]);
+ glColor4f(1, 1, 1, alpha / 255.1);
+ glPushMatrix();
+ glBegin(GL_QUADS);
+@@ -596,7 +596,7 @@ void drawImage( SDL_Surface* image, SDL_Rect* src, SDL
+ }
+
+ // draw a textured quad
+- glBindTexture(GL_TEXTURE_2D, texid[image->refcount]);
++ glBindTexture(GL_TEXTURE_2D, texid[(long int)image->userdata]);
+ glColor4f(1, 1, 1, 1);
+ glPushMatrix();
+ glBegin(GL_QUADS);
+@@ -646,7 +646,7 @@ void drawImageRing(SDL_Surface* image, SDL_Rect* src,
+ }
+
+ // draw a textured quad
+- glBindTexture(GL_TEXTURE_2D, texid[image->refcount]);
++ glBindTexture(GL_TEXTURE_2D, texid[(long int)image->userdata]);
+ glColor4f(1, 1, 1, alpha / 255.f);
+ glPushMatrix();
+
+@@ -771,7 +771,7 @@ void drawImageScaled( SDL_Surface* image, SDL_Rect* sr
+ }
+
+ // draw a textured quad
+- glBindTexture(GL_TEXTURE_2D, texid[image->refcount]);
++ glBindTexture(GL_TEXTURE_2D, texid[(long int)image->userdata]);
+ glColor4f(1, 1, 1, 1);
+ glPushMatrix();
+ glBegin(GL_QUADS);
+@@ -826,7 +826,7 @@ void drawImageScaledPartial(SDL_Surface* image, SDL_Re
+ }
+
+ // draw a textured quad
+- glBindTexture(GL_TEXTURE_2D, texid[image->refcount]);
++ glBindTexture(GL_TEXTURE_2D, texid[(long int)image->userdata]);
+ glColor4f(1, 1, 1, 1);
+ glPushMatrix();
+ glBegin(GL_QUADS);
+@@ -889,7 +889,7 @@ void drawImageScaledColor(SDL_Surface* image, SDL_Rect
+ }
+
+ // draw a textured quad
+- glBindTexture(GL_TEXTURE_2D, texid[image->refcount]);
++ glBindTexture(GL_TEXTURE_2D, texid[(long int)image->userdata]);
+ real_t r = ((Uint8)(color >> mainsurface->format->Rshift)) / 255.f;
+ real_t g = ((Uint8)(color >> mainsurface->format->Gshift)) / 255.f;
+ real_t b = ((Uint8)(color >> mainsurface->format->Bshift)) / 255.f;
+@@ -985,7 +985,7 @@ void drawImageFancy( SDL_Surface* image, Uint32 color,
+ }
+
+ // draw a textured quad
+- glBindTexture(GL_TEXTURE_2D, texid[image->refcount]);
++ glBindTexture(GL_TEXTURE_2D, texid[(long int)image->userdata]);
+ real_t r = ((Uint8)(color >> mainsurface->format->Rshift)) / 255.f;
+ real_t g = ((Uint8)(color >> mainsurface->format->Gshift)) / 255.f;
+ real_t b = ((Uint8)(color >> mainsurface->format->Bshift)) / 255.f;
+@@ -2186,7 +2186,7 @@ void drawWindowFancy(int x1, int y1, int x2, int y2)
+ glVertex2f(x2 - 1, yres - y1 - 1);
+ glEnd();
+ glColor3f(.75, .75, .75);
+- glBindTexture(GL_TEXTURE_2D, texid[fancyWindow_bmp->refcount]); // wood texture
++ glBindTexture(GL_TEXTURE_2D, texid[(long int)fancyWindow_bmp->userdata]); // wood texture
+ glBegin(GL_QUADS);
+ glTexCoord2f(0, 0);
+ glVertex2f(x1 + 2, yres - y1 - 2);
+@@ -2322,7 +2322,7 @@ SDL_Rect ttfPrintTextColor( TTF_Font* font, int x, int
+ SDL_BlitSurface(textSurf, NULL, surf, &pos);
+ // load the text outline surface as a GL texture
+ allsurfaces[imgref] = surf;
+- allsurfaces[imgref]->refcount = imgref;
++ allsurfaces[imgref]->userdata = (void*) imgref;
+ glLoadTexture(allsurfaces[imgref], imgref);
+ imgref++;
+ // store the surface in the text surface cache
Index: patches/patch-src_files_cpp
===================================================================
RCS file: patches/patch-src_files_cpp
diff -N patches/patch-src_files_cpp
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-src_files_cpp 10 Apr 2021 22:39:24 -0000
@@ -0,0 +1,19 @@
+$OpenBSD$
+
+Use 'userdata' instead of 'refcount'. 'refcount' is a private SDL_Surface field.
+Use 'imgref' and not 'imgref + 1'
+
+see: https://github.com/TurningWheel/Barony/pull/582
+
+Index: src/files.cpp
+--- src/files.cpp.orig
++++ src/files.cpp
+@@ -591,7 +591,7 @@ SDL_Surface* loadImage(char const * const filename)
+
+ // load the new surface as a GL texture
+ allsurfaces[imgref] = newSurface;
+- allsurfaces[imgref]->refcount = imgref + 1;
++ allsurfaces[imgref]->userdata = (void *)(imgref);
+ glLoadTexture(allsurfaces[imgref], imgref);
+
+ // free the translated surface
Index: patches/patch-src_opengl_cpp
===================================================================
RCS file: patches/patch-src_opengl_cpp
diff -N patches/patch-src_opengl_cpp
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-src_opengl_cpp 10 Apr 2021 22:39:24 -0000
@@ -0,0 +1,102 @@
+$OpenBSD$
+
+Use 'userdata' instead of 'refcount'. 'refcount' is a private SDL_Surface field.
+Use 'imgref' and not 'imgref + 1'
+
+see: https://github.com/TurningWheel/Barony/pull/582
+
+Index: src/opengl.cpp
+--- src/opengl.cpp.orig
++++ src/opengl.cpp
+@@ -500,7 +500,7 @@ void glDrawSprite(view_t* camera, Entity* entity, int
+ }
+ if ( mode == REALCOLORS )
+ {
+- glBindTexture(GL_TEXTURE_2D, texid[sprite->refcount]);
++ glBindTexture(GL_TEXTURE_2D, texid[(long int)sprite->userdata]);
+ }
+ else
+ {
+@@ -586,7 +586,7 @@ void glDrawSpriteFromImage(view_t* camera, Entity* ent
+ //int x, y;
+ real_t s = 1;
+ SDL_Surface* image = sprites[0];
+- GLuint textureId = texid[sprites[0]->refcount];
++ GLuint textureId = texid[(long int)sprites[0]->userdata];
+ char textToRetrieve[128];
+
+ if ( text.compare("") == 0 )
+@@ -603,7 +603,7 @@ void glDrawSpriteFromImage(view_t* camera, Entity* ent
+ textToRetrieve[std::min(static_cast<int>(strlen(text.c_str())), 22)] = '\0';
+ if ( (image = ttfTextHashRetrieve(ttfTextHash, textToRetrieve, ttf12, true)) != NULL )
+ {
+- textureId = texid[image->refcount];
++ textureId = texid[(long int)image->userdata];
+ }
+ else
+ {
+@@ -627,7 +627,7 @@ void glDrawSpriteFromImage(view_t* camera, Entity* ent
+ SDL_BlitSurface(textSurf, NULL, image, &pos);
+ // load the text outline surface as a GL texture
+ allsurfaces[imgref] = image;
+- allsurfaces[imgref]->refcount = imgref;
++ allsurfaces[imgref]->userdata = (void *)((long int)imgref);
+ glLoadTexture(allsurfaces[imgref], imgref);
+ imgref++;
+ // store the surface in the text surface cache
+@@ -635,7 +635,7 @@ void glDrawSpriteFromImage(view_t* camera, Entity* ent
+ {
+ printlog("warning: failed to store text outline surface with imgref %d\n", imgref - 1);
+ }
+- textureId = texid[image->refcount];
++ textureId = texid[(long int)image->userdata];
+ }
+ // setup projection
+ glMatrixMode(GL_PROJECTION);
+@@ -864,7 +864,7 @@ void glDrawWorld(view_t* camera, int mode)
+
+ // first (higher) sky layer
+ glColor4f(1.f, 1.f, 1.f, .5);
+- glBindTexture(GL_TEXTURE_2D, texid[tiles[cloudtile]->refcount]); // sky tile
++ glBindTexture(GL_TEXTURE_2D, texid[(long int)tiles[cloudtile]->userdata]); // sky tile
+ glBegin( GL_QUADS );
+ glTexCoord2f((real_t)(ticks % 60) / 60, (real_t)(ticks % 60) / 60);
+ glVertex3f(-CLIPFAR * 16, 64, -CLIPFAR * 16);
+@@ -881,7 +881,7 @@ void glDrawWorld(view_t* camera, int mode)
+
+ // second (closer) sky layer
+ glColor4f(1.f, 1.f, 1.f, .5);
+- glBindTexture(GL_TEXTURE_2D, texid[tiles[cloudtile]->refcount]); // sky tile
++ glBindTexture(GL_TEXTURE_2D, texid[(long int)tiles[cloudtile]->userdata]); // sky tile
+ glBegin( GL_QUADS );
+ glTexCoord2f((real_t)(ticks % 240) / 240, (real_t)(ticks % 240) / 240);
+ glVertex3f(-CLIPFAR * 16, 32, -CLIPFAR * 16);
+@@ -954,13 +954,13 @@ void glDrawWorld(view_t* camera, int mode)
+ {
+ if ( map.tiles[index] < 0 || map.tiles[index] >= numtiles )
+ {
+- new_tex = texid[sprites[0]->refcount];
+- //glBindTexture(GL_TEXTURE_2D, texid[sprites[0]->refcount]);
++ new_tex = texid[(long int)sprites[0]->userdata];
++ //glBindTexture(GL_TEXTURE_2D, texid[(long int)sprites[0]->userdata]);
+ }
+ else
+ {
+- new_tex = texid[tiles[map.tiles[index]]->refcount];
+- //glBindTexture(GL_TEXTURE_2D, texid[tiles[map.tiles[index]]->refcount]);
++ new_tex = texid[(long int)tiles[map.tiles[index]]->userdata];
++ //glBindTexture(GL_TEXTURE_2D, texid[(long int)tiles[map.tiles[index]]->userdata]);
+ }
+ }
+ else
+@@ -1282,8 +1282,8 @@ void glDrawWorld(view_t* camera, int mode)
+ // bind texture
+ if ( mode == REALCOLORS )
+ {
+- new_tex = texid[tiles[mapceilingtile]->refcount];
+- //glBindTexture(GL_TEXTURE_2D, texid[tiles[50]->refcount]); // rock tile
++ new_tex = texid[(long int)tiles[mapceilingtile]->userdata];
++ //glBindTexture(GL_TEXTURE_2D, texid[(long int)tiles[50]->userdata]); // rock tile
+ if (cur_tex!=new_tex)
+ {
+ glEnd();
Index: patches/patch-src_savepng_cpp
===================================================================
RCS file: patches/patch-src_savepng_cpp
diff -N patches/patch-src_savepng_cpp
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-src_savepng_cpp 10 Apr 2021 22:39:24 -0000
@@ -0,0 +1,19 @@
+$OpenBSD$
+
+Use 'userdata' instead of 'refcount'. 'refcount' is a private SDL_Surface field.
+Use 'imgref' and not 'imgref + 1'
+
+see: https://github.com/TurningWheel/Barony/pull/582
+
+Index: src/savepng.cpp
+--- src/savepng.cpp.orig
++++ src/savepng.cpp
+@@ -59,7 +59,7 @@ SDL_Surface* SDL_PNGFormatAlpha(SDL_Surface* src)
+ /* NO-OP for images < 32bpp and 32bpp images that already have Alpha channel */
+ if (src->format->BitsPerPixel <= 24 || src->format->Amask)
+ {
+- src->refcount++;
++ src->userdata = (void *)((long int) src->userdata + 1);
+ return src;
+ }
+
No comments:
Post a Comment