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

Jump to bottom

refactor: Replace gil::detail::copy_n with std::copy_n #686

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

refactor: Replace gil::detail::copy_n with std::copy_n #686

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0ce4215
refactor: Replace gil::detail::copy_n with std::copy_n
marco-langer Jun 11, 2022
46497d0
Merge remote-tracking branch 'origin/develop' into remove-copy-n
mloskot Jun 25, 2022
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
6 changes: 3 additions & 3 deletions include/boost/gil/algorithm.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ struct copier_n<iterator_from_2d<IL>,O> {
while (n>0) {
diff_t l=src.width()-src.x_pos();
diff_t numToCopy=(n<l ? n:l);
detail::copy_n(src.x(), numToCopy, dst);
std::copy_n(src.x(), numToCopy, dst);
dst+=numToCopy;
src+=numToCopy;
n-=numToCopy;
Expand All @@ -218,7 +218,7 @@ struct copier_n<I,iterator_from_2d<OL>> {
while (n>0) {
diff_t l=dst.width()-dst.x_pos();
diff_t numToCopy=(n<l ? n:l);
detail::copy_n(src, numToCopy, dst.x());
std::copy_n(src, numToCopy, dst.x());
dst+=numToCopy;
src+=numToCopy;
n-=numToCopy;
Expand All @@ -241,7 +241,7 @@ struct copier_n<iterator_from_2d<IL>,iterator_from_2d<OL>> {
while (n>0) {
diff_t l=dst.width()-dst.x_pos();
diff_t numToCopy=(n<l ? n : l);
detail::copy_n(src.x(), numToCopy, dst.x());
std::copy_n(src.x(), numToCopy, dst.x());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added line #L244 was not covered by tests

Hmm, this one is interesting. I'd have expected those to be fully covered.

Copy link
Contributor Author

@marco-langer marco-langer Jun 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any test cases for core\algorithm\copy_pixels. The other specializations of struct copier_n are apparently triggered by some unrelated test cases from extension or image processing tests AFAIS.

Edit: this is not true, there are some legacy tests, which do not cover this case. However, the Jamfile in test\legacy says these tests should not be refactored or extended. Thus, should we rather add new copy_pixels tests in core\algorithm\copy_pixels.cpp?

dst+=numToCopy;
src+=numToCopy;
n-=numToCopy;
Expand Down
9 changes: 4 additions & 5 deletions include/boost/gil/image_processing/kernel.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#ifndef BOOST_GIL_IMAGE_PROCESSING_KERNEL_HPP
#define BOOST_GIL_IMAGE_PROCESSING_KERNEL_HPP

#include <boost/gil/utilities.hpp>
#include <boost/gil/point.hpp>

#include <boost/assert.hpp>
Expand Down Expand Up @@ -110,7 +109,7 @@ class kernel_1d : public detail::kernel_1d_adaptor<std::vector<T, Allocator>>
kernel_1d(FwdIterator elements, std::size_t size, std::size_t center)
: parent_t(size, center)
{
detail::copy_n(elements, size, this->begin());
std::copy_n(elements, size, this->begin());
}

kernel_1d(kernel_1d const& other) : parent_t(other) {}
Expand All @@ -134,7 +133,7 @@ class kernel_1d_fixed : public detail::kernel_1d_adaptor<std::array<T, Size>>
explicit kernel_1d_fixed(FwdIterator elements, std::size_t center)
: parent_t(center)
{
detail::copy_n(elements, Size, this->begin());
std::copy_n(elements, Size, this->begin());
}

kernel_1d_fixed(kernel_1d_fixed const& other) : parent_t(other) {}
Expand Down Expand Up @@ -284,7 +283,7 @@ class kernel_2d : public detail::kernel_2d_adaptor<std::vector<T, Allocator>>
kernel_2d(FwdIterator elements, std::size_t size, std::size_t center_y, std::size_t center_x)
: parent_t(static_cast<int>(std::sqrt(size)), center_y, center_x)
{
detail::copy_n(elements, size, this->begin());
std::copy_n(elements, size, this->begin());
}

kernel_2d(kernel_2d const& other) : parent_t(other) {}
Expand Down Expand Up @@ -318,7 +317,7 @@ class kernel_2d_fixed :
: parent_t(center_y, center_x)
{
this->square_size = Size;
detail::copy_n(elements, Size * Size, this->begin());
std::copy_n(elements, Size * Size, this->begin());
}

kernel_2d_fixed(kernel_2d_fixed const& other) : parent_t(other) {}
Expand Down
41 changes: 1 addition & 40 deletions include/boost/gil/utilities.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
#pragma GCC diagnostic pop
#endif

#include <algorithm>
#include <cmath>
#include <cstddef>
#include <functional>
Expand Down Expand Up @@ -165,45 +164,6 @@ const OutPtr gil_reinterpret_cast_c(const In* p)

namespace detail {

////////////////////////////////////////////////////////////////////////////////
/// \brief copy_n taken from SGI STL.
////////////////////////////////////////////////////////////////////////////////

template <class InputIter, class Size, class OutputIter>
std::pair<InputIter, OutputIter> _copy_n(InputIter first, Size count,
OutputIter result, std::input_iterator_tag)
{
for ( ; count > 0; --count)
{
*result = *first;
++first;
++result;
}
return std::pair<InputIter, OutputIter>(first, result);
}

template <class RAIter, class Size, class OutputIter>
inline std::pair<RAIter, OutputIter>
_copy_n(RAIter first, Size count, OutputIter result, std::random_access_iterator_tag)
{
RAIter last = first + count;
return std::pair<RAIter, OutputIter>(last, std::copy(first, last, result));
}

template <class InputIter, class Size, class OutputIter>
inline std::pair<InputIter, OutputIter>
_copy_n(InputIter first, Size count, OutputIter result)
{
return _copy_n(first, count, result, typename std::iterator_traits<InputIter>::iterator_category());
}

template <class InputIter, class Size, class OutputIter>
inline std::pair<InputIter, OutputIter>
copy_n(InputIter first, Size count, OutputIter result)
{
return detail::_copy_n(first, count, result);
}

/// \brief identity taken from SGI STL.
template <typename T>
struct identity
Expand All @@ -213,6 +173,7 @@ struct identity
const T& operator()(const T& val) const { return val; }
};

// TODO replace with transparent std::plus in C++14
/// \brief plus function object whose arguments may be of different type.
template <typename T1, typename T2>
struct plus_asymmetric {
Expand Down