widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

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

Requested reviews:
  Widelands Developers (widelands-dev)

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

Suggested commit message:

Cleaned up base/geometry to use templated classes and non specific integers -
this gets rid of some casting.

In general it is not advised to use sized and/or unsigned integer types unless
absolutely necessary. 
See http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Integer_Types.

-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/cleanup_geometry into lp:widelands.

=== modified file 'src/base/CMakeLists.txt'
--- src/base/CMakeLists.txt	2014-10-13 15:04:50 +0000
+++ src/base/CMakeLists.txt	2014-11-26 09:40:20 +0000
@@ -35,7 +35,6 @@
     point.h
     point.cc
     rect.h
-    rect.cc
 )
 
 wl_library(base_md5

=== modified file 'src/base/point.cc'
--- src/base/point.cc	2014-07-20 07:47:15 +0000
+++ src/base/point.cc	2014-11-26 09:40:20 +0000
@@ -19,47 +19,6 @@
 
 #include "base/point.h"
 
-#include <limits>
-
-Point::Point() : x(0), y(0) {}
-
-Point::Point(const int32_t px, const int32_t py) : x(px), y(py) {}
-
-Point Point::invalid() {
-	return Point(std::numeric_limits<int32_t>::max(), std::numeric_limits<int32_t>::max());
-}
-
-bool Point::operator == (const Point& other) const {
-	return x == other.x && y == other.y;
-}
-bool Point::operator != (const Point& other) const {
-	return !(*this == other);
-}
-
-Point Point::operator +(const Point& other) const {
-	return Point(x + other.x, y + other.y);
-}
-
-Point Point::operator -() const {
-	return Point(-x, -y);
-}
-
-Point Point::operator -(const Point& other) const {
-	return Point(x - other.x, y - other.y);
-}
-
-Point& Point::operator += (const Point& other) {
-	x += other.x;
-	y += other.y;
-	return *this;
-}
-
-Point& Point::operator -= (const Point& other) {
-	x -= other.x;
-	y -= other.y;
-	return *this;
-}
-
 Point middle(const Point& a, const Point& b) {
-	return Point((a.x + b.x) >> 1, (a.y + b.y) >> 1);
+	return Point((a.x + b.x) / 2, (a.y + b.y) / 2);
 }

=== modified file 'src/base/point.h'
--- src/base/point.h	2014-07-05 16:41:51 +0000
+++ src/base/point.h	2014-11-26 09:40:20 +0000
@@ -20,27 +20,57 @@
 #ifndef WL_BASE_POINT_H
 #define WL_BASE_POINT_H
 
+#include <limits>
+
 #include <stdint.h>
 
-struct Point {
-	// Initializes the Point to (0,0).
-	Point();
-	Point(int32_t px, int32_t py);
+template <typename T> struct GenericPoint {
+	GenericPoint(const T& px, const T& py) : x(px), y(py) {
+	}
+	GenericPoint() : GenericPoint(T(0), T(0)) {
+	}
 
 	// Returns an invalid point.
-	static Point invalid();
-
-	bool operator == (const Point& other) const;
-	bool operator != (const Point& other) const;
-	Point operator + (const Point& other) const;
-	Point operator - () const;
-	Point operator - (const Point& other) const;
-	Point& operator += (const Point& other);
-	Point& operator -= (const Point& other);
-
-	int32_t x, y;
+	static GenericPoint invalid() {
+		return GenericPoint(std::numeric_limits<T>::max(), std::numeric_limits<T>::max());
+	}
+
+	bool operator == (const GenericPoint& other) const {
+		return x == other.x && y == other.y;
+	}
+	bool operator != (const GenericPoint& other) const {
+		return !(*this == other);
+	}
+
+	GenericPoint operator + (const GenericPoint& other) const {
+		return GenericPoint(x + other.x, y + other.y);
+	}
+
+	GenericPoint operator - () const {
+		return GenericPoint(-x, -y);
+	}
+
+	GenericPoint operator - (const GenericPoint& other) const {
+		return GenericPoint(x - other.x, y - other.y);
+	}
+
+	GenericPoint& operator += (const GenericPoint& other) {
+		x += other.x;
+		y += other.y;
+		return *this;
+	}
+
+	GenericPoint& operator -= (const GenericPoint& other) {
+		x -= other.x;
+		y -= other.y;
+		return *this;
+	}
+
+	int x, y;
 };
 
+using Point = GenericPoint<int>;
+
 /// Returns the point in the middle between a and b (rounded to integer
 /// values).
 Point middle(const Point& a, const Point& b);

=== removed file 'src/base/rect.cc'
--- src/base/rect.cc	2014-06-25 05:42:44 +0000
+++ src/base/rect.cc	1970-01-01 00:00:00 +0000
@@ -1,42 +0,0 @@
-/*
- * Copyright (C) 2006-2014 by the Widelands Development Team
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version 2
- * of the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
- *
- */
-
-#include "base/rect.h"
-
-Rect::Rect() : x(0), y(0), w(0), h(0) {
-}
-
-Rect::Rect(int32_t gx, int32_t gy, uint32_t W, uint32_t H) : x(gx), y(gy), w(W), h(H) {
-}
-
-Rect::Rect(const Point& p, uint32_t W, uint32_t H) : Rect(p.x, p.y, W, H) {
-}
-
-Point Rect::top_left() const {
-	return Point(x, y);
-}
-
-Point Rect::bottom_right() const {
-	return top_left() + Point(w, h);
-}
-
-bool Rect::contains(const Point& pt) const {
-	return pt.x >= x && pt.x < x + static_cast<int32_t>(w) && pt.y >= y &&
-	       pt.y < y + static_cast<int32_t>(h);
-}

=== modified file 'src/base/rect.h'
--- src/base/rect.h	2014-11-02 15:02:30 +0000
+++ src/base/rect.h	2014-11-26 09:40:20 +0000
@@ -22,39 +22,40 @@
 
 #include "base/point.h"
 
-struct Rect {
+template <typename T>
+struct GenericRect {
 	/// Generates a degenerate Rect at (0, 0) with no height or width.
-	Rect();
-
-	Rect(int32_t x, int32_t y, uint32_t width, uint32_t height);
-	Rect(const Point& p, uint32_t width, uint32_t height);
+	GenericRect() : GenericRect(T(0), T(0), T(0), T(0)) {
+	}
+
+	GenericRect(const T& gx, const T& gy, const T& W, const T& H) : x(gx), y(gy), w(W), h(H) {
+	}
+
+	template <typename PointType>
+	GenericRect(const GenericPoint<PointType>& p, const T& width, const T& height)
+	   : GenericRect(T(p.x), T(p.y), width, height) {
+	}
 
 	/// The top left point of this rectangle.
-	Point top_left() const;
+	GenericPoint<T> top_left() const {
+		return GenericPoint<T>(x, y);
+	}
 
 	/// The bottom right point of this rectangle.
-	Point bottom_right() const;
+	GenericPoint<T> bottom_right() const {
+		return top_left() + Point(w, h);
+	}
 
 	/// Returns true if this rectangle contains the given point.
 	/// The bottom and right borders of the rectangle are considered to be excluded.
-	bool contains(const Point& pt) const;
-
-	int32_t x, y;
-	uint32_t w, h;
-};
-
-// TODO(sirver): Use a templated type for all kinds of rects.
-struct FloatRect {
-	FloatRect() = default;
-
-	FloatRect(float init_x, float init_y, float init_w, float init_h)
-	   : x(init_x), y(init_y), w(init_w), h(init_h) {
+	template <typename PointType> bool contains(const GenericPoint<PointType>& pt) const {
+		return T(pt.x) >= x && T(pt.x) < x + w && T(pt.y) >= y && T(pt.y) < y + h;
 	}
 
-	float x = 0.;
-	float y = 0.;
-	float w = 0.;
-	float h = 0.;
+	T x, y, w, h;
 };
 
+using Rect = GenericRect<int>;
+using FloatRect = GenericRect<float>;
+
 #endif  // end of include guard: WL_BASE_RECT_H

=== modified file 'src/graphic/image_transformations.cc'
--- src/graphic/image_transformations.cc	2014-11-24 07:12:35 +0000
+++ src/graphic/image_transformations.cc	2014-11-26 09:40:20 +0000
@@ -64,7 +64,7 @@
 	uint8_t * srcpix = texture.get_pixels() + srcpitch * srcrect.y + fmt.BytesPerPixel * srcrect.x;
 	uint8_t * dstpix = static_cast<uint8_t *>(dest->pixels);
 
-	for (uint32_t y = 0; y < srcrect.h; ++y) {
+	for (int y = 0; y < srcrect.h; ++y) {
 		memcpy(dstpix, srcpix, rowsize);
 		srcpix += srcpitch;
 		dstpix += dest->pitch;

=== modified file 'src/graphic/minimap_renderer.h'
--- src/graphic/minimap_renderer.h	2014-11-24 07:10:03 +0000
+++ src/graphic/minimap_renderer.h	2014-11-26 09:40:20 +0000
@@ -22,9 +22,10 @@
 
 #include <memory>
 
+#include "base/point.h"
+
 class StreamWrite;
 class Texture;
-struct Point;
 
 namespace Widelands {
 	class Player;

=== modified file 'src/graphic/rendertarget.cc'
--- src/graphic/rendertarget.cc	2014-11-24 07:25:21 +0000
+++ src/graphic/rendertarget.cc	2014-11-26 09:40:20 +0000
@@ -231,7 +231,7 @@
 			ofs.y += srch;
 
 		// Blit the image into the rectangle
-		uint32_t ty = 0;
+		int ty = 0;
 
 		while (ty < r.h) {
 			uint32_t tx = 0;
@@ -337,7 +337,7 @@
 	r.y += m_offset.y;
 
 	if (r.x < 0) {
-		if (r.w <= static_cast<uint32_t>(-r.x))
+		if (r.w <= -r.x)
 			return false;
 
 		r.w += r.x;
@@ -346,20 +346,20 @@
 	}
 
 	if (r.x + r.w > m_rect.w) {
-		if (static_cast<int32_t>(m_rect.w) <= r.x)
+		if (m_rect.w <= r.x)
 			return false;
 		r.w = m_rect.w - r.x;
 	}
 
 	if (r.y < 0) {
-		if (r.h <= static_cast<uint32_t>(-r.y))
+		if (r.h <= -r.y)
 			return false;
 		r.h += r.y;
 		r.y = 0;
 	}
 
 	if (r.y + r.h > m_rect.h) {
-		if (static_cast<int32_t>(m_rect.h) <= r.y)
+		if (m_rect.h <= r.y)
 			return false;
 		r.h = m_rect.h - r.y;
 	}
@@ -382,7 +382,7 @@
 
 	// Clipping
 	if (dst->x < 0) {
-		if (srcrc->w <= static_cast<uint32_t>(-dst->x))
+		if (srcrc->w <= -dst->x)
 			return false;
 		srcrc->x -= dst->x;
 		srcrc->w += dst->x;
@@ -390,13 +390,13 @@
 	}
 
 	if (dst->x + srcrc->w > m_rect.w) {
-		if (static_cast<int32_t>(m_rect.w) <= dst->x)
+		if (m_rect.w <= dst->x)
 			return false;
 		srcrc->w = m_rect.w - dst->x;
 	}
 
 	if (dst->y < 0) {
-		if (srcrc->h <= static_cast<uint32_t>(-dst->y))
+		if (srcrc->h <= -dst->y)
 			return false;
 		srcrc->y -= dst->y;
 		srcrc->h += dst->y;
@@ -404,7 +404,7 @@
 	}
 
 	if (dst->y + srcrc->h > m_rect.h) {
-		if (static_cast<int32_t>(m_rect.h) <= dst->y)
+		if (m_rect.h <= dst->y)
 			return false;
 		srcrc->h = m_rect.h - dst->y;
 	}

=== modified file 'src/graphic/richtext.cc'
--- src/graphic/richtext.cc	2014-09-10 14:48:40 +0000
+++ src/graphic/richtext.cc	2014-11-26 09:40:20 +0000
@@ -347,7 +347,7 @@
 			bbox.w = image->width();
 			bbox.h = image->height();
 
-			text.images_height = std::max(text.images_height, bbox.h);
+			text.images_height = std::max<int>(text.images_height, bbox.h);
 			text.images_width += bbox.w;
 
 			m->elements.push_back(new ImageElement(bbox, image));