boostorg
Defined a geometry for polyhedron
As suggested by @barendgehrels and @vissarion in comments #683, I defined geometry for polyhedral surfaces.
As far as I can judge the code, it won't work yet, so you will probably need to modify parts of the source code too.
I checked the code, after using it in one of the examples and tried to make sure that it doesn't throw any error, but I think there is a need to implement wkt support for a polyhedron, to see its working.
After spending a week on the codebase, it seems to me that wkt input and output depends on multiple header files, including dimensions check and concept check.
To read inputs, I added the following codes:-
read.hpp-
template<typename PolyhedralSurface>
struct polyhderal_surface_parser
{
typedef typename ring_return_type<PolyhedralSurface>::type ring_return_type;
typedef container_appender<ring_return_type> appender;
static inline void apply(tokenizer::iterator& it,
tokenizer::iterator const& end,
std::string const& wkt,
ring_return_type& ring)
{
handle_open_parenthesis(it, end, wkt);
int i = 0;
while(it != end && *it != ")"){
appender::apply(it, end, wkt, ring);
i++;
}
handle_close_parenthesis(it, end, wkt);
}
};
ring_type.hpp-
template <typename PolyhedralSurface>
struct ring_return_type<polyhedral_surface_tag, PolyhedralSurface>
{
typedef PolyhedralSurface& type;
};
template <typename PolyhedralSurface>
struct ring_type<polyhedral_surface_tag, PolyhedralSurface>
{
typedef typename std::remove_reference
<
typename ring_return_type<polyhedral_surface_tag, PolyhedralSurface>::type
>::type type;
};
PolyhedralSurface_concept.hpp-
#ifndef BOOST_GEOMETRY_GEOMETRIES_CONCEPTS_POLYHEDRALSURFACE_HPP
#define BOOST_GEOMETRY_GEOMETRIES_CONCEPTS_POLYHEDRALSURFACE_HPP
#include <boost/concept_check.hpp>
#include <boost/range/concepts.hpp>
#include <boost/geometry/core/access.hpp>
#include <boost/geometry/core/ring_type.hpp>
#include <boost/geometry/geometries/concepts/ring_concept.hpp>
namespace boost { namespace geometry { namespace concepts
{
template <typename Geometry>
class PolyhedralSurface
{
#ifndef DOXYGEN_NO_CONCEPT_MEMBERS
typedef typename ring_type<Geometry>::type ring_type;
//BOOST_CONCEPT_ASSERT( (concepts::Ring<ring_type>) );
public:
BOOST_CONCEPT_USAGE(PolyhedralSurface)
{
Geometry* polyhedralSurface = 0;
ring_type* ring = 0;
}
#endif
};
} // namepspace concepts
} // namespace geometry
} // namespace boost
#endif
I don't know why I am getting this error every time I build
file included from ../../../boost/geometry/core/ring_type.hpp:27,
from ../../../boost/geometry/core/closure.hpp:24,
from ../../../boost/geometry/geometry.hpp:34,
from 01_point_example.cpp:15:
../../../boost/geometry/core/coordinate_dimension.hpp: In instantiation of ‘struct
boost::geometry::traits::dimension<boost::geometry::model::ring<boost::geometry::model::point<double, 3,
boost::geometry::cs::cartesian>, true, true>, void>’:
../../../boost/geometry/core/coordinate_dimension.hpp:63:8: required from ‘struct
boost::geometry::core_dispatch::dimension<boost::geometry::point_tag,
boost::geometry::model::ring<boost::geometry::model::point<double, 3, boost::geometry::cs::cartesian>, true, true> >’
../../../boost/geometry/core/coordinate_dimension.hpp:58:8: required from ‘struct
boost::geometry::core_dispatch::dimension<boost::geometry::polyhedral_surface_tag,
boost::geometry::model::PolyhedralSurface<boost::geometry::model::ring<boost::geometry::model::point<double, 3,
boost::geometry::cs::cartesian>, true, true> > >’
../../../boost/geometry/core/coordinate_dimension.hpp:88:8: required from ‘struct
boost::geometry::dimension<boost::geometry::model::PolyhedralSurface<boost::geometry::model::ring<boost::geometry::
model::point<double, 3, boost::geometry::cs::cartesian>, true, true> > >’
../../../boost/geometry/io/wkt/read.hpp:567:51: required from ‘bool boost::geometry::detail::wkt::initialize(const tokenizer&,
const string&, const string&, boost::tokenizer<boost::char_separator<char> >::iterator&,
boost::tokenizer<boost::char_separator<char> >::iterator&) [with Geometry =
boost::geometry::model::PolyhedralSurface<boost::geometry::model::ring<boost::geometry::model::point<double, 3,
boost::geometry::cs::cartesian>, true, true> >; boost::geometry::detail::wkt::tokenizer =
boost::tokenizer<boost::char_separator<char> >; std::string = std::__cxx11::basic_string<char>;
boost::tokenizer<boost::char_separator<char> >::iterator = boost::token_iterator<boost::char_separator<char>,
__gnu_cxx::__normal_iterator<const char*, std::__cxx11::basic_string<char> >, std::__cxx11::basic_string<char> >]’
../../../boost/geometry/io/wkt/read.hpp:596:33: required from ‘static void
boost::geometry::detail::wkt::geometry_parser<Geometry, Parser, PrefixPolicy>::apply(const string&, Geometry&) [with
Geometry = boost::geometry::model::PolyhedralSurface<boost::geometry::model::ring<boost::geometry::model::point<double,
3, boost::geometry::cs::cartesian>, true, true> >; Parser = boost::geometry::detail::wkt::polyhderal_surface_parser;
PrefixPolicy = boost::geometry::detail::wkt::prefix_polyhedral_surface; std::string = std::__cxx11::basic_string<char>]’
../../../boost/geometry/io/wkt/read.hpp:942:70: required from ‘void boost::geometry::read_wkt(const string&, Geometry&) [with
Geometry = boost::geometry::model::PolyhedralSurface<boost::geometry::model::ring<boost::geometry::model::point<double,
3, boost::geometry::cs::cartesian>, true, true> >; std::string = std::__cxx11::basic_string<char>]’
01_point_example.cpp:124:282: required from here
../../../boost/geometry/core/static_assert.hpp:28:81: error: static assertion failed: Not implemented for this Point type.
28 | static_assert(boost::geometry::detail::static_assert_check<false, __VA_ARGS__>::value, MESSAGE)
| ^~~~~
../../../boost/geometry/core/coordinate_dimension.hpp:45:5: note: in expansion of macro
‘BOOST_GEOMETRY_STATIC_ASSERT_FALSE’
45 | BOOST_GEOMETRY_STATIC_ASSERT_FALSE(
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
I am also getting more error like the above, I need some help in understanding why this error turns up every time.
The message indicates that the code expects a point, or an instance implementing the Point Concept, but it is not a point.
Glancing through your code, the problem is probably here.
appender::apply(it, end, wkt, ring);
and it wants to append a point to a ring - but you get a ring, because you've a collection of rings here. Is that possible? It's just a guess, I didn't compile your code. You might comment that line and see if the error disappears (of course it won't work correctly then, but at least you know the line which needs some fixing).
Hey, @barendgehrels will you please review this PR, as I have implemented wkt read feature for polyhedral surfaces and I will also implement wkt write feature in a new PR, once this one gets merged.
@Siddharth-coder13 Somehow this PR #789 and PR #782 have become a mess and they both contain the same commits (e.g. Defined a geometry for polyhedron that is 0961662e6e45878eda378f85dd734ae1922f7110). I have to admit this makes it quite impossible to review. I don't know what happened.
@Siddharth-coder13 Somehow this PR #789 and PR #782 have become a mess and they both contain the same commits (e.g.
Defined a geometry for polyhedronthat is0961662e6e45878eda378f85dd734ae1922f7110). I have to admit this makes it quite impossible to review. I don't know what happened.
Actually, this PR is the most current version of my local repo, it contains all the changes I have done so far, since my last PR doesn't gets merged, so I have to make a new branch in order to contribute. There are three main commits :-
- Prefix wkt:z for 3d geometries.
- Definition of polyhedral surfaces.
- Wkt read implementation.
It's unfortunate. The 1 needs to be in the other separate PR and 2+3 as a separate feature in this PR.
I understand this may make your Git operations less convenient but these two are distinct features, they should be treated independently.
It's unfortunate. The 1 needs to be in the other separate PR and 2+3 as a separate feature in this PR.
I understand this may make your Git operations less convenient but these two are distinct features, they should be treated independently.
Yeah, I know both the things are different, using git sometimes is quite complex. I guess the only thing I can do now is to revert 1, and then this PR will become a separate PR for polyhedral surfaces.
You may review 2 and 3 if you have any changes to suggest, as I want this PR to get merged, for 1 I will submit another PR after discussion if Z is to add or not.
I'd strongly suggest to not to close any existing PRs and to not to open any new PRs.
- Prefix wkt:z for 3d geometries.
This should be in the PR #782
- Definition of polyhedral surfaces.
- Wkt read implementation.
These two should be in this PR #789
Since there should be no dependency between the two PRs, that is, the changes in #782 are not required by the #789, then I'd simply suggest to:
- In each of your local branches from which you submitted these PRS remove all commits that do not belong.
- You can
git rebase -i, or - You can rename this broken
3d-geometrieslocal branch to e.g.old/3d-geometries, create new branch named3d-geometriesbased ondevelop, thengit cherry-pickcorrect commits fromold/3d-geometriesto3d-geometries`
- You can
- Finally,
git push --forceto update the PRs.
By the way, this situation should also give you why it is a bad idea to develop any contributions in your fork's develop branch (like you did in PR #782). Instead, always create a topic branch and submit PR from a topic branch. That way, you can easily 'replace' PR branches in your remote without affecting your local develop branch. Topic branches save time!
@Siddharth-coder13 as an addition to the excellent comments from @mloskot you could also have a look at https://github.com/boostorg/geometry/wiki/Contribution-Tutorial
Hey @mloskot, I think this PR is able to merge now.
or suggest changes if any.
Hey @mloskot, I think this PR is able to merge now.
or suggest changes if any.
It’s big, important and needs to be reviewed and approved properly, by multiple people.
@Siddharth-coder13 Please, hit the "Resolve conversation" for the items that you have addressed. This way, we can see what's been done and what's not.
Hey, @barendgehrels Thanks for your reviews, but I think this point needs a separate PR, because a single PR can not define everything about polyhedral surfaces.
- documentation including an example for the user (
doc/src/examples/geometries/polyhedral_surface.cpp)
I will do the rest changes asap.
Hey, @barendgehrels Thanks for your reviews, but I think this point needs a separate PR, because a single PR can not define everything about polyhedral surfaces.
- documentation including an example for the user (
doc/src/examples/geometries/polyhedral_surface.cpp)I will do the rest changes asap.
Thank you! Success! I think a separate PR for documentation will be fine.
Hello @mloskot @barendgehrels @vissarion @awulkiew, as I have extended this PR as a project in GSOC'21 including other features of polyhedral surfaces. I want to get this PR merged before the GSOC period starts. It would be of great help if you leave your comment on this.
Hi @Siddharth-coder13 , sorry for the late reply. I think it looks good now!
There is no unit test yet, did you plan to do that in this PR or in a next one (along with the documentation)? Documentation we can do later but a unit test is normally included in a PR (especially for new stuff).
Hi @barendgehrels , thanks for the review. Actually, I added the example file of polyhedral surfaces, if unit test is needed in this PR, then I will add it asap.