Skip to content

Commit

Permalink
Disable dbd6695 and bc911f8 with SDL < 2.0.6
Browse files Browse the repository at this point in the history
Requested by @gfgtdf. There is some hope that this commit would fix
crash on exit that a couple of developers have reported.
  • Loading branch information
jyrkive committed Sep 24, 2017
1 parent 563d080 commit 33fe982
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 22 deletions.
21 changes: 21 additions & 0 deletions src/sdl/surface.cpp
Expand Up @@ -14,8 +14,29 @@
#include "sdl/surface.hpp"

#include "sdl/rect.hpp"
#include "sdl/utils.hpp"
#include "video.hpp"

void surface::free_surface()
{
if(surface_) {
/* Workaround for an SDL bug.
* SDL 2.0.6 frees the blit map unconditionally in SDL_FreeSurface() without checking
* if the reference count has fallen to zero. However, many SDL functions such as
* SDL_ConvertSurface() assume that the blit map is present.
* Thus, we only call SDL_FreeSurface() if this is the last reference to the surface.
* Otherwise we just decrement the reference count ourselves.
*
* - Jyrki, 2017-09-23
*/
if(surface_->refcount > 1 && sdl_get_version() >= version_info(2, 0, 6)) {
--surface_->refcount;
} else {
SDL_FreeSurface(surface_);
}
}
}

surface_restorer::surface_restorer()
: target_(nullptr)
, rect_(sdl::empty_rect)
Expand Down
20 changes: 1 addition & 19 deletions src/sdl/surface.hpp
Expand Up @@ -80,25 +80,7 @@ class surface
surface_ = surf;
}

void free_surface()
{
if(surface_) {
/* Workaround for an SDL bug.
* SDL 2.0.6 frees the blit map unconditionally in SDL_FreeSurface() without checking
* if the reference count has fallen to zero. However, many SDL functions such as
* SDL_ConvertSurface() assume that the blit map is present.
* Thus, we only call SDL_FreeSurface() if this is the last reference to the surface.
* Otherwise we just decrement the reference count ourselves.
*
* - Jyrki, 2017-09-23
*/
if(surface_->refcount > 1) {
--surface_->refcount;
} else {
SDL_FreeSurface(surface_);
}
}
}
void free_surface();

SDL_Surface* surface_;
};
Expand Down
7 changes: 7 additions & 0 deletions src/sdl/utils.cpp
Expand Up @@ -32,6 +32,13 @@

#include <boost/math/constants/constants.hpp>

version_info sdl_get_version()
{
SDL_version sdl_version;
SDL_GetVersion(&sdl_version);
return version_info(sdl_version.major, sdl_version.minor, sdl_version.patch);
}

bool is_neutral(const surface& surf)
{
return (surf->format->BytesPerPixel == 4 &&
Expand Down
3 changes: 3 additions & 0 deletions src/sdl/utils.hpp
Expand Up @@ -20,13 +20,16 @@
#include "color.hpp"
#include "sdl/surface.hpp"
#include "utils/math.hpp"
#include "version.hpp"

#include <SDL.h>

#include <cstdlib>
#include <map>
#include <string>

version_info sdl_get_version();

inline void sdl_blit(const surface& src, SDL_Rect* src_rect, surface& dst, SDL_Rect* dst_rect){
SDL_BlitSurface(src, src_rect, dst, dst_rect);
}
Expand Down
10 changes: 7 additions & 3 deletions src/video.cpp
Expand Up @@ -19,6 +19,7 @@
#include "image.hpp"
#include "log.hpp"
#include "preferences/general.hpp"
#include "sdl/utils.hpp"
#include "sdl/window.hpp"
#include "sdl/userevent.hpp"

Expand Down Expand Up @@ -196,9 +197,12 @@ void CVideo::update_framebuffer()
if(!frameBuffer) {
frameBuffer = fb;
} else {
// Because SDL has already freed the old framebuffer,
// ensure that we won't attempt to free it.
frameBuffer.clear_without_free();
if(sdl_get_version() >= version_info(2, 0, 6)) {
// Because SDL has already freed the old framebuffer,
// ensure that we won't attempt to free it.
frameBuffer.clear_without_free();
}

frameBuffer.assign(fb);
}
}
Expand Down

5 comments on commit 33fe982

@newfrenchy83
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jyrkive i just tested and the gamw crash in begun scenario. i have windows 7 ver 42 bits on my computer. i hope what you correct this.

@GregoryLundberg
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to have fixed it for me.

@newfrenchy83
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't know why, but when i apply this commit, i have an crash before objective appears.

@newfrenchy83
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jyrkive have you compile wesnoth with sdl 2.0.6 and replace version 2.0.4 of sdl2.dll by 2.0.6? else it's normal what you're not crash in begin scenario but when the codeblocks include and libraries sdl move up to 2.0.6 then you see the problems. i cancel this fix for sdl\surface.cpp and hpp for avoid the crash

@jyrkive
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have compiled Wesnoth with SDL 2.0.6 on GNU/Linux, and it works fine for me.

On Windows I haven't tried SDL 2.0.6.

Please sign in to comment.