widelands-dev team mailing list archive
-
widelands-dev team
-
Mailing list archive
-
Message #01262
[Merge] lp:~widelands-dev/widelands/graphic_resetting into lp:widelands
SirVer has proposed merging lp:~widelands-dev/widelands/graphic_resetting into lp:widelands.
Requested reviews:
Tino (tino79)
Related bugs:
Bug #1159968 in widelands: "Crash in opengl fonthandler_cc:99"
https://bugs.launchpad.net/widelands/+bug/1159968
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/graphic_resetting/+merge/159985
There is at least one critical bug lurking in this code - but I wasn't able to produce a reliable crash scenario, not did I see the bug. Reaching out for a bunch more eyes :)
--
https://code.launchpad.net/~widelands-dev/widelands/graphic_resetting/+merge/159985
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/graphic_resetting.
=== modified file 'src/graphic/font_handler.cc'
--- src/graphic/font_handler.cc 2013-06-15 13:38:19 +0000
+++ src/graphic/font_handler.cc 2013-07-16 20:53:40 +0000
@@ -111,9 +111,13 @@
Font_Handler::~Font_Handler() {
+ flush();
Font::shutdown();
}
+void Font_Handler::flush() {
+ d.reset(new Data);
+}
/*
* Returns the height of the font, in pixels.
=== modified file 'src/graphic/font_handler.h'
--- src/graphic/font_handler.h 2013-06-15 13:38:19 +0000
+++ src/graphic/font_handler.h 2013-07-16 20:53:40 +0000
@@ -20,6 +20,7 @@
#ifndef FONT_HANDLER_H
#define FONT_HANDLER_H
+#include <string>
#include <boost/scoped_ptr.hpp>
#include "point.h"
@@ -59,6 +60,9 @@
uint32_t wrap = std::numeric_limits<uint32_t>::max());
uint32_t get_fontheight(const std::string & name, int32_t size);
+ // Delete the whole cache.
+ void flush();
+
private:
struct Data;
boost::scoped_ptr<Data> d;
=== modified file 'src/graphic/graphic.cc'
--- src/graphic/graphic.cc 2013-07-14 17:41:02 +0000
+++ src/graphic/graphic.cc 2013-07-16 20:53:40 +0000
@@ -44,6 +44,7 @@
#include "animation.h"
#include "animation_gfx.h"
+#include "font_handler.h"
#include "image.h"
#include "image_loader_impl.h"
#include "image_transformations.h"
@@ -62,14 +63,9 @@
/**
* Initialize the SDL video mode.
*/
-Graphic::Graphic
- (int32_t w, int32_t h,
- int32_t bpp,
- bool fullscreen,
- bool opengl)
+Graphic::Graphic()
:
m_fallback_settings_in_effect (false),
- m_rendertarget (0),
m_nr_update_rects (0),
m_update_fullscreen(true),
image_loader_(new ImageLoaderImpl()),
@@ -88,9 +84,12 @@
SDL_Surface * s = IMG_Load_RW(SDL_RWFromMem(fr.Data(0), fr.GetSize()), 1);
SDL_WM_SetIcon(s, 0);
SDL_FreeSurface(s);
+}
+
+void Graphic::initialize(int32_t w, int32_t h, int32_t bpp, bool fullscreen, bool opengl) {
+ cleanup();
// Set video mode using SDL. First collect the flags
-
int32_t flags = 0;
g_opengl = false;
SDL_Surface * sdlsurface = 0;
@@ -306,7 +305,6 @@
glEnable(GL_TEXTURE_2D);
GLSurfaceTexture::Initialize(use_arb);
-
}
if (g_opengl)
@@ -315,11 +313,11 @@
}
else
{
- screen_.reset(new SDLSurface(sdlsurface));
+ screen_.reset(new SDLSurface(sdlsurface, false));
}
m_sdl_screen = sdlsurface;
- m_rendertarget = new RenderTarget(screen_.get());
+ m_rendertarget.reset(new RenderTarget(screen_.get()));
}
bool Graphic::check_fallback_settings_in_effect()
@@ -327,25 +325,27 @@
return m_fallback_settings_in_effect;
}
-/**
- * Free the surface
-*/
-Graphic::~Graphic()
-{
- BOOST_FOREACH(Texture* texture, m_maptextures)
- delete texture;
- delete m_rendertarget;
-
+void Graphic::cleanup() {
+ flush_maptextures();
flush_animations();
+ surface_cache_->flush();
+ // TODO: this should really not be needed, but currently is :(
+ if (UI::g_fh)
+ UI::g_fh->flush();
if (g_opengl)
GLSurfaceTexture::Cleanup();
}
+Graphic::~Graphic()
+{
+ cleanup();
+}
+
/**
* Return the screen x resolution
*/
-int32_t Graphic::get_xres() const
+int32_t Graphic::get_xres()
{
return screen_->width();
}
@@ -353,11 +353,21 @@
/**
* Return the screen x resolution
*/
-int32_t Graphic::get_yres() const
+int32_t Graphic::get_yres()
{
return screen_->height();
}
+int32_t Graphic::get_bpp()
+{
+ return m_sdl_screen->format->BitsPerPixel;
+}
+
+bool Graphic::is_fullscreen()
+{
+ return m_sdl_screen->flags & SDL_FULLSCREEN;
+}
+
/**
* Return a pointer to the RenderTarget representing the screen
*/
@@ -365,7 +375,7 @@
{
m_rendertarget->reset();
- return m_rendertarget;
+ return m_rendertarget.get();
}
/**
=== modified file 'src/graphic/graphic.h'
--- src/graphic/graphic.h 2013-03-09 08:07:22 +0000
+++ src/graphic/graphic.h 2013-07-16 20:53:40 +0000
@@ -86,13 +86,18 @@
*/
class Graphic {
public:
- Graphic
- (int32_t w, int32_t h, int32_t bpp,
- bool fullscreen, bool opengl);
+ Graphic();
~Graphic();
- int32_t get_xres() const;
- int32_t get_yres() const;
+ // Initialize or reinitialize the graphics system. Throws on error.
+ void initialize
+ (int32_t w, int32_t h, int32_t bpp, bool fullscreen, bool opengl);
+
+ int32_t get_xres();
+ int32_t get_yres();
+ int32_t get_bpp();
+ bool is_fullscreen();
+
RenderTarget * get_render_target();
void toggle_fullscreen();
void update_fullscreen();
@@ -133,6 +138,7 @@
bool check_fallback_settings_in_effect();
private:
+ void cleanup();
void save_png_(Surface & surf, StreamWrite*) const;
bool m_fallback_settings_in_effect;
@@ -153,7 +159,7 @@
/// manipulation the screen context.
SDL_Surface * m_sdl_screen;
/// A RenderTarget for screen_. This is initialized during init()
- RenderTarget * m_rendertarget;
+ boost::scoped_ptr<RenderTarget> m_rendertarget;
/// keeps track which screen regions needs to be redrawn during the next
/// update(). Only used for SDL rendering.
SDL_Rect m_update_rects[MAX_RECTS];
=== modified file 'src/graphic/image_cache.h'
--- src/graphic/image_cache.h 2013-02-10 16:08:37 +0000
+++ src/graphic/image_cache.h 2013-07-16 20:53:40 +0000
@@ -25,6 +25,7 @@
#include <boost/utility.hpp>
#include "image.h"
+
class IImageLoader;
class SurfaceCache;
=== modified file 'src/graphic/render/sdl_surface.cc'
--- src/graphic/render/sdl_surface.cc 2013-07-12 15:11:32 +0000
+++ src/graphic/render/sdl_surface.cc 2013-07-16 20:53:40 +0000
@@ -26,7 +26,8 @@
SDLSurface::~SDLSurface() {
assert(m_surface);
- SDL_FreeSurface(m_surface);
+ if (m_free_surface_on_delete)
+ SDL_FreeSurface(m_surface);
}
const SDL_PixelFormat & SDLSurface::format() const {
=== modified file 'src/graphic/render/sdl_surface.h'
--- src/graphic/render/sdl_surface.h 2013-02-10 16:41:12 +0000
+++ src/graphic/render/sdl_surface.h 2013-07-16 20:53:40 +0000
@@ -33,10 +33,13 @@
*/
class SDLSurface : public Surface {
public:
- SDLSurface(SDL_Surface* surface) :
+ // The surface set by SetVideoMode must not be freed according to the SDL
+ // docs, so we need 'free_surface_on_delete'.
+ SDLSurface(SDL_Surface* surface, bool free_surface_on_delete = true) :
m_surface(surface),
m_offsx(0), m_offsy(0),
- m_w(surface->w), m_h(surface->h)
+ m_w(surface->w), m_h(surface->h),
+ m_free_surface_on_delete(free_surface_on_delete)
{}
virtual ~SDLSurface();
@@ -72,6 +75,7 @@
int32_t m_offsx;
int32_t m_offsy;
uint16_t m_w, m_h;
+ bool m_free_surface_on_delete;
};
=== modified file 'src/graphic/surface_cache.cc'
--- src/graphic/surface_cache.cc 2013-07-13 14:32:49 +0000
+++ src/graphic/surface_cache.cc 2013-07-16 20:53:40 +0000
@@ -39,6 +39,7 @@
virtual ~SurfaceCacheImpl();
// Implements SurfaceCache.
+ virtual void flush();
virtual Surface* get(const string& hash);
virtual Surface* insert(const string& hash, Surface*, bool);
@@ -64,9 +65,16 @@
};
SurfaceCacheImpl::~SurfaceCacheImpl() {
+ flush();
+}
+
+void SurfaceCacheImpl::flush() {
for (Container::iterator it = entries_.begin(); it != entries_.end(); ++it) {
delete it->second;
}
+ entries_.clear();
+ access_history_.clear();
+ used_transient_memory_ = 0;
}
Surface* SurfaceCacheImpl::get(const string& hash) {
=== modified file 'src/graphic/surface_cache.h'
--- src/graphic/surface_cache.h 2013-07-13 14:32:49 +0000
+++ src/graphic/surface_cache.h 2013-07-16 20:53:40 +0000
@@ -38,6 +38,9 @@
SurfaceCache() {};
virtual ~SurfaceCache() {};
+ /// Deletes all surfaces in the cache leaving it as if it were just created.
+ virtual void flush() = 0;
+
/// Returns an entry if it is cached, NULL otherwise.
virtual Surface* get(const std::string& hash) = 0;
=== modified file 'src/helper.cc'
--- src/helper.cc 2013-02-10 19:36:24 +0000
+++ src/helper.cc 2013-07-16 20:53:40 +0000
@@ -17,9 +17,16 @@
*
*/
+#include <cstdarg>
+#include <string>
+
+#include <boost/random.hpp>
+#include <boost/scoped_array.hpp>
+
#include "helper.h"
-#include <cstdarg>
+using namespace std;
+
/// Split a string by separators.
/// \note This ignores empty elements, so do not use this for example to split
@@ -71,3 +78,14 @@
((k.sym >= SDLK_WORLD_0) && (k.sym <= SDLK_WORLD_95)) ||
((k.sym >= SDLK_KP0) && (k.sym <= SDLK_KP_EQUALS));
}
+
+static boost::random::mt19937 random_generator;
+string random_string(const string& chars, int nlen) {
+ boost::random::uniform_int_distribution<> index_dist(0, chars.size() - 1);
+ boost::scoped_array<char> buffer(new char[nlen - 1]);
+ for (int i = 0; i < nlen; ++i) {
+ buffer[i] = chars[index_dist(random_generator)];
+ }
+ return string(buffer.get(),nlen);
+}
+
=== modified file 'src/helper.h'
--- src/helper.h 2013-02-10 19:36:24 +0000
+++ src/helper.h 2013-07-16 20:53:40 +0000
@@ -158,4 +158,7 @@
bool is_printable(SDL_keysym k);
+/// Generate a random string of given size out of the given alphabet.
+std::string random_string(const std::string& chars, int nlen);
+
#endif
=== modified file 'src/ui_basic/panel.cc'
--- src/ui_basic/panel.cc 2013-03-02 20:35:18 +0000
+++ src/ui_basic/panel.cc 2013-07-16 20:53:40 +0000
@@ -17,15 +17,14 @@
*
*/
-#include <boost/concept_check.hpp>
-
#include "constants.h"
#include "graphic/font_handler.h"
#include "graphic/font_handler1.h"
#include "graphic/graphic.h"
-#include "graphic/in_memory_image.h"
#include "graphic/rendertarget.h"
#include "graphic/surface.h"
+#include "graphic/surface_cache.h"
+#include "helper.h"
#include "log.h"
#include "profile/profile.h"
#include "sound/sound_handler.h"
@@ -34,8 +33,37 @@
#include "panel.h"
+using namespace std;
+
namespace UI {
+namespace {
+class CacheImage : public Image {
+public:
+ CacheImage(uint16_t w, uint16_t h) :
+ width_(w), height_(h),
+ hash_("cache_image_" + random_string("0123456789ABCDEFGH", 32)) {}
+ virtual ~CacheImage() {}
+
+ // Implements Image.
+ virtual uint16_t width() const {return width_;}
+ virtual uint16_t height() const {return height_;}
+ virtual const string& hash() const {return hash_;}
+ virtual Surface* surface() const {
+ Surface* rv = g_gr->surfaces().get(hash_);
+ if (rv)
+ return rv;
+
+ rv = g_gr->surfaces().insert(hash_, Surface::create(width_, height_), true);
+ return rv;
+ }
+
+private:
+ const int16_t width_, height_;
+ const string hash_;
+};
+
+} // namespace
Panel * Panel::_modal = 0;
Panel * Panel::_g_mousegrab = 0;
Panel * Panel::_g_mousein = 0;
@@ -835,12 +863,8 @@
uint32_t innerw = _w - (_lborder + _rborder);
uint32_t innerh = _h - (_tborder + _bborder);
- if
- (!_cache ||
- _cache.get()->width() != innerw ||
- _cache.get()->height() != innerh)
- {
- _cache.reset(new_in_memory_image("dummy_hash", Surface::create(innerw, innerh)));
+ if (!_cache || _cache->width() != innerw || _cache->height() != innerh) {
+ _cache.reset(new CacheImage(innerw, innerh));
_needdraw = true;
}
=== modified file 'src/wlapplication.cc'
--- src/wlapplication.cc 2013-07-15 05:18:12 +0000
+++ src/wlapplication.cc 2013-07-16 20:53:40 +0000
@@ -263,9 +263,6 @@
m_mouse_locked (0),
m_mouse_compensate_warp(0, 0),
m_should_die (false),
-m_gfx_w(0), m_gfx_h(0), m_gfx_bpp(0),
-m_gfx_fullscreen (false),
-m_gfx_opengl (true),
m_default_datadirs (true),
#ifdef WIN32
m_homedir(FileSystem::GetHomedir() + "\\.widelands"),
@@ -470,7 +467,6 @@
throw;
}
} else {
-
g_sound_handler.start_music("intro");
{
@@ -810,41 +806,31 @@
}
/**
- * Initialize the graphics subsystem (or shutdown, if system == GFXSYS_NONE)
+ * Initialize the graphics subsystem (or shutdown, if w and h are 0)
* with the given resolution.
* Throws an exception on failure.
- *
- * \note Because of the way pictures are handled now, this function must not be
- * called while UI elements are active.
- *
- * \todo Ensure that calling this with active UI elements does barf
- * \todo Document parameters
*/
-
void WLApplication::init_graphics
(const int32_t w, const int32_t h, const int32_t bpp,
const bool fullscreen, const bool opengl)
{
- if
- (w == m_gfx_w && h == m_gfx_h && bpp == m_gfx_bpp &&
- fullscreen == m_gfx_fullscreen &&
- opengl == m_gfx_opengl)
+ if (!w && !h) { // shutdown.
+ delete g_gr;
+ g_gr = 0;
return;
-
- delete g_gr;
- g_gr = 0;
-
- m_gfx_w = w;
- m_gfx_h = h;
- m_gfx_bpp = bpp;
- m_gfx_fullscreen = fullscreen;
- m_gfx_opengl = opengl;
-
-
- // If we are not to be shut down
- if (w && h) {
- g_gr = new Graphic
- (w, h, bpp, fullscreen, opengl);
+ }
+ assert(w > 0 && h > 0);
+
+ if (!g_gr) {
+ g_gr = new Graphic();
+ g_gr->initialize(w, h, bpp, fullscreen, opengl);
+ } else {
+ if
+ (g_gr->get_xres() != w || g_gr->get_yres() != h || g_gr->get_bpp() != bpp
+ || g_gr->is_fullscreen() != fullscreen || g_opengl != opengl)
+ {
+ g_gr->initialize(w, h, bpp, fullscreen, opengl);
+ }
}
}
@@ -856,7 +842,7 @@
init_graphics
(s.get_int("xres", XRES),
s.get_int("yres", YRES),
- s.get_int("depth", 16),
+ s.get_int("depth", 32),
s.get_bool("fullscreen", false),
s.get_bool("opengl", true));
}
@@ -881,14 +867,12 @@
set_input_grab(s.get_bool("inputgrab", false));
set_mouse_swap(s.get_bool("swapmouse", false));
- m_gfx_fullscreen = s.get_bool("fullscreen", false);
-
- m_gfx_opengl = s.get_bool("opengl", true);
-
// KLUDGE!
// Without this the following config options get dropped by check_used().
// Profile needs support for a Syntax definition to solve this in a
// sensible way
+ s.get_bool("fullscreen");
+ s.get_bool("opengl");
s.get_int("xres");
s.get_int("yres");
s.get_int("border_snap_distance");
@@ -1085,12 +1069,7 @@
SDL_EnableUNICODE(1); //needed by helper.h:is_printable()
SDL_EnableKeyRepeat(SDL_DEFAULT_REPEAT_DELAY, SDL_DEFAULT_REPEAT_INTERVAL);
- uint32_t xres = s.get_int("xres", XRES);
- uint32_t yres = s.get_int("yres", YRES);
-
- init_graphics
- (xres, yres, s.get_int("depth", 16),
- m_gfx_fullscreen, m_gfx_opengl);
+ refresh_graphics();
// Start the audio subsystem
// must know the locale before calling this!
=== modified file 'src/wlapplication.h'
--- src/wlapplication.h 2013-05-20 19:40:13 +0000
+++ src/wlapplication.h 2013-07-16 20:53:40 +0000
@@ -287,21 +287,6 @@
///true if an external entity wants us to quit
bool m_should_die;
- ///The Widelands window's width in pixels
- int32_t m_gfx_w;
-
- ///The Widelands window's height in pixels
- int32_t m_gfx_h;
-
- ///The Widelands window's bits per pixel
- int32_t m_gfx_bpp;
-
- ///If true Widelands is (should be, we never know ;-) running
- ///in a fullscreen window
- bool m_gfx_fullscreen;
-
- bool m_gfx_opengl;
-
//do we want to search the default places for widelands installs
bool m_default_datadirs;
std::string m_homedir;
Follow ups