Skip to content
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

Add move semantics to image class #457

Merged
merged 6 commits into from
Apr 6, 2020
Merged

Add move semantics to image class #457

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
64019f7
Add move semantic to image
sdebionne Mar 19, 2020
144d645
Fix coding style
sdebionne Mar 30, 2020
b3cf36e
Update test using boost/lightweight_test.hpp
mloskot Mar 26, 2020
1fa9d3a
Fix move assignement test
sdebionne Mar 30, 2020
ae47f06
Update move assignement with suggestion from mloskot
sdebionne Mar 30, 2020
a9ee91e
Revert to tag dispatching implementation with appropriate used of pro…
sdebionne Apr 5, 2020
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
127 changes: 111 additions & 16 deletions include/boost/gil/image.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <boost/gil/detail/mp11.hpp>

#include <boost/assert.hpp>
#include <boost/core/exchange.hpp>

#include <cstddef>
#include <memory>
Expand All @@ -37,7 +38,8 @@ namespace boost { namespace gil {
////////////////////////////////////////////////////////////////////////////////////////

template< typename Pixel, bool IsPlanar = false, typename Alloc=std::allocator<unsigned char> >
class image {
class image
{
public:
#if defined(BOOST_NO_CXX11_ALLOCATOR)
using allocator_type = typename Alloc::template rebind<unsigned char>::other;
Expand All @@ -64,73 +66,163 @@ class image {
image(const point_t& dimensions,
std::size_t alignment=0,
const Alloc alloc_in = Alloc()) : _memory(nullptr), _align_in_bytes(alignment), _alloc(alloc_in)
, _allocated_bytes( 0 ) {
, _allocated_bytes( 0 )
{
allocate_and_default_construct(dimensions);
}

image(x_coord_t width, y_coord_t height,
std::size_t alignment=0,
const Alloc alloc_in = Alloc()) : _memory(nullptr), _align_in_bytes(alignment), _alloc(alloc_in)
, _allocated_bytes( 0 ) {
, _allocated_bytes( 0 )
{
allocate_and_default_construct(point_t(width,height));
}

image(const point_t& dimensions,
const Pixel& p_in,
std::size_t alignment = 0,
const Alloc alloc_in = Alloc()) : _memory(nullptr), _align_in_bytes(alignment), _alloc(alloc_in)
, _allocated_bytes( 0 ) {
, _allocated_bytes( 0 )
{
allocate_and_fill(dimensions, p_in);
}

image(x_coord_t width, y_coord_t height,
const Pixel& p_in,
std::size_t alignment = 0,
const Alloc alloc_in = Alloc()) : _memory(nullptr), _align_in_bytes(alignment), _alloc(alloc_in)
, _allocated_bytes ( 0 ) {
, _allocated_bytes ( 0 )
{
allocate_and_fill(point_t(width,height),p_in);
}

image(const image& img) : _memory(nullptr), _align_in_bytes(img._align_in_bytes), _alloc(img._alloc)
, _allocated_bytes( img._allocated_bytes ) {
, _allocated_bytes( img._allocated_bytes )
{
allocate_and_copy(img.dimensions(),img._view);
}

template <typename P2, bool IP2, typename Alloc2>
image(const image<P2,IP2,Alloc2>& img) : _memory(nullptr), _align_in_bytes(img._align_in_bytes), _alloc(img._alloc)
, _allocated_bytes( img._allocated_bytes ) {
, _allocated_bytes( img._allocated_bytes )
{
allocate_and_copy(img.dimensions(),img._view);
}

image& operator=(const image& img) {
// TODO Optimization: use noexcept (requires _view to be nothrow copy constructible)
image(image&& img) :
_view(img._view),
_memory(img._memory),
_align_in_bytes(img._align_in_bytes),
_alloc(std::move(img._alloc)),
mloskot marked this conversation as resolved.
Show resolved Hide resolved
_allocated_bytes(img._allocated_bytes)
{
img._view = view_t();
img._memory = nullptr;
img._align_in_bytes = 0;
img._allocated_bytes = 0;
mloskot marked this conversation as resolved.
Show resolved Hide resolved
}

image& operator=(const image& img)
{
if (dimensions() == img.dimensions())
copy_pixels(img._view,_view);
else {
else
{
image tmp(img);
swap(tmp);
}
return *this;
}

template <typename Img>
image& operator=(const Img& img) {
image& operator=(const Img& img)
{
if (dimensions() == img.dimensions())
copy_pixels(img._view,_view);
else {
else
{
image tmp(img);
swap(tmp);
}
return *this;
}

~image() {
private:
using propagate_allocators = std::true_type;
using no_propagate_allocators = std::false_type;

template <class Alloc2>
using choose_pocma = typename std::conditional<
// TODO: Use std::allocator_traits<Allocator>::is_always_equal if available
std::is_empty<Alloc2>::value,
std::true_type,
typename std::allocator_traits<Alloc2>::propagate_on_container_move_assignment::type
>::type;

static void exchange_memory(image& lhs, image& rhs)
{
lhs._memory = boost::exchange(rhs._memory, nullptr);
lhs._align_in_bytes = boost::exchange(rhs._align_in_bytes, 0);
lhs._allocated_bytes = boost::exchange(rhs._allocated_bytes, 0);
lhs._view = boost::exchange(rhs._view, image::view_t{});
};

void move_assign(image& img, propagate_allocators) noexcept {
// non-sticky allocator, can adopt the memory, fast
destruct_pixels(_view);
this->deallocate();
this->_alloc = img._alloc;
exchange_memory(*this, img);
}

void move_assign(image& img, no_propagate_allocators) {
if (_alloc == img._alloc) {
// allocator stuck to the rhs, but it's equivalent of ours, we can still adopt the memory
destruct_pixels(_view);
this->deallocate();
exchange_memory(*this, img);
} else {
// cannot propagate the allocator and cannot adopt the memory
if (img._memory)
{
allocate_and_copy(img.dimensions(), img._view);
destruct_pixels(img._view);
img.deallocate();
img._view = image::view_t{};
}
else
{
destruct_pixels(this->_view);
this->deallocate();
this->_view = view_t{};
}
}
}

public:
// TODO: Use noexcept(noexcept(move_assign(img, choose_pocma<allocator_type>{})))
// But https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52869 prevents it (fixed in GCC > 9)
image& operator=(image&& img) {
if (this != std::addressof(img))
// Use rebinded alloc to choose pocma
move_assign(img, choose_pocma<allocator_type>{});
mloskot marked this conversation as resolved.
Show resolved Hide resolved

return *this;
}

~image()
{
destruct_pixels(_view);
deallocate();
}

Alloc& allocator() { return _alloc; }
Alloc const& allocator() const { return _alloc; }

void swap(image& img) { // required by MutableContainerConcept
void swap(image& img) // required by MutableContainerConcept
{
using std::swap;
swap(_align_in_bytes, img._align_in_bytes);
swap(_memory, img._memory);
Expand Down Expand Up @@ -419,12 +511,14 @@ class image {
};

template <typename Pixel, bool IsPlanar, typename Alloc>
void swap(image<Pixel, IsPlanar, Alloc>& im1,image<Pixel, IsPlanar, Alloc>& im2) {
void swap(image<Pixel, IsPlanar, Alloc>& im1,image<Pixel, IsPlanar, Alloc>& im2)
{
im1.swap(im2);
}

template <typename Pixel1, bool IsPlanar1, typename Alloc1, typename Pixel2, bool IsPlanar2, typename Alloc2>
bool operator==(const image<Pixel1,IsPlanar1,Alloc1>& im1,const image<Pixel2,IsPlanar2,Alloc2>& im2) {
bool operator==(const image<Pixel1,IsPlanar1,Alloc1>& im1,const image<Pixel2,IsPlanar2,Alloc2>& im2)
{
if ((void*)(&im1)==(void*)(&im2)) return true;
if (const_view(im1).dimensions()!=const_view(im2).dimensions()) return false;
return equal_pixels(const_view(im1),const_view(im2));
Expand All @@ -444,7 +538,8 @@ const typename image<Pixel,IsPlanar,Alloc>::view_t& view(image<Pixel,IsPlanar,Al

/// \brief Returns the constant-pixel view of an image
template <typename Pixel, bool IsPlanar, typename Alloc> inline
const typename image<Pixel,IsPlanar,Alloc>::const_view_t const_view(const image<Pixel,IsPlanar,Alloc>& img) {
const typename image<Pixel,IsPlanar,Alloc>::const_view_t const_view(const image<Pixel,IsPlanar,Alloc>& img)
{
return static_cast<const typename image<Pixel,IsPlanar,Alloc>::const_view_t>(img._view);
}
///@}
Expand Down
45 changes: 45 additions & 0 deletions test/core/image/image.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,54 @@ struct test_constructor_with_dimensions_pixel
}
};

struct test_move_constructor
{
template <typename Image>
void operator()(Image const&)
{
using image_t = Image;
gil::point_t const dimensions{256, 128};
{
image_t image(fixture::create_image<image_t>(dimensions.x, dimensions.y, 0));

image_t image2(std::move(image));
BOOST_TEST_EQ(image2.dimensions(), dimensions);
BOOST_TEST_EQ(image.dimensions(), gil::point_t{});
}
}
static void run()
{
boost::mp11::mp_for_each<fixture::image_types>(test_move_constructor{});
}
};

struct test_move_assignement
{
template <typename Image>
void operator()(Image const&)
{
using image_t = Image;
gil::point_t const dimensions{256, 128};
{
image_t image = fixture::create_image<image_t>(dimensions.x, dimensions.y, 0);
image_t image2 = fixture::create_image<image_t>(dimensions.x * 2, dimensions.y * 2, 1);
image2 = std::move(image);
BOOST_TEST_EQ(image2.dimensions(), dimensions);
BOOST_TEST_EQ(image.dimensions(), gil::point_t{});
}
}
static void run()
{
boost::mp11::mp_for_each<fixture::image_types>(test_move_assignement{});
}
};

int main()
{
test_constructor_with_dimensions_pixel::run();

test_move_constructor::run();
test_move_assignement::run();

return ::boost::report_errors();
}