Test failure: `test_SDL_GetSetClipRect`

[yzhou216]

What doesn't work?

=================================== FAILURES ===================================
____________________ TestSDLSurface.test_SDL_GetSetClipRect ____________________

self = <sdl2.test.surface_test.TestSDLSurface object at 0x7fff4f9b2140>

    def test_SDL_GetSetClipRect(self):
        rectlist = ((rect.SDL_Rect(0, 0, 0, 0), SDL_FALSE, True),
                    (rect.SDL_Rect(2, 2, 0, 0), SDL_FALSE, False),
                    (rect.SDL_Rect(2, 2, 5, 1), SDL_TRUE, True),
                    (rect.SDL_Rect(6, 5, 10, 3), SDL_TRUE, False),
                    (rect.SDL_Rect(0, 0, 10, 10), SDL_TRUE, True),
                    (rect.SDL_Rect(0, 0, -10, 10), SDL_FALSE, False),
                    (rect.SDL_Rect(0, 0, -10, -10), SDL_FALSE, False),
                    (rect.SDL_Rect(-10, -10, 10, 10), SDL_FALSE, False),
                    (rect.SDL_Rect(10, -10, 10, 10), SDL_FALSE, False),
                    (rect.SDL_Rect(10, 10, 10, 10), SDL_TRUE, False)
                    )

        clip = rect.SDL_Rect()
        sf = sdl2.SDL_CreateRGBSurface(0, 15, 15, 32, 0, 0, 0, 0)
        assert isinstance(sf.contents, sdl2.SDL_Surface)
        sdl2.SDL_GetClipRect(sf, byref(clip))
        assert clip == rect.SDL_Rect(0, 0, 15, 15)

        for r, clipsetval, cmpval in rectlist:
            retval = sdl2.SDL_SetClipRect(sf, r)
            sdl2.SDL_GetClipRect(sf, byref(clip))
            err = "Could not set clip rect %s" % r
            assert retval == clipsetval, "retval: " + err
>           assert (clip == r) == cmpval, "clip: " + err
E           AssertionError: clip: Could not set clip rect SDL_Rect(x=2, y=2, w=0, h=0)
E           assert (SDL_Rect(x=2, y=2, w=0, h=0) == SDL_Rect(x=2, y=2, w=0, h=0)
E
E             Use -v to get more diff) == False

sdl2/test/surface_test.py:493: AssertionError
=========================== short test summary info ============================
FAILED sdl2/test/surface_test.py::TestSDLSurface::test_SDL_GetSetClipRect - AssertionError: clip: Could not set clip rect SDL_Rect(x=2, y=2, w=0, h=0)
===== 1 failed, 773 passed, 324 skipped, 1 deselected, 3 xpassed in 11.64s =====

How To Reproduce

Build using nix

{
  stdenv,
  lib,
  replaceVars,
  fetchFromGitHub,
  buildPythonPackage,
  setuptools,

  # native dependencies
  SDL2,
  SDL2_ttf,
  SDL2_image,
  SDL2_gfx,
  SDL2_mixer,

  # tests
  numpy,
  pillow,
  pytestCheckHook,
}:

buildPythonPackage rec {
  pname = "pysdl2";
  version = "0.9.17-unstable-2025-04-03";
  pyproject = true;

  pythonImportsCheck = [ "sdl2" ];

  src = fetchFromGitHub {
    owner = "py-sdl";
    repo = "py-sdl2";
    rev = "6414ee1c5f4a6eb91b71f5f9e35d469eee395b9f";
    hash = "sha256-E6Jpuin4bqDkvFTaZTsTNkNQJd2e5fuTf2oLsQ71uQ0=";
  };

  patches = [
    (replaceVars ./PySDL2-dll.patch (
      (builtins.mapAttrs
        (_: pkg: "${pkg}/lib/lib${pkg.pname}${stdenv.hostPlatform.extensions.sharedLibrary}")
        {
          inherit
            SDL2_ttf
            SDL2_image
            SDL2_gfx
            SDL2_mixer
            ;
        }
      )
      // {
        # sdl2-compat has the pname sdl2-compat,
        # but the shared object is named libSDL2.so for compatibility reasons.
        # This requires making the shared object path for SDL2 not depend on pname.
        SDL2 = (pkg: "${pkg}/lib/libSDL2${stdenv.hostPlatform.extensions.sharedLibrary}") SDL2;
      }
    ))
  ];

  build-system = [ setuptools ];

  buildInputs = [
    SDL2
    SDL2_ttf
    SDL2_image
    SDL2_gfx
    SDL2_mixer
  ];

  env = {
    SDL_VIDEODRIVER = "dummy";
    SDL_AUDIODRIVER = "dummy";
    SDL_RENDER_DRIVER = "software";
    PYTHONFAULTHANDLER = "1";
  };

  nativeCheckInputs = [
    numpy
    pillow
    pytestCheckHook
  ];

  disabledTests = [
    # GetPrefPath for OrgName/AppName is None
    "test_SDL_GetPrefPath"
  ];

  meta = {
    changelog = "https://github.com/py-sdl/py-sdl2/compare/0.9.17..${src.rev}";
    description = "Wrapper around the SDL2 library and as such similar to the discontinued PySDL project";
    homepage = "https://github.com/py-sdl/py-sdl2";
    license = lib.licenses.publicDomain;
    maintainers = with lib.maintainers; [ pmiddend ];
  };
}

Platform (if relevant):

• OS: Linux 6.18.0-rc3 #1-NixOS SMP PREEMPT_DYNAMIC Tue Jan 1 00:00:00 UTC 1980 x86_64 GNU/Linux
• Python Version: Python 3.13.9
• SDL2 Version: sdl2-compat-2.32.58
• Using pysdl2-dll: [Yes / No]

[a-hurst]

@yzhou216 Thanks for the bug report! However, can you confirm that this is a bug in the py-sdl2 tests (i.e. they're testing the API incorrectly) and not an upstream bug with sdl2-compat?

[yzhou216]

@yzhou216 Thanks for the bug report! However, can you confirm that this is a bug in the py-sdl2 tests (i.e. they're testing the API incorrectly) and not an upstream bug with sdl2-compat?

Sorry, I'm a package maintainer for Nixpkgs, and I'm not familiar with py-sdl2. Could you please direct me to the upstream repo so I can open the issue there? Thanks!

[a-hurst]

Absolutely, the upstream would be here! https://github.com/libsdl-org/sdl2-compat

py-sdl2 simply provides Python bindings for the SDL2 library. Now that SDL3 has been released, many distros (like Nix, apparently) are migrating over to it and providing their SDL2 binaries via SDL3 using the sdl2-compat API compatibility layer, however this compatibility layer is still a work in progress and the upstream devs are still working to make it as compatible as possible with actual SDL2.

If the sdl2-compat team say that my test is using the API incorrectly I'll fix it here, but otherwise they'll likely want to fix it on their end!

[yzhou216]

Absolutely, the upstream would be here! https://github.com/libsdl-org/sdl2-compat

py-sdl2 simply provides Python bindings for the SDL2 library. Now that SDL3 has been released, many distros (like Nix, apparently) are migrating over to it and providing their SDL2 binaries via SDL3 using the sdl2-compat API compatibility layer, however this compatibility layer is still a work in progress and the upstream devs are still working to make it as compatible as possible with actual SDL2.

If the sdl2-compat team say that my test is using the API incorrectly I'll fix it here, but otherwise they'll likely want to fix it on their end!

Thank you! I'm keeping this issue open for right now until I hear back from libsdl-org/sdl2-compat#551.

[slouken]

Are you using the latest main code for sdl2-compat? We fixed a rectangle clipping issue recently that may be related.

[slouken]

Sorry, the fix is in SDL release-3.2.26, commit libsdl-org/SDL@95977f4. Are you testing with that version of SDL?

[yzhou216]

Sorry, the fix is in SDL release-3.2.26, commit libsdl-org/SDL@95977f4. Are you testing with that version of SDL?

The issue seems to be caused by NixOS/nixpkgs#457099, which updates to sdl3 3.2.26 and sdl2-compat 2.32.58.

[slouken]

So I think the problem is that this test was adjusted to match the previous sdl2-compat behavior which was incorrect, and that this failure is actually expected.

Here's a C version of the test you provided:

#include <stdbool.h>

#include "SDL2/SDL.h"

int main()
{
    struct {
        SDL_Rect r;
        SDL_bool clipsetval;
        SDL_bool cmpval;
    } rectlist[] = {{{0, 0, 0, 0}, SDL_FALSE, true},
                    {{2, 2, 0, 0}, SDL_FALSE, false},
                    {{2, 2, 5, 1}, SDL_TRUE, true},
                    {{6, 5, 10, 3}, SDL_TRUE, false},
                    {{0, 0, 10, 10}, SDL_TRUE, true},
                    {{0, 0, -10, 10}, SDL_FALSE, false},
                    {{0, 0, -10, -10}, SDL_FALSE, false},
                    {{-10, -10, 10, 10}, SDL_FALSE, false},
                    {{10, -10, 10, 10}, SDL_FALSE, false},
                    {{10, 10, 10, 10}, SDL_TRUE, false}
                    };

        SDL_Rect clip;
        SDL_Surface *sf = SDL_CreateRGBSurface(0, 15, 15, 32, 0, 0, 0, 0);
        //SDL_GetClipRect(sf, &clip);

        for(int i = 0; i < SDL_arraysize(rectlist); ++i) {
            SDL_Rect r = rectlist[i].r;
            SDL_bool clipsetval = rectlist[i].clipsetval;
            bool cmpval = rectlist[i].cmpval;

            SDL_bool retval = SDL_SetClipRect(sf, &r);
            SDL_GetClipRect(sf, &clip);
            if (retval != clipsetval) {
                SDL_Log("retval: Could not set clip rect {%d,%d,%d,%d}", r.x, r.y, r.w, r.h);
            }
            if ((SDL_memcmp(&r, &clip, sizeof(r)) == 0) != cmpval) {
                SDL_Log("clip: Could not set clip rect ({%d,%d,%d,%d} == {%d,%d,%d,%d}) != %s", r.x, r.y, r.w, r.h, clip.x, clip.y, clip.w, clip.h, cmpval ? "true" : "false");
            }
        }
        return 0;
}

Here is the output with the latest sdl2-compat and the latest SDL3:

clip: Could not set clip rect ({2,2,0,0} == {2,2,0,0}) != false
clip: Could not set clip rect ({0,0,-10,10} == {0,0,-10,10}) != false
clip: Could not set clip rect ({0,0,-10,-10} == {0,0,-10,-10}) != false

Here is the output with SDL2:

INFO: clip: Could not set clip rect ({2,2,0,0} == {2,2,0,0}) != false
INFO: clip: Could not set clip rect ({0,0,-10,10} == {0,0,-10,10}) != false
INFO: clip: Could not set clip rect ({0,0,-10,-10} == {0,0,-10,-10}) != false

Here is a debug session showing the SDL2 code in action:

SDL_SetClipRect_REAL (surface=0x5555555592a0, rect=0x7fffffffdbe0)
    at /home/slouken/projects/SDL2/src/video/SDL_surface.c:640
640	{
(gdb) 
644	    if (!surface) {
(gdb) 
652	    full_rect.h = surface->h;
(gdb) 
655	    if (!rect) {
(gdb) 
659	    return SDL_IntersectRect(rect, &full_rect, &surface->clip_rect);
(gdb) p rect
$6 = (const SDL_Rect *) 0x7fffffffdbe0
(gdb) p *rect
$7 = {x = 2, y = 2, w = 0, h = 0}
(gdb) p full_rect
$8 = {x = 0, y = 0, w = 15, h = 15}
(gdb) n
main () at t.c:34
34	            SDL_GetClipRect(sf, &clip);
(gdb) p sf->clip_rect
$9 = {x = 2, y = 2, w = 0, h = 0}

[slouken]

So I think the fix in the SDL 3.2.26 release makes sdl2-compat match SDL2 behavior and is correct.

[madebr]

Current SDL2 git is also failing this test. All 2.32.x releases are good.

libsdl-org/SDL@1c19bee introduced the failure

[slouken]

Current SDL2 git is also failing this test. All 2.32.x releases are good.

libsdl-org/SDL@1c19bee introduced the failure

Okay, so we need to root cause why SDL2 is failing.

[slouken]

So after investigation, this change was introduced in libsdl-org/SDL@80cbc90, which is a legitimate bug fix where the x and y values of the rectangle were left uninitialized, and this caused issues in some older games.

The test should be updated to reflect the new behavior, and if there's a real application that is broken because of this, we can add a quirk to sdl2-compat for that specific case.