Skip to content

Conversation

Tejtex
Copy link

@Tejtex Tejtex commented Sep 15, 2025

This PR refactors math utilities and related code to use Point2 tuples consistently instead of mixing x, y float parameters and Point2:

Changes

  • Updated core math functions:

    • get_distance(x1, y1, x2, y2)get_distance(pos1: Point2, pos2: Point2)

    • rotate_point(x, y, cx, cy, angle)rotate_point(point: Point2, center: Point2, angle)

    • get_angle_degrees and get_angle_radians now take Point2 pairs.

  • Fixed get_angle_radians: previously used math.atan2(x, y), now corrected to math.atan2(y, x).

  • Updated all call sites across the codebase (drawing functions, physics, easing, examples, etc.) to use the new signatures.

  • Improved readability and reduced chance of mismatching parameters.

@Cleptomania
Copy link
Member

Cleptomania commented Sep 15, 2025

I'm not sure if I'm in favor of requiring the use of Point2 objects here, the current API leaves it up to the user if they want to have tuple positions, but changing to this would require them to do it. While tuples are pretty performant it's not exactly a zero cost to add tuples into functions which are likely to be used in heavily performance sensitive pieces of code

@Tejtex
Copy link
Author

Tejtex commented Sep 15, 2025

Yes but the issue is not not using Point2 but the incosistency. In some function x,y is used and in other Point2. This is really confusing, because in one function you put something in a tuple and in the other x, y. If the performance is an issue shouldn't all Point2's be replaced by x, y?

@pushfoo
Copy link
Member

pushfoo commented Sep 16, 2025

This is really confusing

I agree it's not ideal. Please consider:

  1. There are good reasons for things being the way they are now
  2. We can probably document the quick fixes before attempting a longer-term option

See the end of comment after the break for details beyond "document it" with a call-out (:tip:?). This'll be a long comment, too. 😅

Quick Fixes

You Want Tuples as Points:

  1. Python's math.dist offers the API you want for get_distance
  2. Iterable unpacking with * makes using rotate_point easier:
point_to_rotate = (0, 1)
center = (1,1)
rotated = rotate_point(*point_to_rotate, *center, angle_degrees=45)

You Want OOP / Modifiable Shapes

The pyglet.shapes module may help. For maximum efficiency and customization, you will want custom shaders instead.

Why These Functions Look Different

TL;DR: A middle-ground design, history, and the way GPUs work

The Design Goals Of These Functions

TL;DR: Balance reliability and debugging ease against performance

  1. Type checking
  2. Bug resistance (avoiding copy-and-paste inlining)
    • Non-Python languages might inline small functions during compilation
    • In pyglet, manual inlining is worth the risk to gain perf (pure Python + a hard "zero dependencies" rule)
    • In Arcade, we ship Pymunk bindings in the box to avoid risking desync of copy-and-pasted code
  3. Good-Enough Performance
    • As Clepto said, packing and repacking tuples isn't free
    • It adds up when you shuffle a lot of them around
    • Doing it this way runs faster
  4. Readability
    • Densely inlined code is hard to read
    • It's hard to tell if what's wrong if we can't read it at a glance
History

The get_distance and rotate_point functions existed in another module before Python 3.8 added math.dist.

For example, rotate_point was in arcade.geometry in Arcade 2.6.7. That hints at why it looks so strange: it helps shape drawing and pure-Python collisions be reliable "get started fast" tools.

The pyglet.shapes Module Does The Same Thing Inside

If you look at pyglet.shapes:

  • The pyget.shapes.Line class takes the same arguments for the start and end points
  • Each shape type has:
    • no separate "point" tuples inside
    • an internal _vertex_list of axis values

That vertex list is how GPUs store numbers. They don't bother structuring it because:

  • it'd run slower
  • they leave it to higher-level abstractions to handle it

The pyglet.math.Vec* types are that abstraction for Python-side math.

What to Do?

These are the easiest options I see:

A Short-Term Fix: Document It

Add better explanations to the docstrings of:

  • arcade.math.get_distance
  • arcade.math.rotate_point

A Longer-Term Option: Upstream It

The pyglet.math module provides an OOP-based approach to math. It might be worthwhile to:

  1. Add rotation around points to the Vec2 and Vec3 types in pyglet.math
    • I've asked the maintainers about this over in their Discord
    • It seems like a reasonable helper set to add
    • We'd have to address how 2D and 3D points interact + type annotation concerns
  2. If all goes well, we'd then mark this specific rotate_point function as either:
    • deprecated
    • purpose-specific as outlined above

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