Skip to content
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

Add Float 2d snap rounding #8797

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

LeoValque
Copy link
Contributor

Please use the following template to help us managing pull requests.

Summary of Changes

Add to function that given a range of segments, round their coordinates to double while preserving the topology up to collapse

double_snap_rounding_2 output a range of Polyline corresponding to the input segments

compute_snapped_subcurves_2 output a range of segments

Release Management

  • Affected package(s): Snap rounding 2
  • Feature/Small Feature (if any):
  • Link to compiled documentation (obligatory for small feature) wrong link name to be changed
  • License and copyright ownership: GF

};

// Generic template signature of boxes, specialized for ID_FROM_HANDLE policy
#if 0
Copy link
Member

Choose a reason for hiding this comment

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

remove

#define CGAL_FLOAT_SNAP_ROUNDING_2_H


#include <iostream>
Copy link
Member

Choose a reason for hiding this comment

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

hidden behind debug macro?

}

template <class PointsRange , class PolylineRange>
void double_snap_rounding_2_disjoint(PointsRange &pts,
Copy link
Member

Choose a reason for hiding this comment

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

align arguments

PolylineRange &polylines)
{
using Point_2 = std::remove_cv_t<typename std::iterator_traits<typename PointsRange::iterator>::value_type>;
using Kernel = typename Kernel_traits<Point_2>::Kernel;
Copy link
Member

Choose a reason for hiding this comment

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

does it make sense to have named parameters for the traits? + point map maybe? Also we will need a traits concept with all the requirements of the function

Copy link
Member

Choose a reason for hiding this comment

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

If the traits is a parameter you won't a point map (can be integrated in a model of the concept of the traits), so no NP for now I'd say.

using PBox = CGAL::Box_intersection_d::Box_with_index_d<double,2>;
using SBox = CGAL::Box_intersection_d::Box_with_index_d<double,2>;

auto comp_by_x=[&](size_t ai, size_t bi){
Copy link
Member

Choose a reason for hiding this comment

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

size_t -> std::size_t everywhere

auto comp_by_x=[&](size_t ai, size_t bi){
return compare(pts[ai].x(),pts[bi].x())==SMALLER;
};
auto comp_by_y=[&](size_t ai, size_t bi){
Copy link
Member

Choose a reason for hiding this comment

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

You can use Less_x_2, Less_y_2 and Less_xy_2, part of your new concept ;)

std::set<size_t, decltype(comp_by_y_first)> p_sort_by_y(comp_by_y_first);
for(size_t i=0; i!=pts.size(); ++i)
{
p_sort_by_x.insert(i);
Copy link
Member

Choose a reason for hiding this comment

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

what about using std::vector + std::iota + std::sort?

for(size_t i=0; i<pts.size(); ++i)
points_boxes.emplace_back(pts[i].bbox(),i);
for(size_t i=0; i<polylines.size(); ++i)
segs_boxes.emplace_back(pts[polylines[i][0]].bbox()+pts[polylines[i][1]].bbox(),i);
Copy link
Member

Choose a reason for hiding this comment

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

use Construct_source_2 and Construct_target_2


// Bound the maximum squared distance between a point and its rounded value
auto round_bound=[](Point_2& p){
return std::pow(std::nextafter(p.x().approx().sup(),std::numeric_limits<double>::infinity())-p.x().approx().inf(),2)+
Copy link
Member

Choose a reason for hiding this comment

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

To_interval?

#ifdef DOUBLE_2D_SNAP_VERBOSE
std::cout << points_boxes.size()-size_before << " subdivisions performed" << std::endl;
#endif
for(size_t i=size_before; i<pts.size(); ++i)
Copy link
Member

Choose a reason for hiding this comment

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

reserve()?

@sloriot
Copy link
Member

sloriot commented Apr 1, 2025

Also, I think the user will be interested in having a correspondence between input and output segments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants