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.

Sign up for GitHub

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

Jump to bottom

Interpolated Curvature remaining TODOs #7919

Merged
merged 11 commits into from
Jan 24, 2024
Merged

Interpolated Curvature remaining TODOs #7919

merged 11 commits into from
Jan 24, 2024

Conversation

hoskillua
Copy link
Member

@hoskillua hoskillua commented Dec 8, 2023 •
edited by sloriot
Loading

Summary of Changes

Remaining issue #7859 to be handled before 6.0 in order not to block the integration of the small feature branch on which two other PRs depend on.

Todos (taken from the issue)

  • Update Results & Performance section
  • Add a comment in the user manual about the relationship to the package Estimation of Local Differential Properties of Point-Sampled Surfaces
  • Move average_edge_length() and maybe other helper function in measure.h or other files.
  • Check that the display plugin works as expected
  • plugin for principal curvature directions?
  • plugin for principal curvature directions -> make sure the directions make sense in the concave (both negative) surface areas
  • remove Surface_mesh properties created in the plugin not specific to this method and not a big deal.
  • restrict named parameters (for @sloriot it is easy to make a mistake with and without the_map suffix)
  • Why is ball radius default -1 and not 0

@hoskillua
Copy link
Member Author

hoskillua commented Dec 8, 2023 •
edited
Loading

  • Why is ball radius default -1 and not 0

This was discussed with the authors during the project. There are mainly 3 cases for the input:

  • val > 0 -> the general case and a ball radius of val is assigned
  • val = 0 -> a small epsilon (avg_edge_length * 1e-6) is used (to account for convergance of values at infinitely small balls)
  • va < 0 -> no ball raduis is used. The sum is instead computed over the incident faces of the vertex
    So, a ball radius is used only if the user provides a sensical value.

@sloriot
Copy link
Member

sloriot commented Dec 8, 2023

We should add in the doc what happens if val = 0 as it is not documented for now.

@hoskillua
Copy link
Member Author

/force-build:v1

@github-actions GitHub Actions
Copy link

The documentation is built. It will be available, after a few minutes, here: https://cgal.github.io/7919/v1/Manual/index.html

@hoskillua
Copy link
Member Author

We should add in the doc what happens if val = 0 as it is not documented for now.

This was added here: https://cgal.github.io/7919/v1/Polygon_mesh_processing/index.html#ICCBackground

@hoskillua
Copy link
Member Author

/force-build:v1

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 6, 2024

There was an error while building the doc:

/home/runner/work/cgal/cgal/Polygon_mesh_processing/doc/Polygon_mesh_processing/Polygon_mesh_processing.txt:1097: warning: expected <tr> or <caption> tag but found <colgroup> instead!
/home/runner/work/cgal/cgal/Polygon_mesh_processing/doc/Polygon_mesh_processing/Polygon_mesh_processing.txt:1098: warning: Unsupported xml/html tag <col> found
/home/runner/work/cgal/cgal/Polygon_mesh_processing/doc/Polygon_mesh_processing/Polygon_mesh_processing.txt:1099: warning: Unsupported xml/html tag <col> found
/home/runner/work/cgal/cgal/Polygon_mesh_processing/doc/Polygon_mesh_processing/Polygon_mesh_processing.txt:1100: warning: Unsupported xml/html tag <col> found
/home/runner/work/cgal/cgal/Polygon_mesh_processing/doc/Polygon_mesh_processing/Polygon_mesh_processing.txt:1101: warning: Unsupported xml/html tag <col> found
/home/runner/work/cgal/cgal/Polygon_mesh_processing/doc/Polygon_mesh_processing/Polygon_mesh_processing.txt:1102: warning: Unsupported xml/html tag <col> found
/home/runner/work/cgal/cgal/Polygon_mesh_processing/doc/Polygon_mesh_processing/Polygon_mesh_processing.txt:1103: warning: Unsupported xml/html tag <col> found
/home/runner/work/cgal/cgal/Polygon_mesh_processing/doc/Polygon_mesh_processing/Polygon_mesh_processing.txt:1104: warning: Unsupported xml/html tag </colgroup> found
/home/runner/work/cgal/cgal/Polygon_mesh_processing/doc/Polygon_mesh_processing/Polygon_mesh_processing.txt:1105: warning: Unsupported xml/html tag <thead> found
/home/runner/work/cgal/cgal/Polygon_mesh_processing/doc/Polygon_mesh_processing/Polygon_mesh_processing.txt:1114: warning: Unsupported xml/html tag </thead> found
/home/runner/work/cgal/cgal/Polygon_mesh_processing/doc/Polygon_mesh_processing/Polygon_mesh_processing.txt:1115: warning: Unsupported xml/html tag <tbody> found
/home/runner/work/cgal/cgal/Polygon_mesh_processing/doc/Polygon_mesh_processing/Polygon_mesh_processing.txt:1203: warning: Unsupported xml/html tag </tbody> found

https://github.com/CGAL/cgal/actions/runs/7431644792

@hoskillua
Copy link
Member Author

/force-build:v1

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 6, 2024

The documentation is built. It will be available, after a few minutes, here: https://cgal.github.io/7919/v1/Manual/index.html

@hoskillua
Copy link
Member Author

/force-build:v1

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 6, 2024

The documentation is built. It will be available, after a few minutes, here: https://cgal.github.io/7919/v1/Manual/index.html

@hoskillua
Copy link
Member Author

/force-build:v1

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 6, 2024

The documentation is built. It will be available, after a few minutes, here: https://cgal.github.io/7919/v1/Manual/index.html

@hoskillua
Copy link
Member Author

/force-build:v1

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 6, 2024

The documentation is built. It will be available, after a few minutes, here: https://cgal.github.io/7919/v1/Manual/index.html

@sloriot sloriot mentioned this pull request Jan 8, 2024
Open
@sloriot
Copy link
Member

sloriot commented Jan 24, 2024

Successfully tested in CGAL-6.0-Ic-156

@sloriot sloriot linked an issue Jan 24, 2024 that may be closed by this pull request
Interpolated Curvature remaining TODOs #7859
Closed
8 tasks
@lrineau lrineau added this to the 6.0-beta milestone Jan 24, 2024
@lrineau lrineau self-assigned this Jan 24, 2024
@lrineau lrineau added the rm only: ready for master For the release team only: that indicates that a PR is about to be merged in 'master' label Jan 24, 2024
@lrineau lrineau merged commit 47324c1 into CGAL:master Jan 24, 2024
9 checks passed
@lrineau lrineau removed the rm only: ready for master For the release team only: that indicates that a PR is about to be merged in 'master' label Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@lrineau lrineau

Labels
Enhancement Merged_in_6.0 Pkg::PMP Tested
Projects
None yet
Milestone
6.0-beta
Development

Successfully merging this pull request may close these issues.

Interpolated Curvature remaining TODOs
3 participants
@hoskillua @sloriot @lrineau