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

Isosurfacing_3 (new package) #6849

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

Isosurfacing_3 (new package) #6849

wants to merge 379 commits into from

Conversation

JulyCode
Copy link
Member

@JulyCode JulyCode commented Sep 15, 2022 •
edited by MaelRL
Loading

Summary of Changes

Extracting isosurfaces from scalar functions is a common method for generating 3D meshes. An isosurface is a subspace with equal values of the function.

ToDo:
Preview Give feedback

Extra:
Preview Give feedback

Release Management

  • Affected package(s): Isosurfacing_3
  • Issue(s) solved (if any): -
  • Feature/Small Feature (if any): Isosurfacing_3
  • License and copyright ownership: see feature

@JulyCode

This comment was marked as outdated.

Sign in to view
@MaelRL MaelRL marked this pull request as ready for review September 15, 2022 12:14
@afabri
Copy link
Member

afabri commented Sep 15, 2022

/build:v1

@MaelRL MaelRL added Not yet approved The feature or pull-request has not yet been approved. Feature Work in progress labels Sep 15, 2022
@github-actions
Copy link

There was an error while building the doc:
/home/runner/work/cgal/cgal/Isosurfacing_3/include/CGAL/Octree_wrapper.h:330: warning: The following parameters of CGAL::Isosurfacing::Octree_wrapper::e_glIndex(const std::size_t &e, const std::size_t &i_idx, const std::size_t &j_idx, const std::size_t &k_idx, const std::size_t &depth) const are not documented:

@afabri

This comment was marked as outdated.

Sign in to view
@afabri
Copy link
Member

afabri commented Sep 15, 2022 •
edited
Loading

The data files have a problem with rights. I think they also need o+r
But more importantly they should be in the package Data so that we do not have millions of bunnies and skulls.

@sloriot
Copy link
Member

sloriot commented Oct 14, 2024

Successfully tested in CGAL-6.0.1-Ic-345

@lrineau
Copy link
Member

lrineau commented Oct 14, 2024

@afabri You requested change (negative review0; a few months ago: #6849 (review).

Can you please re-review, if you now agree with the changes, so that the red flag on the PR disappears?

@lrineau
Copy link
Member

lrineau commented Oct 14, 2024

@soesau Can you please merge master and fix the conflict in the file Orthtree/include/CGAL/Orthtree.h?

@github-actions github-actions bot removed the Tested label Oct 14, 2024
@github-actions GitHub Actions
Copy link

This pull-request was previously marked with the label Tested, but has been modified with new commits. That label has been removed.

@afabri
Copy link
Member

afabri commented Dec 16, 2024

@MaelRL we have to take a look at the dependencies. Why Mesh_3 for example ?

@sloriot
Copy link
Member

sloriot commented Dec 24, 2024

@JulyCode the test you added uses some data that you did not add. Could you please commit them or shall I remove the test?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@janetournois janetournois janetournois left review comments

@lrineau lrineau lrineau left review comments

@MaelRL MaelRL MaelRL left review comments

@afabri afabri afabri requested changes

@sloriot sloriot sloriot left review comments

Assignees
No one assigned
Labels
CHANGES.md not updated Feature gsoc gsoc-2022 Not yet approved The feature or pull-request has not yet been approved. Pkg::Isosurfacing_3 TODO (web) TODO Under Testing
Projects
None yet
Milestone
6.1-beta
Development

Successfully merging this pull request may close these issues.
9 participants
@JulyCode @afabri @sloriot @MaelRL @soesau @ange-clement @lrineau @janetournois @palliez