From 89f3991351185b365ccd10525e74d35d7bb2da46 Mon Sep 17 00:00:00 2001 From: Lars Müller <34514239+appgurueu@users.noreply.github.com> Date: Sun, 30 May 2021 20:23:12 +0200 Subject: Fix base64 validation and add unittests (#10515) Implement proper padding character checks --- src/util/base64.cpp | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) (limited to 'src/util') diff --git a/src/util/base64.cpp b/src/util/base64.cpp index 6e1584410..0c2455222 100644 --- a/src/util/base64.cpp +++ b/src/util/base64.cpp @@ -33,18 +33,39 @@ static const std::string base64_chars = "abcdefghijklmnopqrstuvwxyz" "0123456789+/"; +static const std::string base64_chars_padding_1 = "AEIMQUYcgkosw048"; +static const std::string base64_chars_padding_2 = "AQgw"; static inline bool is_base64(unsigned char c) { - return isalnum(c) || c == '+' || c == '/' || c == '='; + return (c >= '0' && c <= '9') + || (c >= 'A' && c <= 'Z') + || (c >= 'a' && c <= 'z') + || c == '+' || c == '/'; } bool base64_is_valid(std::string const& s) { - for (char i : s) - if (!is_base64(i)) + size_t i = 0; + for (; i < s.size(); ++i) + if (!is_base64(s[i])) + break; + unsigned char padding = 3 - ((i + 3) % 4); + if ((padding == 1 && base64_chars_padding_1.find(s[i - 1]) == std::string::npos) + || (padding == 2 && base64_chars_padding_2.find(s[i - 1]) == std::string::npos) + || padding == 3) + return false; + int actual_padding = s.size() - i; + // omission of padding characters is allowed + if (actual_padding == 0) + return true; + + // remaining characters (max. 2) may only be padding + for (; i < s.size(); ++i) + if (s[i] != '=') return false; - return true; + // number of padding characters needs to match + return padding == actual_padding; } std::string base64_encode(unsigned char const* bytes_to_encode, unsigned int in_len) { -- cgit v1.2.3 From f5706d444b02ccc1fcd854968087172d50cfcca2 Mon Sep 17 00:00:00 2001 From: x2048 Date: Sun, 11 Jul 2021 17:15:19 +0200 Subject: Improve shadow rendering with non-default camera FOV (#11385) * Adjust minimum filter radius for perspective * Expand shadow frustum when camera FOV changes, reuse FOV distance adjustment from numeric.cpp * Read shadow_soft_radius setting as float * Use adaptive filter radius to accomodate for PSM distortion * Adjust filter radius for texture resolution --- client/shaders/nodes_shader/opengl_fragment.glsl | 16 +++++++----- src/client/shader.cpp | 2 +- src/client/shadows/dynamicshadows.cpp | 31 ++++++++++++++---------- src/util/numeric.cpp | 11 ++++++--- 4 files changed, 37 insertions(+), 23 deletions(-) (limited to 'src/util') diff --git a/client/shaders/nodes_shader/opengl_fragment.glsl b/client/shaders/nodes_shader/opengl_fragment.glsl index 43a8b1f25..9f8a21d09 100644 --- a/client/shaders/nodes_shader/opengl_fragment.glsl +++ b/client/shaders/nodes_shader/opengl_fragment.glsl @@ -181,9 +181,14 @@ float getDeltaPerspectiveFactor(float l) float getPenumbraRadius(sampler2D shadowsampler, vec2 smTexCoord, float realDistance, float multiplier) { + float baseLength = getBaseLength(smTexCoord); + float perspectiveFactor; + // Return fast if sharp shadows are requested - if (SOFTSHADOWRADIUS <= 1.0) - return SOFTSHADOWRADIUS; + if (SOFTSHADOWRADIUS <= 1.0) { + perspectiveFactor = getDeltaPerspectiveFactor(baseLength); + return max(2 * length(smTexCoord.xy) * 2048 / f_textureresolution / pow(perspectiveFactor, 3), SOFTSHADOWRADIUS); + } vec2 clampedpos; float texture_size = 1.0 / (2048 /*f_textureresolution*/ * 0.5); @@ -192,8 +197,6 @@ float getPenumbraRadius(sampler2D shadowsampler, vec2 smTexCoord, float realDist float pointDepth; float maxRadius = SOFTSHADOWRADIUS * 5.0 * multiplier; - float baseLength = getBaseLength(smTexCoord); - float perspectiveFactor; float bound = clamp(PCFBOUND * (1 - baseLength), 0.5, PCFBOUND); int n = 0; @@ -211,9 +214,10 @@ float getPenumbraRadius(sampler2D shadowsampler, vec2 smTexCoord, float realDist } depth = depth / n; - depth = pow(clamp(depth, 0.0, 1000.0), 1.6) / 0.001; - return max(0.5, depth * maxRadius); + + perspectiveFactor = getDeltaPerspectiveFactor(baseLength); + return max(length(smTexCoord.xy) * 2 * 2048 / f_textureresolution / pow(perspectiveFactor, 3), depth * maxRadius); } #ifdef POISSON_FILTER diff --git a/src/client/shader.cpp b/src/client/shader.cpp index 355366bd3..0b35c37af 100644 --- a/src/client/shader.cpp +++ b/src/client/shader.cpp @@ -740,7 +740,7 @@ ShaderInfo ShaderSource::generateShader(const std::string &name, s32 shadow_filter = g_settings->getS32("shadow_filters"); shaders_header << "#define SHADOW_FILTER " << shadow_filter << "\n"; - float shadow_soft_radius = g_settings->getS32("shadow_soft_radius"); + float shadow_soft_radius = g_settings->getFloat("shadow_soft_radius"); if (shadow_soft_radius < 1.0f) shadow_soft_radius = 1.0f; shaders_header << "#define SOFTSHADOWRADIUS " << shadow_soft_radius << "\n"; diff --git a/src/client/shadows/dynamicshadows.cpp b/src/client/shadows/dynamicshadows.cpp index 775cdebce..17b711a61 100644 --- a/src/client/shadows/dynamicshadows.cpp +++ b/src/client/shadows/dynamicshadows.cpp @@ -33,29 +33,34 @@ void DirectionalLight::createSplitMatrices(const Camera *cam) v3f newCenter; v3f look = cam->getDirection(); + // camera view tangents + float tanFovY = tanf(cam->getFovY() * 0.5f); + float tanFovX = tanf(cam->getFovX() * 0.5f); + + // adjusted frustum boundaries + float sfNear = shadow_frustum.zNear; + float sfFar = adjustDist(shadow_frustum.zFar, cam->getFovY()); + + // adjusted camera positions v3f camPos2 = cam->getPosition(); v3f camPos = v3f(camPos2.X - cam->getOffset().X * BS, camPos2.Y - cam->getOffset().Y * BS, camPos2.Z - cam->getOffset().Z * BS); - camPos += look * shadow_frustum.zNear; - camPos2 += look * shadow_frustum.zNear; - float end = shadow_frustum.zNear + shadow_frustum.zFar; - newCenter = camPos + look * (shadow_frustum.zNear + 0.05f * end); - v3f world_center = camPos2 + look * (shadow_frustum.zNear + 0.05f * end); - // Create a vector to the frustum far corner - // @Liso: move all vars we can outside the loop. - float tanFovY = tanf(cam->getFovY() * 0.5f); - float tanFovX = tanf(cam->getFovX() * 0.5f); + camPos += look * sfNear; + camPos2 += look * sfNear; - const v3f &viewUp = cam->getCameraNode()->getUpVector(); - // viewUp.normalize(); + // center point of light frustum + float end = sfNear + sfFar; + newCenter = camPos + look * (sfNear + 0.05f * end); + v3f world_center = camPos2 + look * (sfNear + 0.05f * end); + // Create a vector to the frustum far corner + const v3f &viewUp = cam->getCameraNode()->getUpVector(); v3f viewRight = look.crossProduct(viewUp); - // viewRight.normalize(); v3f farCorner = look + viewRight * tanFovX + viewUp * tanFovY; // Compute the frustumBoundingSphere radius - v3f boundVec = (camPos + farCorner * shadow_frustum.zFar) - newCenter; + v3f boundVec = (camPos + farCorner * sfFar) - newCenter; radius = boundVec.getLength() * 2.0f; // boundVec.getLength(); float vvolume = radius * 2.0f; diff --git a/src/util/numeric.cpp b/src/util/numeric.cpp index 99e4cfb5c..702ddce95 100644 --- a/src/util/numeric.cpp +++ b/src/util/numeric.cpp @@ -159,7 +159,7 @@ bool isBlockInSight(v3s16 blockpos_b, v3f camera_pos, v3f camera_dir, return true; } -s16 adjustDist(s16 dist, float zoom_fov) +inline float adjustDist(float dist, float zoom_fov) { // 1.775 ~= 72 * PI / 180 * 1.4, the default FOV on the client. // The heuristic threshold for zooming is half of that. @@ -167,8 +167,13 @@ s16 adjustDist(s16 dist, float zoom_fov) if (zoom_fov < 0.001f || zoom_fov > threshold_fov) return dist; - return std::round(dist * std::cbrt((1.0f - std::cos(threshold_fov)) / - (1.0f - std::cos(zoom_fov / 2.0f)))); + return dist * std::cbrt((1.0f - std::cos(threshold_fov)) / + (1.0f - std::cos(zoom_fov / 2.0f))); +} + +s16 adjustDist(s16 dist, float zoom_fov) +{ + return std::round(adjustDist((float)dist, zoom_fov)); } void setPitchYawRollRad(core::matrix4 &m, const v3f &rot) -- cgit v1.2.3 From 80d12dbedb67191a5eb3e4f3c36b04baed1f8afb Mon Sep 17 00:00:00 2001 From: hecks <42101236+hecktest@users.noreply.github.com> Date: Thu, 29 Jul 2021 05:10:10 +0200 Subject: Add a simple PNG image encoder with Lua API (#11485) * Add a simple PNG image encoder with Lua API Add ColorSpec to RGBA converter Make a safety wrapper for the encoder Create devtest examples Co-authored-by: hecktest <> Co-authored-by: sfan5 --- .gitignore | 1 + builtin/game/misc.lua | 39 ++++++++++++++++ doc/lua_api.txt | 19 +++++++- games/devtest/mods/testnodes/textures.lua | 75 +++++++++++++++++++++++++++++++ src/script/lua_api/l_util.cpp | 43 ++++++++++++++++++ src/script/lua_api/l_util.h | 6 +++ src/util/CMakeLists.txt | 1 + src/util/png.cpp | 68 ++++++++++++++++++++++++++++ src/util/png.h | 27 +++++++++++ 9 files changed, 278 insertions(+), 1 deletion(-) create mode 100755 src/util/png.cpp create mode 100755 src/util/png.h (limited to 'src/util') diff --git a/.gitignore b/.gitignore index df1386bce..a83a3718f 100644 --- a/.gitignore +++ b/.gitignore @@ -87,6 +87,7 @@ src/test_config.h src/cmake_config.h src/cmake_config_githash.h src/unittest/test_world/world.mt +games/devtest/mods/testnodes/textures/testnodes_generated_*.png /locale/ .directory *.cbp diff --git a/builtin/game/misc.lua b/builtin/game/misc.lua index c13a583f0..cee95dd23 100644 --- a/builtin/game/misc.lua +++ b/builtin/game/misc.lua @@ -290,3 +290,42 @@ function core.dynamic_add_media(filepath, callback) end return true end + + +-- PNG encoder safety wrapper + +local o_encode_png = core.encode_png +function core.encode_png(width, height, data, compression) + if type(width) ~= "number" then + error("Incorrect type for 'width', expected number, got " .. type(width)) + end + if type(height) ~= "number" then + error("Incorrect type for 'height', expected number, got " .. type(height)) + end + + local expected_byte_count = width * height * 4; + + if type(data) ~= "table" and type(data) ~= "string" then + error("Incorrect type for 'height', expected table or string, got " .. type(height)); + end + + local data_length = type(data) == "table" and #data * 4 or string.len(data); + + if data_length ~= expected_byte_count then + error(string.format( + "Incorrect length of 'data', width and height imply %d bytes but %d were provided", + expected_byte_count, + data_length + )) + end + + if type(data) == "table" then + local dataBuf = {} + for i = 1, #data do + dataBuf[i] = core.colorspec_to_bytes(data[i]) + end + data = table.concat(dataBuf) + end + + return o_encode_png(width, height, data, compression or 6) +end diff --git a/doc/lua_api.txt b/doc/lua_api.txt index 7ee9a3f2c..21e34b1ec 100644 --- a/doc/lua_api.txt +++ b/doc/lua_api.txt @@ -4611,6 +4611,23 @@ Utilities * `minetest.colorspec_to_colorstring(colorspec)`: Converts a ColorSpec to a ColorString. If the ColorSpec is invalid, returns `nil`. * `colorspec`: The ColorSpec to convert +* `minetest.colorspec_to_bytes(colorspec)`: Converts a ColorSpec to a raw + string of four bytes in an RGBA layout, returned as a string. + * `colorspec`: The ColorSpec to convert +* `minetest.encode_png(width, height, data, [compression])`: Encode a PNG + image and return it in string form. + * `width`: Width of the image + * `height`: Height of the image + * `data`: Image data, one of: + * array table of ColorSpec, length must be width*height + * string with raw RGBA pixels, length must be width*height*4 + * `compression`: Optional zlib compression level, number in range 0 to 9. + The data is one-dimensional, starting in the upper left corner of the image + and laid out in scanlines going from left to right, then top to bottom. + Please note that it's not safe to use string.char to generate raw data, + use `colorspec_to_bytes` to generate raw RGBA values in a predictable way. + The resulting PNG image is always 32-bit. Palettes are not supported at the moment. + You may use this to procedurally generate textures during server init. Logging ------- @@ -7631,7 +7648,7 @@ Used by `minetest.register_node`. leveled_max = 127, -- Maximum value for `leveled` (0-127), enforced in -- `minetest.set_node_level` and `minetest.add_node_level`. - -- Values above 124 might causes collision detection issues. + -- Values above 124 might causes collision detection issues. liquid_range = 8, -- Maximum distance that flowing liquid nodes can spread around diff --git a/games/devtest/mods/testnodes/textures.lua b/games/devtest/mods/testnodes/textures.lua index f6e6a0c2a..4652007d9 100644 --- a/games/devtest/mods/testnodes/textures.lua +++ b/games/devtest/mods/testnodes/textures.lua @@ -65,3 +65,78 @@ for a=1,#alphas do }) end +-- Generate PNG textures + +local function mandelbrot(w, h, iterations) + local r = {} + for y=0, h-1 do + for x=0, w-1 do + local re = (x - w/2) * 4/w + local im = (y - h/2) * 4/h + -- zoom in on a nice view + re = re / 128 - 0.23 + im = im / 128 - 0.82 + + local px, py = 0, 0 + local i = 0 + while px*px + py*py <= 4 and i < iterations do + px, py = px*px - py*py + re, 2 * px * py + im + i = i + 1 + end + r[w*y+x+1] = i / iterations + end + end + return r +end + +local function gen_checkers(w, h, tile) + local r = {} + for y=0, h-1 do + for x=0, w-1 do + local hori = math.floor(x / tile) % 2 == 0 + local vert = math.floor(y / tile) % 2 == 0 + r[w*y+x+1] = hori ~= vert and 1 or 0 + end + end + return r +end + +local fractal = mandelbrot(512, 512, 128) +local checker = gen_checkers(512, 512, 32) + +local floor = math.floor +local abs = math.abs +local data_mb = {} +local data_ck = {} +for i=1, #fractal do + data_mb[i] = { + r = floor(fractal[i] * 255), + g = floor(abs(fractal[i] * 2 - 1) * 255), + b = floor(abs(1 - fractal[i]) * 255), + a = 255, + } + data_ck[i] = checker[i] > 0 and "#F80" or "#000" +end + +local textures_path = minetest.get_modpath( minetest.get_current_modname() ) .. "/textures/" +minetest.safe_file_write( + textures_path .. "testnodes_generated_mb.png", + minetest.encode_png(512,512,data_mb) +) +minetest.safe_file_write( + textures_path .. "testnodes_generated_ck.png", + minetest.encode_png(512,512,data_ck) +) + +minetest.register_node("testnodes:generated_png_mb", { + description = S("Generated Mandelbrot PNG Test Node"), + tiles = { "testnodes_generated_mb.png" }, + + groups = { dig_immediate = 2 }, +}) +minetest.register_node("testnodes:generated_png_ck", { + description = S("Generated Checker PNG Test Node"), + tiles = { "testnodes_generated_ck.png" }, + + groups = { dig_immediate = 2 }, +}) diff --git a/src/script/lua_api/l_util.cpp b/src/script/lua_api/l_util.cpp index 8de2d67c8..87436fce0 100644 --- a/src/script/lua_api/l_util.cpp +++ b/src/script/lua_api/l_util.cpp @@ -40,6 +40,7 @@ with this program; if not, write to the Free Software Foundation, Inc., #include "version.h" #include "util/hex.h" #include "util/sha1.h" +#include "util/png.h" #include #include @@ -497,6 +498,43 @@ int ModApiUtil::l_colorspec_to_colorstring(lua_State *L) return 0; } +// colorspec_to_bytes(colorspec) +int ModApiUtil::l_colorspec_to_bytes(lua_State *L) +{ + NO_MAP_LOCK_REQUIRED; + + video::SColor color(0); + if (read_color(L, 1, &color)) { + u8 colorbytes[4] = { + (u8) color.getRed(), + (u8) color.getGreen(), + (u8) color.getBlue(), + (u8) color.getAlpha(), + }; + lua_pushlstring(L, (const char*) colorbytes, 4); + return 1; + } + + return 0; +} + +// encode_png(w, h, data, level) +int ModApiUtil::l_encode_png(lua_State *L) +{ + NO_MAP_LOCK_REQUIRED; + + // The args are already pre-validated on the lua side. + u32 width = readParam(L, 1); + u32 height = readParam(L, 2); + const char *data = luaL_checklstring(L, 3, NULL); + s32 compression = readParam(L, 4); + + std::string out = encodePNG((const u8*)data, width, height, compression); + + lua_pushlstring(L, out.data(), out.size()); + return 1; +} + void ModApiUtil::Initialize(lua_State *L, int top) { API_FCT(log); @@ -532,6 +570,9 @@ void ModApiUtil::Initialize(lua_State *L, int top) API_FCT(get_version); API_FCT(sha1); API_FCT(colorspec_to_colorstring); + API_FCT(colorspec_to_bytes); + + API_FCT(encode_png); LuaSettings::create(L, g_settings, g_settings_path); lua_setfield(L, top, "settings"); @@ -557,6 +598,7 @@ void ModApiUtil::InitializeClient(lua_State *L, int top) API_FCT(get_version); API_FCT(sha1); API_FCT(colorspec_to_colorstring); + API_FCT(colorspec_to_bytes); } void ModApiUtil::InitializeAsync(lua_State *L, int top) @@ -585,6 +627,7 @@ void ModApiUtil::InitializeAsync(lua_State *L, int top) API_FCT(get_version); API_FCT(sha1); API_FCT(colorspec_to_colorstring); + API_FCT(colorspec_to_bytes); LuaSettings::create(L, g_settings, g_settings_path); lua_setfield(L, top, "settings"); diff --git a/src/script/lua_api/l_util.h b/src/script/lua_api/l_util.h index 6943a6afb..54d2be619 100644 --- a/src/script/lua_api/l_util.h +++ b/src/script/lua_api/l_util.h @@ -104,6 +104,12 @@ private: // colorspec_to_colorstring(colorspec) static int l_colorspec_to_colorstring(lua_State *L); + // colorspec_to_bytes(colorspec) + static int l_colorspec_to_bytes(lua_State *L); + + // encode_png(w, h, data, level) + static int l_encode_png(lua_State *L); + public: static void Initialize(lua_State *L, int top); static void InitializeAsync(lua_State *L, int top); diff --git a/src/util/CMakeLists.txt b/src/util/CMakeLists.txt index cd2e468d1..6bc97915f 100644 --- a/src/util/CMakeLists.txt +++ b/src/util/CMakeLists.txt @@ -15,4 +15,5 @@ set(UTIL_SRCS ${CMAKE_CURRENT_SOURCE_DIR}/string.cpp ${CMAKE_CURRENT_SOURCE_DIR}/srp.cpp ${CMAKE_CURRENT_SOURCE_DIR}/timetaker.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/png.cpp PARENT_SCOPE) diff --git a/src/util/png.cpp b/src/util/png.cpp new file mode 100755 index 000000000..7ac2e94a1 --- /dev/null +++ b/src/util/png.cpp @@ -0,0 +1,68 @@ +/* +Minetest +Copyright (C) 2021 hecks + +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 "png.h" +#include +#include +#include +#include +#include "util/serialize.h" +#include "serialization.h" +#include "irrlichttypes.h" + +static void writeChunk(std::ostringstream &target, const std::string &chunk_str) +{ + assert(chunk_str.size() >= 4); + assert(chunk_str.size() - 4 < U32_MAX); + writeU32(target, chunk_str.size() - 4); // Write length minus the identifier + target << chunk_str; + writeU32(target, crc32(0,(const u8*)chunk_str.data(), chunk_str.size())); +} + +std::string encodePNG(const u8 *data, u32 width, u32 height, s32 compression) +{ + auto file = std::ostringstream(std::ios::binary); + file << "\x89PNG\r\n\x1a\n"; + + { + auto IHDR = std::ostringstream(std::ios::binary); + IHDR << "IHDR"; + writeU32(IHDR, width); + writeU32(IHDR, height); + // 8 bpp, color type 6 (RGBA) + IHDR.write("\x08\x06\x00\x00\x00", 5); + writeChunk(file, IHDR.str()); + } + + { + auto IDAT = std::ostringstream(std::ios::binary); + IDAT << "IDAT"; + auto scanlines = std::ostringstream(std::ios::binary); + for(u32 i = 0; i < height; i++) { + scanlines.write("\x00", 1); // Null predictor + scanlines.write((const char*) data + width * 4 * i, width * 4); + } + compressZlib(scanlines.str(), IDAT, compression); + writeChunk(file, IDAT.str()); + } + + file.write("\x00\x00\x00\x00IEND\xae\x42\x60\x82", 12); + + return file.str(); +} diff --git a/src/util/png.h b/src/util/png.h new file mode 100755 index 000000000..92387aef0 --- /dev/null +++ b/src/util/png.h @@ -0,0 +1,27 @@ +/* +Minetest +Copyright (C) 2021 hecks + +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. +*/ + +#pragma once + +#include +#include "irrlichttypes.h" + +/* Simple PNG encoder. Encodes an RGBA image with no predictors. + Returns a binary string. */ +std::string encodePNG(const u8 *data, u32 width, u32 height, s32 compression); -- cgit v1.2.3 From 75bf9b75caba5fc876f20eabea3fabc142d1b51e Mon Sep 17 00:00:00 2001 From: sfan5 Date: Sat, 11 Sep 2021 21:06:57 +0200 Subject: Make sure relevant std::stringstreams are set to binary --- src/client/client.cpp | 8 ++++---- src/client/game.cpp | 2 +- src/database/database-leveldb.cpp | 16 ++++++++-------- src/database/database-postgresql.cpp | 14 ++++++++------ src/database/database-redis.cpp | 6 ++---- src/database/database-sqlite3.cpp | 13 +++++++++---- src/gui/guiChatConsole.cpp | 1 - src/gui/guiFormSpecMenu.cpp | 10 +++------- src/inventory.cpp | 1 - src/itemstackmetadata.cpp | 2 +- src/network/clientpackethandler.cpp | 12 +++++------- src/script/common/c_converter.cpp | 5 +---- src/script/lua_api/l_mapgen.cpp | 4 +--- src/script/lua_api/l_util.cpp | 12 ++++++------ src/settings.cpp | 5 +---- src/unittest/test_areastore.cpp | 4 ++-- src/util/serialize.cpp | 5 ++--- 17 files changed, 54 insertions(+), 66 deletions(-) (limited to 'src/util') diff --git a/src/client/client.cpp b/src/client/client.cpp index 13ff22e8e..45cc62a33 100644 --- a/src/client/client.cpp +++ b/src/client/client.cpp @@ -1693,13 +1693,13 @@ float Client::mediaReceiveProgress() return 1.0; // downloader only exists when not yet done } -typedef struct TextureUpdateArgs { +struct TextureUpdateArgs { gui::IGUIEnvironment *guienv; u64 last_time_ms; u16 last_percent; const wchar_t* text_base; ITextureSource *tsrc; -} TextureUpdateArgs; +}; void Client::showUpdateProgressTexture(void *args, u32 progress, u32 max_progress) { @@ -1718,8 +1718,8 @@ void Client::showUpdateProgressTexture(void *args, u32 progress, u32 max_progres if (do_draw) { targs->last_time_ms = time_ms; - std::basic_stringstream strm; - strm << targs->text_base << " " << targs->last_percent << "%..."; + std::wostringstream strm; + strm << targs->text_base << L" " << targs->last_percent << L"%..."; m_rendering_engine->draw_load_screen(strm.str(), targs->guienv, targs->tsrc, 0, 72 + (u16) ((18. / 100.) * (double) targs->last_percent), true); } diff --git a/src/client/game.cpp b/src/client/game.cpp index 18df5cc58..a24ded844 100644 --- a/src/client/game.cpp +++ b/src/client/game.cpp @@ -1618,7 +1618,7 @@ bool Game::getServerContent(bool *aborted) dtime, progress); delete[] text; } else { - std::stringstream message; + std::ostringstream message; std::fixed(message); message.precision(0); float receive = client->mediaReceiveProgress() * 100; diff --git a/src/database/database-leveldb.cpp b/src/database/database-leveldb.cpp index 73cd63f6d..39f4c8442 100644 --- a/src/database/database-leveldb.cpp +++ b/src/database/database-leveldb.cpp @@ -70,11 +70,11 @@ bool Database_LevelDB::saveBlock(const v3s16 &pos, const std::string &data) void Database_LevelDB::loadBlock(const v3s16 &pos, std::string *block) { - std::string datastr; leveldb::Status status = m_database->Get(leveldb::ReadOptions(), - i64tos(getBlockAsInteger(pos)), &datastr); + i64tos(getBlockAsInteger(pos)), block); - *block = (status.ok()) ? datastr : ""; + if (!status.ok()) + block->clear(); } bool Database_LevelDB::deleteBlock(const v3s16 &pos) @@ -131,7 +131,7 @@ void PlayerDatabaseLevelDB::savePlayer(RemotePlayer *player) std::string (long) serialized_inventory */ - std::ostringstream os; + std::ostringstream os(std::ios_base::binary); writeU8(os, 1); PlayerSAO *sao = player->getPlayerSAO(); @@ -142,7 +142,7 @@ void PlayerDatabaseLevelDB::savePlayer(RemotePlayer *player) writeF32(os, sao->getRotation().Y); writeU16(os, sao->getBreath()); - StringMap stringvars = sao->getMeta().getStrings(); + const auto &stringvars = sao->getMeta().getStrings(); writeU32(os, stringvars.size()); for (const auto &it : stringvars) { os << serializeString16(it.first); @@ -170,7 +170,7 @@ bool PlayerDatabaseLevelDB::loadPlayer(RemotePlayer *player, PlayerSAO *sao) player->getName(), &raw); if (!s.ok()) return false; - std::istringstream is(raw); + std::istringstream is(raw, std::ios_base::binary); if (readU8(is) > 1) return false; @@ -230,7 +230,7 @@ bool AuthDatabaseLevelDB::getAuth(const std::string &name, AuthEntry &res) leveldb::Status s = m_database->Get(leveldb::ReadOptions(), name, &raw); if (!s.ok()) return false; - std::istringstream is(raw); + std::istringstream is(raw, std::ios_base::binary); /* u8 version = 1 @@ -262,7 +262,7 @@ bool AuthDatabaseLevelDB::getAuth(const std::string &name, AuthEntry &res) bool AuthDatabaseLevelDB::saveAuth(const AuthEntry &authEntry) { - std::ostringstream os; + std::ostringstream os(std::ios_base::binary); writeU8(os, 1); os << serializeString16(authEntry.password); diff --git a/src/database/database-postgresql.cpp b/src/database/database-postgresql.cpp index 29ecd4223..3469f4242 100644 --- a/src/database/database-postgresql.cpp +++ b/src/database/database-postgresql.cpp @@ -274,10 +274,10 @@ void MapDatabasePostgreSQL::loadBlock(const v3s16 &pos, std::string *block) PGresult *results = execPrepared("read_block", ARRLEN(args), args, argLen, argFmt, false); - *block = ""; - if (PQntuples(results)) - *block = std::string(PQgetvalue(results, 0, 0), PQgetlength(results, 0, 0)); + block->assign(PQgetvalue(results, 0, 0), PQgetlength(results, 0, 0)); + else + block->clear(); PQclear(results); } @@ -496,6 +496,7 @@ void PlayerDatabasePostgreSQL::savePlayer(RemotePlayer *player) execPrepared("remove_player_inventory_items", 1, rmvalues); std::vector inventory_lists = sao->getInventory()->getLists(); + std::ostringstream oss; for (u16 i = 0; i < inventory_lists.size(); i++) { const InventoryList* list = inventory_lists[i]; const std::string &name = list->getName(); @@ -512,9 +513,10 @@ void PlayerDatabasePostgreSQL::savePlayer(RemotePlayer *player) execPrepared("add_player_inventory", 5, inv_values); for (u32 j = 0; j < list->getSize(); j++) { - std::ostringstream os; - list->getItem(j).serialize(os); - std::string itemStr = os.str(), slotId = itos(j); + oss.str(""); + oss.clear(); + list->getItem(j).serialize(oss); + std::string itemStr = oss.str(), slotId = itos(j); const char* invitem_values[] = { player->getName(), diff --git a/src/database/database-redis.cpp b/src/database/database-redis.cpp index 096ea504d..5ffff67b7 100644 --- a/src/database/database-redis.cpp +++ b/src/database/database-redis.cpp @@ -127,8 +127,7 @@ void Database_Redis::loadBlock(const v3s16 &pos, std::string *block) switch (reply->type) { case REDIS_REPLY_STRING: { - *block = std::string(reply->str, reply->len); - // std::string copies the memory so this won't cause any problems + block->assign(reply->str, reply->len); freeReplyObject(reply); return; } @@ -141,8 +140,7 @@ void Database_Redis::loadBlock(const v3s16 &pos, std::string *block) "Redis command 'HGET %s %s' errored: ") + errstr); } case REDIS_REPLY_NIL: { - *block = ""; - // block not found in database + block->clear(); freeReplyObject(reply); return; } diff --git a/src/database/database-sqlite3.cpp b/src/database/database-sqlite3.cpp index 4560743b9..898acc265 100644 --- a/src/database/database-sqlite3.cpp +++ b/src/database/database-sqlite3.cpp @@ -302,7 +302,10 @@ void MapDatabaseSQLite3::loadBlock(const v3s16 &pos, std::string *block) const char *data = (const char *) sqlite3_column_blob(m_stmt_read, 0); size_t len = sqlite3_column_bytes(m_stmt_read, 0); - *block = (data) ? std::string(data, len) : ""; + if (data) + block->assign(data, len); + else + block->clear(); sqlite3_step(m_stmt_read); // We should never get more than 1 row, so ok to reset @@ -491,6 +494,7 @@ void PlayerDatabaseSQLite3::savePlayer(RemotePlayer *player) sqlite3_reset(m_stmt_player_remove_inventory_items); std::vector inventory_lists = sao->getInventory()->getLists(); + std::ostringstream oss; for (u16 i = 0; i < inventory_lists.size(); i++) { const InventoryList* list = inventory_lists[i]; @@ -503,9 +507,10 @@ void PlayerDatabaseSQLite3::savePlayer(RemotePlayer *player) sqlite3_reset(m_stmt_player_add_inventory); for (u32 j = 0; j < list->getSize(); j++) { - std::ostringstream os; - list->getItem(j).serialize(os); - std::string itemStr = os.str(); + oss.str(""); + oss.clear(); + list->getItem(j).serialize(oss); + std::string itemStr = oss.str(); str_to_sqlite(m_stmt_player_add_inventory_items, 1, player->getName()); int_to_sqlite(m_stmt_player_add_inventory_items, 2, i); diff --git a/src/gui/guiChatConsole.cpp b/src/gui/guiChatConsole.cpp index 049e21a16..0610c85cc 100644 --- a/src/gui/guiChatConsole.cpp +++ b/src/gui/guiChatConsole.cpp @@ -729,7 +729,6 @@ void GUIChatConsole::middleClick(s32 col, s32 row) msg << gettext("Failed to open webpage"); } msg << " '" << weblink << "'"; - msg.flush(); m_chat_backend->addUnparsedMessage(utf8_to_wide(msg.str())); } } diff --git a/src/gui/guiFormSpecMenu.cpp b/src/gui/guiFormSpecMenu.cpp index 09b004f8f..797fd3ff6 100644 --- a/src/gui/guiFormSpecMenu.cpp +++ b/src/gui/guiFormSpecMenu.cpp @@ -3933,9 +3933,7 @@ void GUIFormSpecMenu::acceptInput(FormspecQuitMode quitmode) } if (e != 0) { - std::stringstream ss; - ss << (e->getActiveTab() +1); - fields[name] = ss.str(); + fields[name] = itos(e->getActiveTab() + 1); } } else if (s.ftype == f_CheckBox) { // No dynamic cast possible due to some distributions shipped @@ -3961,12 +3959,10 @@ void GUIFormSpecMenu::acceptInput(FormspecQuitMode quitmode) e = static_cast(element); if (e) { - std::stringstream os; - os << e->getPos(); if (s.fdefault == L"Changed") - fields[name] = "CHG:" + os.str(); + fields[name] = "CHG:" + itos(e->getPos()); else - fields[name] = "VAL:" + os.str(); + fields[name] = "VAL:" + itos(e->getPos()); } } else if (s.ftype == f_AnimatedImage) { // No dynamic cast possible due to some distributions shipped diff --git a/src/inventory.cpp b/src/inventory.cpp index b3bed623a..da6517e62 100644 --- a/src/inventory.cpp +++ b/src/inventory.cpp @@ -460,7 +460,6 @@ void InventoryList::deSerialize(std::istream &is) std::getline(is, line, '\n'); std::istringstream iss(line); - //iss.imbue(std::locale("C")); std::string name; std::getline(iss, name, ' '); diff --git a/src/itemstackmetadata.cpp b/src/itemstackmetadata.cpp index 7a26fbb0e..529e0149f 100644 --- a/src/itemstackmetadata.cpp +++ b/src/itemstackmetadata.cpp @@ -60,7 +60,7 @@ bool ItemStackMetadata::setString(const std::string &name, const std::string &va void ItemStackMetadata::serialize(std::ostream &os) const { - std::ostringstream os2; + std::ostringstream os2(std::ios_base::binary); os2 << DESERIALIZE_START; for (const auto &stringvar : m_stringvars) { if (!stringvar.first.empty() || !stringvar.second.empty()) diff --git a/src/network/clientpackethandler.cpp b/src/network/clientpackethandler.cpp index 9c9c59d13..128240c02 100644 --- a/src/network/clientpackethandler.cpp +++ b/src/network/clientpackethandler.cpp @@ -261,7 +261,7 @@ void Client::handleCommand_NodemetaChanged(NetworkPacket *pkt) return; std::istringstream is(pkt->readLongString(), std::ios::binary); - std::stringstream sstr; + std::stringstream sstr(std::ios::binary); decompressZlib(is, sstr); NodeMetadataList meta_updates_list(false); @@ -760,12 +760,11 @@ void Client::handleCommand_NodeDef(NetworkPacket* pkt) // Decompress node definitions std::istringstream tmp_is(pkt->readLongString(), std::ios::binary); - std::ostringstream tmp_os; + std::stringstream tmp_os(std::ios::binary | std::ios::in | std::ios::out); decompressZlib(tmp_is, tmp_os); // Deserialize node definitions - std::istringstream tmp_is2(tmp_os.str()); - m_nodedef->deSerialize(tmp_is2); + m_nodedef->deSerialize(tmp_os); m_nodedef_received = true; } @@ -780,12 +779,11 @@ void Client::handleCommand_ItemDef(NetworkPacket* pkt) // Decompress item definitions std::istringstream tmp_is(pkt->readLongString(), std::ios::binary); - std::ostringstream tmp_os; + std::stringstream tmp_os(std::ios::binary | std::ios::in | std::ios::out); decompressZlib(tmp_is, tmp_os); // Deserialize node definitions - std::istringstream tmp_is2(tmp_os.str()); - m_itemdef->deSerialize(tmp_is2); + m_itemdef->deSerialize(tmp_os); m_itemdef_received = true; } diff --git a/src/script/common/c_converter.cpp b/src/script/common/c_converter.cpp index d848b75b8..19734b913 100644 --- a/src/script/common/c_converter.cpp +++ b/src/script/common/c_converter.cpp @@ -76,10 +76,7 @@ static void set_vector_metatable(lua_State *L) void push_float_string(lua_State *L, float value) { - std::stringstream ss; - std::string str; - ss << value; - str = ss.str(); + auto str = ftos(value); lua_pushstring(L, str.c_str()); } diff --git a/src/script/lua_api/l_mapgen.cpp b/src/script/lua_api/l_mapgen.cpp index eb3d49a5e..f173bd162 100644 --- a/src/script/lua_api/l_mapgen.cpp +++ b/src/script/lua_api/l_mapgen.cpp @@ -752,9 +752,7 @@ int ModApiMapgen::l_get_mapgen_params(lua_State *L) lua_setfield(L, -2, "mgname"); settingsmgr->getMapSetting("seed", &value); - std::istringstream ss(value); - u64 seed; - ss >> seed; + u64 seed = from_string(value); lua_pushinteger(L, seed); lua_setfield(L, -2, "seed"); diff --git a/src/script/lua_api/l_util.cpp b/src/script/lua_api/l_util.cpp index 9152b5f7f..2405cd90d 100644 --- a/src/script/lua_api/l_util.cpp +++ b/src/script/lua_api/l_util.cpp @@ -272,11 +272,11 @@ int ModApiUtil::l_compress(lua_State *L) const char *data = luaL_checklstring(L, 1, &size); int level = -1; - if (!lua_isnone(L, 3) && !lua_isnil(L, 3)) - level = readParam(L, 3); + if (!lua_isnoneornil(L, 3)) + level = readParam(L, 3); - std::ostringstream os; - compressZlib(std::string(data, size), os, level); + std::ostringstream os(std::ios_base::binary); + compressZlib(reinterpret_cast(data), size, os, level); std::string out = os.str(); @@ -292,8 +292,8 @@ int ModApiUtil::l_decompress(lua_State *L) size_t size; const char *data = luaL_checklstring(L, 1, &size); - std::istringstream is(std::string(data, size)); - std::ostringstream os; + std::istringstream is(std::string(data, size), std::ios_base::binary); + std::ostringstream os(std::ios_base::binary); decompressZlib(is, os); std::string out = os.str(); diff --git a/src/settings.cpp b/src/settings.cpp index 4def46112..f4de5bec9 100644 --- a/src/settings.cpp +++ b/src/settings.cpp @@ -537,11 +537,8 @@ float Settings::getFloat(const std::string &name) const u64 Settings::getU64(const std::string &name) const { - u64 value = 0; std::string s = get(name); - std::istringstream ss(s); - ss >> value; - return value; + return from_string(s); } diff --git a/src/unittest/test_areastore.cpp b/src/unittest/test_areastore.cpp index 691cd69d2..2af3ca90c 100644 --- a/src/unittest/test_areastore.cpp +++ b/src/unittest/test_areastore.cpp @@ -135,7 +135,7 @@ void TestAreaStore::testSerialization() b.data = "Area BB"; store.insertArea(&b); - std::ostringstream os; + std::ostringstream os(std::ios_base::binary); store.serialize(os); std::string str = os.str(); @@ -157,7 +157,7 @@ void TestAreaStore::testSerialization() UASSERTEQ(const std::string &, str, str_wanted); - std::istringstream is(str); + std::istringstream is(str, std::ios_base::binary); store.deserialize(is); // deserialize() doesn't clear the store diff --git a/src/util/serialize.cpp b/src/util/serialize.cpp index d770101f2..281061229 100644 --- a/src/util/serialize.cpp +++ b/src/util/serialize.cpp @@ -248,7 +248,7 @@ std::string serializeJsonStringIfNeeded(const std::string &s) std::string deSerializeJsonStringIfNeeded(std::istream &is) { - std::ostringstream tmp_os; + std::stringstream tmp_os(std::ios_base::binary | std::ios_base::in | std::ios_base::out); bool expect_initial_quote = true; bool is_json = false; bool was_backslash = false; @@ -280,8 +280,7 @@ std::string deSerializeJsonStringIfNeeded(std::istream &is) expect_initial_quote = false; } if (is_json) { - std::istringstream tmp_is(tmp_os.str(), std::ios::binary); - return deSerializeJsonString(tmp_is); + return deSerializeJsonString(tmp_os); } return tmp_os.str(); -- cgit v1.2.3 From fd8a8501bc26dfca2a93d51000867b8592210040 Mon Sep 17 00:00:00 2001 From: sfan5 Date: Fri, 17 Sep 2021 18:14:25 +0200 Subject: Shave off buffer copies in networking code (#11607) --- src/network/connection.cpp | 72 +++++++++++++++++++++------------------ src/network/connection.h | 40 +++++++++------------- src/network/connectionthreads.cpp | 41 +++++++++++----------- src/network/networkpacket.cpp | 9 ++--- src/network/networkpacket.h | 3 +- src/unittest/test_connection.cpp | 12 +++---- src/util/container.h | 7 ++++ src/util/pointer.h | 34 ++++++++++++++++++ 8 files changed, 126 insertions(+), 92 deletions(-) (limited to 'src/util') diff --git a/src/network/connection.cpp b/src/network/connection.cpp index 0ba8c36b2..a4970954f 100644 --- a/src/network/connection.cpp +++ b/src/network/connection.cpp @@ -200,17 +200,12 @@ RPBSearchResult ReliablePacketBuffer::findPacket(u16 seqnum) return i; } -RPBSearchResult ReliablePacketBuffer::notFound() -{ - return m_list.end(); -} - bool ReliablePacketBuffer::getFirstSeqnum(u16& result) { MutexAutoLock listlock(m_list_mutex); if (m_list.empty()) return false; - const BufferedPacket &p = *m_list.begin(); + const BufferedPacket &p = m_list.front(); result = readU16(&p.data[BASE_HEADER_SIZE + 1]); return true; } @@ -220,14 +215,14 @@ BufferedPacket ReliablePacketBuffer::popFirst() MutexAutoLock listlock(m_list_mutex); if (m_list.empty()) throw NotFoundException("Buffer is empty"); - BufferedPacket p = *m_list.begin(); - m_list.erase(m_list.begin()); + BufferedPacket p = std::move(m_list.front()); + m_list.pop_front(); if (m_list.empty()) { m_oldest_non_answered_ack = 0; } else { m_oldest_non_answered_ack = - readU16(&m_list.begin()->data[BASE_HEADER_SIZE + 1]); + readU16(&m_list.front().data[BASE_HEADER_SIZE + 1]); } return p; } @@ -241,15 +236,7 @@ BufferedPacket ReliablePacketBuffer::popSeqnum(u16 seqnum) << " not found in reliable buffer"<data[BASE_HEADER_SIZE+1])); - m_oldest_non_answered_ack = s; - } + BufferedPacket p = std::move(*r); m_list.erase(r); @@ -257,12 +244,12 @@ BufferedPacket ReliablePacketBuffer::popSeqnum(u16 seqnum) m_oldest_non_answered_ack = 0; } else { m_oldest_non_answered_ack = - readU16(&m_list.begin()->data[BASE_HEADER_SIZE + 1]); + readU16(&m_list.front().data[BASE_HEADER_SIZE + 1]); } return p; } -void ReliablePacketBuffer::insert(BufferedPacket &p, u16 next_expected) +void ReliablePacketBuffer::insert(const BufferedPacket &p, u16 next_expected) { MutexAutoLock listlock(m_list_mutex); if (p.data.getSize() < BASE_HEADER_SIZE + 3) { @@ -355,7 +342,7 @@ void ReliablePacketBuffer::insert(BufferedPacket &p, u16 next_expected) } /* update last packet number */ - m_oldest_non_answered_ack = readU16(&(*m_list.begin()).data[BASE_HEADER_SIZE+1]); + m_oldest_non_answered_ack = readU16(&m_list.front().data[BASE_HEADER_SIZE+1]); } void ReliablePacketBuffer::incrementTimeouts(float dtime) @@ -367,17 +354,19 @@ void ReliablePacketBuffer::incrementTimeouts(float dtime) } } -std::list ReliablePacketBuffer::getTimedOuts(float timeout, - unsigned int max_packets) +std::list + ReliablePacketBuffer::getTimedOuts(float timeout, u32 max_packets) { MutexAutoLock listlock(m_list_mutex); std::list timed_outs; for (BufferedPacket &bufferedPacket : m_list) { if (bufferedPacket.time >= timeout) { + // caller will resend packet so reset time and increase counter + bufferedPacket.time = 0.0f; + bufferedPacket.resend_count++; + timed_outs.push_back(bufferedPacket); - //this packet will be sent right afterwards reset timeout here - bufferedPacket.time = 0.0f; if (timed_outs.size() >= max_packets) break; } @@ -1051,20 +1040,20 @@ bool UDPPeer::processReliableSendCommand( m_connection->GetProtocolID(), m_connection->GetPeerID(), c.channelnum); - toadd.push(p); + toadd.push(std::move(p)); } if (have_sequence_number) { volatile u16 pcount = 0; while (!toadd.empty()) { - BufferedPacket p = toadd.front(); + BufferedPacket p = std::move(toadd.front()); toadd.pop(); // LOG(dout_con<getDesc() // << " queuing reliable packet for peer_id: " << c.peer_id // << " channel: " << (c.channelnum&0xFF) // << " seqnum: " << readU16(&p.data[BASE_HEADER_SIZE+1]) // << std::endl) - chan.queued_reliables.push(p); + chan.queued_reliables.push(std::move(p)); pcount++; } sanity_check(chan.queued_reliables.size() < 0xFFFF); @@ -1208,12 +1197,19 @@ Connection::~Connection() } /* Internal stuff */ -void Connection::putEvent(ConnectionEvent &e) + +void Connection::putEvent(const ConnectionEvent &e) { assert(e.type != CONNEVENT_NONE); // Pre-condition m_event_queue.push_back(e); } +void Connection::putEvent(ConnectionEvent &&e) +{ + assert(e.type != CONNEVENT_NONE); // Pre-condition + m_event_queue.push_back(std::move(e)); +} + void Connection::TriggerSend() { m_sendThread->Trigger(); @@ -1299,7 +1295,7 @@ ConnectionEvent Connection::waitEvent(u32 timeout_ms) } } -void Connection::putCommand(ConnectionCommand &c) +void Connection::putCommand(const ConnectionCommand &c) { if (!m_shutting_down) { m_command_queue.push_back(c); @@ -1307,6 +1303,14 @@ void Connection::putCommand(ConnectionCommand &c) } } +void Connection::putCommand(ConnectionCommand &&c) +{ + if (!m_shutting_down) { + m_command_queue.push_back(std::move(c)); + m_sendThread->Trigger(); + } +} + void Connection::Serve(Address bind_addr) { ConnectionCommand c; @@ -1408,7 +1412,7 @@ void Connection::Send(session_t peer_id, u8 channelnum, ConnectionCommand c; c.send(peer_id, channelnum, pkt, reliable); - putCommand(c); + putCommand(std::move(c)); } Address Connection::GetPeerAddress(session_t peer_id) @@ -1508,12 +1512,12 @@ u16 Connection::createPeer(Address& sender, MTProtocols protocol, int fd) << "createPeer(): giving peer_id=" << peer_id_new << std::endl); ConnectionCommand cmd; - SharedBuffer reply(4); + Buffer reply(4); writeU8(&reply[0], PACKET_TYPE_CONTROL); writeU8(&reply[1], CONTROLTYPE_SET_PEER_ID); writeU16(&reply[2], peer_id_new); cmd.createPeer(peer_id_new,reply); - putCommand(cmd); + putCommand(std::move(cmd)); // Create peer addition event ConnectionEvent e; @@ -1560,7 +1564,7 @@ void Connection::sendAck(session_t peer_id, u8 channelnum, u16 seqnum) writeU16(&ack[2], seqnum); c.ack(peer_id, channelnum, ack); - putCommand(c); + putCommand(std::move(c)); m_sendThread->Trigger(); } diff --git a/src/network/connection.h b/src/network/connection.h index 24cd4fe4a..49bb65c3e 100644 --- a/src/network/connection.h +++ b/src/network/connection.h @@ -19,7 +19,7 @@ with this program; if not, write to the Free Software Foundation, Inc., #pragma once -#include "irrlichttypes_bloated.h" +#include "irrlichttypes.h" #include "peerhandler.h" #include "socket.h" #include "constants.h" @@ -29,7 +29,6 @@ with this program; if not, write to the Free Software Foundation, Inc., #include "util/numeric.h" #include "networkprotocol.h" #include -#include #include #include @@ -242,20 +241,19 @@ public: BufferedPacket popFirst(); BufferedPacket popSeqnum(u16 seqnum); - void insert(BufferedPacket &p, u16 next_expected); + void insert(const BufferedPacket &p, u16 next_expected); void incrementTimeouts(float dtime); - std::list getTimedOuts(float timeout, - unsigned int max_packets); + std::list getTimedOuts(float timeout, u32 max_packets); void print(); bool empty(); - RPBSearchResult notFound(); u32 size(); private: RPBSearchResult findPacket(u16 seqnum); // does not perform locking + inline RPBSearchResult notFound() { return m_list.end(); } std::list m_list; @@ -329,18 +327,6 @@ struct ConnectionCommand bool raw = false; ConnectionCommand() = default; - ConnectionCommand &operator=(const ConnectionCommand &other) - { - type = other.type; - address = other.address; - peer_id = other.peer_id; - channelnum = other.channelnum; - // We must copy the buffer here to prevent race condition - data = SharedBuffer(*other.data, other.data.getSize()); - reliable = other.reliable; - raw = other.raw; - return *this; - } void serve(Address address_) { @@ -364,7 +350,7 @@ struct ConnectionCommand void send(session_t peer_id_, u8 channelnum_, NetworkPacket *pkt, bool reliable_); - void ack(session_t peer_id_, u8 channelnum_, const SharedBuffer &data_) + void ack(session_t peer_id_, u8 channelnum_, const Buffer &data_) { type = CONCMD_ACK; peer_id = peer_id_; @@ -373,7 +359,7 @@ struct ConnectionCommand reliable = false; } - void createPeer(session_t peer_id_, const SharedBuffer &data_) + void createPeer(session_t peer_id_, const Buffer &data_) { type = CONCMD_CREATE_PEER; peer_id = peer_id_; @@ -707,7 +693,7 @@ struct ConnectionEvent ConnectionEvent() = default; - std::string describe() + const char *describe() const { switch(type) { case CONNEVENT_NONE: @@ -724,7 +710,7 @@ struct ConnectionEvent return "Invalid ConnectionEvent"; } - void dataReceived(session_t peer_id_, const SharedBuffer &data_) + void dataReceived(session_t peer_id_, const Buffer &data_) { type = CONNEVENT_DATA_RECEIVED; peer_id = peer_id_; @@ -763,7 +749,9 @@ public: /* Interface */ ConnectionEvent waitEvent(u32 timeout_ms); - void putCommand(ConnectionCommand &c); + // Warning: creates an unnecessary copy, prefer putCommand(T&&) if possible + void putCommand(const ConnectionCommand &c); + void putCommand(ConnectionCommand &&c); void SetTimeoutMs(u32 timeout) { m_bc_receive_timeout = timeout; } void Serve(Address bind_addr); @@ -802,11 +790,14 @@ protected: } UDPSocket m_udpSocket; + // Command queue: user -> SendThread MutexedQueue m_command_queue; bool Receive(NetworkPacket *pkt, u32 timeout); - void putEvent(ConnectionEvent &e); + // Warning: creates an unnecessary copy, prefer putEvent(T&&) if possible + void putEvent(const ConnectionEvent &e); + void putEvent(ConnectionEvent &&e); void TriggerSend(); @@ -815,6 +806,7 @@ protected: return getPeerNoEx(PEER_ID_SERVER) != nullptr; } private: + // Event queue: ReceiveThread -> user MutexedQueue m_event_queue; session_t m_peer_id = 0; diff --git a/src/network/connectionthreads.cpp b/src/network/connectionthreads.cpp index 7b62bc792..47678dac5 100644 --- a/src/network/connectionthreads.cpp +++ b/src/network/connectionthreads.cpp @@ -174,6 +174,11 @@ void ConnectionSendThread::runTimeouts(float dtime) std::vector timeouted_peers; std::vector peerIds = m_connection->getPeerIDs(); + const u32 numpeers = m_connection->m_peers.size(); + + if (numpeers == 0) + return; + for (session_t &peerId : peerIds) { PeerHelper peer = m_connection->getPeerNoEx(peerId); @@ -209,7 +214,6 @@ void ConnectionSendThread::runTimeouts(float dtime) float resend_timeout = udpPeer->getResendTimeout(); bool retry_count_exceeded = false; for (Channel &channel : udpPeer->channels) { - std::list timed_outs; // Remove timed out incomplete unreliable split packets channel.incoming_splits.removeUnreliableTimedOuts(dtime, m_timeout); @@ -217,13 +221,8 @@ void ConnectionSendThread::runTimeouts(float dtime) // Increment reliable packet times channel.outgoing_reliables_sent.incrementTimeouts(dtime); - unsigned int numpeers = m_connection->m_peers.size(); - - if (numpeers == 0) - return; - // Re-send timed out outgoing reliables - timed_outs = channel.outgoing_reliables_sent.getTimedOuts(resend_timeout, + auto timed_outs = channel.outgoing_reliables_sent.getTimedOuts(resend_timeout, (m_max_data_packets_per_iteration / numpeers)); channel.UpdatePacketLossCounter(timed_outs.size()); @@ -231,16 +230,14 @@ void ConnectionSendThread::runTimeouts(float dtime) m_iteration_packets_avaialble -= timed_outs.size(); - for (std::list::iterator k = timed_outs.begin(); - k != timed_outs.end(); ++k) { - session_t peer_id = readPeerId(*(k->data)); - u8 channelnum = readChannel(*(k->data)); - u16 seqnum = readU16(&(k->data[BASE_HEADER_SIZE + 1])); + for (const auto &k : timed_outs) { + session_t peer_id = readPeerId(*k.data); + u8 channelnum = readChannel(*k.data); + u16 seqnum = readU16(&(k.data[BASE_HEADER_SIZE + 1])); - channel.UpdateBytesLost(k->data.getSize()); - k->resend_count++; + channel.UpdateBytesLost(k.data.getSize()); - if (k->resend_count > MAX_RELIABLE_RETRY) { + if (k.resend_count > MAX_RELIABLE_RETRY) { retry_count_exceeded = true; timeouted_peers.push_back(peer->id); /* no need to check additional packets if a single one did timeout*/ @@ -249,14 +246,14 @@ void ConnectionSendThread::runTimeouts(float dtime) LOG(derr_con << m_connection->getDesc() << "RE-SENDING timed-out RELIABLE to " - << k->address.serializeString() + << k.address.serializeString() << "(t/o=" << resend_timeout << "): " << "from_peer_id=" << peer_id << ", channel=" << ((int) channelnum & 0xff) << ", seqnum=" << seqnum << std::endl); - rawSend(*k); + rawSend(k); // do not handle rtt here as we can't decide if this packet was // lost or really takes more time to transmit @@ -375,7 +372,7 @@ bool ConnectionSendThread::rawSendAsPacket(session_t peer_id, u8 channelnum, << " INFO: queueing reliable packet for peer_id: " << peer_id << " channel: " << (u32)channelnum << " seqnum: " << seqnum << std::endl); - channel->queued_reliables.push(p); + channel->queued_reliables.push(std::move(p)); return false; } @@ -717,13 +714,15 @@ void ConnectionSendThread::sendPackets(float dtime) channel.outgoing_reliables_sent.size() < channel.getWindowSize() && peer->m_increment_packets_remaining > 0) { - BufferedPacket p = channel.queued_reliables.front(); + BufferedPacket p = std::move(channel.queued_reliables.front()); channel.queued_reliables.pop(); + LOG(dout_con << m_connection->getDesc() << " INFO: sending a queued reliable packet " << " channel: " << i << ", seqnum: " << readU16(&p.data[BASE_HEADER_SIZE + 1]) << std::endl); + sendAsPacketReliable(p, &channel); peer->m_increment_packets_remaining--; } @@ -911,7 +910,7 @@ void ConnectionReceiveThread::receive(SharedBuffer &packetdata, if (data_left) { ConnectionEvent e; e.dataReceived(peer_id, resultdata); - m_connection->putEvent(e); + m_connection->putEvent(std::move(e)); } } catch (ProcessedSilentlyException &e) { @@ -1022,7 +1021,7 @@ void ConnectionReceiveThread::receive(SharedBuffer &packetdata, ConnectionEvent e; e.dataReceived(peer_id, resultdata); - m_connection->putEvent(e); + m_connection->putEvent(std::move(e)); } catch (ProcessedSilentlyException &e) { } diff --git a/src/network/networkpacket.cpp b/src/network/networkpacket.cpp index a71e26572..6b8b0f703 100644 --- a/src/network/networkpacket.cpp +++ b/src/network/networkpacket.cpp @@ -549,14 +549,11 @@ NetworkPacket& NetworkPacket::operator<<(video::SColor src) return *this; } -SharedBuffer NetworkPacket::oldForgePacket() +Buffer NetworkPacket::oldForgePacket() { - SharedBuffer sb(m_datasize + 2); + Buffer sb(m_datasize + 2); writeU16(&sb[0], m_command); + memcpy(&sb[2], m_data.data(), m_datasize); - u8* datas = getU8Ptr(0); - - if (datas != NULL) - memcpy(&sb[2], datas, m_datasize); return sb; } diff --git a/src/network/networkpacket.h b/src/network/networkpacket.h index c7ff03b8e..b1c44f055 100644 --- a/src/network/networkpacket.h +++ b/src/network/networkpacket.h @@ -115,7 +115,8 @@ public: NetworkPacket &operator<<(video::SColor src); // Temp, we remove SharedBuffer when migration finished - SharedBuffer oldForgePacket(); + // ^ this comment has been here for 4 years + Buffer oldForgePacket(); private: void checkReadOffset(u32 from_offset, u32 field_size); diff --git a/src/unittest/test_connection.cpp b/src/unittest/test_connection.cpp index c3aacc536..23b7e9105 100644 --- a/src/unittest/test_connection.cpp +++ b/src/unittest/test_connection.cpp @@ -88,7 +88,7 @@ void TestConnection::testNetworkPacketSerialize() }; if (sizeof(wchar_t) == 2) - warningstream << __func__ << " may fail on this platform." << std::endl; + warningstream << __FUNCTION__ << " may fail on this platform." << std::endl; { NetworkPacket pkt(123, 0); @@ -96,7 +96,7 @@ void TestConnection::testNetworkPacketSerialize() // serializing wide strings should do surrogate encoding, we test that here pkt << std::wstring(L"\U00020b9a"); - SharedBuffer buf = pkt.oldForgePacket(); + auto buf = pkt.oldForgePacket(); UASSERTEQ(int, buf.getSize(), sizeof(expected)); UASSERT(!memcmp(expected, &buf[0], buf.getSize())); } @@ -280,7 +280,7 @@ void TestConnection::testConnectSendReceive() NetworkPacket pkt; pkt.putRawPacket((u8*) "Hello World !", 14, 0); - SharedBuffer sentdata = pkt.oldForgePacket(); + auto sentdata = pkt.oldForgePacket(); infostream<<"** running client.Send()"< sentdata = pkt.oldForgePacket(); + auto sentdata = pkt.oldForgePacket(); server.Send(peer_id_client, 0, &pkt, true); //sleep_ms(3000); - SharedBuffer recvdata; + Buffer recvdata; infostream << "** running client.Receive()" << std::endl; session_t peer_id = 132; u16 size = 0; diff --git a/src/util/container.h b/src/util/container.h index 1c4a219f0..001066563 100644 --- a/src/util/container.h +++ b/src/util/container.h @@ -140,6 +140,13 @@ public: m_signal.post(); } + void push_back(T &&t) + { + MutexAutoLock lock(m_mutex); + m_queue.push_back(std::move(t)); + m_signal.post(); + } + /* this version of pop_front returns a empty element of T on timeout. * Make sure default constructor of T creates a recognizable "empty" element */ diff --git a/src/util/pointer.h b/src/util/pointer.h index d29ec8739..7fc5de551 100644 --- a/src/util/pointer.h +++ b/src/util/pointer.h @@ -51,6 +51,19 @@ public: else data = NULL; } + Buffer(Buffer &&buffer) + { + m_size = buffer.m_size; + if(m_size != 0) + { + data = buffer.data; + buffer.data = nullptr; + buffer.m_size = 0; + } + else + data = nullptr; + } + // Copies whole buffer Buffer(const T *t, unsigned int size) { m_size = size; @@ -62,10 +75,12 @@ public: else data = NULL; } + ~Buffer() { drop(); } + Buffer& operator=(const Buffer &buffer) { if(this == &buffer) @@ -81,6 +96,23 @@ public: data = NULL; return *this; } + Buffer& operator=(Buffer &&buffer) + { + if(this == &buffer) + return *this; + drop(); + m_size = buffer.m_size; + if(m_size != 0) + { + data = buffer.data; + buffer.data = nullptr; + buffer.m_size = 0; + } + else + data = nullptr; + return *this; + } + T & operator[](unsigned int i) const { return data[i]; @@ -89,10 +121,12 @@ public: { return data; } + unsigned int getSize() const { return m_size; } + private: void drop() { -- cgit v1.2.3