Broken textures with SDL 2.0.14

rapenne-s commented 3 years ago

Hello,

I (and other people) are experiencing the same issue reported in #568 but the issue was closed so I reopen one.

I'm using Barony on OpenBSD and we now ship the latest SDL2 package with version 2.0.14 and we experience broken textures. We can't use older SDL2 .so files because of regular ABI changes that make them unusable (and it's not a great solution).

I'd be fine importing patches into our packaging to fix the issue if it's possible.

Is there a plan to make Barony compatible with newer SDL2 versions? That would be SO great for the future 👍

DanielGibson commented 3 years ago

Sounds like an issue that should definitely be investigated - it could as well be a regression in SDL2 that should be fixed there.

CyberOnion commented 3 years ago

Running PopOS, I had to add the launch commands that was in issue 568 and afterwards the textures worked

DanielGibson commented 3 years ago

Forcing the game to use an older SDL2 version is not a solution, it's a workaround at best.

namtsui commented 3 years ago

cc @1bsyl

I bisected using the sdl2 hg repo and openbsd port. I found the commit that caused this bug to show up. I get the missing textures as in: https://github.com/TurningWheel/Barony/issues/568

see: https://github.com/libsdl-org/SDL/commit/ebc12a2fd2bb692908884e7db6cc935941a591f2

dust2$ hg bisect --reset
dust2$ hg bisect --bad 14878  
dust2$ hg bisect --good 13608 
Testing changeset 14252:6c2bbcfc3940 (1213 changesets remaining, ~10 tests)
430 files updated, 0 files merged, 12 files removed, 0 files unresolved
dust2$ bisect --bad
ksh: bisect: not found
dust2$ hg bisect --bad
Testing changeset 13914:348d0896fc9e (606 changesets remaining, ~9 tests)
240 files updated, 0 files merged, 55 files removed, 0 files unresolved
dust2$ hg bisect --good
Testing changeset 14087:01d524d4574f (303 changesets remaining, ~8 tests)
107 files updated, 0 files merged, 0 files removed, 0 files unresolved
dust2$ hg bisect --bad  
Testing changeset 13997:738ede671341 (151 changesets remaining, ~7 tests)
54 files updated, 0 files merged, 12 files removed, 0 files unresolved
dust2$ hg bisect --good
Testing changeset 14035:e287d66e4210 (76 changesets remaining, ~6 tests)
25 files updated, 0 files merged, 0 files removed, 0 files unresolved
dust2$ hg bisect --bad  
Testing changeset 14016:e7b77e8ed1e0 (38 changesets remaining, ~5 tests)
12 files updated, 0 files merged, 0 files removed, 0 files unresolved
dust2$ hg bisect --bad 
Testing changeset 14006:8f1abe5e7d8f (19 changesets remaining, ~4 tests)
11 files updated, 0 files merged, 0 files removed, 0 files unresolved
dust2$ hg bisect --good
Testing changeset 14011:458d11a0a3c8 (10 changesets remaining, ~3 tests)
5 files updated, 0 files merged, 0 files removed, 0 files unresolved
dust2$ hg bisect --good 
Testing changeset 14013:53e29c306f45 (5 changesets remaining, ~2 tests)
6 files updated, 0 files merged, 0 files removed, 0 files unresolved
dust2$ hg bisect --bad  
Testing changeset 14012:ee61a055bb11 (2 changesets remaining, ~1 tests)
4 files updated, 0 files merged, 0 files removed, 0 files unresolved
dust2$ hg bisect --good
The first bad revision is:
changeset:   14013:53e29c306f45
user:        Sylvain Becker <>
date:        Mon Sep 07 18:50:30 2020 +0200
summary:     SDL_Surface refcount: destination surface keep track of surfaces

1bsyl commented 3 years ago

Hi, I can have a look. I tried to compile Barony but it doesn't start, (missing .//images/system/fancyWindow.png ) ? Is there some asset to add ?

namtsui commented 3 years ago

Thanks. Yes, it requires the game assets from epic/gog/steam (barony-v3.3.7-010920). The game engine is picky about matching the correct version of the assets; otherwise you might get segfaults starting up with older versions of the game if a store hasn't updated yet. The game engine is open source, but the user must provide game assets.

I see that file in the assets: ./images/system/fancyWindow.png

EDIT: Specify data directory with barony-bin -datadir=~/games/data/barony

1bsyl commented 3 years ago

I got the files with the help of Daniel It crashes .. but I ll try later.

is the "editor" enough to show the issue ? or barony ? do you have a screenshot of what's wrong since I really don't know the game. (eventually the step to reproduce)

namtsui commented 3 years ago

https://github.com/namtsui/imagedump

I uploaded 6 pictures to compare good and bad versions.

barony_bad_startgame.png: click on start game

logo on the top left is incorrect
background of the start a new game text box is messed up.
mouse cursor becomes a dot, but it should be an x (as seen in barony_good_game.png)

barony_bad_settings.png: similar bug to startgame picture

barony_bad_game.png: once you enter the game, press m to pull up the map.

stats box on the left and skills box on the right have the same buggy background
boxes at the bottom of the screen shot lightning bolts instead of squares (1 to 0)

1bsyl commented 3 years ago

OK.

I did a quick try again, and I am missing:

selectedcursor.png (i replaced with cursor.png) but also: image/system/glyphsheet_ns.png

( but the 'editor' stared )

namtsui commented 3 years ago

That's strange. I don't see any glyphsheet_ns.png. I am using epic's assets though. EDIT: It might be fine as long as you can see the buggy textures in the editor I guess. Do they look like the buggy patterns in my pictures?

1bsyl commented 3 years ago

seems added here: https://github.com/TurningWheel/Barony/commit/4d9d60e447712e93177a568feaf8190f1ba5b2c3 https://github.com/TurningWheel/Barony/commit/688569ba953add2e3851ca73c2933597df97cee6

DanielGibson commented 3 years ago

Is this code newer than the one used for the Steam build? Or are those assets from some DLC that's missing?

1bsyl commented 3 years ago

The editor looks fine .. no strange graphics, nor distortion

namtsui commented 3 years ago

Are you using the release? Latest Steam/Epic/GOG build as of 29/01/21

https://github.com/TurningWheel/Barony/releases/tag/v3.3.7

1bsyl commented 3 years ago

no, I'll try the tag, thanks! edit: not sure about the editor, it uses my linux distrib sdl2, so I need to switch to a dev sdl2 instead

DanielGibson commented 3 years ago

Using dev SDL (or 2.0.14) is probably a good idea, esp. in case your distros version doesn't include the commit that apparently broke it ;)

1bsyl commented 3 years ago

Here's a fix https://github.com/TurningWheel/Barony/pull/582

Barony shouldn't use and re-assign refcount, this is a private SDL_Surface field ... There is 'userdata' for that.
And I think there is a mistake in files.cpp, using the imgref vs imgref + 1 as id.

namtsui commented 3 years ago

Thanks. Your fix works and I can see textures again, as in my "good" screenshots. I tested using sdl2-2.0.14 and barony v3.3.7.

1bsyl commented 3 years ago

You're welcome, I have also added another bugfix to make sure the image pitch of the SDL_Surface is taken into account when loading it into a texture. https://github.com/TurningWheel/Barony/pull/582/commits/ce9b618f89535b39a30d6ea0183eeb2f54b5d454 That prevents bad loading of the surface, for instance when using the SDL_ttf current dev version.

1bsyl commented 3 years ago

@rapenne-s Hi, I looked at the patch, and I think you missed the commit https://github.com/TurningWheel/Barony/commit/ce9b618f89535b39a30d6ea0183eeb2f54b5d454

You should it because surface pitch is not guaranteed to be the surface width * BPP (for instance loading assets with SDL_image, or rendering SDL_ttf), and then the data can be loaded incorrectly. This is problematic with next SDL_ttf update, which is faster to render software texture, has bug fixes, and is using complex text shaping with harfbuzz.

TurningWheel / Barony

Broken textures with SDL 2.0.14 #580