widelands-dev team mailing list archive

[Benedikt Straub <benedikt-straub@xxxxxx>]

Benedikt Straub has proposed merging lp:~widelands-dev/widelands/workarea-fixes into lp:widelands.

Commit message:
· Draw a narrow border around workareas for better visibility
· Improve performance by only recalculating the workarea shader data when needed
· Add the hotkey W to enable/disable highlighting workarea overlaps

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1830345 in widelands: "overlapping workareas are very performance hungry"
  https://bugs.launchpad.net/widelands/+bug/1830345
  Bug #1830647 in widelands: "Indicate workarea border more clearly"
  https://bugs.launchpad.net/widelands/+bug/1830647

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/workarea-fixes/+merge/368083
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/workarea-fixes into lp:widelands.

=== added file 'data/images/wui/fieldaction/menu_show_workarea_overlap.png'
Binary files data/images/wui/fieldaction/menu_show_workarea_overlap.png	1970-01-01 00:00:00 +0000 and data/images/wui/fieldaction/menu_show_workarea_overlap.png	2019-05-29 18:00:02 +0000 differ
=== modified file 'src/graphic/gl/fields_to_draw.h'
--- src/graphic/gl/fields_to_draw.h	2019-02-27 19:00:36 +0000
+++ src/graphic/gl/fields_to_draw.h	2019-05-29 18:00:02 +0000
@@ -96,7 +96,6 @@
 		return &fields_[index];
 	}
 
-private:
 	// Calculates the index of the given field with ('fx', 'fy') being geometric
 	// coordinates in the map. Returns INVALID_INDEX if this field is not in the
 	// fields_to_draw.
@@ -112,6 +111,7 @@
 		return yidx * w_ + xidx;
 	}
 
+private:
 	// Minimum and maximum field coordinates (geometric) to render. Can be negative.
 	int min_fx_ = 0;
 	int max_fx_ = 0;

=== modified file 'src/graphic/gl/workarea_program.cc'
--- src/graphic/gl/workarea_program.cc	2019-05-17 07:13:04 +0000
+++ src/graphic/gl/workarea_program.cc	2019-05-29 18:00:02 +0000
@@ -24,7 +24,8 @@
 #include "graphic/gl/utils.h"
 #include "graphic/texture.h"
 
-WorkareaProgram::WorkareaProgram() {
+WorkareaProgram::WorkareaProgram()
+	: cache_(nullptr) {
 	gl_program_.build("workarea");
 
 	attr_position_ = glGetAttribLocation(gl_program_.object(), "attr_position");
@@ -36,22 +37,32 @@
 void WorkareaProgram::gl_draw(int gl_texture, float z_value) {
 	glUseProgram(gl_program_.object());
 
-	auto& gl_state = Gl::State::instance();
-	gl_state.enable_vertex_attrib_array({attr_position_, attr_overlay_});
-
-	gl_array_buffer_.bind();
-	gl_array_buffer_.update(vertices_);
-
-	Gl::vertex_attrib_pointer(
-	   attr_position_, 2, sizeof(PerVertexData), offsetof(PerVertexData, gl_x));
-	Gl::vertex_attrib_pointer(
-	   attr_overlay_, 4, sizeof(PerVertexData), offsetof(PerVertexData, overlay_r));
-
-	gl_state.bind(GL_TEXTURE0, gl_texture);
-
-	glUniform1f(u_z_value_, z_value);
-
-	glDrawArrays(GL_TRIANGLES, 0, vertices_.size());
+	{
+		auto& gl_state = Gl::State::instance();
+		gl_state.enable_vertex_attrib_array({attr_position_, attr_overlay_});
+		gl_array_buffer_.bind();
+		gl_array_buffer_.update(vertices_);
+		Gl::vertex_attrib_pointer(
+		   attr_position_, 2, sizeof(PerVertexData), offsetof(PerVertexData, gl_x));
+		Gl::vertex_attrib_pointer(
+		   attr_overlay_, 4, sizeof(PerVertexData), offsetof(PerVertexData, overlay_r));
+		gl_state.bind(GL_TEXTURE0, gl_texture);
+		glUniform1f(u_z_value_, z_value);
+		glDrawArrays(GL_TRIANGLES, 0, vertices_.size());
+	}
+	{
+		auto& gl_state = Gl::State::instance();
+		gl_state.enable_vertex_attrib_array({attr_position_, attr_overlay_});
+		gl_array_buffer_.bind();
+		gl_array_buffer_.update(outer_vertices_);
+		Gl::vertex_attrib_pointer(
+		   attr_position_, 2, sizeof(PerVertexData), offsetof(PerVertexData, gl_x));
+		Gl::vertex_attrib_pointer(
+		   attr_overlay_, 4, sizeof(PerVertexData), offsetof(PerVertexData, overlay_r));
+		gl_state.bind(GL_TEXTURE0, gl_texture);
+		glUniform1f(u_z_value_, z_value);
+		glDrawArrays(GL_LINES, 0, outer_vertices_.size());
+	}
 }
 
 constexpr uint8_t kWorkareaTransparency = 127;
@@ -81,9 +92,9 @@
 	return RGBAColor(r, g, b, special.a);
 }
 
-void WorkareaProgram::add_vertex(const FieldsToDraw::Field& field, RGBAColor overlay) {
-	vertices_.emplace_back();
-	PerVertexData& back = vertices_.back();
+void WorkareaProgram::add_vertex(const FieldsToDraw::Field& field, RGBAColor overlay, std::vector<PerVertexData>* v) {
+	v->emplace_back();
+	PerVertexData& back = v->back();
 
 	back.gl_x = field.gl_position.x;
 	back.gl_y = field.gl_position.y;
@@ -97,15 +108,34 @@
                            Workareas workarea,
                            const FieldsToDraw& fields_to_draw,
                            float z_value) {
+	const FieldsToDraw::Field& topleft = fields_to_draw.at(0);
+	if (cache_ && cache_->fcoords == topleft.fcoords && cache_->surface_pixel == topleft.surface_pixel &&
+			cache_->workareas == workarea) {
+		return gl_draw(texture_id, z_value);
+	}
+	cache_.reset(new WorkareasCache(workarea, topleft.fcoords, topleft.surface_pixel));
+
 	vertices_.clear();
-	vertices_.reserve(fields_to_draw.size() * 3);
+	outer_vertices_.clear();
+	{
+		size_t estimate_inner = 0;
+		size_t estimate_outer = 0;
+		for (const WorkareasEntry& wa_map : workarea) {
+			estimate_inner += 3 * wa_map.first.size();
+			for (const auto& vector : wa_map.second) {
+				estimate_outer += 2 * vector.size();
+			}
+		}
+		vertices_.reserve(estimate_inner);
+		outer_vertices_.reserve(estimate_outer);
+	}
 
 	auto emplace_triangle = [this, workarea, fields_to_draw](
 	                           const FieldsToDraw::Field& field,
 	                           Widelands::TriangleIndex triangle_index) {
 		RGBAColor color(0, 0, 0, 0);
 		for (const WorkareasEntry& wa_map : workarea) {
-			for (const WorkareaPreviewData& data : wa_map) {
+			for (const WorkareaPreviewData& data : wa_map.first) {
 				if (data.coords == Widelands::TCoords<>(field.fcoords, triangle_index)) {
 					RGBAColor color_to_apply = workarea_colors[data.index];
 					if (data.use_special_coloring) {
@@ -117,12 +147,12 @@
 			}
 		}
 		if (color.a > 0) {
-			add_vertex(field, color);
-			add_vertex(fields_to_draw.at(field.brn_index), color);
+			add_vertex(field, color, &vertices_);
+			add_vertex(fields_to_draw.at(field.brn_index), color, &vertices_);
 			add_vertex(
 			   fields_to_draw.at(triangle_index == Widelands::TriangleIndex::D ? field.bln_index :
 			                                                                     field.rn_index),
-			   color);
+			   color, &vertices_);
 		}
 	};
 
@@ -138,5 +168,41 @@
 		}
 	}
 
+	{
+		for (const WorkareasEntry& wa_map : workarea) {
+			int32_t index = 5;
+			for (const auto& border : wa_map.second) {
+				assert(index == 5 || index == 4 || index == 2);
+				for (auto it = border.begin(); it != border.end(); ++it) {
+					int f1 = fields_to_draw.calculate_index(it->x, it->y);
+					if (f1 == FieldsToDraw::kInvalidIndex) {
+						continue;
+					}
+					int f2;
+					if (it + 1 == border.end()) {
+						f2 = fields_to_draw.calculate_index(border.begin()->x, border.begin()->y);
+					} else {
+						f2 = fields_to_draw.calculate_index((it + 1)->x, (it + 1)->y);
+					}
+					if (f2 != FieldsToDraw::kInvalidIndex) {
+						add_vertex(fields_to_draw.at(f1), workarea_colors[index], &outer_vertices_);
+						add_vertex(fields_to_draw.at(f2), workarea_colors[index], &outer_vertices_);
+					}
+				}
+				switch (index) {
+					case 5:
+						index = 4;
+						break;
+					case 4:
+						index = 2;
+						break;
+					default:
+						index = -1;
+						break;
+				}
+			}
+		}
+	}
+
 	gl_draw(texture_id, z_value);
 }

=== modified file 'src/graphic/gl/workarea_program.h'
--- src/graphic/gl/workarea_program.h	2019-04-26 05:52:49 +0000
+++ src/graphic/gl/workarea_program.h	2019-05-29 18:00:02 +0000
@@ -20,6 +20,7 @@
 #ifndef WL_GRAPHIC_GL_WORKAREA_PROGRAM_H
 #define WL_GRAPHIC_GL_WORKAREA_PROGRAM_H
 
+#include <memory>
 #include <vector>
 
 #include "base/vector.h"
@@ -52,7 +53,7 @@
 
 	// Adds a vertex to the end of vertices with data from 'field' in order to apply the specified
 	// 'overlay'.
-	void add_vertex(const FieldsToDraw::Field& field, RGBAColor overlay);
+	void add_vertex(const FieldsToDraw::Field& field, RGBAColor overlay, std::vector<PerVertexData>*);
 
 	// The program used for drawing the workarea overlay.
 	Gl::Program gl_program_;
@@ -70,6 +71,20 @@
 	// Objects below are kept around to avoid memory allocations on each frame.
 	// They could theoretically also be recreated.
 	std::vector<PerVertexData> vertices_;
+	std::vector<PerVertexData> outer_vertices_;
+
+	// Calculating the workareas is a bit slow, so we only recalculate when we have to
+	struct WorkareasCache {
+		WorkareasCache(const Workareas& wa, const Widelands::FCoords f, const Vector2f v)
+			: workareas(wa), fcoords(f), surface_pixel(v) {
+		}
+
+		const Workareas workareas;
+		// Viewpoint data
+		const Widelands::FCoords fcoords;
+		const Vector2f surface_pixel;
+	};
+	std::unique_ptr<WorkareasCache> cache_;
 
 	DISALLOW_COPY_AND_ASSIGN(WorkareaProgram);
 };

=== modified file 'src/logic/map_objects/tribes/workarea_info.h'
--- src/logic/map_objects/tribes/workarea_info.h	2019-05-17 07:13:04 +0000
+++ src/logic/map_objects/tribes/workarea_info.h	2019-05-29 18:00:02 +0000
@@ -61,6 +61,11 @@
 	WorkareaPreviewData(const WorkareaPreviewData& other) = default;
 	WorkareaPreviewData& operator=(const WorkareaPreviewData&) = default;
 
+	bool operator==(const WorkareaPreviewData& d) const {
+		return index == d.index && coords == d.coords && use_special_coloring == d.use_special_coloring &&
+				(!use_special_coloring || special_coloring == d.special_coloring);
+	}
+
 	// The triangle this data is applied to
 	Widelands::TCoords<> coords;
 	// The underlying workarea color
@@ -71,7 +76,8 @@
 	bool use_special_coloring;
 	uint32_t special_coloring;
 };
-using WorkareasEntry = std::vector<WorkareaPreviewData>;
+// Pair of interior information and a per-circle list of border coords
+using WorkareasEntry = std::pair<std::vector<WorkareaPreviewData>, std::vector<std::vector<Widelands::Coords>>>;
 using Workareas = std::vector<WorkareasEntry>;
 
 #endif  // end of include guard: WL_LOGIC_MAP_OBJECTS_TRIBES_WORKAREA_INFO_H

=== modified file 'src/wui/fieldaction.cc'
--- src/wui/fieldaction.cc	2019-05-26 17:21:15 +0000
+++ src/wui/fieldaction.cc	2019-05-29 18:00:02 +0000
@@ -165,6 +165,7 @@
 	void act_watch();
 	void act_show_census();
 	void act_show_statistics();
+	void act_show_workarea_overlap();
 	void act_debug();
 	void act_buildflag();
 	void act_configure_economy();
@@ -206,6 +207,7 @@
 	uint32_t best_tab_;
 	bool showing_workarea_preview_;
 	std::set<Widelands::Coords> overlapping_workareas_;
+	bool is_showing_workarea_overlaps_;
 
 	/// Variables to use with attack dialog.
 	AttackBox* attack_box_;
@@ -231,6 +233,7 @@
 static const char* const pic_watchfield = "images/wui/fieldaction/menu_watch_field.png";
 static const char* const pic_showcensus = "images/wui/fieldaction/menu_show_census.png";
 static const char* const pic_showstatistics = "images/wui/fieldaction/menu_show_statistics.png";
+static const char* const pic_showworkareaoverlap = "images/wui/fieldaction/menu_show_workarea_overlap.png";
 static const char* const pic_debug = "images/wui/fieldaction/menu_debug.png";
 static const char* const pic_abort = "images/wui/menu_abort.png";
 static const char* const pic_geologist = "images/wui/fieldaction/menu_geologist.png";
@@ -253,6 +256,7 @@
      fastclick_(true),
      best_tab_(0),
      showing_workarea_preview_(false),
+     is_showing_workarea_overlaps_(ib->get_display_flag(InteractiveBase::dfShowWorkareaOverlap)),
      attack_box_(nullptr) {
 	ib->set_sel_freeze(true);
 	set_center_panel(&tabpanel_);
@@ -262,15 +266,28 @@
 	if (showing_workarea_preview_) {
 		ibase().hide_workarea(node_, false);
 	}
+	showing_workarea_preview_ = false;
 	clear_overlapping_workareas();
 	ibase().set_sel_freeze(false);
 	delete attack_box_;
 }
 
 void FieldActionWindow::think() {
-	if (player_ && player_->vision(node_.field - &ibase().egbase().map()[0]) <= 1 &&
-	    !player_->see_all())
+	if (is_showing_workarea_overlaps_ != ibase().get_display_flag(InteractiveBase::dfShowWorkareaOverlap)) {
+		is_showing_workarea_overlaps_ = !is_showing_workarea_overlaps_;
+		if (!is_showing_workarea_overlaps_) {
+			clear_overlapping_workareas();
+		}
+#ifndef NDEBUG
+		else {
+			// Desired overlaps will be shown as soon as the user moves the mouse
+			assert(overlapping_workareas_.empty());
+		}
+#endif
+	}
+	if (player_ && player_->vision(node_.field - &ibase().egbase().map()[0]) <= 1 && !player_->see_all()) {
 		die();
+	}
 }
 
 void FieldActionWindow::clear_overlapping_workareas() {
@@ -372,6 +389,11 @@
 	}
 	add_button(&watchbox, "census", pic_showcensus, &FieldActionWindow::act_show_census,
 	           _("Toggle building label display"));
+	if (is_a(InteractivePlayer, &ibase())) {
+		add_button(&watchbox, "workarea_overlap", pic_showworkareaoverlap,
+				   &FieldActionWindow::act_show_workarea_overlap,
+				   _("Toggle whether overlapping workareas are indicated when placing a constructionsite"));
+	}
 
 	if (ibase().get_display_flag(InteractiveBase::dfDebug))
 		add_button(
@@ -570,6 +592,12 @@
 	reset_mouse_and_die();
 }
 
+void FieldActionWindow::act_show_workarea_overlap() {
+	ibase().set_display_flag(InteractiveBase::dfShowWorkareaOverlap,
+	                         !ibase().get_display_flag(InteractiveBase::dfShowWorkareaOverlap));
+	reset_mouse_and_die();
+}
+
 /*
 ===============
 Show a debug widow for this field.
@@ -697,8 +725,8 @@
 void FieldActionWindow::building_icon_mouse_out(Widelands::DescriptionIndex) {
 	if (showing_workarea_preview_) {
 		ibase().hide_workarea(node_, false);
+		showing_workarea_preview_ = false;
 		clear_overlapping_workareas();
-		showing_workarea_preview_ = false;
 	}
 }
 
@@ -709,6 +737,9 @@
 		const WorkareaInfo& workarea_info = descr.workarea_info();
 		ibase().show_workarea(workarea_info, node_);
 		showing_workarea_preview_ = true;
+		if (!is_showing_workarea_overlaps_) {
+			return;
+		}
 
 		const Widelands::Map& map = ibase().egbase().map();
 		uint32_t workarea_radius = 0;

=== modified file 'src/wui/interactive_base.cc'
--- src/wui/interactive_base.cc	2019-05-26 17:21:15 +0000
+++ src/wui/interactive_base.cc	2019-05-29 18:00:02 +0000
@@ -383,7 +383,8 @@
 	const WorkareaInfo* workarea_info = workarea.info;
 	intermediate_result[coords] = 0;
 	WorkareaInfo::size_type wa_index;
-	switch (workarea_info->size()) {
+	const size_t workarea_size = workarea_info->size();
+	switch (workarea_size) {
 	case 0:
 		return WorkareasEntry();  // no workarea
 	case 1:
@@ -435,7 +436,7 @@
 					break;
 				}
 			}
-			result.push_back(wd);
+			result.first.push_back(wd);
 		}
 		if (rn != intermediate_result.end()) {
 			TCoords<> tc(pair.first, Widelands::TriangleIndex::R);
@@ -446,8 +447,40 @@
 					break;
 				}
 			}
-			result.push_back(wd);
-		}
+			result.first.push_back(wd);
+		}
+	}
+	for (const auto& pair : *workarea_info) {
+		std::vector<Coords> border;
+		Coords c = coords;
+		for (uint32_t i = pair.first; i > 0; --i) {
+			map.get_tln(c, &c);
+		}
+		for (uint32_t i = pair.first; i > 0; --i) {
+			border.push_back(c);
+			map.get_rn(c, &c);
+		}
+		for (uint32_t i = pair.first; i > 0; --i) {
+			border.push_back(c);
+			map.get_brn(c, &c);
+		}
+		for (uint32_t i = pair.first; i > 0; --i) {
+			border.push_back(c);
+			map.get_bln(c, &c);
+		}
+		for (uint32_t i = pair.first; i > 0; --i) {
+			border.push_back(c);
+			map.get_ln(c, &c);
+		}
+		for (uint32_t i = pair.first; i > 0; --i) {
+			border.push_back(c);
+			map.get_tln(c, &c);
+		}
+		for (uint32_t i = pair.first; i > 0; --i) {
+			border.push_back(c);
+			map.get_trn(c, &c);
+		}
+		result.second.push_back(border);
 	}
 	return result;
 }

=== modified file 'src/wui/interactive_base.h'
--- src/wui/interactive_base.h	2019-05-16 09:15:03 +0000
+++ src/wui/interactive_base.h	2019-05-29 18:00:02 +0000
@@ -62,6 +62,7 @@
 		dfShowCensus = 1,      ///< show census report on buildings
 		dfShowStatistics = 2,  ///< show statistics report on buildings
 		dfDebug = 4,           ///< general debugging info
+		dfShowWorkareaOverlap = 8, ///< highlight overlapping workareas when placing a constructionsite
 	};
 
 	// Overlays displayed while a road is under construction.

=== modified file 'src/wui/interactive_player.cc'
--- src/wui/interactive_player.cc	2019-05-04 10:47:44 +0000
+++ src/wui/interactive_player.cc	2019-05-29 18:00:02 +0000
@@ -172,6 +172,8 @@
 		new GameStatisticsMenu(*this, statisticsmenu_, main_windows_);
 	};
 
+	set_display_flag(InteractiveBase::dfShowWorkareaOverlap, true); // enable by default
+
 	toolbar()->add_space(15);
 
 	add_toolbar_button(
@@ -473,6 +475,10 @@
 				set_display_flag(dfShowStatistics, !get_display_flag(dfShowStatistics));
 			return true;
 
+		case SDLK_w:
+			set_display_flag(dfShowWorkareaOverlap, !get_display_flag(dfShowWorkareaOverlap));
+			return true;
+
 		case SDLK_KP_7:
 			if (code.mod & KMOD_NUM)
 				break;