Merge pull request #3513 from lightmare/fix-image-swap

Fix image swap

include/mapnik/image.hpp

@@ -76,7 +76,6 @@ public:
 private:
     detail::image_dimensions<65535> dimensions_;
     detail::buffer buffer_;
-    pixel_type *pData_;
     double offset_;
     double scaling_;
     bool premultiplied_alpha_;

include/mapnik/image_impl.hpp

@@ -62,7 +62,6 @@ template <typename T>
 image<T>::image()
     : dimensions_(0,0),
       buffer_(0),
-      pData_(nullptr),
       offset_(0.0),
       scaling_(1.0),
       premultiplied_alpha_(false),
@@ -73,7 +72,6 @@ template <typename T>
 image<T>::image(int width, int height, unsigned char* data, bool premultiplied, bool painted)
     : dimensions_(width, height),
       buffer_(data, width * height * sizeof(pixel_size)),
-      pData_(reinterpret_cast<pixel_type*>(buffer_.data())),
       offset_(0.0),
       scaling_(1.0),
       premultiplied_alpha_(premultiplied),
@@ -83,15 +81,14 @@ template <typename T>
 image<T>::image(int width, int height, bool initialize, bool premultiplied, bool painted)
     : dimensions_(width, height),
       buffer_(dimensions_.width() * dimensions_.height() * pixel_size),
-      pData_(reinterpret_cast<pixel_type*>(buffer_.data())),
       offset_(0.0),
       scaling_(1.0),
       premultiplied_alpha_(premultiplied),
       painted_(painted)
 {
-    if (pData_ && initialize)
+    if (initialize)
     {
-        std::fill(pData_, pData_ + dimensions_.width() * dimensions_.height(), 0);
+        std::fill(begin(), end(), 0);
     }
 }
 
@@ -99,7 +96,6 @@ template <typename T>
 image<T>::image(image<T> const& rhs)
     : dimensions_(rhs.dimensions_),
       buffer_(rhs.buffer_),
-      pData_(reinterpret_cast<pixel_type*>(buffer_.data())),
       offset_(rhs.offset_),
       scaling_(rhs.scaling_),
       premultiplied_alpha_(rhs.premultiplied_alpha_),
@@ -109,14 +105,12 @@ template <typename T>
 image<T>::image(image<T> && rhs) noexcept
     : dimensions_(std::move(rhs.dimensions_)),
       buffer_(std::move(rhs.buffer_)),
-      pData_(reinterpret_cast<pixel_type*>(buffer_.data())),
       offset_(rhs.offset_),
       scaling_(rhs.scaling_),
       premultiplied_alpha_(rhs.premultiplied_alpha_),
       painted_(rhs.painted_)
 {
     rhs.dimensions_ = { 0, 0 };
-    rhs.pData_ = nullptr;
 }
 
 template <typename T>
@@ -153,14 +147,14 @@ template <typename T>
 inline typename image<T>::pixel_type& image<T>::operator() (std::size_t i, std::size_t j)
 {
     assert(i < dimensions_.width() && j < dimensions_.height());
-    return pData_[j * dimensions_.width() + i];
+    return *get_row(j, i);
 }
 
 template <typename T>
 inline const typename image<T>::pixel_type& image<T>::operator() (std::size_t i, std::size_t j) const
 {
     assert(i < dimensions_.width() && j < dimensions_.height());
-    return pData_[j * dimensions_.width() + i];
+    return *get_row(j, i);
 }
 
 template <typename T>
@@ -190,19 +184,19 @@ inline std::size_t image<T>::row_size() const
 template <typename T>
 inline void image<T>::set(pixel_type const& t)
 {
-    std::fill(pData_, pData_ + dimensions_.width() * dimensions_.height(), t);
+    std::fill(begin(), end(), t);
 }
 
 template <typename T>
 inline const typename image<T>::pixel_type* image<T>::data() const
 {
-    return pData_;
+    return reinterpret_cast<const pixel_type*>(buffer_.data());
 }
 
 template <typename T>
 inline typename image<T>::pixel_type* image<T>::data()
 {
-    return pData_;
+    return reinterpret_cast<pixel_type*>(buffer_.data());
 }
 
 template <typename T>
@@ -219,40 +213,40 @@ inline unsigned char* image<T>::bytes()
 
 // iterator interface
 template <typename T>
-inline typename image<T>::iterator image<T>::begin() { return pData_; }
+inline typename image<T>::iterator image<T>::begin() { return data(); }
 
 template <typename T>
-inline typename image<T>::iterator image<T>::end() { return pData_ + dimensions_.width() * dimensions_.height(); }
+inline typename image<T>::iterator image<T>::end() { return data() + dimensions_.width() * dimensions_.height(); }
 
 template <typename T>
-inline typename image<T>::const_iterator image<T>::begin() const { return pData_; }
+inline typename image<T>::const_iterator image<T>::begin() const { return data(); }
 
 template <typename T>
-inline typename image<T>::const_iterator image<T>::end() const{ return pData_ + dimensions_.width() * dimensions_.height(); }
+inline typename image<T>::const_iterator image<T>::end() const{ return data() + dimensions_.width() * dimensions_.height(); }
 
 
 template <typename T>
 inline typename image<T>::pixel_type const* image<T>::get_row(std::size_t row) const
 {
-    return pData_ + row * dimensions_.width();
+    return data() + row * dimensions_.width();
 }
 
 template <typename T>
 inline const typename image<T>::pixel_type* image<T>::get_row(std::size_t row, std::size_t x0) const
 {
-    return pData_ + row * dimensions_.width() + x0;
+    return data() + row * dimensions_.width() + x0;
 }
 
 template <typename T>
 inline typename image<T>::pixel_type* image<T>::get_row(std::size_t row)
 {
-    return pData_ + row * dimensions_.width();
+    return data() + row * dimensions_.width();
 }
 
 template <typename T>
 inline typename image<T>::pixel_type* image<T>::get_row(std::size_t row, std::size_t x0)
 {
-    return pData_ + row * dimensions_.width() + x0;
+    return data() + row * dimensions_.width() + x0;
 }
 
 template <typename T>
@@ -260,7 +254,7 @@ inline void image<T>::set_row(std::size_t row, pixel_type const* buf, std::size_
 {
     assert(row < dimensions_.height());
     assert(size <= dimensions_.width());
-    std::copy(buf, buf + size, pData_ + row * dimensions_.width());
+    std::copy(buf, buf + size, get_row(row));
 }
 
 template <typename T>
@@ -268,7 +262,7 @@ inline void image<T>::set_row(std::size_t row, std::size_t x0, std::size_t x1, p
 {
     assert(row < dimensions_.height());
     assert ((x1 - x0) <= dimensions_.width() );
-    std::copy(buf, buf + (x1 - x0), pData_ + row * dimensions_.width() + x0);
+    std::copy(buf, buf + (x1 - x0), get_row(row, x0));
 }
 
 template <typename T>

test/unit/imaging/image.cpp

@@ -363,4 +363,30 @@ SECTION("Image copy/move")
     }
 }
 
+SECTION("image::swap")
+{
+    auto blue = mapnik::color(50, 50, 250).rgba();
+    auto orange = mapnik::color(250, 150, 0).rgba();
+
+    mapnik::image_rgba8 im;
+    mapnik::image_rgba8 im2(16, 16);
+    mapnik::image_rgba8 im3(16, 16);
+
+    im2.set(blue);
+    im3.set(orange);
+
+    // swap two non-empty images
+    CHECK_NOTHROW(im2.swap(im3));
+    CHECK(im2(0, 0) == orange);
+    CHECK(im3(0, 0) == blue);
+
+    // swap empty <-> non-empty
+    CHECK_NOTHROW(im.swap(im3));
+    CHECK(im3.data() == nullptr);
+    CHECKED_IF(im.data() != nullptr)
+    {
+        CHECK(im(0, 0) == blue);
+    }
+}
+
 } // END TEST CASE