widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

SirVer has proposed merging lp:~widelands-dev/widelands/remove_caching_in_ui into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/remove_caching_in_ui/+merge/242559

In preparation of removing the software rendering: Removes caching from the UI implementation. It was not used often (only for complete windows) and will likely be slower on OpenGL anyways.
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/remove_caching_in_ui into lp:widelands.

=== modified file 'src/ui_basic/CMakeLists.txt'
--- src/ui_basic/CMakeLists.txt	2014-11-22 10:23:33 +0000
+++ src/ui_basic/CMakeLists.txt	2014-11-22 10:39:42 +0000
@@ -60,7 +60,6 @@
     graphic
     graphic_color
     graphic_image
-    graphic_surface
     helper
     io_filesystem
     # TODO(sirver): should not depend on logic

=== modified file 'src/ui_basic/button.cc'
--- src/ui_basic/button.cc	2014-11-02 20:31:40 +0000
+++ src/ui_basic/button.cc	2014-11-22 10:39:42 +0000
@@ -59,9 +59,6 @@
 	m_draw_caret    (false)
 {
 	set_think(false);
-
-	if (m_pic_background)
-		set_cache(true);
 }
 
 
@@ -91,9 +88,6 @@
 	m_draw_caret    (false)
 {
 	set_think(false);
-
-	if (m_pic_background)
-		set_cache(true);
 }
 
 

=== modified file 'src/ui_basic/panel.cc'
--- src/ui_basic/panel.cc	2014-10-28 12:53:29 +0000
+++ src/ui_basic/panel.cc	2014-11-22 10:39:42 +0000
@@ -19,13 +19,12 @@
 
 #include "ui_basic/panel.h"
 
+// NOCOM(#sirver): check includes
 #include "base/log.h"
 #include "graphic/font_handler.h"
 #include "graphic/font_handler1.h"
 #include "graphic/graphic.h"
 #include "graphic/rendertarget.h"
-#include "graphic/surface.h"
-#include "graphic/surface_cache.h"
 #include "helper.h"
 #include "profile/profile.h"
 #include "sound/sound_handler.h"
@@ -38,40 +37,6 @@
 
 namespace UI {
 
-// Caches the image of the inner of a panel. Will redraw the Panel on cache misses.
-class Panel::CacheImage : public Image {
-public:
-	CacheImage(Panel* const panel, uint16_t w, uint16_t h) :
-		width_(w),
-		height_(h),
-		panel_(panel),
-		hash_("cache_image_" + random_string("0123456789ABCDEFGH", 32)) {}
-	virtual ~CacheImage() {}
-
-	// Implements Image.
-	uint16_t width() const override {return width_;}
-	uint16_t height() const override {return height_;}
-	const string& hash() const override {return hash_;}
-	Surface* surface() const override {
-		Surface* rv = g_gr->surfaces().get(hash_);
-		if (rv)
-			return rv;
-
-		rv = g_gr->surfaces().insert(hash_, Surface::create(width_, height_), true);
-
-		// Cache miss! We have to redraw our panel onto our surface.
-		RenderTarget inner(rv);
-		panel_->do_draw_inner(inner);
-
-		return rv;
-	}
-
-private:
-	const int16_t width_, height_;
-	Panel* const panel_;  // not owned.
-	const string hash_;
-};
-
 Panel * Panel::_modal       = nullptr;
 Panel * Panel::_g_mousegrab = nullptr;
 Panel * Panel::_g_mousein   = nullptr;
@@ -93,7 +58,6 @@
 	:
 	_parent(nparent), _fchild(nullptr), _lchild(nullptr), _mousein(nullptr), _focus(nullptr),
 	_flags(pf_handle_mouse|pf_think|pf_visible),
-	_needdraw(false),
 	_x(nx), _y(ny), _w(nw), _h(nh),
 	_lborder(0), _rborder(0), _tborder(0), _bborder(0),
 	_border_snap_distance(0), _panel_snap_distance(0),
@@ -308,12 +272,10 @@
  * Move the panel. Panel's position is relative to the parent.
  */
 void Panel::set_pos(const Point n) {
-	bool nd = _needdraw;
 	update(0, 0, _w, _h);
 	_x = n.x;
 	_y = n.y;
 	update(0, 0, _w, _h);
-	_needdraw = nd;
 }
 
 /**
@@ -510,8 +472,6 @@
 		 y >= static_cast<int32_t>(_h) || y + h <= 0)
 		return;
 
-	_needdraw = true;
-
 	if (_parent) {
 		_parent->update_inner(x + _x, y + _y, w, h);
 	} else {
@@ -558,24 +518,6 @@
 }
 
 /**
- * Enable/Disable the drawing cache.
- * When the drawing cache is enabled, draw() is only called after an update()
- * has been called explicitly. Otherwise, the contents of the panel are copied
- * from an \ref Surface containing the cached image.
- *
- * \note Caching only works properly for solid panels that have no transparency.
- */
-void Panel::set_cache(bool cache)
-{
-	if (cache) {
-		_flags |= pf_cache;
-	} else {
-		_flags &= ~pf_cache;
-		_cache.reset();
-	}
-}
-
-/**
  * Called once per event loop pass, unless set_think(false) has
  * been called. It is intended to be used for animations and game logic.
  */
@@ -934,31 +876,12 @@
 
 	draw_border(dst);
 
-	if (_flags & pf_cache) {
-		uint32_t innerw = _w - (_lborder + _rborder);
-		uint32_t innerh = _h - (_tborder + _bborder);
-
-	if (!_cache || _cache->width() != innerw || _cache->height() != innerh) {
-			_cache.reset(new CacheImage(this, innerw, innerh));
-			_needdraw = true;
-		}
-
-		if (_needdraw) {
-			RenderTarget inner(_cache->surface());
-			do_draw_inner(inner);
-
-			_needdraw = false;
-		}
-
-		dst.blit(Point(_lborder, _tborder), _cache.get(), CM_Copy);
-	} else {
-		Rect innerwindow
-			(Point(_lborder, _tborder),
-				_w - (_lborder + _rborder), _h - (_tborder + _bborder));
-
-		if (dst.enter_window(innerwindow, nullptr, nullptr))
-			do_draw_inner(dst);
-	}
+	Rect innerwindow
+		(Point(_lborder, _tborder),
+			_w - (_lborder + _rborder), _h - (_tborder + _bborder));
+
+	if (dst.enter_window(innerwindow, nullptr, nullptr))
+		do_draw_inner(dst);
 
 	dst.set_window(outerrc, outerofs);
 }

=== modified file 'src/ui_basic/panel.h'
--- src/ui_basic/panel.h	2014-10-27 08:37:54 +0000
+++ src/ui_basic/panel.h	2014-11-22 10:39:42 +0000
@@ -23,7 +23,6 @@
 
 #include <cassert>
 #include <cstring>
-#include <memory>
 #include <string>
 
 #include <SDL_keyboard.h>
@@ -72,8 +71,6 @@
 		pf_dock_windows_to_edges = 256,
 		/// whether any change in the desired size should propagate to the actual size
 		pf_layout_toplevel = 512,
-		/// whether widget panels should be cached when possible
-		pf_cache = 1024,
 		/// whether widget wants to receive unicode textinput messages
 		pf_textinput = 2048,
 	}; // TODO(unknown): Turn this into separate bool flags
@@ -174,7 +171,6 @@
 	void update(int32_t x, int32_t y, int32_t w, int32_t h);
 	void update();
 	void update_inner(int32_t x, int32_t y, int32_t w, int32_t h);
-	void set_cache(bool enable);
 
 	// Events
 	virtual void think();
@@ -256,9 +252,6 @@
 	static bool draw_tooltip(RenderTarget &, const std::string & text);
 
 private:
-	class CacheImage;
-	friend class CacheImage;
-
 	void check_child_death();
 
 	void do_draw(RenderTarget &);
@@ -293,8 +286,6 @@
 	Panel * _focus; //  keyboard focus
 
 	uint32_t _flags;
-	std::unique_ptr<const Image> _cache;
-	bool _needdraw;
 
 	/**
 	 * The outer rectangle is defined by (_x, _y, _w, _h)

=== modified file 'src/ui_basic/window.cc'
--- src/ui_basic/window.cc	2014-11-02 20:31:40 +0000
+++ src/ui_basic/window.cc	2014-11-22 10:39:42 +0000
@@ -101,7 +101,6 @@
 	set_border
 		(VT_B_PIXMAP_THICKNESS, VT_B_PIXMAP_THICKNESS,
 		 TP_B_PIXMAP_THICKNESS, BT_B_PIXMAP_THICKNESS);
-	set_cache(true);
 	set_top_on_click(true);
 	set_layout_toplevel(true);
 }

=== modified file 'src/wui/general_statistics_menu.cc'
--- src/wui/general_statistics_menu.cc	2014-10-27 10:14:10 +0000
+++ src/wui/general_statistics_menu.cc	2014-11-22 10:39:42 +0000
@@ -63,8 +63,6 @@
 	set_center_panel(&m_box);
 	m_box.set_border(5, 5, 5, 5);
 
-	set_cache(false);
-
 	// Setup plot data
 	m_plot.set_sample_rate(STATISTICS_SAMPLE_TIME);
 	m_plot.set_plotmode(WuiPlotArea::PLOTMODE_ABSOLUTE);

=== modified file 'src/wui/minimap.cc'
--- src/wui/minimap.cc	2014-09-27 18:53:55 +0000
+++ src/wui/minimap.cc	2014-11-22 10:39:42 +0000
@@ -187,8 +187,6 @@
 	   boost::bind(&MiniMap::toggle, boost::ref(*this), MiniMapLayer::Building));
 	button_zoom.sigclicked.connect(boost::bind(&MiniMap::toggle, boost::ref(*this), MiniMapLayer::Zoom2));
 
-	set_cache(false);
-
 	resize();
 
 	update_button_permpressed();

=== modified file 'src/wui/transport_ui.cc'
--- src/wui/transport_ui.cc	2014-09-10 16:57:31 +0000
+++ src/wui/transport_ui.cc	2014-11-22 10:39:42 +0000
@@ -64,11 +64,6 @@
 			 g_gr->images().get(pic_tab_workers),
 			 new EconomyOptionsWorkerPanel(&m_tabpanel, parent, economy),
 			 _("Workers"));
-
-		// Until we can find a non-stupid way of automatically updating
-		// the window when one of the target quantities changes,
-		// simply disable caching.
-		set_cache(false);
 	}
 
 private:

=== modified file 'src/wui/ware_statistics_menu.cc'
--- src/wui/ware_statistics_menu.cc	2014-09-10 14:48:40 +0000
+++ src/wui/ware_statistics_menu.cc	2014-11-22 10:39:42 +0000
@@ -139,8 +139,6 @@
 	(&parent, "ware_statistics", &registry, 400, 270, _("Ware Statistics")),
 m_parent(&parent)
 {
-	set_cache(false);
-
 	uint8_t const nr_wares = parent.get_player()->tribe().get_nrwares();
 
 	//init color sets

=== modified file 'src/wui/watchwindow.cc'
--- src/wui/watchwindow.cc	2014-11-13 08:25:45 +0000
+++ src/wui/watchwindow.cc	2014-11-22 10:39:42 +0000
@@ -147,7 +147,6 @@
 
 	add_view(coords);
 	next_view(true);
-	set_cache(false);
 }
 
 /**