Skip to content

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

Merged
ahcorde merged 1 commit into
gz-math9from
mergify/bp/gz-math9/pr-809
Aug 25, 2026
Merged

Line2: use T instead of double in APIs (backport #809)#830
ahcorde merged 1 commit into
gz-math9from
mergify/bp/gz-math9/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:29
@ahcorde
ahcorde merged commit 6c2f718 into gz-math9 Aug 25, 2026
16 checks passed
@ahcorde
ahcorde deleted the mergify/bp/gz-math9/pr-809 branch August 25, 2026 14:37
@github-project-automation github-project-automation Bot moved this from In review to Done in Core development Aug 25, 2026
@scpeters

Copy link
Copy Markdown
Member

@ahcorde I marked the parent PR as unsure whether to backport it

Did you investigate whether it is safe to do so?

@scpeters

Copy link
Copy Markdown
Member

I marked the parent PR as unsure whether to backport it

Did you investigate whether it is safe to do so?

I discussed with @caguero, who believes that it is safe

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

Labels

🪵 jetty Gazebo Jetty

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants