pygame._render: Changing/getting the line rendering methods used by the SDL2 render API#3710
Open
BatteRaquette581 wants to merge 8 commits intopygame-community:mainfrom
Open
pygame._render: Changing/getting the line rendering methods used by the SDL2 render API#3710BatteRaquette581 wants to merge 8 commits intopygame-community:mainfrom
BatteRaquette581 wants to merge 8 commits intopygame-community:mainfrom
Conversation
added 2 commits
February 9, 2026 18:12
…game._render, LINE_RENDER_* constants
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds line-rendering configuration: two functions Changes
Sequence DiagramsequenceDiagram
actor Python as Python Caller
participant RenderAPI as pygame._render (C)
participant SDL as SDL Library
rect rgba(100, 150, 200, 0.5)
Python->>RenderAPI: set_line_render_method(method:int)
RenderAPI->>RenderAPI: validate (0–3) and convert to string
RenderAPI->>SDL: SDL_SetHint(SDL_HINT_RENDER_LINE_METHOD, "N")
SDL-->>RenderAPI: success/failure
RenderAPI-->>Python: bool
end
rect rgba(150, 200, 100, 0.5)
Python->>RenderAPI: get_line_render_method()
RenderAPI->>SDL: SDL_GetHint(SDL_HINT_RENDER_LINE_METHOD)
SDL-->>RenderAPI: hint_string ("0"/"1"/"2"/"3")
RenderAPI->>RenderAPI: parse to int
RenderAPI-->>Python: int (0–3)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src_c/render.c`:
- Around line 1193-1214: In set_line_render_method, stop masking
PyArg_ParseTuple failures: when PyArg_ParseTuple(args, "i", &method) returns
false, return NULL instead of calling RAISE(...) so the original TypeError is
propagated; also replace the magic 48 with the character literal '0' when
building the hint string (hint[0] = method + '0'). Apply the same replacement of
48→'0' in the corresponding get_line_render_method function to keep behavior
consistent.
- Around line 1186-1191: get_line_render_method currently dereferences the
pointer returned by SDL_GetHint(SDL_HINT_RENDER_LINE_METHOD) which can be NULL;
update get_line_render_method to check if hint is NULL before accessing hint[0]
and return a sane default (e.g., 0) when NULL. Locate the get_line_render_method
function and change the logic to handle a NULL hint (e.g., if hint == NULL then
return PyLong_FromLong(0); otherwise parse the hint string safely—use atoi or
PyLong_FromLong((long)(hint[0] - '0'))—and return via PyLong_FromLong). Ensure
you still return a PyLong and avoid any dereference when SDL_GetHint returns
NULL.
- Around line 1186-1214: Guard uses of SDL_HINT_RENDER_LINE_METHOD with SDL
version checks: wrap the implementations/uses in get_line_render_method and
set_line_render_method with `#if` SDL_VERSION_ATLEAST(2,0,20) ... `#else` ... `#endif`
so the symbol is only referenced when available; in the `#else` branch provide a
safe fallback (e.g., return a default value or raise a Python exception
indicating the feature is unavailable) to avoid referencing the hint on SDL <
2.0.20. Ensure both the getter and setter are covered by the same conditional
and keep their public API behavior consistent in the fallback.
In `@test/render_test.py`:
- Around line 488-496: Update the test expectations to match the actual Python
exception raised by PyArg_ParseTuple: in test_correct_argument change the two
assertions for _render.set_line_render_method("foo") and
_render.set_line_render_method(None) to use assertRaises(TypeError) instead of
assertRaises(ValueError), while keeping the integer-bound checks as ValueError;
this aligns the test with the parsing behavior of the C API and the intended
render.c fix for argument validation in set_line_render_method.
🧹 Nitpick comments (1)
test/render_test.py (1)
498-506: Conditional assertions silently skip verification whenset_line_render_methodreturnsFalse.If
SDL_SetHintfails and returnsFalse, theassertEqualis skipped and the test passes vacuously. Consider asserting on the return value to ensure the hint was actually applied:♻️ Proposed refactor
def test_get_set_line_render_method(self): - if _render.set_line_render_method(pygame.LINE_RENDER_DEFAULT): - self.assertEqual(pygame.LINE_RENDER_DEFAULT, _render.get_line_render_method()) - if _render.set_line_render_method(pygame.LINE_RENDER_POINT): - self.assertEqual(pygame.LINE_RENDER_POINT, _render.get_line_render_method()) - if _render.set_line_render_method(pygame.LINE_RENDER_LINE): - self.assertEqual(pygame.LINE_RENDER_LINE, _render.get_line_render_method()) - if _render.set_line_render_method(pygame.LINE_RENDER_GEOMETRY): - self.assertEqual(pygame.LINE_RENDER_GEOMETRY, _render.get_line_render_method()) + self.assertTrue(_render.set_line_render_method(pygame.LINE_RENDER_DEFAULT)) + self.assertEqual(pygame.LINE_RENDER_DEFAULT, _render.get_line_render_method()) + self.assertTrue(_render.set_line_render_method(pygame.LINE_RENDER_POINT)) + self.assertEqual(pygame.LINE_RENDER_POINT, _render.get_line_render_method()) + self.assertTrue(_render.set_line_render_method(pygame.LINE_RENDER_LINE)) + self.assertEqual(pygame.LINE_RENDER_LINE, _render.get_line_render_method()) + self.assertTrue(_render.set_line_render_method(pygame.LINE_RENDER_GEOMETRY)) + self.assertEqual(pygame.LINE_RENDER_GEOMETRY, _render.get_line_render_method())
Author
|
New commit has been pushed, fixes SDL_GetHint possibly being NULL, removes that magic number, and ensures SDL2 >= 2.0.20. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@test.py`:
- Around line 1-7: Remove this accidental debug script (test.py) from the PR:
delete the file so it no longer shadows the standard library test package and
cannot be imported, and remove any references to
pygame._render.set_line_render_method("test") which incorrectly passes a string
to set_line_render_method (expects an int). If you intended to add a real test,
move it into the repository's tests/ directory and call set_line_render_method
with a valid integer in a proper test function instead of keeping a top-level
script.
Author
|
Temporary script left in now removed, oops! :P |
Contributor
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src_c/render.c`:
- Around line 1199-1202: The PyArg_ParseTuple failure is being masked by calling
RAISE(PyExc_ValueError,...); instead, preserve the original Python error by
removing that RAISE and simply return NULL when PyArg_ParseTuple(args, "i",
&method) fails. Update the code around the PyArg_ParseTuple call (remove the
RAISE(...) branch) so the function returns NULL directly, leaving the original
TypeError from PyArg_ParseTuple intact.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src_c/render.c`:
- Around line 1198-1201: The code calls PyArg_ParseTuple(args, "i", &method) and
on failure uses Py_RETURN_NONE which returns a non-NULL value while an exception
(TypeError) is active; change the failure path to return NULL so the Python
exception can propagate (replace Py_RETURN_NONE with return NULL in the
PyArg_ParseTuple failure branch handling the 'method' parse).
- Around line 1186-1218: The SDL hint functions get_line_render_method and
set_line_render_method use SDL_HINT_RENDER_LINE_METHOD which exists in SDL
2.0.20+ and SDL 3.2.0+ but not in SDL 3.0/3.1 previews; update the compile guard
around these functions to exclude 3.0.x and 3.1.x preview versions by changing
the `#if` condition to: (SDL_VERSION_ATLEAST(2, 0, 20) && !SDL_VERSION_ATLEAST(3,
0, 0)) || SDL_VERSION_ATLEAST(3, 2, 0) (or split into separate blocks like
similar guards in render.c) so the get_line_render_method and
set_line_render_method definitions are only built when the hint is actually
available.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src_c/render.c (1)
1190-1192: Handle empty-string hint defensively.If
SDL_GetHintreturns a non-NULL but empty string (""), thenhint[0] - '0'evaluates to'\0' - '0'= −48, which is out of the expected 0–3 range. While unlikely in practice, addinghint[0] == '\0'to the NULL check would be more robust.🛡️ Suggested fix
const char *hint = SDL_GetHint(SDL_HINT_RENDER_LINE_METHOD); return PyLong_FromLong( - hint == NULL ? 0 : hint[0] - '0'); // hint is a char between 0-3 + (hint == NULL || hint[0] == '\0') ? 0 : hint[0] - '0'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src_c/render.c` around lines 1190 - 1192, The current return uses hint[0]-'0' without guarding against an empty string; update the check after calling SDL_GetHint(SDL_HINT_RENDER_LINE_METHOD) to ensure hint is non-NULL and hint[0] != '\0' before computing hint[0] - '0', otherwise treat it as 0 (or a safe default) when creating the PyLong via PyLong_FromLong; reference the local variable hint, the SDL_GetHint call and PyLong_FromLong in your change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src_c/render.c`:
- Around line 1344-1352: The method table _render_methods currently exposes
get_line_render_method and set_line_render_method under a different `#if` guard
than their definitions, which causes undefined symbol errors on SDL 3.0.x/3.1.x;
update the preprocessor guard around the two method entries in _render_methods
to exactly match the compound condition used where the functions are defined:
(SDL_VERSION_ATLEAST(2, 0, 20) && !SDL_VERSION_ATLEAST(3, 0, 0)) ||
SDL_VERSION_ATLEAST(3, 2, 0), so the entries for get_line_render_method and
set_line_render_method are only compiled when those functions are actually
defined.
---
Nitpick comments:
In `@src_c/render.c`:
- Around line 1190-1192: The current return uses hint[0]-'0' without guarding
against an empty string; update the check after calling
SDL_GetHint(SDL_HINT_RENDER_LINE_METHOD) to ensure hint is non-NULL and hint[0]
!= '\0' before computing hint[0] - '0', otherwise treat it as 0 (or a safe
default) when creating the PyLong via PyLong_FromLong; reference the local
variable hint, the SDL_GetHint call and PyLong_FromLong in your change.
…orted SDL versions
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request adds 2 methods to get/set the SDL2 SDL_HINT_RENDER_LINE_METHOD
hint:
pygame._render.get_line_render_method() -> int: returns an integer (between 0 and 3) of the current line rendering method being used (can be any of the constants described below).pygame._render.set_line_render_method(method: int) -> bool: sets the current line rendering method with an integer (between 0 and 3, one of the constants described below), and returnsTrueif the hint has been applied, else returnsFalse.As described above, 4 new constants have been added for the line rendering method hint (see the SDL2 SDL_HINT_RENDER_LINE_METHOD page):
pygame.LINE_RENDER_DEFAULT = 0: default behavior, equivalent topygame.LINE_RENDER_POINT = 1as of SDL 2.0.20.pygame.LINE_RENDER_POINT = 1: uses the driver's point API and the Bresenham line algorithm to draw the line (always correct).pygame.LINE_RENDER_LINE = 2: uses the driver's line API (can sometimes skip some points with some drivers' quirks).pygame.LINE_RENDER_GEOMETRY = 3: uses the driver's geometry API (can appear thicker on diagonal lines).Snippet to try out:
Comparison image:
Thanks in advance!