Skip to content

Add pmr image typedefs - #529

Merged
mloskot merged 5 commits into
boostorg:developfrom
sdebionne:add-pmr-image
Jun 28, 2022
Merged

Add pmr image typedefs#529
mloskot merged 5 commits into
boostorg:developfrom
sdebionne:add-pmr-image

Conversation

@sdebionne

@sdebionnesdebionne commented Nov 9, 2020

Copy link
Copy Markdown
Contributor

Description

when building with C++17 standard, adds pmr (polymorphic memroy resource) image typedefs (e.g. gil::pmr::gray8_image_t = using image<gray8_pixel_t, false, std::pmr::polymorphic_allocator<unsigned char>).

Fixes#528.

Tasklist

  • Conditionally include <memory_resource> and typedefs
  • Add test case(s)
  • Ensure all CI builds pass
  • Review and approve

@sdebionne
sdebionneforce-pushed the add-pmr-image branch 2 times, most recently from 56f968d to 979b7f1CompareMay 4, 2022 10:08
@codecov

codecovBot commented May 4, 2022

Copy link
Copy Markdown

Codecov Report

Merging #529 (59703e6) into develop (8d7034c) will increase coverage by 0.00%.
The diff coverage is n/a.

@@ Coverage Diff @@## develop #529 +/- ##
========================================
Coverage 80.84% 80.84% ========================================
Files 116 116 Lines 5121 5127 +6 ========================================
+ Hits 4140 4145 +5 - Misses 981 982 +1 

@sdebionne
sdebionne marked this pull request as ready for review May 4, 2022 10:38
@sdebionne
sdebionne requested a review from mloskotMay 4, 2022 11:55
@sdebionne

Copy link
Copy Markdown
ContributorAuthor

I would be happy to remove the #if !defined(BOOST_NO_CXX17_HDR_MEMORY_RESOURCE) guards if we decide to move to C++17 😁

@sdebionnesdebionne added the cat/annoyance Not a bug, not a feature, but something that should be improved label May 12, 2022
@sdebionnesdebionne added this to the Boost 1.80 milestone May 12, 2022
Comment threadinclude/boost/gil/image.hpp

@mloskotmloskot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

By the way, see #676

@mloskot

Copy link
Copy Markdown
Member

I have updated this PR with latest develop and I am going to merge it if no major CI issues are revealed.

@mloskot

mloskot commented Jun 27, 2022

Copy link
Copy Markdown
Member

The GCC 11 drops the warning:

include/boost/gil/image.hpp:241:12: warning: ‘if constexpr’ only available with ‘-std=c++17’ or ‘-std=gnu++17’
241 | if constexpr (std::allocator_traits<Alloc>::propagate_on_container_swap::value)
| ^~~~~~~~~

and C++14 compilation mode is failing, obviously. Some #ifdef-s are missing :)

@mloskot
mloskot merged commit 4dbf35a into boostorg:developJun 28, 2022
@sdebionne

sdebionne commented Jun 28, 2022

Copy link
Copy Markdown
ContributorAuthor

Thanks for looking into the issue, std::allocator_traits is C++11 but indeed if constexpr is C++17... I guess I would need to re-implement that part with SFINAE. But since we are switching to C++17 anyway...

@sdebionne
sdebionne deleted the add-pmr-image branch June 28, 2022 07:53
mloskot added a commit that referenced this pull request Jun 29, 2022
* develop:
test: Add more basic cases for image class (#423)
test: Add virtual_2d_locator fixture; is_2d_traversable test case
test: Check more properties of indexed_image_view from extension/toolbox
test: Add basic is_1d_traversable cases for image_view
chore: Update CMakeSettings.json sample [ci skip]
chore: Update CMake to use latest cmake-conan/0.18.1 [ci skip]
Add pmr image typedefs (#529)
test: Add test cases for image with empty dimensions (#702)
test: Case test_constructor_from_view was not called
fix: Memory leak in image class for empty dimensions (#649)
docs: Bump C++11 to C++14 as current required (#700)
ci: Remove C++11 build jobs after C++14 switch (#698)
build: Fix CMake source file extensions must be explicit
refactor: Switch to trailing return types (#599)
build: Bump Boost required by CMake from 1.72 to 1.80
build: Update CMAKE_CXX_STANDARD from 11 to 14
@mloskotmloskot mentioned this pull request Jul 5, 2022
6 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cat/annoyanceNot a bug, not a feature, but something that should be improved

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for pmr::polymorphic_allocator

2 participants

@sdebionne@mloskot