Clean up numeric.h and split FacePositionCache from it

I also optiized FacePositionCache a bit: I removed a map lookup and vector copy from both branches of getFacePosition.
minetest · May 6, 2017 · 77597c4 · 77597c4
1 parent b6f4a9c
commit 77597c4
Show file tree

Hide file tree

Showing 17 changed files with 205 additions and 157 deletions.
diff --git a/build/android/jni/Android.mk b/build/android/jni/Android.mk
@@ -143,6 +143,7 @@ LOCAL_SRC_FILES := \
 		jni/src/dungeongen.cpp                    \
 		jni/src/emerge.cpp                        \
 		jni/src/environment.cpp                   \
+		jni/src/face_position_cache.cpp           \
 		jni/src/filecache.cpp                     \
 		jni/src/filesys.cpp                       \
 		jni/src/fontengine.cpp                    \

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -388,6 +388,7 @@ set(common_SRCS
 	dungeongen.cpp
 	emerge.cpp
 	environment.cpp
+	face_position_cache.cpp
 	filesys.cpp
 	genericobject.cpp
 	gettext.cpp

diff --git a/src/clientenvironment.cpp b/src/clientenvironment.cpp
@@ -30,6 +30,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "raycast.h"
 #include "voxelalgorithms.h"
 #include "settings.h"
+#include <algorithm>
 
 /*
 	ClientEnvironment

diff --git a/src/clientiface.cpp b/src/clientiface.cpp
@@ -20,7 +20,6 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include <sstream>
 
 #include "clientiface.h"
-#include "util/numeric.h"
 #include "remoteplayer.h"
 #include "settings.h"
 #include "mapblock.h"
@@ -32,6 +31,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "log.h"
 #include "network/serveropcodes.h"
 #include "util/srp.h"
+#include "face_position_cache.h"
 
 const char *ClientInterface::statenames[] = {
 	"Invalid",

diff --git a/src/content_cao.cpp b/src/content_cao.cpp
@@ -44,6 +44,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "camera.h" // CameraModes
 #include "wieldmesh.h"
 #include "log.h"
+#include <algorithm>
 
 class Settings;
 struct ToolCapabilities;

diff --git a/src/face_position_cache.cpp b/src/face_position_cache.cpp
@@ -0,0 +1,110 @@
+/*
+Minetest
+Copyright (C) 2015 Nerzhul, Loic Blot <loic.blot@unix-experience.fr>
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU Lesser General Public License as published by
+the Free Software Foundation; either version 2.1 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 Lesser General Public License for more details.
+
+You should have received a copy of the GNU Lesser General Public License along
+with this program; if not, write to the Free Software Foundation, Inc.,
+51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+*/
+
+#include "face_position_cache.h"
+#include "threading/mutex_auto_lock.h"
+
+
+UNORDERED_MAP<u16, std::vector<v3s16> > FacePositionCache::cache;
+Mutex FacePositionCache::cache_mutex;
+
+// Calculate the borders of a "d-radius" cube
+const std::vector<v3s16> &FacePositionCache::getFacePositions(u16 d)
+{
+	MutexAutoLock lock(cache_mutex);
+	UNORDERED_MAP<u16, std::vector<v3s16> >::iterator it = cache.find(d);
+	if (it != cache.end())
+		return it->second;
+
+	return generateFacePosition(d);
+}
+
+const std::vector<v3s16> &FacePositionCache::generateFacePosition(u16 d)
+{
+	cache[d] = std::vector<v3s16>();
+	std::vector<v3s16> &c = cache[d];
+	if (d == 0) {
+		c.push_back(v3s16(0,0,0));
+		return c;
+	}
+	if (d == 1) {
+		// This is an optimized sequence of coordinates.
+		c.push_back(v3s16( 0, 1, 0)); // Top
+		c.push_back(v3s16( 0, 0, 1)); // Back
+		c.push_back(v3s16(-1, 0, 0)); // Left
+		c.push_back(v3s16( 1, 0, 0)); // Right
+		c.push_back(v3s16( 0, 0,-1)); // Front
+		c.push_back(v3s16( 0,-1, 0)); // Bottom
+		// 6
+		c.push_back(v3s16(-1, 0, 1)); // Back left
+		c.push_back(v3s16( 1, 0, 1)); // Back right
+		c.push_back(v3s16(-1, 0,-1)); // Front left
+		c.push_back(v3s16( 1, 0,-1)); // Front right
+		c.push_back(v3s16(-1,-1, 0)); // Bottom left
+		c.push_back(v3s16( 1,-1, 0)); // Bottom right
+		c.push_back(v3s16( 0,-1, 1)); // Bottom back
+		c.push_back(v3s16( 0,-1,-1)); // Bottom front
+		c.push_back(v3s16(-1, 1, 0)); // Top left
+		c.push_back(v3s16( 1, 1, 0)); // Top right
+		c.push_back(v3s16( 0, 1, 1)); // Top back
+		c.push_back(v3s16( 0, 1,-1)); // Top front
+		// 18
+		c.push_back(v3s16(-1, 1, 1)); // Top back-left
+		c.push_back(v3s16( 1, 1, 1)); // Top back-right
+		c.push_back(v3s16(-1, 1,-1)); // Top front-left
+		c.push_back(v3s16( 1, 1,-1)); // Top front-right
+		c.push_back(v3s16(-1,-1, 1)); // Bottom back-left
+		c.push_back(v3s16( 1,-1, 1)); // Bottom back-right
+		c.push_back(v3s16(-1,-1,-1)); // Bottom front-left
+		c.push_back(v3s16( 1,-1,-1)); // Bottom front-right
+		// 26
+		return c;
+	}
+
+	// Take blocks in all sides, starting from y=0 and going +-y
+	for (s16 y = 0; y <= d - 1; y++) {
+		// Left and right side, including borders
+		for (s16 z =- d; z <= d; z++) {
+			c.push_back(v3s16( d, y, z));
+			c.push_back(v3s16(-d, y, z));
+			if (y != 0) {
+				c.push_back(v3s16( d, -y, z));
+				c.push_back(v3s16(-d, -y, z));
+			}
+		}
+		// Back and front side, excluding borders
+		for (s16 x = -d + 1; x <= d - 1; x++) {
+			c.push_back(v3s16(x, y, d));
+			c.push_back(v3s16(x, y, -d));
+			if (y != 0) {
+				c.push_back(v3s16(x, -y, d));
+				c.push_back(v3s16(x, -y, -d));
+			}
+		}
+	}
+
+	// Take the bottom and top face with borders
+	// -d < x < d, y = +-d, -d < z < d
+	for (s16 x = -d; x <= d; x++)
+	for (s16 z = -d; z <= d; z++) {
+		c.push_back(v3s16(x, -d, z));
+		c.push_back(v3s16(x, d, z));
+	}
+	return c;
+}
diff --git a/src/face_position_cache.h b/src/face_position_cache.h
@@ -0,0 +1,44 @@
+/*
+Minetest
+Copyright (C) 2015 Nerzhul, Loic Blot <loic.blot@unix-experience.fr>
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU Lesser General Public License as published by
+the Free Software Foundation; either version 2.1 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 Lesser General Public License for more details.
+
+You should have received a copy of the GNU Lesser General Public License along
+with this program; if not, write to the Free Software Foundation, Inc.,
+51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+*/
+
+#ifndef FACE_POSITION_CACHE_HEADER
+#define FACE_POSITION_CACHE_HEADER
+
+#include "irr_v3d.h"
+#include "threading/mutex.h"
+#include "util/cpp11_container.h"
+
+#include <map>
+#include <vector>
+
+/*
+ * This class permits caching getFacePosition call results.
+ * This reduces CPU usage and vector calls.
+ */
+class FacePositionCache {
+public:
+	static const std::vector<v3s16> &getFacePositions(u16 d);
+
+private:
+	static const std::vector<v3s16> &generateFacePosition(u16 d);
+	static UNORDERED_MAP<u16, std::vector<v3s16> > cache;
+	static Mutex cache_mutex;
+};
+
+#endif
diff --git a/src/mg_decoration.cpp b/src/mg_decoration.cpp
@@ -24,6 +24,8 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "map.h"
 #include "log.h"
 #include "util/numeric.h"
+#include <algorithm>
+
 
 FlagDesc flagdesc_deco[] = {
 	{"place_center_x",  DECO_PLACE_CENTER_X},

diff --git a/src/mg_ore.cpp b/src/mg_ore.cpp
@@ -20,9 +20,10 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "mg_ore.h"
 #include "mapgen.h"
 #include "noise.h"
-#include "util/numeric.h"
 #include "map.h"
 #include "log.h"
+#include <algorithm>
+
 
 FlagDesc flagdesc_ore[] = {
 	{"absheight",                 OREFLAG_ABSHEIGHT},

diff --git a/src/script/lua_api/l_env.cpp b/src/script/lua_api/l_env.cpp
@@ -35,6 +35,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "treegen.h"
 #include "emerge.h"
 #include "pathfinder.h"
+#include "face_position_cache.h"
 
 struct EnumString ModApiEnvMod::es_ClearObjectsMode[] =
 {

diff --git a/src/script/lua_api/l_server.cpp b/src/script/lua_api/l_server.cpp
@@ -26,6 +26,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "environment.h"
 #include "player.h"
 #include "log.h"
+#include <algorithm>
 
 // request_shutdown()
 int ModApiServer::l_request_shutdown(lua_State *L)

diff --git a/src/unittest/test_noderesolver.cpp b/src/unittest/test_noderesolver.cpp
@@ -24,6 +24,8 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "gamedef.h"
 #include "nodedef.h"
 
+#include <algorithm>
+
 
 class TestNodeResolver : public TestBase {
 public:

diff --git a/src/util/basic_macros.h b/src/util/basic_macros.h
@@ -20,14 +20,13 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #ifndef BASICMACROS_HEADER
 #define BASICMACROS_HEADER
 
-#include <algorithm>
-
 #define ARRLEN(x) (sizeof(x) / sizeof((x)[0]))
 
 #define MYMIN(a, b) ((a) < (b) ? (a) : (b))
 
 #define MYMAX(a, b) ((a) > (b) ? (a) : (b))
 
+// Requires <algorithm>
 #define CONTAINS(c, v) (std::find((c).begin(), (c).end(), (v)) != (c).end())
 
 // To disable copy constructors and assignment operations for some class

diff --git a/src/util/numeric.cpp b/src/util/numeric.cpp
@@ -24,100 +24,9 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "../noise.h" // PseudoRandom, PcgRandom
 #include "../threading/mutex_auto_lock.h"
 #include <string.h>
-#include <iostream>
 
-UNORDERED_MAP<u16, std::vector<v3s16> > FacePositionCache::m_cache;
-Mutex FacePositionCache::m_cache_mutex;
-// Calculate the borders of a "d-radius" cube
-// TODO: Make it work without mutex and data races, probably thread-local
-std::vector<v3s16> FacePositionCache::getFacePositions(u16 d)
-{
-	MutexAutoLock cachelock(m_cache_mutex);
-	if (m_cache.find(d) != m_cache.end())
-		return m_cache[d];
-
-	generateFacePosition(d);
-	return m_cache[d];
-
-}
-
-void FacePositionCache::generateFacePosition(u16 d)
-{
-	m_cache[d] = std::vector<v3s16>();
-	if(d == 0) {
-		m_cache[d].push_back(v3s16(0,0,0));
-		return;
-	}
-	if(d == 1) {
-		/*
-			This is an optimized sequence of coordinates.
-		*/
-		m_cache[d].push_back(v3s16( 0, 1, 0)); // top
-		m_cache[d].push_back(v3s16( 0, 0, 1)); // back
-		m_cache[d].push_back(v3s16(-1, 0, 0)); // left
-		m_cache[d].push_back(v3s16( 1, 0, 0)); // right
-		m_cache[d].push_back(v3s16( 0, 0,-1)); // front
-		m_cache[d].push_back(v3s16( 0,-1, 0)); // bottom
-		// 6
-		m_cache[d].push_back(v3s16(-1, 0, 1)); // back left
-		m_cache[d].push_back(v3s16( 1, 0, 1)); // back right
-		m_cache[d].push_back(v3s16(-1, 0,-1)); // front left
-		m_cache[d].push_back(v3s16( 1, 0,-1)); // front right
-		m_cache[d].push_back(v3s16(-1,-1, 0)); // bottom left
-		m_cache[d].push_back(v3s16( 1,-1, 0)); // bottom right
-		m_cache[d].push_back(v3s16( 0,-1, 1)); // bottom back
-		m_cache[d].push_back(v3s16( 0,-1,-1)); // bottom front
-		m_cache[d].push_back(v3s16(-1, 1, 0)); // top left
-		m_cache[d].push_back(v3s16( 1, 1, 0)); // top right
-		m_cache[d].push_back(v3s16( 0, 1, 1)); // top back
-		m_cache[d].push_back(v3s16( 0, 1,-1)); // top front
-		// 18
-		m_cache[d].push_back(v3s16(-1, 1, 1)); // top back-left
-		m_cache[d].push_back(v3s16( 1, 1, 1)); // top back-right
-		m_cache[d].push_back(v3s16(-1, 1,-1)); // top front-left
-		m_cache[d].push_back(v3s16( 1, 1,-1)); // top front-right
-		m_cache[d].push_back(v3s16(-1,-1, 1)); // bottom back-left
-		m_cache[d].push_back(v3s16( 1,-1, 1)); // bottom back-right
-		m_cache[d].push_back(v3s16(-1,-1,-1)); // bottom front-left
-		m_cache[d].push_back(v3s16( 1,-1,-1)); // bottom front-right
-		// 26
-		return;
-	}
 
-	// Take blocks in all sides, starting from y=0 and going +-y
-	for(s16 y=0; y<=d-1; y++) {
-		// Left and right side, including borders
-		for(s16 z=-d; z<=d; z++) {
-			m_cache[d].push_back(v3s16(d,y,z));
-			m_cache[d].push_back(v3s16(-d,y,z));
-			if(y != 0) {
-				m_cache[d].push_back(v3s16(d,-y,z));
-				m_cache[d].push_back(v3s16(-d,-y,z));
-			}
-		}
-		// Back and front side, excluding borders
-		for(s16 x=-d+1; x<=d-1; x++) {
-			m_cache[d].push_back(v3s16(x,y,d));
-			m_cache[d].push_back(v3s16(x,y,-d));
-			if(y != 0) {
-				m_cache[d].push_back(v3s16(x,-y,d));
-				m_cache[d].push_back(v3s16(x,-y,-d));
-			}
-		}
-	}
-
-	// Take the bottom and top face with borders
-	// -d<x<d, y=+-d, -d<z<d
-	for(s16 x=-d; x<=d; x++)
-	for(s16 z=-d; z<=d; z++) {
-		m_cache[d].push_back(v3s16(x,-d,z));
-		m_cache[d].push_back(v3s16(x,d,z));
-	}
-}
-
-/*
-    myrand
-*/
+// myrand
 
 PcgRandom g_pcgrand;