Skip to content

feat(RasterImage): contiguous access to all the colors #1375

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
BenPinet merged 3 commits into from
Oct 22, 2026
+66 −0
Merged

feat(RasterImage): contiguous access to all the colors #1375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0617b4e
feat(RasterImage): contiguous access to all the colors
BotellaA Oct 21, 2026
eab4fbc
Merge remote-tracking branch 'origin/next' into fix/image-io-robustness
BotellaA Oct 21, 2026
b371055
Merge branch 'next' of https://github.com/Geode-solutions/OpenGeode i…
BotellaA Oct 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions include/geode/image/core/raster_image.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@

#pragma once

#include <absl/types/span.h>

#include <geode/basic/cell_array.hpp>
#include <geode/basic/identifier.hpp>
#include <geode/basic/pimpl.hpp>
Expand All @@ -37,7 +39,7 @@
namespace geode
{
template < index_t dimension >
class RasterImage : public CellArray< dimension >, public Identifier

Check warning on line 42 in include/geode/image/core/raster_image.hpp

View workflow job for this annotation

GitHub Actions / test / tidy

include/geode/image/core/raster_image.hpp:42:11 [cppcoreguidelines-special-member-functions] 

class 'RasterImage' defines a destructor, a move constructor and a move assignment operator but does not define a copy constructor or a copy assignment operator
{
friend class bitsery::Access;

Expand Down Expand Up @@ -72,6 +74,17 @@

void set_color( index_t index, RGBColor color );

/*!
* All the cell colors, stored contiguously in cell index order
*/
[[nodiscard]] absl::Span< const RGBColor > colors() const;

/*!
* All the cell colors, stored contiguously in cell index order, for
* bulk modifications
*/
[[nodiscard]] absl::Span< RGBColor > modifiable_colors();

[[nodiscard]] RasterImage clone() const;

private:
Expand Down
22 changes: 22 additions & 0 deletions src/geode/image/core/raster_image.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
friend class bitsery::Access;

public:
const RGBColor& color( index_t index ) const

Check warning on line 42 in src/geode/image/core/raster_image.cpp

View workflow job for this annotation

GitHub Actions / test / tidy

src/geode/image/core/raster_image.cpp:42:9 [modernize-use-nodiscard] 

function 'color' should be marked [[nodiscard]]
{
OpenGeodeImageException::check_assertion( index < colors_.size(),
"[RasterImage::color] Accessing a "
Expand All @@ -52,9 +52,19 @@
OpenGeodeImageException::check_assertion( index < colors_.size(),
"[RasterImage::set_color] Accessing a "
"cell that does not exist" );
colors_[index] = std::move( color );

Check warning on line 55 in src/geode/image/core/raster_image.cpp

View workflow job for this annotation

GitHub Actions / test / tidy

src/geode/image/core/raster_image.cpp:55:30 [hicpp-move-const-arg] 

std::move of the variable 'color' of the trivially-copyable type 'RGBColor' has no effect; remove std::move()
}

[[nodiscard]] absl::Span< const RGBColor > colors() const
{
return colors_;
}

[[nodiscard]] absl::Span< RGBColor > modifiable_colors()
{
return absl::MakeSpan( colors_ );
}

[[nodiscard]] index_t nb_colors() const
{
return static_cast< index_t >( colors_.size() );
Expand Down Expand Up @@ -150,6 +160,18 @@
impl_->set_color( index, std::move( color ) );
}

template < index_t dimension >
absl::Span< const RGBColor > RasterImage< dimension >::colors() const
{
return impl_->colors();
}

template < index_t dimension >
absl::Span< RGBColor > RasterImage< dimension >::modifiable_colors()
{
return impl_->modifiable_colors();
}

template < index_t dimension >
RasterImage< dimension > RasterImage< dimension >::clone() const
{
Expand Down
31 changes: 31 additions & 0 deletions tests/image/test-raster-image.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,15 @@

#include <geode/tests/common.hpp>

void test_raster( const geode::RasterImage2D& raster )

Check warning on line 37 in tests/image/test-raster-image.cpp

View workflow job for this annotation

GitHub Actions / test / tidy

tests/image/test-raster-image.cpp:37:6 [misc-use-internal-linkage] 

function 'test_raster' can be made static or moved into an anonymous namespace to enforce internal linkage
{
geode::OpenGeodeImageException::test(
raster.nb_cells() == 100, "Wrong number of cells" );

Check warning on line 40 in tests/image/test-raster-image.cpp

View workflow job for this annotation

GitHub Actions / test / tidy

tests/image/test-raster-image.cpp:40:30 [cppcoreguidelines-avoid-magic-numbers] 

100 is a magic number; consider replacing it with a named constant
geode::OpenGeodeImageException::test(
raster.nb_cells_in_direction( 0 ) == 10,

Check warning on line 42 in tests/image/test-raster-image.cpp

View workflow job for this annotation

GitHub Actions / test / tidy

tests/image/test-raster-image.cpp:42:46 [cppcoreguidelines-avoid-magic-numbers] 

10 is a magic number; consider replacing it with a named constant
"Wrong number of cells in direction 0" );
geode::OpenGeodeImageException::test(
raster.nb_cells_in_direction( 1 ) == 10,

Check warning on line 45 in tests/image/test-raster-image.cpp

View workflow job for this annotation

GitHub Actions / test / tidy

tests/image/test-raster-image.cpp:45:46 [cppcoreguidelines-avoid-magic-numbers] 

10 is a magic number; consider replacing it with a named constant
"Wrong number of cells in direction 1" );
for( const auto i : geode::LRange{ raster.nb_cells() } )
{
Expand All @@ -52,9 +52,9 @@
}
}

void test_save_failure( const geode::RasterImage2D& raster )

Check warning on line 55 in tests/image/test-raster-image.cpp

View workflow job for this annotation

GitHub Actions / test / tidy

tests/image/test-raster-image.cpp:55:6 [misc-use-internal-linkage] 

function 'test_save_failure' can be made static or moved into an anonymous namespace to enforce internal linkage
{
const auto folder = "raster_folder.og_img2d";

Check warning on line 57 in tests/image/test-raster-image.cpp

View workflow job for this annotation

GitHub Actions / test / tidy

tests/image/test-raster-image.cpp:57:5 [llvm-qualified-auto] 

'const auto folder' can be declared as 'const auto *const folder'
std::filesystem::create_directory( folder );
bool thrown{ false };
try
Expand All @@ -68,7 +68,7 @@
geode::OpenGeodeImageException::test(
thrown, "[TEST] Saving into a directory should throw" );

const auto file = "raster_regular_file";

Check warning on line 71 in tests/image/test-raster-image.cpp

View workflow job for this annotation

GitHub Actions / test / tidy

tests/image/test-raster-image.cpp:71:5 [llvm-qualified-auto] 

'const auto file' can be declared as 'const auto *const file'
std::ofstream{ file };
thrown = false;
try
Expand Down Expand Up @@ -135,6 +135,36 @@
"[TEST] Loading more colors than cells should throw" );
}

void test_colors_access()
{
geode::RasterImage2D raster{ { 4, 3 } };
auto colors = raster.modifiable_colors();
geode::OpenGeodeImageException::test( colors.size() == raster.nb_cells(),
"[TEST] Wrong number of modifiable colors" );
for( const auto i : geode::Indices{ colors } )
{
const auto value = static_cast< geode::local_index_t >( 10 * i );
colors[i] = { value, static_cast< geode::local_index_t >( value + 1 ),
static_cast< geode::local_index_t >( value + 2 ) };
}
const auto& const_raster = raster;
const auto stored_colors = const_raster.colors();
geode::OpenGeodeImageException::test(
stored_colors.size() == raster.nb_cells(),
"[TEST] Wrong number of colors" );
for( const auto i : geode::Range{ raster.nb_cells() } )
{
const auto value = static_cast< geode::local_index_t >( 10 * i );
const geode::RGBColor expected{ value,
static_cast< geode::local_index_t >( value + 1 ),
static_cast< geode::local_index_t >( value + 2 ) };
geode::OpenGeodeImageException::test( raster.color( i ) == expected,
"[TEST] Wrong color set through modifiable colors for ", i );
geode::OpenGeodeImageException::test(
stored_colors[i] == expected, "[TEST] Wrong stored color for ", i );
}
}

template < geode::index_t dimension >
void test_io( std::array< geode::index_t, dimension > cells_number )
{
Expand Down Expand Up @@ -190,6 +220,7 @@
test_missing_colors();
test_too_many_cells();
test_extra_colors();
test_colors_access();
auto raster2 = std::move( raster );
geode::OpenGeodeImageException::test(
raster2.nb_cells() == 100, "Wrong number of cells after move" );
Expand Down
Loading