Skip to content

Line2: use T instead of double in APIs (backport #809) - #832

Merged
caguero merged 1 commit into
gz-math7from
mergify/bp/gz-math7/pr-809
Aug 25, 2026
Merged

Line2: use T instead of double in APIs (backport #809)#832
caguero merged 1 commit into
gz-math7from
mergify/bp/gz-math7/pr-809

Conversation

@mergify

@mergify mergify Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

🎉 New feature

Fixes some windows compiler warnings noticed in #804 and #807

Summary

While testing support for python bindings on Windows in #804, I noticed a bunch of C4244 compiler warnings from Line2.hh:

conversion from 'double' to 'T', possible loss of data

Then I noticed that Line2 is a templated class but still uses double in many API arguments. This updates the API to use T more consistently. There are still some instances of double arguments in the Slope function and for tolerance arguments, so static_casts are added to handle these type conversions.

Summary from Gemini edited by me:

  1. Constructor & Set arguments changed from double to T.
  2. CrossProduct return type changed from double to T.
  3. Updated Collinear, Parallel, Within, and Intersect to cast zero constants and _epsilon to T, avoiding mixed double/T arithmetic.

Since these are template APIs, it might be safe to backport, but I'm not sure.

Backport Policy

  • This is safe to backport to the following versions:
    • Jetty
    • Ionic
    • Harmonic
    • Fortress
  • This should not be backported
  • I am not sure
  • Other (fill in yourself)

Test it

I don't see any uses of this class outside of gz-math, so verifying that CI passes should be sufficient

Checklist

  • Signed all commits for DCO
  • Added a screen capture or video to the PR description that demonstrates the feature
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • Updated Bazel files (if adding new files). Created an issue otherwise.
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers
  • Was GenAI used to generate this PR? If so, make sure to add "Assisted-by" to your commits. (See this policy for more info.)

Assisted-by: Gemini 3.6 Flash

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by and Generated-by messages.

Backports: If this is a backport, please use Rebase and Merge instead.


This is an automatic backport of pull request #809 done by [Mergify](https://mergify.com).

1. Set with Vector2 parameters: Verified setting start/end points
   using Vector2 instances.
2. CrossProduct: Added explicit tests for both Line2 and Vector2
   cross products. Expect CrossProduct of parallel lines is 0.
3. OnSegment and Within: Tested points on segment, collinear points
   outside bounds, non-collinear points within bounding box, and endpoints.
4. Intersect Branch Coverage:
    • Parallel collinear non-overlapping line segments.
    • Collinear lines where only the second endpoint lies on the segment.
    • Line extensions that intersect outside X or Y bounding ranges.
    • Intersect overload taking no output point.
5. Operators: Expanded operator== and operator!= self-comparison checks.
6. Template Types: Added test cases for Line2i (int) and Line2f (float).

Assisted-by: Gemini 3.6 Flash

Signed-off-by: Steve Peters <scpeters@intrinsic.ai>
(cherry picked from commit 59ed16c)
@github-project-automation github-project-automation Bot moved this from Inbox to In review in Core development Aug 25, 2026
@ahcorde
ahcorde enabled auto-merge (squash) August 25, 2026 14:30
@scpeters
scpeters disabled auto-merge August 25, 2026 14:47
@scpeters

Copy link
Copy Markdown
Member

Cancelling auto+merge until confirming that it is safe to backport

@ahcorde ahcorde closed this Aug 25, 2026
@github-project-automation github-project-automation Bot moved this from In review to Done in Core development Aug 25, 2026
@caguero caguero reopened this Aug 25, 2026
@github-project-automation github-project-automation Bot moved this from Done to Inbox in Core development Aug 25, 2026
@github-project-automation github-project-automation Bot moved this from Inbox to In review in Core development Aug 25, 2026
@caguero
caguero merged commit 96c53cc into gz-math7 Aug 25, 2026
17 checks passed
@caguero
caguero deleted the mergify/bp/gz-math7/pr-809 branch August 25, 2026 19:48
@github-project-automation github-project-automation Bot moved this from In review to Done in Core development Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🎵 harmonic Gazebo Harmonic

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants