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.

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

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

Conversation

marco-langer
Copy link
Contributor

Description

This PR replaces Gil's gil::detail::copy_n with std::copy_n.

Tasklist

  • Ensure all CI builds pass
  • Review and approve

@codecov
Copy link

codecov bot commented Jun 11, 2022

Codecov Report

Merging #686 (46497d0) into develop (151fd9c) will decrease coverage by 0.05%.
The diff coverage is 85.71%.

@@             Coverage Diff             @@
##           develop     #686      +/-   ##
===========================================
- Coverage    80.32%   80.27%   -0.06%     
===========================================
  Files          117      117              
  Lines         5032     5019      -13     
===========================================
- Hits          4042     4029      -13     
  Misses         990      990              

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

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?

@mloskot mloskot added cat/refactoring Any nonfunctional changes core boost/gil ext/image_processing boost/gil/extension/image_processing labels Jun 28, 2022
@mloskot mloskot added this to the Boost 1.81+ milestone Jul 7, 2022
@mloskot mloskot modified the milestones: Boost 1.82, Boost 1.83+ Mar 31, 2023
@mloskot mloskot modified the milestones: Boost 1.86, 1.87.0 Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cat/refactoring Any nonfunctional changes core boost/gil ext/image_processing boost/gil/extension/image_processing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants