From df3c2b8748be7f0c40331a9ecdba1786199a2819 Mon Sep 17 00:00:00 2001 From: TimTheBig <132001783+TimTheBig@users.noreply.github.com> Date: Fri, 9 May 2025 13:59:57 -0400 Subject: [PATCH 1/4] Fix review comments --- Cargo.toml | 9 ++++----- src/expansion/expansion.rs | 1 - src/types.rs | 7 ++++++- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 9dd8c68..130cb1d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [workspace] members = ["test_utils"] exclude = ["fuzz"] -# 2 so msrv does not hold up dependence versions like in not 3 +# v2 so msrv does not hold up dependence versions like in v3 resolver = "2" [package] @@ -13,7 +13,7 @@ rust-version = "1.85.1" repository = "https://github.com/glennDittmann/geogram_predicates" authors = ["Glenn Dittmann", "TimTheBig"] -license = "LGPL-3.0" +license = "LGPL-3.0 OR MIT" keywords = ["computer-graphics", "math", "geometry", "predicates", "robust"] categories = ["no-std::no-alloc", "mathematics", "graphics"] @@ -29,9 +29,8 @@ cxx = { version = "1.0", features = ["alloc", "c++20"], default-features = false cxx-build = { version = "1.0", optional = true } [dev-dependencies] -float_extras = "0.1.6" +float_extras = "0.1" test_utils = { path = "test_utils" } [features] -alloc = ["cxx?/alloc"] -legacy = ["dep:cxx", "dep:cxx-build", "alloc"] +legacy = ["dep:cxx", "dep:cxx-build"] diff --git a/src/expansion/expansion.rs b/src/expansion/expansion.rs index 9ccd3c8..d596956 100644 --- a/src/expansion/expansion.rs +++ b/src/expansion/expansion.rs @@ -24,7 +24,6 @@ impl fmt::Display for Expansion { } impl PartialEq for Expansion { - // todo check if this should be `self.data == other.data` fn eq(&self, other: &Self) -> bool { self.equals(other) } diff --git a/src/types.rs b/src/types.rs index ee3b039..a413c0a 100644 --- a/src/types.rs +++ b/src/types.rs @@ -1,6 +1,9 @@ use core::cmp::Ordering; -/// A helper for functions that return signs +/// A helper for functions that return signs. +/// +/// Note: Sign's memory layout is `repr(i8)` so it can be freely converted to i8, +/// and back in some cases like multiply. #[derive(PartialEq, Eq, PartialOrd, Ord, Debug, Clone)] #[repr(i8)] pub enum Sign { @@ -54,6 +57,8 @@ impl PartialOrd for Sign { impl core::ops::Mul for Sign { type Output = Sign; + // It's more efficient then the safe alternative a match, + // as a integer multiply is faster then a 9 branch match. #[inline] fn mul(self, rhs: Self) -> Self::Output { // SAFETY: Sign is repr(i8) and will be (-1, 0, 1) so the product must be (1, 0, -1) From 441884d98ae2e40140e28d913910e7ceae3264dd Mon Sep 17 00:00:00 2001 From: TimTheBig <132001783+TimTheBig@users.noreply.github.com> Date: Fri, 9 May 2025 14:03:55 -0400 Subject: [PATCH 2/4] Update README.md --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index ee41654..8a0a0d6 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ Be sure to check it out [here](https://github.com/BrunoLevy/geogram). It yields easy access to dependency-free parts of its code base, as so called _Pluggable Software Modules_ (PSM), which in turn make it easy to write `cxx_bridges` for these. -This library is licenced under the LGPL-3.0, when the legacy feature is enabled the c++ code it uses is under Apache-2.0. +This library is licenced under the `LGPL-3.0 OR MIT`, when the legacy feature is enabled the c++ code it uses is under `Apache-2.0`. ## Example @@ -71,7 +71,7 @@ Below are visualizations comparing naive and robust `orient_2d` & `in_circle_2d` ### Other - [x] det_4d() (not in rust) -- [x] geo_sgn() +- [x] geo_sgn() (renamed to geo_sign) - [x] initialize() (not in rust) - [x] show_stats() (not in rust) - [x] terminate() (not in rust) From 9d804cc78ea5897491d1c057ff03c7f7f8e56481 Mon Sep 17 00:00:00 2001 From: TimTheBig <132001783+TimTheBig@users.noreply.github.com> Date: Fri, 9 May 2025 14:06:40 -0400 Subject: [PATCH 3/4] Update workflow --- .github/workflows/rust.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 7c0230f..325f7a2 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -2,7 +2,9 @@ name: Rust on: push: - branches: [ "main" ] + branches: ["main"] + pull_request: + branches: ["*"] env: CARGO_TERM_COLOR: always From 0b30b59c39c28183bc51047f0ce1c752ef79b3af Mon Sep 17 00:00:00 2001 From: Ryan <132001783+TimTheBig@users.noreply.github.com> Date: Fri, 9 May 2025 14:09:56 -0400 Subject: [PATCH 4/4] Rename CI jobs --- .github/workflows/rust.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 325f7a2..71b8296 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -10,7 +10,7 @@ env: CARGO_TERM_COLOR: always jobs: - build: + test: runs-on: ubuntu-latest steps: @@ -23,7 +23,7 @@ jobs: - name: Test run: cargo test - build-legacy: + test-legacy: runs-on: ubuntu-latest steps: