invlerp and remap implementation
#2654
Conversation
It's because I had to use |
I'm just saying to not touch Also, not sure how much it really matters, but you could |
Starbuck5
added
New API
src_c/math.c
Outdated
|
|
||
| if (!PyNumber_Check(min) || !PyNumber_Check(max) || !PyNumber_Check(value)) | ||
| return RAISE(PyExc_TypeError, | ||
| "invlerp requires all the arguments to be numbers"); |
There was a problem hiding this comment.
improvement suggestion: instead of checking this explicitly, we can instead call PyFloat_AsDouble directly, and then handle the error on a per-argument basis? That we can also tell which of the arguments is incorrect
There was a problem hiding this comment.
So If I understood, I should write something like this ? :
double v = PyFloat_AsDouble(value);
if (PyErr_Occurred())
return RAISE(PyExc_ValueError,
"invalid `arg` values passed to remap, the value might "
"be too small or too big");
double a = PyFloat_AsDouble(i_min);
// same here
double b = PyFloat_AsDouble(i_max);
// same here
double c = PyFloat_AsDouble(o_min);
// same here
double d = PyFloat_AsDouble(o_max);
// same hereThere was a problem hiding this comment.
Well, we could do a simple return NULL as the error is already set. But the error won't be really specific
Good question, one approach would of course be a macro that does that, just add it after every double conversion and give it the argument number or name or whatever and make it just add the error message with that name in that place using the macro (make sure you undef it after usage). I guess another approach is a function, but that seems somewhat unnecessary for this. So yeah, probably a macro if you want to do that. |
|
Why is this implemented in C? |
Why not? I think for this current PR it would be a bit awkward & somewhat confusing to have 90% of the current math library in C and just two functions not written in C. It is a worthwhile question to ask though and I would like more of pygame's code to be written in python. If you are pondering re-implementing the whole of |
Because bigwhoop's feature request had the functions written in Python. It would be trivial to just monkey patch them in in Less code on our end, might even be more performant. |
I'd like to see a pull request like this - both to see how it looks as an example case, and to see some performance testing. I think performance is definitely relevant for a math function like this that could be called repeatedly in tight loop drawing code. |
|
Here is a simple performance test script for testing the import pygame
import random
import time
use_python = False
objs = [
(
random.uniform(-100, 100),
random.uniform(-100, 100),
random.uniform(0, 1),
)
for _ in range(5000)
]
def py_invlerp(a, b, v, /):
return (v - a) / (b - a)
start = time.time()
if use_python:
for arg in objs:
py_invlerp(*arg)
else:
for arg in objs:
pygame.math.invlerp(*arg)
print(time.time() - start)I'm on python 3.12 and the C version implemented in this PR is more performant than the python version. However, at the rate at which the language is evolving (with the jit compiler and stuff), the difference is only going to lessen over the years, seems like. On this specific case, I still would support the C implementation because it is simple enough, and if we ever decide to do a python re-implementation: it's a one-liner. |
|
I highly believe that at this stage of implementation, and because the rest of math lib is written in C, that we keep this implementation. |
Implementation of #2649
Docs will come in a short time.