Skip to content

[WIP] GraphicsView: Add General_polygon_2 to QPainterPath converter. … #2059

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chgans
Copy link
Contributor

@chgans chgans commented Apr 16, 2017

Summary of Changes

This adds a converter function to convert from General_polygon_2 to QPainterPath.

I couldn't get the template parameter right, eg, express the CGAL::General_polygon_2 concept that depends on the class template parameter K, without having to include CGAL/General_polygon_2.h directly.
I have tried to define it with:

template < template < typename K_> class  T> 
QPainterPath operator()(const typename T<K>::General_polygon_2 &pgn) const;

And using it this way:

typedef CGAL::Epeck                               Kernel;
typedef CGAL::Gps_circle_segment_traits_2<Kernel> Traits_2;
typedef Traits_2::General_polygon_2               Polygon_2;
typedef CGAL::Qt::Converter<Kernel>               QtConvert;

Polygon_2 pgn;
QtConverter convert;
QPainterPath path = convert(pgn);

I have tried other ways to define it, but in the best case, I always ended up with errors along
"template argument deduction/substitution failed". The only other way around was to call the converter this way convert.operator()<...>(); But I think this defeat the purpose of the functor approach of the converter...

I've marked this PR as 'WIP', as I am not sure how to solve this extra template parameter.
As well, do you want me to add an example code to the GraphicsView module? Eg, export to image (PNG), or maybe export to SVG?

Release Management

@chgans
Copy link
Contributor Author

chgans commented Apr 16, 2017

Last thing, the converter itself cannot be used to convert the curve's own points, since they are not CGAL::Point_2<K>, hence all calls to CGAL::to_double()
Maybe this converter function shouldn't even be part of CGAL::Qt::Converter, maybe it should be moved to a specialisation for the circular GPS traits, or a generic 'Polygon converter' class templated on a GPS trait class...

@lrineau
Copy link
Member

lrineau commented Apr 19, 2017

For the moment, this pull-request triggers compilation errors, because the declaration of CGAL::General_polygon_2 is not known.

I suggest you use a forward declaration

template <typename Traits> class General_polygon_2;

in the CGAL namespace. That should fix the compilation issue.

As for the need for an example: yes, we would need an example, so that the new feature is tested daily, and does not rot in the future.

As far as I can see, your code can only deal with a CGAL::General_polygon_2 templated with CGAL::Arr_circle_segment_traits_2< Kernel >, am I right?

@lrineau
Copy link
Member

lrineau commented May 15, 2017

@chgans Have you seen my message (#2059 (comment)) about a month ago? Do you still have time to deal with this PR?

@chgans
Copy link
Contributor Author

chgans commented May 25, 2017

@lrineau I will update the PR, including the build fix and an example. I just need to find some time.
And yes, the code will only work with CGAL::Arr_circle_segment_traits_2< Kernel >, I tried to make it more general, but i've hit some issues with that (sorry can't remember the details).

@lrineau lrineau modified the milestone: 4.12-beta Jul 17, 2017
@lrineau lrineau modified the milestones: 4.12-beta, 4.13-beta Jan 24, 2018
@lrineau lrineau removed this from the 4.13-beta milestone Jun 26, 2018
@lrineau lrineau added this to the Trash / Attic milestone Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFC] GraphicsView: general polygon to QPainterPath converter
2 participants