Skip to content

Jf/standardize cpp params#61

Merged
tomvanmele merged 2 commits into
compas-dev:mainfrom
jf---:jf/standardize-cpp-params
Dec 13, 2025
Merged

Jf/standardize cpp params#61
tomvanmele merged 2 commits into
compas-dev:mainfrom
jf---:jf/standardize-cpp-params

Conversation

@jf---

@jf--- jf--- commented Dec 11, 2025

Copy link
Copy Markdown
Collaborator

implementation of #59

@jf---

jf--- commented Dec 11, 2025

Copy link
Copy Markdown
Collaborator Author

gosh need to rebase when #57 #58 are merged

@jf---

jf--- commented Dec 11, 2025

Copy link
Copy Markdown
Collaborator Author

hi @petrasvestartas I guess this is about applying the architecture you developed with foolish consistency

@petrasvestartas

Copy link
Copy Markdown
Collaborator

Since multiple files were changed, we need to merge to see if this works locally.
Let's start from the last pr.

@jf---

jf--- commented Dec 12, 2025

Copy link
Copy Markdown
Collaborator Author

hi @petrasvestartas your right the structure of the commits overlapped with other PR's I'll submit the precise changeset( the last 2 commits)

- subdivision: pass-by-value → Eigen::Ref<const>
- straight_skeleton_2: const& → Eigen::Ref<const>
- triangulation: Eigen::Ref → Eigen::Ref<const>
- meshing: remove extra & on Eigen::Ref, add const

eliminates unnecessary copies, ~240KB+ per call for large datasets
@jf--- jf--- force-pushed the jf/standardize-cpp-params branch from da29e0d to 040eb63 Compare December 13, 2025 09:07
@jf---

jf--- commented Dec 13, 2025

Copy link
Copy Markdown
Collaborator Author

now only the 2 relevant commits

@jf---

jf--- commented Dec 13, 2025

Copy link
Copy Markdown
Collaborator Author

@tomvanmele congrats with the move to mkdocs, great to get rid of rst directives in the python code.
after merging this PR, a good moment to cut a release?
@petrasvestartas curious to have your perspective.

@tomvanmele

Copy link
Copy Markdown
Member

@jf--- can i merge this?

@jf---

jf--- commented Dec 13, 2025

Copy link
Copy Markdown
Collaborator Author

@petrasvestartas perspective is highly relevant here

@petrasvestartas

Copy link
Copy Markdown
Collaborator

Thank you.

Yes we can merge.
All these changes for inputs, which we only read, so "const" is a better option.

P.S. it would be great in mk docs to bring the examples :)

1 similar comment
@petrasvestartas

Copy link
Copy Markdown
Collaborator

Thank you.

Yes we can merge.
All these changes for inputs, which we only read, so "const" is a better option.

P.S. it would be great in mk docs to bring the examples :)

@tomvanmele

Copy link
Copy Markdown
Member

Thank you.

Yes we can merge. All these changes for inputs, which we only read, so "const" is a better option.

P.S. it would be great in mk docs to bring the examples :)

the examples are already there

@tomvanmele tomvanmele merged commit 397ba98 into compas-dev:main Dec 13, 2025
8 checks passed
@tomvanmele

Copy link
Copy Markdown
Member

https://compas.dev/compas_cgal/examples/example_booleans/

@jf---

jf--- commented Dec 13, 2025

Copy link
Copy Markdown
Collaborator Author

was an exciting week; this PR and especially the squeaky fresh mkdocs are a perfect way to wrap up!

@petrasvestartas

Copy link
Copy Markdown
Collaborator

Thank you for these cool contributions!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants