Skip to content

Implement scalar Point operations#71

Merged
simonask merged 5 commits intosimonask:mainfrom
Netzwerk2:point-scalar-ops
May 2, 2025
Merged

Implement scalar Point operations#71
simonask merged 5 commits intosimonask:mainfrom
Netzwerk2:point-scalar-ops

Conversation

@Netzwerk2
Copy link
Contributor

@Netzwerk2 Netzwerk2 commented May 1, 2025

Implement scalar operations similar to Vector. I saw there was a test which stated, that scalar multiplication should not compile. I decided to implement it anyway, because I don't really see a reason against it.

Addition of two points was also implemented.

Fixes #70

Note: This PR is based on #69, so it should be merged after #69 is merged.

@simonask
Copy link
Owner

simonask commented May 2, 2025

Thank you for this!

The reason it wasn't implemented is that scaling a point doesn't make geometric sense. Perhaps translation (addition) does, though.

I'm wondering what your use case is here. :-)

(Don't mind the CI failures, it's a problem on the main branch, I'm fixing it.)

@Netzwerk2
Copy link
Contributor Author

One of my use cases is calculating the center of a bounding box.

let center = 0.5 * bounds.min + 0.5 * bounds.max;

Another example would be calculating the vertex "center of mass" (i.e. the average of all vertices).

@simonask
Copy link
Owner

simonask commented May 2, 2025

For bounding boxes, I would recommend using Box2<T>, which has a center() method doing basically that. (Looking at it I'm realizing it may not be optimal - it probably won't use fmuladd, that should be checked and fixed if so.)

Vertex average is interesting. I'm wondering if it makes sense to have specific functionality for it, but there don't seem to be any specific prior art in the ecosystem, and I'm guessing that a generic implementation may not be sufficient in many cases, like if you wanted a scaled or weighted average.

I think I've talked myself into this. :-)

@simonask
Copy link
Owner

simonask commented May 2, 2025

#69 went in squashed and with a merge commit, so this may need rebasing. Sorry for the churn.

@Netzwerk2
Copy link
Contributor Author

#69 went in squashed and with a merge commit, so this may need rebasing. Sorry for the churn.

No worries, I'll rebase it soon.

For bounding boxes, I would recommend using Box2, which has a center() method doing basically that.

I'm using 3D bounds. Box3<T> is currently missing a lot of functionality from the Box2<T>, including center(). I'll open another issue for that :)

@Netzwerk2 Netzwerk2 force-pushed the point-scalar-ops branch from f1fdbdf to 51f7a7e Compare May 2, 2025 12:24
@codecov
Copy link

codecov bot commented May 2, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.31%. Comparing base (85e7da0) to head (1c51c8d).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #71   +/-   ##
=======================================
  Coverage   99.30%   99.31%           
=======================================
  Files          22       22           
  Lines        5919     5946   +27     
=======================================
+ Hits         5878     5905   +27     
  Misses         41       41           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@simonask simonask self-requested a review May 2, 2025 12:34
Copy link
Owner

@simonask simonask left a comment

Choose a reason for hiding this comment

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

Looks great, I'll add a CHANGELOG entry and merge ✨

@simonask simonask merged commit 6b4281b into simonask:main May 2, 2025
15 checks passed
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.

Scalar Point operations

2 participants