Added pg_IntFromObjEx(), pg_TwoIntsFromObjEx() and pg_IntFromSeqIndexEx()

[itzpr3d4t0r]

Added three new C API functions:

• pg_IntFromObjEx
• pg_IntFromSeqIndexEx
• pg_TwoIntsFromObjEx

These three C API functions are supposed to be slightly faster than the current ones and have a different behaviour as they raise errors on their own, something that their current couterparts do not do. Errors should be more precise and provide more information, epecifically about what the error is, what it expected and what it got in terms of value/type/numebr of values.

You can either use a custom error message to override the default ones or pass NULL to use the predefined error messages and formatting. I recommend passing NULL most of the time as it will give you more precise and informative error messages. Only use a custom message when you need to add more description.

Modified:

• Rect.collidepoint() to use the new functionality as an example.

I'd like to higlight that in the case of collidepoint, passing a tupleof 2 items is about 83% faster and a list of 2 items about 38% faster. This could entail similar benefits for every function that prefers tuples/lists of 2 items.

Rect.collidepoint() main:

pygame-ce 2.1.4.dev1 (SDL 2.0.22, Python 3.11.0)

====||SEQUENCES||====
-) tup inside tup: 0.6059378 s
-) coll tup: 0.56530388 s
-) not coll tup: 0.5557564 s
-) coll list: 0.8165761 s
-) not coll list: 0.80312168 s
-) coll 2 args: 0.3694743 s
-) not coll 2 args: 0.36558976 s
-) coll unpack tuple 1->2: 0.63821076 s
-) not coll unpack tuple 1->2: 0.62786246 s
-) coll unpack list 1->2: 1.00910418 s
-) not coll unpack list 1->2: 0.98168378 s
______________
-) Mean time: 0.66714737

====||VECTORS||====
-) coll vector: 0.60935196 s
-) not coll vector: 0.61871136 s
______________
-) Mean time: 0.61403166

This PR:

pygame-ce 2.1.4.dev1 (SDL 2.0.22, Python 3.11.0)

====||SEQUENCES||====
-) tup inside tup: 0.31907396 s
-) coll tup: 0.30785382 s
-) not coll tup: 0.31080586 s
-) coll list: 0.59054886 s
-) not coll list: 0.59381622 s
-) coll 2 args: 0.31044572 s
-) not coll 2 args: 0.3121588 s
-) coll unpack tuple 1->2: 0.56166662 s
-) not coll unpack tuple 1->2: 0.5666255 s
-) coll unpack list 1->2: 0.9124658 s
-) not coll unpack list 1->2: 0.89807034 s
______________
-) Mean time: 0.51668468

====||VECTORS||====
-) coll vector: 0.51883372 s
-) not coll vector: 0.51085838 s
______________
-) Mean time: 0.51484605

I used this program to test the new collidepoint with the new C functions:

import pygame

from statistics import fmean
import timeit


def test(test_name: str, func: str, globs: dict, num: int,
         mode: str = "mean") -> float:
    value = 0
    if mode == "mean":
        value = fmean(timeit.repeat(func, globals=globs, number=num))
    elif mode == "cumulative":
        value = timeit.timeit(func, globals=globs, number=num)
    else:
        raise NotImplementedError("Incorrect test mode")
    print("-) " + test_name + f": {round(value, 8)} s")
    return value


def test_group(group_name: str, sequence: list[str],
               globs: dict,
               tests_names: tuple[str, ...] = None, num: int = 10_000_000,
               include_tot: bool = False, include_mean: bool = True,
               test_mode: str = "mean") -> float:
    print("\n====||" + group_name.upper() + "||====")

    test_data = [
        test(tests_names[i] if tests_names else str(i + 1), test_obj, globs,
             num, test_mode)
        for
        i, test_obj in
        enumerate(sequence)]

    print("______________")
    if include_tot:
        print(f"-) Total time: {round(sum(test_data), 8)}")

    if include_mean:
        print(f"-) Mean time: {round(fmean(test_data), 8)}")

    return fmean(test_data)


pygame.init()

tottime = 0
r1 = pygame.rect.Rect(0, 0, 10, 10)

tup = (5.0, 5.0)
lst = [5.0, 5.0]

ntup = (30.0, 30.0)
nlst = [30.0, 30.0]

vec1 = pygame.math.Vector2(5.0, 5.0)
vec2 = pygame.math.Vector2(30.0, 30.0)

NUMBER = 10000000
GLOB = {"r1": r1, "ntup": ntup, "nlst": nlst, "tup": tup, "lst": lst,
        "vec1": vec1, "vec2": vec2}

test_names = (
    "tup inside tup", "coll tup", "not coll tup", "coll list", "not coll list",
    "coll 2 args", "not coll 2 args", "coll unpack tuple 1->2",
    "not coll unpack tuple 1->2", "coll unpack list 1->2",
    "not coll unpack list 1->2")

test_group(
    "Sequences", [
        "r1.collidepoint(((5.0, 5.0), ))",
        "r1.collidepoint((5.0, 5.0))",
        "r1.collidepoint((30.0, 30.0))",
        "r1.collidepoint([5.0, 5.0])",
        "r1.collidepoint([30.0, 30.0])",
        "r1.collidepoint(5.0, 5.0)",
        "r1.collidepoint(30.0, 30.0)",
        "r1.collidepoint(*tup)",
        "r1.collidepoint(*ntup)",
        "r1.collidepoint(*lst)",
        "r1.collidepoint(*nlst)"
    ],
    GLOB,
    test_names
)

test_group(
    "Vectors", [
        "r1.collidepoint(vec1)",
        "r1.collidepoint(vec2)"
    ],
    GLOB,
    ("coll vector", "not coll vector")
)

[MyreMylar]

LGTM 👍

I guess time will see if we get some good speed ups from these.

[MyreMylar]

I get it going from (without this PR):

====||SEQUENCES||====
-) tup inside tup: 0.89526932 s
-) coll tup: 0.87893394 s
-) not coll tup: 0.8794718 s
-) coll list: 1.09282864 s
-) not coll list: 1.10122356 s
-) coll 2 args: 1.03318158 s
-) not coll 2 args: 1.06660838 s
-) coll unpack tuple 1->2: 1.11719524 s
-) not coll unpack tuple 1->2: 1.1178481 s
-) coll unpack list 1->2: 1.31008738 s
-) not coll unpack list 1->2: 1.24720098 s
______________
-) Mean time: 1.06725899

====||VECTORS||====
-) coll vector: 0.95589292 s
-) not coll vector: 0.90819832 s
______________
-) Mean time: 0.93204562

to (with this PR):

====||SEQUENCES||====
-) tup inside tup: 0.8181443 s
-) coll tup: 0.75252392 s
-) not coll tup: 0.74443626 s
-) coll list: 1.00662154 s
-) not coll list: 0.95291206 s
-) coll 2 args: 0.85712456 s
-) not coll 2 args: 0.85301936 s
-) coll unpack tuple 1->2: 1.01843844 s
-) not coll unpack tuple 1->2: 1.01575796 s
-) coll unpack list 1->2: 1.18465654 s
-) not coll unpack list 1->2: 1.18172594 s
______________
-) Mean time: 0.94412372

====||VECTORS||====
-) coll vector: 0.8472964 s
-) not coll vector: 0.85649714 s
______________
-) Mean time: 0.85189677

Process finished with exit code 0

Seems about 10% faster all over.

[itzpr3d4t0r]

I'll be reverting the implementation in rect.collidepoint as an example because of the changes that were made to the rect module.

[MyreMylar]

I see no harm in adding these into the internal pygame API for other PRs and developers to use. In the previous example usage (now removed from this PR because of other changes to Rect) there was a demonstrated 10% performance improvement using these functions over current implementations.

If using them doesn't turn out to provide a benefit long-term then developers won't use them and we could eventually clean them out of the developer API. Right now there seems to be a proven benefit and no change to our public facing API.

[yunline]

Thank you for this PR. I have found some problems.

I use this test function:

static PyObject*
int_cvt_test(PyObject* self, PyObject *arg) // METH_O
{   
    int val;
    if(!pg_IntFromObjEx(arg, &val, "This is an error msg.")){
        return NULL;
    }
    printf("Got value %d\n", val);
    Py_RETURN_NONE;
}

Problems

• TypeError if input object only has __int__

  A TypeError is raised if the input object has __int__or __float__ method but doesn't have __index__method. (Such as decimal.Decimal and fractionsFraction)

  Code

  class Num2:
      def __int__(self):
          return 1
  
  int_cvt_test(Num2())    # Should print "Got value 1"

  Current behavior:

  Traceback (most recent call last):
      File "x:\python\pygame_test\pgint.py", line 20, in <module>
          int_cvt_test(Num2())    # Got value 1
  TypeError: This is an error msg.
  

  From python 3.10+, PyLong_AsLong would no longer use __int__ to get the int value of object. This is the cause of the problem. In fact, this is also why you can't pass a float object to PyLong_AsLong.
  To check if object is able to be converted as int object, use PyNumber_Check.

• Some errors is still replaced with TypeError

  If any exception is raised inside of the __index__() method of the input object, a TypeError is always thrown out.

  Code

  class FooError(Exception):
      pass
  
  class Foo:
      def __index__(self):
          raise FooError("Fooooo.")
  
  int_cvt_test(Foo())    # Should raise "__main__.FooError: Fooooo."

  Current behavior:

  Traceback (most recent call last):
      File "x:\python\pygame_test\pgint.py", line 23, in <module>
          int_cvt_test(Foo())     # __main__.FooError: Fooooo.     
  TypeError: This is an error msg.
  

  Those exceptions should have a higher priority than the customized TypeError.

• Float input should also be able to cause OverflowError

  Code

  int_cvt_test(1e44) # should raise OverflowError

  Current behavior:

  No error is raised.
  

Solution

I had written another version in #2405 before I found this PR.

In this version, you don't need to override any exception raised inside. (Override is not good)
Only when the input object can't pass the PyNumber_Check, will the msg argument be used.

I hope my code would be helpful for you.

static int
pg_IntFromObjEx(PyObject *obj, int *val, const char *msg)
{
    int tmp_val;
    PyObject *tmp_obj;
    SDL_bool do_decref = SDL_FALSE;

    if (PyLong_Check(obj)) {
        tmp_obj = obj;
    }
    else if (PyNumber_Check(obj)) {
        tmp_obj = PyNumber_Long(obj);
        if (!tmp_obj)
            return 0;

        do_decref = SDL_TRUE;
    }
    else {
        PyErr_SetString(PyExc_TypeError ,msg);
        return 0;  // Not a number type
    }

    tmp_val = PyLong_AsLong(tmp_obj);

    if (do_decref)
        Py_DECREF(tmp_obj);

    if (tmp_val == -1 && PyErr_Occurred()) {
        return 0;
    }
    *val = tmp_val;
    return 1;
}

[Starbuck5]

This PR is not ready for merge, so I'm marking it as a draft.

[yunline]

All problems I mentioned have been fixed. LGTM👍