Result change in geometry::centroid between version 1.53 and 1.58

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

Result change in geometry::centroid between version 1.53 and 1.58

eric.noirfalise
Dear All,

I'm having a result change issue in geometry::centroid between boost version
1.53 and 1.58 when I use input polygon composed of integer points and an output
double point.

Here is my code :

#include <boost/geometry.hpp>
#include <boost/geometry/geometries/point_xy.hpp>
#include <boost/geometry/geometries/polygon.hpp>


int main()
{
    typedef boost::geometry::model::d2::point_xy<int> PointInt;
    typedef boost::geometry::model::d2::point_xy<double> PointDouble;
    typedef boost::geometry::model::polygon<PointInt> polygon_type;

    polygon_type poly;
    boost::geometry::read_wkt(
        "POLYGON((1 1, 2 1, 2 2, 1 2, 1 1))", poly);

    PointDouble p;
    typedef boost::geometry::strategy::centroid::bashein_detmer<PointDouble,
PointInt, double> Strategy;
    boost::geometry::centroid(poly, p, Strategy());

    std::cout << "centroid: " << boost::geometry::dsv(p) << std::endl;

    return 0;
}

The result for boost 1.53 (which seems good for me) :
centroid: (1.5, 1.5)

And the result for boost 1.58 (which seems wrong for me) :
centroid: (1, 1)
I can also see new warnings during compilation with version 1.58.

Could you confirm me that this is a bug ?

Best Regards,
Eric Noirfalise

_______________________________________________
Geometry mailing list
[hidden email]
http://lists.boost.org/mailman/listinfo.cgi/geometry
Reply | Threaded
Open this post in threaded view
|

Re: Result change in geometry::centroid between version 1.53 and 1.58

Adam Wulkiewicz
Hi Eric,

Eric.noirfalise wrote:

> Dear All,
>
> I'm having a result change issue in geometry::centroid between boost version
> 1.53 and 1.58 when I use input polygon composed of integer points and an output
> double point.
>
> Here is my code :
>
> #include <boost/geometry.hpp>
> #include <boost/geometry/geometries/point_xy.hpp>
> #include <boost/geometry/geometries/polygon.hpp>
>
>
> int main()
> {
>      typedef boost::geometry::model::d2::point_xy<int> PointInt;
>      typedef boost::geometry::model::d2::point_xy<double> PointDouble;
>      typedef boost::geometry::model::polygon<PointInt> polygon_type;
>
>      polygon_type poly;
>      boost::geometry::read_wkt(
>          "POLYGON((1 1, 2 1, 2 2, 1 2, 1 1))", poly);
>
>      PointDouble p;
>      typedef boost::geometry::strategy::centroid::bashein_detmer<PointDouble,
> PointInt, double> Strategy;
>      boost::geometry::centroid(poly, p, Strategy());
>
>      std::cout << "centroid: " << boost::geometry::dsv(p) << std::endl;
>
>      return 0;
> }
>
> The result for boost 1.53 (which seems good for me) :
> centroid: (1.5, 1.5)
>
> And the result for boost 1.58 (which seems wrong for me) :
> centroid: (1, 1)
> I can also see new warnings during compilation with version 1.58.
>
> Could you confirm me that this is a bug ?

Yes, it's a bug.
Introduced in this PR https://github.com/boostorg/geometry/pull/155
This was released with Boost 1.57.

Could you please create a ticket?

Regards,
Adam
_______________________________________________
Geometry mailing list
[hidden email]
http://lists.boost.org/mailman/listinfo.cgi/geometry
Reply | Threaded
Open this post in threaded view
|

Re: Result change in geometry::centroid between version 1.53 and 1.58

eric.noirfalise
Hi Adam,

Done for ticket : https://svn.boost.org/trac/boost/ticket/11236

Thank you,
Eric

Selon Adam Wulkiewicz <[hidden email]>:

> Hi Eric,
>
> Eric.noirfalise wrote:
> > Dear All,
> >
> > I'm having a result change issue in geometry::centroid between boost
> version
> > 1.53 and 1.58 when I use input polygon composed of integer points and an
> output
> > double point.
> >
> > Here is my code :
> >
> > #include <boost/geometry.hpp>
> > #include <boost/geometry/geometries/point_xy.hpp>
> > #include <boost/geometry/geometries/polygon.hpp>
> >
> >
> > int main()
> > {
> >      typedef boost::geometry::model::d2::point_xy<int> PointInt;
> >      typedef boost::geometry::model::d2::point_xy<double> PointDouble;
> >      typedef boost::geometry::model::polygon<PointInt> polygon_type;
> >
> >      polygon_type poly;
> >      boost::geometry::read_wkt(
> >          "POLYGON((1 1, 2 1, 2 2, 1 2, 1 1))", poly);
> >
> >      PointDouble p;
> >      typedef
> boost::geometry::strategy::centroid::bashein_detmer<PointDouble,
> > PointInt, double> Strategy;
> >      boost::geometry::centroid(poly, p, Strategy());
> >
> >      std::cout << "centroid: " << boost::geometry::dsv(p) << std::endl;
> >
> >      return 0;
> > }
> >
> > The result for boost 1.53 (which seems good for me) :
> > centroid: (1.5, 1.5)
> >
> > And the result for boost 1.58 (which seems wrong for me) :
> > centroid: (1, 1)
> > I can also see new warnings during compilation with version 1.58.
> >
> > Could you confirm me that this is a bug ?
>
> Yes, it's a bug.
> Introduced in this PR https://github.com/boostorg/geometry/pull/155
> This was released with Boost 1.57.
>
> Could you please create a ticket?
>
> Regards,
> Adam
> _______________________________________________
> Geometry mailing list
> [hidden email]
> http://lists.boost.org/mailman/listinfo.cgi/geometry
>


_______________________________________________
Geometry mailing list
[hidden email]
http://lists.boost.org/mailman/listinfo.cgi/geometry
Reply | Threaded
Open this post in threaded view
|

Re: Result change in geometry::centroid between version 1.53 and 1.58

Adam Wulkiewicz
Eric.noirfalise wrote:
> Hi Adam,
>
> Done for ticket : https://svn.boost.org/trac/boost/ticket/11236

Thanks!

Actually it's a manifestation of a different bug, in arithmetic
functions, in this particular case in add_point().
See: https://github.com/boostorg/geometry/pull/283

You wrote in a ticket that the compiler reports a warning.
Could you paste it here or there?

Regards,
Adam

P.S. Top-posting is discouraged on this list

>
> Thank you,
> Eric
>
> Selon Adam Wulkiewicz <[hidden email]>:
>
>> Hi Eric,
>>
>> Eric.noirfalise wrote:
>>> Dear All,
>>>
>>> I'm having a result change issue in geometry::centroid between boost
>> version
>>> 1.53 and 1.58 when I use input polygon composed of integer points and an
>> output
>>> double point.
>>>
>>> Here is my code :
>>>
>>> #include <boost/geometry.hpp>
>>> #include <boost/geometry/geometries/point_xy.hpp>
>>> #include <boost/geometry/geometries/polygon.hpp>
>>>
>>>
>>> int main()
>>> {
>>>       typedef boost::geometry::model::d2::point_xy<int> PointInt;
>>>       typedef boost::geometry::model::d2::point_xy<double> PointDouble;
>>>       typedef boost::geometry::model::polygon<PointInt> polygon_type;
>>>
>>>       polygon_type poly;
>>>       boost::geometry::read_wkt(
>>>           "POLYGON((1 1, 2 1, 2 2, 1 2, 1 1))", poly);
>>>
>>>       PointDouble p;
>>>       typedef
>> boost::geometry::strategy::centroid::bashein_detmer<PointDouble,
>>> PointInt, double> Strategy;
>>>       boost::geometry::centroid(poly, p, Strategy());
>>>
>>>       std::cout << "centroid: " << boost::geometry::dsv(p) << std::endl;
>>>
>>>       return 0;
>>> }
>>>
>>> The result for boost 1.53 (which seems good for me) :
>>> centroid: (1.5, 1.5)
>>>
>>> And the result for boost 1.58 (which seems wrong for me) :
>>> centroid: (1, 1)
>>> I can also see new warnings during compilation with version 1.58.
>>>
>>> Could you confirm me that this is a bug ?
>> Yes, it's a bug.
>> Introduced in this PR https://github.com/boostorg/geometry/pull/155
>> This was released with Boost 1.57.
>>
>> Could you please create a ticket?
>>
>> Regards,
>> Adam
>> _______________________________________________
>> Geometry mailing list
>> [hidden email]
>> http://lists.boost.org/mailman/listinfo.cgi/geometry
>>
>
> _______________________________________________
> Geometry mailing list
> [hidden email]
> http://lists.boost.org/mailman/listinfo.cgi/geometry
>


_______________________________________________
Geometry mailing list
[hidden email]
http://lists.boost.org/mailman/listinfo.cgi/geometry
Reply | Threaded
Open this post in threaded view
|

Re: Result change in geometry::centroid between version 1.53 and 1.58

eric.noirfalise
Selon Adam Wulkiewicz <[hidden email]>:

> Eric.noirfalise wrote:
> > Hi Adam,
> >
> > Done for ticket : https://svn.boost.org/trac/boost/ticket/11236
>
> Thanks!
>
> Actually it's a manifestation of a different bug, in arithmetic
> functions, in this particular case in add_point().
> See: https://github.com/boostorg/geometry/pull/283
>
> You wrote in a ticket that the compiler reports a warning.
> Could you paste it here or there?
>
> Regards,
> Adam
>
> P.S. Top-posting is discouraged on this list
>
> >
> > Thank you,
> > Eric
> >
> > Selon Adam Wulkiewicz <[hidden email]>:
> >
> >> Hi Eric,
> >>
> >> Eric.noirfalise wrote:
> >>> Dear All,
> >>>
> >>> I'm having a result change issue in geometry::centroid between boost
> >> version
> >>> 1.53 and 1.58 when I use input polygon composed of integer points and an
> >> output
> >>> double point.
> >>>
> >>> Here is my code :
> >>>
> >>> #include <boost/geometry.hpp>
> >>> #include <boost/geometry/geometries/point_xy.hpp>
> >>> #include <boost/geometry/geometries/polygon.hpp>
> >>>
> >>>
> >>> int main()
> >>> {
> >>>       typedef boost::geometry::model::d2::point_xy<int> PointInt;
> >>>       typedef boost::geometry::model::d2::point_xy<double> PointDouble;
> >>>       typedef boost::geometry::model::polygon<PointInt> polygon_type;
> >>>
> >>>       polygon_type poly;
> >>>       boost::geometry::read_wkt(
> >>>           "POLYGON((1 1, 2 1, 2 2, 1 2, 1 1))", poly);
> >>>
> >>>       PointDouble p;
> >>>       typedef
> >> boost::geometry::strategy::centroid::bashein_detmer<PointDouble,
> >>> PointInt, double> Strategy;
> >>>       boost::geometry::centroid(poly, p, Strategy());
> >>>
> >>>       std::cout << "centroid: " << boost::geometry::dsv(p) << std::endl;
> >>>
> >>>       return 0;
> >>> }
> >>>
> >>> The result for boost 1.53 (which seems good for me) :
> >>> centroid: (1.5, 1.5)
> >>>
> >>> And the result for boost 1.58 (which seems wrong for me) :
> >>> centroid: (1, 1)
> >>> I can also see new warnings during compilation with version 1.58.
> >>>
> >>> Could you confirm me that this is a bug ?
> >> Yes, it's a bug.
> >> Introduced in this PR https://github.com/boostorg/geometry/pull/155
> >> This was released with Boost 1.57.
> >>
> >> Could you please create a ticket?
> >>
> >> Regards,
> >> Adam
> >> _______________________________________________
> >> Geometry mailing list
> >> [hidden email]
> >> http://lists.boost.org/mailman/listinfo.cgi/geometry
> >>
> >
> > _______________________________________________
> > Geometry mailing list
> > [hidden email]
> > http://lists.boost.org/mailman/listinfo.cgi/geometry
> >
>
>
> _______________________________________________
> Geometry mailing list
> [hidden email]
> http://lists.boost.org/mailman/listinfo.cgi/geometry
>

Hi Adam,
Sorry for previous top posting

Here is the (awful) cascaded warning stack under visual studio 2012 :

1>C:\dev\Externals\Package\boost\boost_1_58_0\include\boost/geometry/arithmetic/arithmetic.hpp(75):
warning C4244: 'argument' : conversion from 'double' to 'const int', possible
loss of data
1>        
C:\dev\Externals\Package\boost\boost_1_58_0\include\boost/geometry/util/for_each_coordinate.hpp(38)
: see reference to function template instantiation 'void
boost::geometry::detail::point_operation<PointSrc,Function>::apply<Point,0>(PointDst
&) const' being compiled
1>          with
1>          [
1>              PointSrc=boost::geometry::model::d2::point_xy<int>,
1>              Function=std::plus,
1>              Point=PointDouble,
1>              PointDst=PointDouble
1>          ]
1>        
C:\dev\Externals\Package\boost\boost_1_58_0\include\boost/geometry/util/for_each_coordinate.hpp(38)
: see reference to function template instantiation 'void
boost::geometry::detail::point_operation<PointSrc,Function>::apply<Point,0>(PointDst
&) const' being compiled
1>          with
1>          [
1>              PointSrc=boost::geometry::model::d2::point_xy<int>,
1>              Function=std::plus,
1>              Point=PointDouble,
1>              PointDst=PointDouble
1>          ]
1>        
C:\dev\Externals\Package\boost\boost_1_58_0\include\boost/geometry/util/for_each_coordinate.hpp(76)
: see reference to function template instantiation 'Op
boost::geometry::detail::coordinates_scanner<Point,Dimension,DimensionCount,IsConst>::apply<Op>(boost::geometry::model::d2::point_xy<CoordinateType>
&,Op)' being compiled
1>          with
1>          [
1>            
Op=boost::geometry::detail::point_operation<boost::geometry::model::d2::point_xy<int>,std::plus>,
1>              Point=PointDouble,
1>              Dimension=0,
1>              DimensionCount=2,
1>              IsConst=false,
1>              CoordinateType=double
1>          ]
1>        
C:\dev\Externals\Package\boost\boost_1_58_0\include\boost/geometry/util/for_each_coordinate.hpp(76)
: see reference to function template instantiation 'Op
boost::geometry::detail::coordinates_scanner<Point,Dimension,DimensionCount,IsConst>::apply<Op>(boost::geometry::model::d2::point_xy<CoordinateType>
&,Op)' being compiled
1>          with
1>          [
1>            
Op=boost::geometry::detail::point_operation<boost::geometry::model::d2::point_xy<int>,std::plus>,
1>              Point=PointDouble,
1>              Dimension=0,
1>              DimensionCount=2,
1>              IsConst=false,
1>              CoordinateType=double
1>          ]
1>        
C:\dev\Externals\Package\boost\boost_1_58_0\include\boost/geometry/arithmetic/arithmetic.hpp(148)
: see reference to function template instantiation 'void
boost::geometry::for_each_coordinate<Point1,boost::geometry::detail::point_operation<PointSrc,Function>>(Point
&,Op)' being compiled
1>          with
1>          [
1>              Point1=PointDouble,
1>              PointSrc=boost::geometry::model::d2::point_xy<int>,
1>              Function=std::plus,
1>              Point=PointDouble,
1>            
Op=boost::geometry::detail::point_operation<boost::geometry::model::d2::point_xy<int>,std::plus>
1>          ]
1>        
C:\dev\Externals\Package\boost\boost_1_58_0\include\boost/geometry/algorithms/detail/centroid/translating_transformer.hpp(106)
: see reference to function template instantiation 'void
boost::geometry::add_point<ResPt,boost::geometry::model::d2::point_xy<CoordinateType>>(Point1
&,const Point2 &)' being compiled
1>          with
1>          [
1>              ResPt=PointDouble,
1>              CoordinateType=int,
1>              Point1=PointDouble,
1>              Point2=boost::geometry::model::d2::point_xy<int>
1>          ]
1>        
C:\dev\Externals\Package\boost\boost_1_58_0\include\boost/geometry/algorithms/centroid.hpp(316)
: see reference to function template instantiation 'void
boost::geometry::detail::centroid::translating_transformer<Geometry>::apply_reverse<Point>(ResPt
&) const' being compiled
1>          with
1>          [
1>              Geometry=polygon_type,
1>              Point=PointDouble,
1>              ResPt=PointDouble
1>          ]
1>        
C:\dev\Externals\Package\boost\boost_1_58_0\include\boost/geometry/algorithms/centroid.hpp(316)
: see reference to function template instantiation 'void
boost::geometry::detail::centroid::translating_transformer<Geometry>::apply_reverse<Point>(ResPt
&) const' being compiled
1>          with
1>          [
1>              Geometry=polygon_type,
1>              Point=PointDouble,
1>              ResPt=PointDouble
1>          ]
1>        
C:\dev\Externals\Package\boost\boost_1_58_0\include\boost/geometry/algorithms/centroid.hpp(403)
: see reference to function template instantiation 'bool
boost::geometry::detail::centroid::centroid_polygon::apply<Geometry,Point,Strategy>(const
Polygon &,Point &,const Strategy &)' being compiled
1>          with
1>          [
1>              Geometry=polygon_type,
1>              Point=PointDouble,
1>            
Strategy=boost::geometry::strategy::centroid::bashein_detmer<PointDouble,PointInt,double>,
1>              Polygon=polygon_type
1>          ]
1>        
C:\dev\Externals\Package\boost\boost_1_58_0\include\boost/geometry/algorithms/centroid.hpp(509)
: see reference to function template instantiation 'void
boost::geometry::detail::centroid::centroid_linear_areal<Algorithm>::apply<Geometry,Point,Strategy>(const
Geometry &,Point &,const Strategy &)' being compiled
1>          with
1>          [
1>              Algorithm=boost::geometry::detail::centroid::centroid_polygon,
1>              Geometry=polygon_type,
1>              Point=PointDouble,
1>            
Strategy=boost::geometry::strategy::centroid::bashein_detmer<PointDouble,PointInt,double>
1>          ]
1>        
C:\dev\Externals\Package\boost\boost_1_58_0\include\boost/geometry/algorithms/centroid.hpp(509)
: see reference to function template instantiation 'void
boost::geometry::detail::centroid::centroid_linear_areal<Algorithm>::apply<Geometry,Point,Strategy>(const
Geometry &,Point &,const Strategy &)' being compiled
1>          with
1>          [
1>              Algorithm=boost::geometry::detail::centroid::centroid_polygon,
1>              Geometry=polygon_type,
1>              Point=PointDouble,
1>            
Strategy=boost::geometry::strategy::centroid::bashein_detmer<PointDouble,PointInt,double>
1>          ]
1>        
C:\dev\Externals\Package\boost\boost_1_58_0\include\boost/geometry/algorithms/centroid.hpp(546)
: see reference to function template instantiation 'void
boost::geometry::resolve_strategy::centroid<Geometry>::apply<Point,Strategy>(const
Geometry &,Point &,const Strategy &)' being compiled
1>          with
1>          [
1>              Geometry=polygon_type,
1>              Point=PointDouble,
1>            
Strategy=boost::geometry::strategy::centroid::bashein_detmer<PointDouble,PointInt,double>
1>          ]
1>        
C:\dev\Externals\Package\boost\boost_1_58_0\include\boost/geometry/algorithms/centroid.hpp(546)
: see reference to function template instantiation 'void
boost::geometry::resolve_strategy::centroid<Geometry>::apply<Point,Strategy>(const
Geometry &,Point &,const Strategy &)' being compiled
1>          with
1>          [
1>              Geometry=polygon_type,
1>              Point=PointDouble,
1>            
Strategy=boost::geometry::strategy::centroid::bashein_detmer<PointDouble,PointInt,double>
1>          ]
1>        
C:\dev\Externals\Package\boost\boost_1_58_0\include\boost/geometry/algorithms/centroid.hpp(604)
: see reference to function template instantiation 'void
boost::geometry::resolve_variant::centroid<Geometry>::apply<Point,Strategy>(const
Geometry &,Point &,const Strategy &)' being compiled
1>          with
1>          [
1>              Geometry=polygon_type,
1>              Point=PointDouble,
1>            
Strategy=boost::geometry::strategy::centroid::bashein_detmer<PointDouble,PointInt,double>
1>          ]
1>        
C:\dev\Externals\Package\boost\boost_1_58_0\include\boost/geometry/algorithms/centroid.hpp(604)
: see reference to function template instantiation 'void
boost::geometry::resolve_variant::centroid<Geometry>::apply<Point,Strategy>(const
Geometry &,Point &,const Strategy &)' being compiled
1>          with
1>          [
1>              Geometry=polygon_type,
1>              Point=PointDouble,
1>            
Strategy=boost::geometry::strategy::centroid::bashein_detmer<PointDouble,PointInt,double>
1>          ]
1>          TestCGAL.cpp(196) : see reference to function template instantiation
'void
boost::geometry::centroid<polygon_type,PointDouble,boost::geometry::strategy::centroid::bashein_detmer<Point,PointOfSegment,CalculationType>>(const
Geometry &,Point &,const Strategy &)' being compiled
1>          with
1>          [
1>              Point=PointDouble,
1>              PointOfSegment=PointInt,
1>              CalculationType=double,
1>              Geometry=polygon_type,
1>            
Strategy=boost::geometry::strategy::centroid::bashein_detmer<PointDouble,PointInt,double>
1>          ]

Regards,
Eric
_______________________________________________
Geometry mailing list
[hidden email]
http://lists.boost.org/mailman/listinfo.cgi/geometry
Reply | Threaded
Open this post in threaded view
|

Re: Result change in geometry::centroid between version 1.53 and 1.58

Adam Wulkiewicz
Eric.noirfalise wrote:

> Selon Adam Wulkiewicz <[hidden email]>:
>
>> Eric.noirfalise wrote:
>>> Hi Adam,
>>>
>>> Done for ticket : https://svn.boost.org/trac/boost/ticket/11236
>> Thanks!
>>
>> Actually it's a manifestation of a different bug, in arithmetic
>> functions, in this particular case in add_point().
>> See: https://github.com/boostorg/geometry/pull/283
>>
>> You wrote in a ticket that the compiler reports a warning.
>> Could you paste it here or there?
<snip>
> Here is the (awful) cascaded warning stack under visual studio 2012 :
>
> 1>C:\dev\Externals\Package\boost\boost_1_58_0\include\boost/geometry/arithmetic/arithmetic.hpp(75):
> warning C4244: 'argument' : conversion from 'double' to 'const int', possible
> loss of data
>

Ok, so after the fix is applied you shouldn't see this warning.
Could you verify that it works for you?
E.g. grab this file
https://github.com/awulkiew/geometry/blob/42d56f6116659336551e0b85a6a68dd908379923/include/boost/geometry/arithmetic/arithmetic.hpp
and simply replace in your copy of Boost 1.58.

Regards,
Adam
_______________________________________________
Geometry mailing list
[hidden email]
http://lists.boost.org/mailman/listinfo.cgi/geometry
Reply | Threaded
Open this post in threaded view
|

Re: Result change in geometry::centroid between version 1.53 and 1.58

eric.noirfalise
In reply to this post by eric.noirfalise

Eric.noirfalise wrote:

> Selon Adam Wulkiewicz <[hidden email]>:
>
>> Eric.noirfalise wrote:
>>> Hi Adam,
>>>
>>> Done for ticket : https://svn.boost.org/trac/boost/ticket/11236
>> Thanks!
>>
>> Actually it's a manifestation of a different bug, in arithmetic
>> functions, in this particular case in add_point().
>> See: https://github.com/boostorg/geometry/pull/283
>>
>> You wrote in a ticket that the compiler reports a warning.
>> Could you paste it here or there?
... [show rest of quote]
<snip>
> Here is the (awful) cascaded warning stack under visual studio 2012 :
>
>
1>C:\dev\Externals\Package\boost\boost_1_58_0\include\boost/geometry/arithmetic/arithmetic.hpp(75):
> warning C4244: 'argument' : conversion from 'double' to 'const int', possible
> loss of data
>

Ok, so after the fix is applied you shouldn't see this warning.
Could you verify that it works for you?
E.g. grab this file
https://github.com/awulkiew/geometry/blob/42d56f6116659336551e0b85a6a68dd908379923/include/boost/geometry/arithmetic/arithmetic.hpp
and simply replace in your copy of Boost 1.58.

Regards,
Adam
_______________________________________________
Geometry mailing list
[hidden email]
http://lists.boost.org/mailman/listinfo.cgi/geometry


Hi Adam,

Your patch seems great :
- no more warning under Visual studio 2012.
- corrected result in my case

Thank you very much,
Regards,
Eric Noirfalise
_______________________________________________
Geometry mailing list
[hidden email]
http://lists.boost.org/mailman/listinfo.cgi/geometry