Skip to content

Commit

Permalink
Add move semantics to image class (#457)
Browse files Browse the repository at this point in the history 
Since we do not require C++17 yet with its `if constexpr
that would discard instantiation of allocator's `operator=`
when the statement is not active, we have to use the tag
dispatching with appropriate used of
`propagate_on_container_move_assignment` and
rebound `allocator_type` to choose the POCMA.

Minor coding style fixes.
  • Loading branch information
@sdebionne
sdebionne authored Apr 6, 2020
1 parent b9011e1 commit 9b055f2
Show file tree
Hide file tree
Showing 2 changed files with 156 additions and 16 deletions.
127 changes: 111 additions & 16 deletions include/boost/gil/image.hpp
View file
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)),
_allocated_bytes(img._allocated_bytes)
{
img._view = view_t();
img._memory = nullptr;
img._align_in_bytes = 0;
img._allocated_bytes = 0;
}

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>{});

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
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();
}

0 comments on commit 9b055f2

Please sign in to comment.