From 5ebfd429c02fc9a94989de248d0af19196ed87ec Mon Sep 17 00:00:00 2001 From: Albin Johansson Date: Wed, 15 May 2024 00:19:05 +0200 Subject: [PATCH] Make matrix types use signed integers --- .../base/inc/tactile/base/util/sign_cast.hpp | 26 +++++++++ .../tactile/core/layer/dense_tile_matrix.hpp | 4 +- .../tactile/core/layer/dense_tile_matrix.cpp | 56 +++++++++++-------- .../tactile/core/layer/sparse_tile_matrix.cpp | 29 +++++----- .../src/tactile/core/layer/tile_layer.cpp | 31 +--------- source/core/src/tactile/core/tile/tileset.cpp | 6 +- .../core/test/src/layer/tile_layer_test.cpp | 4 +- .../core/test/src/layer/tile_matrix_test.cpp | 16 ++---- 8 files changed, 91 insertions(+), 81 deletions(-) create mode 100644 source/base/inc/tactile/base/util/sign_cast.hpp diff --git a/source/base/inc/tactile/base/util/sign_cast.hpp b/source/base/inc/tactile/base/util/sign_cast.hpp new file mode 100644 index 0000000000..f049756534 --- /dev/null +++ b/source/base/inc/tactile/base/util/sign_cast.hpp @@ -0,0 +1,26 @@ +// Copyright (C) 2024 Albin Johansson (GNU General Public License v3.0) + +#pragma once + +#include // signed_integral, unsigned_integral +#include // make_unsigned_t, make_signed_t + +#include "tactile/base/prelude.hpp" + +namespace tactile { + +template +[[nodiscard]] constexpr auto sign_cast(const T value) noexcept + -> std::make_unsigned_t +{ + return static_cast>(value); +} + +template +[[nodiscard]] constexpr auto sign_cast(const T value) noexcept + -> std::make_signed_t +{ + return static_cast>(value); +} + +} // namespace tactile diff --git a/source/core/inc/tactile/core/layer/dense_tile_matrix.hpp b/source/core/inc/tactile/core/layer/dense_tile_matrix.hpp index 9fd3a7c1ca..efeac44a8a 100644 --- a/source/core/inc/tactile/core/layer/dense_tile_matrix.hpp +++ b/source/core/inc/tactile/core/layer/dense_tile_matrix.hpp @@ -102,9 +102,9 @@ class DenseTileMatrix final MatrixExtent mExtent {0, 0}; Vector> mRows {}; - void _set_row_count(usize rows); + void _set_row_count(MatrixExtent::value_type rows); - void _set_column_count(usize cols); + void _set_column_count(MatrixExtent::value_type cols); }; } // namespace tactile diff --git a/source/core/src/tactile/core/layer/dense_tile_matrix.cpp b/source/core/src/tactile/core/layer/dense_tile_matrix.cpp index 561cae9117..7f6fb32844 100644 --- a/source/core/src/tactile/core/layer/dense_tile_matrix.cpp +++ b/source/core/src/tactile/core/layer/dense_tile_matrix.cpp @@ -2,42 +2,48 @@ #include "tactile/core/layer/dense_tile_matrix.hpp" +#include "tactile/base/util/sign_cast.hpp" #include "tactile/core/debug/assert.hpp" #include "tactile/core/debug/exception.hpp" namespace tactile { namespace { -void _add_rows(Vector>& rows, const usize n, const usize cols) +void _add_rows(Vector>& rows, + const MatrixExtent::value_type n, + const MatrixExtent::value_type cols) { - rows.reserve(rows.size() + n); - for (usize i = 0; i < n; ++i) { + rows.reserve(rows.size() + sign_cast(n)); + for (MatrixExtent::value_type i = 0; i < n; ++i) { rows.emplace_back(cols, kEmptyTile); } } -void _add_columns(Vector>& rows, const usize n) +void _add_columns(Vector>& rows, + const MatrixExtent::value_type n) { for (auto& row : rows) { - row.reserve(row.size() + n); - for (usize i = 0; i < n; ++i) { + row.reserve(row.size() + sign_cast(n)); + for (MatrixExtent::value_type i = 0; i < n; ++i) { row.push_back(kEmptyTile); } } } -void _remove_rows(Vector>& rows, const usize n) +void _remove_rows(Vector>& rows, + const MatrixExtent::value_type n) { - for (usize i = 0; i < n; ++i) { + for (MatrixExtent::value_type i = 0; i < n; ++i) { TACTILE_ASSERT(!rows.empty()); rows.pop_back(); } } -void _remove_columns(Vector>& rows, const usize n) +void _remove_columns(Vector>& rows, + const MatrixExtent::value_type n) { for (auto& row : rows) { - for (usize i = 0; i < n; ++i) { + for (MatrixExtent::value_type i = 0; i < n; ++i) { TACTILE_ASSERT(!row.empty()); row.pop_back(); } @@ -49,8 +55,9 @@ void _remove_columns(Vector>& rows, const usize n) DenseTileMatrix::DenseTileMatrix(const MatrixExtent& extent) : mExtent {extent} { - mRows.reserve(mExtent.rows); - mRows.assign(mExtent.rows, Vector(mExtent.cols, kEmptyTile)); + mRows.reserve(sign_cast(mExtent.rows)); + mRows.assign(sign_cast(mExtent.rows), + Vector(sign_cast(mExtent.cols), kEmptyTile)); } void DenseTileMatrix::resize(const MatrixExtent& new_extent) @@ -59,7 +66,7 @@ void DenseTileMatrix::resize(const MatrixExtent& new_extent) _set_row_count(new_extent.rows); } -void DenseTileMatrix::_set_row_count(const usize rows) +void DenseTileMatrix::_set_row_count(const MatrixExtent::value_type rows) { if (rows != mExtent.rows) { if (rows > mExtent.rows) { @@ -73,7 +80,7 @@ void DenseTileMatrix::_set_row_count(const usize rows) } } -void DenseTileMatrix::_set_column_count(const usize cols) +void DenseTileMatrix::_set_column_count(const MatrixExtent::value_type cols) { if (cols != mExtent.cols) { if (cols > mExtent.cols) { @@ -89,32 +96,37 @@ void DenseTileMatrix::_set_column_count(const usize cols) auto DenseTileMatrix::at(const MatrixIndex index) -> TileID { - if (is_valid(index)) [[likely]] { - return mRows[index.row][index.col]; + if (!is_valid(index)) [[unlikely]] { + throw Exception {"bad matrix index"}; } - throw Exception {"bad matrix index"}; + return mRows[sign_cast(index.row)][sign_cast(index.col)]; } auto DenseTileMatrix::at(const MatrixIndex index) const -> const TileID& { - if (is_valid(index)) [[likely]] { - return mRows[index.row][index.col]; + if (!is_valid(index)) [[unlikely]] { + throw Exception {"bad matrix index"}; } - throw Exception {"bad matrix index"}; + return mRows[sign_cast(index.row)][sign_cast(index.col)]; } auto DenseTileMatrix::operator[](const MatrixIndex index) noexcept -> TileID& { + TACTILE_ASSERT_MSG(index.row >= 0, "negative row index"); + TACTILE_ASSERT_MSG(index.col >= 0, "negative column index"); TACTILE_ASSERT_MSG(index.row < mExtent.rows, "bad row index"); TACTILE_ASSERT_MSG(index.col < mExtent.cols, "bad column index"); - return mRows[index.row][index.col]; + return mRows[sign_cast(index.row)][sign_cast(index.col)]; } auto DenseTileMatrix::is_valid(const MatrixIndex& index) const noexcept -> bool { - return (index.row < mExtent.rows) && (index.col < mExtent.cols); + return (index.row >= 0) && // + (index.col >= 0) && // + (index.row < mExtent.rows) && // + (index.col < mExtent.cols); } auto DenseTileMatrix::get_extent() const noexcept -> const MatrixExtent& diff --git a/source/core/src/tactile/core/layer/sparse_tile_matrix.cpp b/source/core/src/tactile/core/layer/sparse_tile_matrix.cpp index ccbdd43e29..5a54278957 100644 --- a/source/core/src/tactile/core/layer/sparse_tile_matrix.cpp +++ b/source/core/src/tactile/core/layer/sparse_tile_matrix.cpp @@ -21,28 +21,28 @@ void SparseTileMatrix::resize(const MatrixExtent& new_extent) auto SparseTileMatrix::at(const MatrixIndex index) -> TileID { - if (is_valid(index)) [[likely]] { - if (const auto* tile_id = find_in(mTiles, index)) { - return *tile_id; - } + if (is_valid(index)) [[unlikely]] { + throw Exception {"bad matrix index"}; + } - return kEmptyTile; + if (const auto* tile_id = find_in(mTiles, index)) { + return *tile_id; } - throw Exception {"bad matrix index"}; + return kEmptyTile; } auto SparseTileMatrix::at(const MatrixIndex index) const -> TileID { - if (is_valid(index)) [[likely]] { - if (const auto* tile_id = find_in(mTiles, index)) { - return *tile_id; - } + if (is_valid(index)) [[unlikely]] { + throw Exception {"bad matrix index"}; + } - return kEmptyTile; + if (const auto* tile_id = find_in(mTiles, index)) { + return *tile_id; } - throw Exception {"bad matrix index"}; + return kEmptyTile; } auto SparseTileMatrix::operator[](const MatrixIndex index) -> TileID& @@ -53,7 +53,10 @@ auto SparseTileMatrix::operator[](const MatrixIndex index) -> TileID& auto SparseTileMatrix::is_valid(const MatrixIndex& index) const noexcept -> bool { - return index.row < mExtent.rows && index.col < mExtent.cols; + return (index.row >= 0) && // + (index.col >= 0) && // + (index.row < mExtent.rows) && // + (index.col < mExtent.cols); } auto SparseTileMatrix::get_extent() const noexcept -> const MatrixExtent& diff --git a/source/core/src/tactile/core/layer/tile_layer.cpp b/source/core/src/tactile/core/layer/tile_layer.cpp index c171c6a676..f78284a9ed 100644 --- a/source/core/src/tactile/core/layer/tile_layer.cpp +++ b/source/core/src/tactile/core/layer/tile_layer.cpp @@ -16,8 +16,8 @@ void _copy_tile_matrix(const TileMatrix1& from, TileMatrix2& to) const auto extent = from.get_extent(); to.resize(extent); - for (usize row = 0; row < extent.rows; ++row) { - for (usize col = 0; col < extent.cols; ++col) { + for (MatrixExtent::value_type row = 0; row < extent.rows; ++row) { + for (MatrixExtent::value_type col = 0; col < extent.cols; ++col) { const MatrixIndex index {row, col}; const auto tile_id = from.at(index); @@ -59,33 +59,6 @@ void destroy_tile_layer(Registry& registry, const EntityID layer_entity) registry.destroy(layer_entity); } -#if 0 - -template -void convert_tile_matrix_types(Registry& registry, const EntityID layer_entity) -{ - const auto from = registry.detach(layer_entity); - - if (from.has_value()) { - const auto extent = from->tiles.get_extent(); - - auto& to = registry.add(layer_entity); - to.tiles.resize(from->tiles.get_extent()); - - for (usize row = 0; row < extent.rows; ++row) { - for (usize col = 0; col < extent.cols; ++col) { - const MatrixIndex index {row, col}; - const auto tile_id = from->tiles.at(index); - - if (tile_id != kEmptyTile) { - to.tiles[index] = tile_id; - } - } - } - } -} -#endif - void to_dense_tile_layer(Registry& registry, const EntityID layer_entity) { TACTILE_ASSERT(is_tile_layer(registry, layer_entity)); diff --git a/source/core/src/tactile/core/tile/tileset.cpp b/source/core/src/tactile/core/tile/tileset.cpp index 3e03b2602c..98862bf38e 100644 --- a/source/core/src/tactile/core/tile/tileset.cpp +++ b/source/core/src/tactile/core/tile/tileset.cpp @@ -56,8 +56,10 @@ auto make_tileset(Registry& registry, const TilesetSpec& spec) -> EntityID } const MatrixExtent extent { - .rows = static_cast(spec.texture.size.y() / spec.tile_size.y()), - .cols = static_cast(spec.texture.size.x() / spec.tile_size.x()), + .rows = static_cast(spec.texture.size.y() / + spec.tile_size.y()), + .cols = static_cast(spec.texture.size.x() / + spec.tile_size.x()), }; if (extent.rows <= 0 || extent.cols <= 0) { diff --git a/source/core/test/src/layer/tile_layer_test.cpp b/source/core/test/src/layer/tile_layer_test.cpp index b70e493476..4c238db6bf 100644 --- a/source/core/test/src/layer/tile_layer_test.cpp +++ b/source/core/test/src/layer/tile_layer_test.cpp @@ -56,8 +56,8 @@ TEST_F(TileLayerTest, MakeTileLayer) EXPECT_TRUE(layer.visible); EXPECT_EQ(dense_tile_layer.tiles.get_extent(), extent); - for (usize row = 0; row < extent.rows; ++row) { - for (usize col = 0; col < extent.cols; ++col) { + for (MatrixExtent::value_type row = 0; row < extent.rows; ++row) { + for (MatrixExtent::value_type col = 0; col < extent.cols; ++col) { const MatrixIndex index {row, col}; EXPECT_EQ(dense_tile_layer.tiles.at(index), kEmptyTile); } diff --git a/source/core/test/src/layer/tile_matrix_test.cpp b/source/core/test/src/layer/tile_matrix_test.cpp index 198ac6f251..73b330239b 100644 --- a/source/core/test/src/layer/tile_matrix_test.cpp +++ b/source/core/test/src/layer/tile_matrix_test.cpp @@ -15,8 +15,8 @@ void _validate_tiles(const MatrixType& tile_matrix, { EXPECT_EQ(tile_matrix.get_extent(), expected_extent); - for (usize row = 0; row < expected_extent.rows; ++row) { - for (usize col = 0; col < expected_extent.cols; ++col) { + for (MatrixExtent::value_type row = 0; row < expected_extent.rows; ++row) { + for (MatrixExtent::value_type col = 0; col < expected_extent.cols; ++col) { const MatrixIndex index {row, col}; EXPECT_TRUE(tile_matrix.is_valid(index)) << "index is " << index; } @@ -45,9 +45,7 @@ TYPED_TEST(TileMatrixTest, DefaultConstructor) /// \trace tactile::SparseTileMatrix::SparseTileMatrix [MatrixExtent] TYPED_TEST(TileMatrixTest, ExtentConstructor) { - const TypeParam tile_matrix { - MatrixExtent {12, 24} - }; + const TypeParam tile_matrix {MatrixExtent {12, 24}}; _validate_tiles(tile_matrix, MatrixExtent {12, 24}); } @@ -103,9 +101,7 @@ TYPED_TEST(TileMatrixTest, Resize) /// \trace tactile::SparseTileMatrix::at TYPED_TEST(TileMatrixTest, At) { - TypeParam tile_matrix { - MatrixExtent {2, 2} - }; + TypeParam tile_matrix {MatrixExtent {2, 2}}; tile_matrix[{0, 0}] = TileID {42}; tile_matrix[{0, 1}] = TileID {99}; tile_matrix[{1, 1}] = TileID {123}; @@ -130,9 +126,7 @@ TYPED_TEST(TileMatrixTest, At) /// \trace tactile::SparseTileMatrix::operator[] TYPED_TEST(TileMatrixTest, SubscriptOperator) { - TypeParam tile_matrix { - MatrixExtent {3, 4} - }; + TypeParam tile_matrix {MatrixExtent {3, 4}}; const auto assign_and_check = [&](const MatrixIndex& index, const TileID tile_id) {