widelands

Merge lp:~widelands-dev/widelands/avoid_wraparound into lp:widelands

  1. avoid_wraparound
  2. Merge into trunk
Proposed by SirVer
Status: Merged
Merged at revision: 7749
Proposed branch: lp:~widelands-dev/widelands/avoid_wraparound
Merge into: lp:widelands
Diff against target: 489 lines (+81/-88)
12 files modified
src/graphic/rendertarget.cc (+2/-2)
src/logic/editor_game_base.cc (+2/-3)
src/logic/map_objects/tribes/ship.h (+0/-1)
src/ui_basic/box.cc (+14/-14)
src/ui_basic/box.h (+7/-7)
src/ui_basic/panel.cc (+14/-14)
src/ui_basic/panel.h (+18/-18)
src/ui_basic/table.cc (+3/-3)
src/ui_basic/tabpanel.cc (+4/-7)
src/ui_basic/textarea.cc (+2/-2)
src/ui_basic/window.cc (+13/-15)
src/wui/ware_statistics_menu.cc (+2/-2)
To merge this branch: bzr merge lp:~widelands-dev/widelands/avoid_wraparound
Reviewer Review Type Date Requested Status
kaputtnik (community) Approve
GunChleoc Approve
Review via email: mp+284067@code.launchpad.net

Commit message

Switched overzealous uint32_t -> int to avoid underflow errors on minus arithmetic.

Description of the change

Fixes stripes of the port window.

The problem is that we do unsigned d = (unsized(0) - unsized(50)) / 2, which is is a huge number.

later we compare

2 *(d + ~50) < 1000

which is true, because we wrap around the other way again.

The solution is to never use unsigned integers unless absolutely needed. I changed a lot of pixel size to be int instead of uint32_t.

To post a comment you must log in.
Revision history for this message
bunnybot (widelandsofficial) wrote :

Hi, I am bunnybot (https://github.com/widelands/bunnybot).

I am keeping the source branch lp:~widelands-dev/widelands/avoid_wraparound mirrored to https://github.com/widelands/widelands/tree/_widelands_dev_widelands_avoid_wraparound

You can give me commands by starting a line with @bunnybot <command>. I understand:
 merge: Merges the source branch into the target branch, closing the merge proposal. I will use the proposed commit message if it is set.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Code LGTM, not tested yet.

review: Approve
Revision history for this message
kaputtnik (franku) wrote :

No vertical stripes anymore. I tested also some other windows, no failure.

@bunnybot merge

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/graphic/rendertarget.cc'
2--- src/graphic/rendertarget.cc 2016-01-23 14:54:44 +0000
3+++ src/graphic/rendertarget.cc 2016-01-27 08:11:43 +0000
4@@ -329,10 +329,10 @@
5 Rect destination_rect(dst.x - animation.hotspot().x + source_rect.x,
6 dst.y - animation.hotspot().y + source_rect.y, source_rect.w,
7 source_rect.h);
8-
9 Rect srcrc(source_rect);
10- if (to_surface_geometry(&destination_rect, &srcrc))
11+ if (to_surface_geometry(&destination_rect, &srcrc)) {
12 animation.blit(time, destination_rect.origin(), srcrc, player_color, m_surface);
13+ }
14
15 // Look if there is a sound effect registered for this frame and trigger the
16 // effect (see SoundHandler::stereo_position).
17
18=== modified file 'src/logic/editor_game_base.cc'
19--- src/logic/editor_game_base.cc 2016-01-17 09:54:31 +0000
20+++ src/logic/editor_game_base.cc 2016-01-27 08:11:43 +0000
21@@ -634,9 +634,8 @@
22 assert (player_area.x < map().get_width());
23 assert(0 <= player_area.y);
24 assert (player_area.y < map().get_height());
25- const Field & first_field = map()[0];
26- assert(&first_field <= player_area.field);
27- assert(player_area.field < &first_field + map().max_index());
28+ assert(&map()[0] <= player_area.field);
29+ assert(player_area.field < &map()[0] + map().max_index());
30 assert(0 < player_area.player_number);
31 assert (player_area.player_number <= map().get_nrplayers());
32 MapRegion<Area<FCoords> > mr(map(), player_area);
33
34=== modified file 'src/logic/map_objects/tribes/ship.h'
35--- src/logic/map_objects/tribes/ship.h 2016-01-18 19:08:41 +0000
36+++ src/logic/map_objects/tribes/ship.h 2016-01-27 08:11:43 +0000
37@@ -280,7 +280,6 @@
38 uint32_t m_destination;
39 uint8_t m_ship_state;
40 std::string m_shipname;
41- uint32_t m_shipname_index;
42 std::unique_ptr<Expedition> m_expedition;
43 std::vector<ShippingItem::Loader> m_items;
44 };
45
46=== modified file 'src/ui_basic/box.cc'
47--- src/ui_basic/box.cc 2016-01-17 08:29:59 +0000
48+++ src/ui_basic/box.cc 2016-01-27 08:11:43 +0000
49@@ -88,12 +88,12 @@
50 */
51 void Box::update_desired_size()
52 {
53- uint32_t totaldepth = 0;
54- uint32_t maxbreadth = m_mindesiredbreadth;
55+ int totaldepth = 0;
56+ int maxbreadth = m_mindesiredbreadth;
57
58 for (uint32_t idx = 0; idx < m_items.size(); ++idx) {
59- uint32_t depth, breadth;
60- get_item_desired_size(idx, depth, breadth);
61+ int depth, breadth;
62+ get_item_desired_size(idx, &depth, &breadth);
63
64 totaldepth += depth;
65 if (breadth > maxbreadth)
66@@ -131,11 +131,11 @@
67 void Box::layout()
68 {
69 // First pass: compute the depth and adjust whether we have a scrollbar
70- uint32_t totaldepth = 0;
71+ int totaldepth = 0;
72
73- for (uint32_t idx = 0; idx < m_items.size(); ++idx) {
74- uint32_t depth, tmp;
75- get_item_desired_size(idx, depth, tmp);
76+ for (size_t idx = 0; idx < m_items.size(); ++idx) {
77+ int depth, tmp;
78+ get_item_desired_size(idx, &depth, &tmp);
79
80 totaldepth += depth;
81 }
82@@ -223,8 +223,8 @@
83 totalbreadth -= Scrollbar::Size;
84
85 for (uint32_t idx = 0; idx < m_items.size(); ++idx) {
86- uint32_t depth, breadth;
87- get_item_size(idx, depth, breadth);
88+ int depth, breadth;
89+ get_item_size(idx, &depth, &breadth);
90
91 if (m_items[idx].type == Item::ItemPanel) {
92 set_item_size
93@@ -317,7 +317,7 @@
94 * to the orientation axis.
95 */
96 void Box::get_item_desired_size
97- (uint32_t const idx, uint32_t & depth, uint32_t & breadth)
98+ (uint32_t const idx, int* depth, int* breadth)
99 {
100 assert(idx < m_items.size());
101
102@@ -333,7 +333,7 @@
103 break;
104
105 case Item::ItemSpace:
106- depth = it.u.space;
107+ *depth = it.u.space;
108 breadth = 0;
109 break;
110 }
111@@ -344,7 +344,7 @@
112 * for expanding items, at least for now.
113 */
114 void Box::get_item_size
115- (uint32_t const idx, uint32_t & depth, uint32_t & breadth)
116+ (uint32_t const idx, int* depth, int* breadth)
117 {
118 assert(idx < m_items.size());
119
120@@ -357,7 +357,7 @@
121 /**
122 * Set the given items actual size.
123 */
124-void Box::set_item_size(uint32_t idx, uint32_t depth, uint32_t breadth)
125+void Box::set_item_size(uint32_t idx, int depth, int breadth)
126 {
127 assert(idx < m_items.size());
128
129
130=== modified file 'src/ui_basic/box.h'
131--- src/ui_basic/box.h 2014-07-26 10:43:23 +0000
132+++ src/ui_basic/box.h 2016-01-27 08:11:43 +0000
133@@ -71,15 +71,15 @@
134 void update_desired_size() override;
135
136 private:
137- void get_item_desired_size(uint32_t idx, uint32_t & depth, uint32_t & breadth);
138- void get_item_size(uint32_t idx, uint32_t & depth, uint32_t & breadth);
139- void set_item_size(uint32_t idx, uint32_t depth, uint32_t breadth);
140+ void get_item_desired_size(uint32_t idx, int* depth, int* breadth);
141+ void get_item_size(uint32_t idx, int* depth, int* breadth);
142+ void set_item_size(uint32_t idx, int depth, int breadth);
143 void set_item_pos(uint32_t idx, int32_t pos);
144 void scrollbar_moved(int32_t);
145 void update_positions();
146
147 //don't resize beyond this size
148- uint32_t m_max_x, m_max_y;
149+ int m_max_x, m_max_y;
150
151 private:
152 struct Item {
153@@ -93,14 +93,14 @@
154 union {
155 struct {
156 Panel * panel;
157- uint32_t align;
158+ int align;
159 bool fullsize;
160 } panel;
161- uint32_t space;
162+ int space;
163 } u;
164
165 bool fillspace;
166- uint32_t assigned_var_depth;
167+ int assigned_var_depth;
168 };
169
170 bool m_scrolling;
171
172=== modified file 'src/ui_basic/panel.cc'
173--- src/ui_basic/panel.cc 2015-11-28 11:04:12 +0000
174+++ src/ui_basic/panel.cc 2016-01-27 08:11:43 +0000
175@@ -50,7 +50,7 @@
176 */
177 Panel::Panel
178 (Panel * const nparent,
179- const int32_t nx, const int32_t ny, const uint32_t nw, const uint32_t nh,
180+ const int nx, const int ny, const int nw, const int nh,
181 const std::string & tooltip_text)
182 :
183 _parent(nparent), _fchild(nullptr), _lchild(nullptr), _mousein(nullptr), _focus(nullptr),
184@@ -154,7 +154,7 @@
185
186 uint32_t minTime;
187 {
188- int32_t maxfps = g_options.pull_section("global").get_int("maxfps", 25);
189+ int maxfps = g_options.pull_section("global").get_int("maxfps", 25);
190 if (maxfps < 5)
191 maxfps = 5;
192 minTime = 1000 / maxfps;
193@@ -237,13 +237,13 @@
194 * \note NEVER override this function. If you feel the urge to override this
195 * function, you probably want to override \ref layout.
196 */
197-void Panel::set_size(const uint32_t nw, const uint32_t nh)
198+void Panel::set_size(const int nw, const int nh)
199 {
200 if (nw == _w && nh == _h)
201 return;
202
203- uint32_t const upw = std::min(nw, _w);
204- uint32_t const uph = std::min(nh, _h);
205+ int const upw = std::min(nw, _w);
206+ int const uph = std::min(nh, _h);
207 _w = nw;
208 _h = nh;
209
210@@ -269,10 +269,10 @@
211 * Set \p w and \p h to the desired
212 * width and height of this panel, respectively.
213 */
214-void Panel::get_desired_size(uint32_t & w, uint32_t & h) const
215+void Panel::get_desired_size(int* w, int* h) const
216 {
217- w = _desired_w;
218- h = _desired_h;
219+ *w = _desired_w;
220+ *h = _desired_h;
221 }
222
223 /**
224@@ -285,7 +285,7 @@
225 *
226 * \note NEVER override this function
227 */
228-void Panel::set_desired_size(uint32_t w, uint32_t h)
229+void Panel::set_desired_size(int w, int h)
230 {
231 if (_desired_w == w && _desired_h == h)
232 return;
233@@ -367,7 +367,7 @@
234 /**
235 * Set the size of the inner area (total area minus border)
236 */
237-void Panel::set_inner_size(uint32_t const nw, uint32_t const nh)
238+void Panel::set_inner_size(int const nw, int const nh)
239 {
240 set_size(nw + _lborder + _rborder, nh + _tborder + _bborder);
241 }
242@@ -377,7 +377,7 @@
243 * Note that since position and total size aren't changed, so that the size
244 * and position of the inner area will change.
245 */
246-void Panel::set_border(uint32_t l, uint32_t r, uint32_t t, uint32_t b)
247+void Panel::set_border(int l, int r, int t, int b)
248 {
249 _lborder = l;
250 _rborder = r;
251@@ -451,12 +451,12 @@
252 /**
253 * Mark a part of a panel for updating.
254 */
255-void Panel::update(int32_t x, int32_t y, int32_t w, int32_t h)
256+void Panel::update(int x, int y, int w, int h)
257 {
258 if
259- (x >= static_cast<int32_t>(_w) || x + w <= 0
260+ (x >= _w || x + w <= 0
261 ||
262- y >= static_cast<int32_t>(_h) || y + h <= 0)
263+ y >= _h || y + h <= 0)
264 return;
265
266 if (_parent) {
267
268=== modified file 'src/ui_basic/panel.h'
269--- src/ui_basic/panel.h 2016-01-15 21:46:03 +0000
270+++ src/ui_basic/panel.h 2016-01-27 08:11:43 +0000
271@@ -79,7 +79,7 @@
272 Panel
273 (Panel * const nparent,
274 int32_t const nx, int32_t const ny,
275- uint32_t const nw, uint32_t const nh,
276+ int const nw, int const nh,
277 const std::string& tooltip_text = std::string());
278 virtual ~Panel();
279
280@@ -114,8 +114,8 @@
281 virtual void end();
282
283 // Geometry
284- void set_size(uint32_t nw, uint32_t nh);
285- void set_desired_size(uint32_t w, uint32_t h);
286+ void set_size(int nw, int nh);
287+ void set_desired_size(int w, int h);
288 void set_pos(Point);
289 virtual void move_inside_parent();
290 virtual void layout();
291@@ -123,7 +123,7 @@
292 void set_layout_toplevel(bool ltl);
293 bool get_layout_toplevel() const;
294
295- void get_desired_size(uint32_t & w, uint32_t & h) const;
296+ void get_desired_size(int* w, int* h) const;
297
298 int32_t get_x() const {return _x;}
299 int32_t get_y() const {return _y;}
300@@ -151,19 +151,19 @@
301 return _flags & pf_dock_windows_to_edges;
302 }
303 void set_dock_windows_to_edges(const bool on = true);
304- void set_inner_size(uint32_t nw, uint32_t nh);
305- void set_border(uint32_t l, uint32_t r, uint32_t t, uint32_t b);
306-
307- uint32_t get_lborder() const {return _lborder;}
308- uint32_t get_rborder() const {return _rborder;}
309- uint32_t get_tborder() const {return _tborder;}
310- uint32_t get_bborder() const {return _bborder;}
311-
312- uint32_t get_inner_w() const {
313+ void set_inner_size(int nw, int nh);
314+ void set_border(int l, int r, int t, int b);
315+
316+ int get_lborder() const {return _lborder;}
317+ int get_rborder() const {return _rborder;}
318+ int get_tborder() const {return _tborder;}
319+ int get_bborder() const {return _bborder;}
320+
321+ int get_inner_w() const {
322 assert(_lborder + _rborder <= _w);
323 return _w - (_lborder + _rborder);
324 }
325- uint32_t get_inner_h() const {
326+ int get_inner_h() const {
327 assert(_tborder + _bborder <= _h);
328 return _h - (_tborder + _bborder);
329 }
330@@ -321,11 +321,11 @@
331 */
332 /*@{*/
333 int32_t _x, _y;
334- uint32_t _w, _h;
335+ int _w, _h;
336 /*@}*/
337- uint32_t _lborder, _rborder, _tborder, _bborder;
338+ int _lborder, _rborder, _tborder, _bborder;
339 uint8_t _border_snap_distance, _panel_snap_distance;
340- uint32_t _desired_w, _desired_h;
341+ int _desired_w, _desired_h;
342
343 bool _running;
344 int _retcode;
345@@ -359,7 +359,7 @@
346 NamedPanel
347 (Panel * const nparent, const std::string & name,
348 int32_t const nx, int32_t const ny,
349- uint32_t const nw, uint32_t const nh,
350+ int const nw, int const nh,
351 const std::string & tooltip_text = std::string())
352 : Panel(nparent, nx, ny, nw, nh, tooltip_text), m_name(name)
353 {
354
355=== modified file 'src/ui_basic/table.cc'
356--- src/ui_basic/table.cc 2016-01-26 09:10:52 +0000
357+++ src/ui_basic/table.cc 2016-01-27 08:11:43 +0000
358@@ -248,9 +248,9 @@
359 if (entries == 0) {
360 entries = size();
361 }
362- uint32_t tablewidth;
363- uint32_t tableheight;
364- get_desired_size(tablewidth, tableheight);
365+ int tablewidth;
366+ int tableheight;
367+ get_desired_size(&tablewidth, &tableheight);
368 tableheight = m_headerheight + 2 + get_lineheight() * entries;
369 set_desired_size(tablewidth, tableheight);
370 }
371
372=== modified file 'src/ui_basic/tabpanel.cc'
373--- src/ui_basic/tabpanel.cc 2016-01-01 19:04:45 +0000
374+++ src/ui_basic/tabpanel.cc 2016-01-27 08:11:43 +0000
375@@ -139,19 +139,16 @@
376 */
377 void TabPanel::update_desired_size()
378 {
379- uint32_t w;
380- uint32_t h;
381-
382 // size of button row
383- w = kTabPanelButtonHeight * tabs_.size();
384- h = kTabPanelButtonHeight + kTabPanelSeparatorHeight;
385+ int w = kTabPanelButtonHeight * tabs_.size();
386+ int h = kTabPanelButtonHeight + kTabPanelSeparatorHeight;
387
388 // size of contents
389 if (active_ < tabs_.size()) {
390 Panel * const panel = tabs_[active_]->panel;
391- uint32_t panelw, panelh;
392+ int panelw, panelh;
393
394- panel->get_desired_size(panelw, panelh);
395+ panel->get_desired_size(&panelw, &panelh);
396 // TODO(unknown): the panel might be bigger -> add a scrollbar in that case
397 //panel->set_size(panelw, panelh);
398
399
400=== modified file 'src/ui_basic/textarea.cc'
401--- src/ui_basic/textarea.cc 2015-12-11 19:03:45 +0000
402+++ src/ui_basic/textarea.cc 2016-01-27 08:11:43 +0000
403@@ -206,8 +206,8 @@
404 int32_t y = get_y();
405
406 update_desired_size();
407- uint32_t w, h;
408- get_desired_size(w, h);
409+ int w, h;
410+ get_desired_size(&w, &h);
411
412 if (m_align & Align_HCenter)
413 x -= w >> 1;
414
415=== modified file 'src/ui_basic/window.cc'
416--- src/ui_basic/window.cc 2015-08-06 17:14:34 +0000
417+++ src/ui_basic/window.cc 2016-01-27 08:11:43 +0000
418@@ -133,8 +133,8 @@
419 void Window::update_desired_size()
420 {
421 if (m_center_panel) {
422- uint32_t innerw, innerh;
423- m_center_panel->get_desired_size(innerw, innerh);
424+ int innerw, innerh;
425+ m_center_panel->get_desired_size(&innerw, &innerh);
426 set_desired_size
427 (innerw + get_lborder() + get_rborder(),
428 innerh + get_tborder() + get_bborder());
429@@ -201,22 +201,20 @@
430 if (Panel * const parent = get_parent()) {
431 int32_t px = get_x();
432 int32_t py = get_y();
433- if
434- ((parent->get_inner_w() < static_cast<uint32_t>(get_w())) &&
435- (px + get_w() <= static_cast<int32_t>(parent->get_inner_w()) || px >= 0))
436- px = (static_cast<int32_t>(parent->get_inner_w()) - get_w()) / 2;
437- if
438- ((parent->get_inner_h() < static_cast<uint32_t>(get_h())) &&
439- (py + get_h() < static_cast<int32_t>(parent->get_inner_h()) || py > 0))
440+ if ((parent->get_inner_w() < get_w()) && (px + get_w() <= parent->get_inner_w() || px >= 0))
441+ px = (parent->get_inner_w() - get_w()) / 2;
442+ if
443+ ((parent->get_inner_h() < get_h()) &&
444+ (py + get_h() < parent->get_inner_h() || py > 0))
445 py = 0;
446
447- if (parent->get_inner_w() >= static_cast<uint32_t>(get_w())) {
448+ if (parent->get_inner_w() >= get_w()) {
449 if (px < 0) {
450 px = 0;
451 if (parent->get_dock_windows_to_edges() && !_docked_left)
452 _docked_left = true;
453- } else if (px + static_cast<uint32_t>(get_w()) >= parent->get_inner_w()) {
454- px = static_cast<int32_t>(parent->get_inner_w()) - get_w();
455+ } else if (px + get_w() >= parent->get_inner_w()) {
456+ px = parent->get_inner_w() - get_w();
457 if (parent->get_dock_windows_to_edges() && !_docked_right)
458 _docked_right = true;
459 }
460@@ -225,11 +223,11 @@
461 else if (_docked_right)
462 px += VT_B_PIXMAP_THICKNESS;
463 }
464- if (parent->get_inner_h() >= static_cast<uint32_t>(get_h())) {
465+ if (parent->get_inner_h() >= get_h()) {
466 if (py < 0)
467 py = 0;
468- else if (py + static_cast<uint32_t>(get_h()) > parent->get_inner_h()) {
469- py = static_cast<int32_t>(parent->get_inner_h()) - get_h();
470+ else if (py + get_h() > parent->get_inner_h()) {
471+ py = parent->get_inner_h() - get_h();
472 if
473 (!_is_minimal
474 &&
475
476=== modified file 'src/wui/ware_statistics_menu.cc'
477--- src/wui/ware_statistics_menu.cc 2016-01-24 20:11:53 +0000
478+++ src/wui/ware_statistics_menu.cc 2016-01-27 08:11:43 +0000
479@@ -115,8 +115,8 @@
480 AbstractWaresDisplay(parent, x, y, tribe, Widelands::wwWARE, true, callback_function),
481 color_map_(color_map)
482 {
483- uint32_t w, h;
484- get_desired_size(w, h);
485+ int w, h;
486+ get_desired_size(&w, &h);
487 set_size(w, h);
488 }
489 protected:

Subscribers

People subscribed via source and target branches

to status/vote changes: