please integrate patches for Boost.Gil and Boost.Graph

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

please integrate patches for Boost.Gil and Boost.Graph

Boost - Dev mailing list
I am not sure what the correct procedure for this is, but there are at least two old patches that would improve Boost.Gil and Boost.Graph.  I would greatly appreciate it if someone integrates these.  I have been using these for some time now with no detrimental effects.

The patch to Boost.Gil corrects clearly incorrect code.

The patch to Boost.Graph solves a problem that should never have been in the original design in the first place.  Since it expands the acceptable set of arguments to cover the full range of values any user would expect and has the original semantics for cases that worked before, as far as I can see there can be no effect on existing user code.

Thank you very much.

Cheers,
Brook



See the following:

- https://lists.boost.org/Archives/boost/2013/02/200721.php
- https://svn.boost.org/trac10/ticket/7270
- https://svn.boost.org/trac10/ticket/9517
- https://svn.boost.org/trac10/attachment/ticket/9517/gil_channel_algorithm.patch
- https://svn.boost.org/trac10/ticket/13397

--- boost/gil/channel_algorithm.hpp.orig 2018-02-23 22:45:42.000000000 -0700
+++ boost/gil/channel_algorithm.hpp 2018-03-11 22:05:31.000000000 -0600
@@ -25,6 +25,7 @@
 
 #include "gil_config.hpp"
 #include "channel.hpp"
+#include <limits>
 #include <boost/mpl/less.hpp>
 #include <boost/mpl/integral_c.hpp>
 #include <boost/mpl/greater.hpp>
@@ -51,7 +52,7 @@
 
 
 template <typename UnsignedIntegralChannel>
-struct unsigned_integral_max_value : public mpl::integral_c<UnsignedIntegralChannel,-1> {};
+struct unsigned_integral_max_value : public mpl::integral_c<UnsignedIntegralChannel,std::numeric_limits<UnsignedIntegralChannel>::max()> {};
 
 template <>
 struct unsigned_integral_max_value<uint8_t> : public mpl::integral_c<uint32_t,0xFF> {};



Allow dimensions to have length >= 1, rather than > 1.
See https://svn.boost.org/trac10/ticket/11735.

--- boost/graph/grid_graph.hpp.orig 2017-12-13 16:56:43.000000000 -0700
+++ boost/graph/grid_graph.hpp 2018-04-07 09:33:56.000000000 -0600
@@ -630,15 +630,17 @@
 
         // If the vertex is on the edge of this dimension, then its
         // number of out edges is dependent upon whether the dimension
-        // wraps or not.
-        if ((vertex[dimension_index] == 0) ||
-            (vertex[dimension_index] == (length(dimension_index) - 1))) {
-          out_edge_count += (wrapped(dimension_index) ? 2 : 1);
-        }
-        else {
-          // Next and previous edges, regardless or wrapping
-          out_edge_count += 2;
-        }
+        // wraps or not, but only if the length is > 1.
+ if (length(dimension_index) > 1) {
+          if ((vertex[dimension_index] == 0) ||
+              (vertex[dimension_index] == (length(dimension_index) - 1))) {
+            out_edge_count += (wrapped(dimension_index) ? 2 : 1);
+          }
+          else {
+            // Next and previous edges, regardless or wrapping
+            out_edge_count += 2;
+          }
+ }
       }
 
       return (out_edge_count);


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: please integrate patches for Boost.Gil and Boost.Graph

Boost - Dev mailing list
On 9 April 2018 at 17:07, Brook Milligan via Boost
<[hidden email]> wrote:
> I am not sure what the correct procedure for this is, but there are at least two old patches that would improve Boost.Gil and Boost.Graph.  I would greatly appreciate it if someone integrates these.  I have been using these for some time now with no detrimental effects.
>
> The patch to Boost.Gil corrects clearly incorrect code.
>
> - https://svn.boost.org/trac10/ticket/7270
>- https://svn.boost.org/trac10/ticket/9517


I've closed both tickets as closed.

The fixes are in the develop branch and whic his planned to be
released with Boost 1.68

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: please integrate patches for Boost.Gil and Boost.Graph

Boost - Dev mailing list
On 9 April 2018 at 17:20, Mateusz Loskot <[hidden email]> wrote:

> On 9 April 2018 at 17:07, Brook Milligan via Boost
> <[hidden email]> wrote:
>> I am not sure what the correct procedure for this is, but there are at least two old patches that would improve Boost.Gil and Boost.Graph.  I would greatly appreciate it if someone integrates these.  I have been using these for some time now with no detrimental effects.
>>
>> The patch to Boost.Gil corrects clearly incorrect code.
>>
>> - https://svn.boost.org/trac10/ticket/7270
>>- https://svn.boost.org/trac10/ticket/9517
>
>
> I've closed both tickets as closed.

...as fixed, I mean :)

Thanks for the reminder.
--
Mateusz Loskot, http://mateusz.loskot.net

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: please integrate patches for Boost.Gil and Boost.Graph

Boost - Dev mailing list

> On Apr 9, 2018, at 9:21 AM, Mateusz Loskot via Boost <[hidden email]> wrote:
>
> On 9 April 2018 at 17:20, Mateusz Loskot <[hidden email]> wrote:
>> On 9 April 2018 at 17:07, Brook Milligan via Boost
>> <[hidden email]> wrote:
>>> I am not sure what the correct procedure for this is, but there are at least two old patches that would improve Boost.Gil and Boost.Graph.  I would greatly appreciate it if someone integrates these.  I have been using these for some time now with no detrimental effects.
>>>
>>> The patch to Boost.Gil corrects clearly incorrect code.
>>>
>>> - https://svn.boost.org/trac10/ticket/7270
>>> - https://svn.boost.org/trac10/ticket/9517
>>
>>
>> I've closed both tickets as closed.
>
> ...as fixed, I mean :)
>
> Thanks for the reminder.

Sure.  Thanks for the quick fix.

Would you mind taking a look at the patch for Boost.Graph?  The original limitation (fixed by the patch) to dimensions > 1 is entirely artificial and reduces the utility of this library.  Indeed, in generic code it is easily possible to have a dimension == 1.  It would be very nice to get this in.

Thank you very much.

Cheers,
Brook


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: please integrate patches for Boost.Gil and Boost.Graph

Boost - Dev mailing list
On 9 April 2018 at 18:03, Brook Milligan <[hidden email]> wrote:

>> On Apr 9, 2018, at 9:21 AM, Mateusz Loskot via Boost <[hidden email]> wrote:
>> On 9 April 2018 at 17:20, Mateusz Loskot <[hidden email]> wrote:
>>> On 9 April 2018 at 17:07, Brook Milligan via Boost
>>> <[hidden email]> wrote:
>>>> I am not sure what the correct procedure for this is, but there are at least two old patches that would improve Boost.Gil and Boost.Graph.  I would greatly appreciate it if someone integrates these.  I have been using these for some time now with no detrimental effects.
>>>>
>>>> The patch to Boost.Gil corrects clearly incorrect code.
>>>>
>>>> - https://svn.boost.org/trac10/ticket/7270
>>>> - https://svn.boost.org/trac10/ticket/9517
>>>
>>>
>>> I've closed both tickets as closed.
>>
>> ...as fixed, I mean :)
>>
>> Thanks for the reminder.
>
> Sure.  Thanks for the quick fix.
>
> Would you mind taking a look at the patch for Boost.Graph?

I know close to nothing about Boost.Graph. I'm useless here, I'm afraid.

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: please integrate patches for Boost.Gil and Boost.Graph

Boost - Dev mailing list
     On Apr 9, 2018, at 10:16 AM, Mateusz Loskot via Boost
     <[hidden email]> wrote:
     On 9 April 2018 at 18:03, Brook Milligan <[hidden email]> wrote:

     On Apr 9, 2018, at 9:21 AM, Mateusz Loskot via Boost
     <[hidden email]> wrote:
     On 9 April 2018 at 17:20, Mateusz Loskot <[hidden email]> wrote:

     On 9 April 2018 at 17:07, Brook Milligan via Boost
     <[hidden email]> wrote:

     I am not sure what the correct procedure for this is, but there are
     at least two old patches that would improve Boost.Gil and
     Boost.Graph.  I would greatly appreciate it if someone integrates
     these.  I have been using these for some time now with no
     detrimental effects.
     The patch to Boost.Gil corrects clearly incorrect code.
     - https://svn.boost.org/trac10/ticket/7270
     - https://svn.boost.org/trac10/ticket/9517

     I've closed both tickets as closed.

     ...as fixed, I mean :)
     Thanks for the reminder.

     Sure.  Thanks for the quick fix.
     Would you mind taking a look at the patch for Boost.Graph?

     I know close to nothing about Boost.Graph. I'm useless here, I'm
     afraid.

   OK, fair enough.
   So, a question for the broader audience: how does one get an issue like
   this resolved?
   Cheers,
   Brook

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: please integrate patches for Boost.Gil and Boost.Graph

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On 04/09/18 18:07, Brook Milligan via Boost wrote:

> I am not sure what the correct procedure for this is, but there are at least two old patches that would improve Boost.Gil and Boost.Graph.  I would greatly appreciate it if someone integrates these.  I have been using these for some time now with no detrimental effects.
>
> The patch to Boost.Gil corrects clearly incorrect code.
>
> The patch to Boost.Graph solves a problem that should never have been in the original design in the first place.  Since it expands the acceptable set of arguments to cover the full range of values any user would expect and has the original semantics for cases that worked before, as far as I can see there can be no effect on existing user code.
>
> Thank you very much.
>
> Cheers,
> Brook
>
>
>
> See the following:
>
> - https://lists.boost.org/Archives/boost/2013/02/200721.php
> - https://svn.boost.org/trac10/ticket/7270
> - https://svn.boost.org/trac10/ticket/9517
> - https://svn.boost.org/trac10/attachment/ticket/9517/gil_channel_algorithm.patch
> - https://svn.boost.org/trac10/ticket/13397
>
> --- boost/gil/channel_algorithm.hpp.orig 2018-02-23 22:45:42.000000000 -0700
> +++ boost/gil/channel_algorithm.hpp 2018-03-11 22:05:31.000000000 -0600
> @@ -25,6 +25,7 @@
>  
>   #include "gil_config.hpp"
>   #include "channel.hpp"
> +#include <limits>
>   #include <boost/mpl/less.hpp>
>   #include <boost/mpl/integral_c.hpp>
>   #include <boost/mpl/greater.hpp>
> @@ -51,7 +52,7 @@
>  
>  
>   template <typename UnsignedIntegralChannel>
> -struct unsigned_integral_max_value : public mpl::integral_c<UnsignedIntegralChannel,-1> {};
> +struct unsigned_integral_max_value : public mpl::integral_c<UnsignedIntegralChannel,std::numeric_limits<UnsignedIntegralChannel>::max()> {};

This requires `std::numeric_limits<UnsignedIntegralChannel>::max()` to
be constexpr, i.e. a C++11 compiler. It has to be done differently for
compatibility with C++03, for example:

 
static_cast<UnsignedIntegralChannel>(~static_cast<UnsignedIntegralChannel>(0u))

The original code is also correct, although it might generate some
warnings, which could be resolved with a cast:

   static_cast<UnsignedIntegralChannel>(-1)

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: please integrate patches for Boost.Gil and Boost.Graph

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On 9 April 2018 at 19:17, Brook Milligan via Boost
<[hidden email]> wrote:
>
>      I know close to nothing about Boost.Graph. I'm useless here, I'm
>      afraid.
>
>    OK, fair enough.
>    So, a question for the broader audience: how does one get an issue like
>    this resolved?

You need to create a pull request. Unfortunately the graph maintainer
is not available at the moment. I'm helping with the next release, but
I'll probably limit myself to getting the changes in develop ready for
release, and fixing C++17 issues. That said, I'm more likely to merge
a pull request with unit tests and, if necessary, documentation
updates, if the need for it is clear.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost