Skip to content

Conversation

damusss
Copy link
Member

@damusss damusss commented Jul 13, 2025

As I said on discord, pygame.display.get_desktop_sizes exist but doesn't take in consideration the taskbar or any other OS reserved space. The SDL function for it exists, so it makes sense to me to add it to display. It would be very tedious to do this in python cross platform without it. It's a differnt function from the sizes because it returns rects, but the return list uses the same logic so with the same index you get the full size and usable area for the same desktop. Also, it's alredy SDL3 compatible, so there will be no problems when display is ported. There is no conflict with window because this is one of the features that make sense for display, and doesn't concert any windows (display should only hold this functions)

Summary by CodeRabbit

  • New Features

    • Added get_desktop_usable_bounds() to return usable desktop areas (excluding taskbars/docks) as a list of Rects.
    • Returns usable bounds per configured virtual desktop; count may differ from attached monitors and supports multi-monitor mirroring.
  • Documentation

    • Added API docs describing signature and behavior of get_desktop_usable_bounds().
  • Tests

    • Added tests validating returned Rects and that each bound does not exceed corresponding desktop sizes.

@damusss damusss requested a review from a team as a code owner July 13, 2025 07:49
@damusss damusss added New API This pull request may need extra debate as it adds a new class or function to pygame display pygame.display labels Jul 13, 2025
@ankith26
Copy link
Member

@coderabbitai full review

Copy link
Contributor

coderabbitai bot commented Aug 13, 2025

✅ Actions performed

Full review triggered.

Copy link
Contributor

coderabbitai bot commented Aug 13, 2025

Walkthrough

Adds pygame.display.get_desktop_usable_bounds(), implemented in C using SDL2/SDL3 APIs, returns a list of pygame.Rect for each usable desktop area; updates stub typing, adds a docs macro, and includes a unit test validating types and size constraints.

Changes

Cohort / File(s) Summary
Stub typings
buildconfig/stubs/pygame/display.pyi
Declares get_desktop_usable_bounds() -> list[Rect]; adds from pygame.rect import Rect; removes Union from typing imports; places declaration after get_desktop_sizes.
C display module
src_c/display.c
Adds pg_get_desktop_usable_bounds (handles SDL3 and pre-SDL3 paths), gathers per-display usable bounds via SDL, converts to Rect objects, returns list[Rect], and registers the method with DOC_DISPLAY_GETDESKTOPUSABLEBOUNDS.
Docs macro
src_c/doc/display_doc.h
Adds DOC_DISPLAY_GETDESKTOPUSABLEBOUNDS documenting the new function signature and purpose.
Tests
test/display_test.py
Adds test_get_desktop_usable_bounds asserting return is a list of pygame.Rect and each rect's width/height do not exceed corresponding get_desktop_sizes() values.

Sequence Diagram(s)

sequenceDiagram
    participant Py as Python caller
    participant Display as pygame.display
    participant C as pg_get_desktop_usable_bounds (C)
    participant SDL as SDL Video API

    Py->>Display: get_desktop_usable_bounds()
    Display->>C: invoke (no args)
    alt SDL3
        C->>SDL: SDL_GetDisplays() / SDL_GetDisplayUsableBounds()
    else pre-SDL3
        C->>SDL: SDL_GetNumVideoDisplays() / SDL_GetDisplayUsableBounds()
    end
    SDL-->>C: usable bounds per display
    C-->>Display: list[Rect]
    Display-->>Py: list[Rect]
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

I hopped across each screen and found,
Little Rects of safe, free ground.
Taskbars fenced and windows clear,
Usable bounds now draw us near.
A thump, a cheer — the API is here. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly states the primary change: adding the new function pygame.display.get_desktop_usable_bounds, succinctly capturing the main API addition without extraneous detail.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
test/display_test.py (1)

700-709: Avoid potential IndexError and assert list alignment

If, for any reason, the two lists differ in length, this loop will raise IndexError. Assert the lengths match and iterate with zip to keep the test robust.

Apply this diff:

 def test_get_desktop_usable_bounds(self):
     bounds = pygame.display.get_desktop_usable_bounds()
     sizes = pygame.display.get_desktop_sizes()
     self.assertIsInstance(bounds, list)
-    for i, bound in enumerate(bounds):
-        self.assertIsInstance(bound, pygame.Rect)
-        size = sizes[i]
-        self.assertLessEqual(bound.w, size[0])
-        self.assertLessEqual(bound.h, size[1])
+    self.assertEqual(len(bounds), len(sizes))
+    for bound, size in zip(bounds, sizes):
+        self.assertIsInstance(bound, pygame.Rect)
+        self.assertLessEqual(bound.w, size[0])
+        self.assertLessEqual(bound.h, size[1])
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a6db5c7 and f1f8177.

📒 Files selected for processing (4)
  • buildconfig/stubs/pygame/display.pyi (2 hunks)
  • src_c/display.c (2 hunks)
  • src_c/doc/display_doc.h (1 hunks)
  • test/display_test.py (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
test/display_test.py (2)
buildconfig/stubs/pygame/display.pyi (2)
  • get_desktop_usable_bounds (425-441)
  • get_desktop_sizes (406-423)
buildconfig/stubs/pygame/rect.pyi (1)
  • Rect (303-303)
buildconfig/stubs/pygame/display.pyi (4)
buildconfig/stubs/pygame/typing.pyi (1)
  • rect (57-57)
src_py/typing.py (1)
  • rect (57-57)
buildconfig/stubs/typing_sample_app.py (1)
  • rect (100-101)
buildconfig/stubs/pygame/rect.pyi (1)
  • Rect (303-303)
🔇 Additional comments (4)
src_c/doc/display_doc.h (1)

14-14: Doc macro addition LGTM

The new macro name and summary align with existing patterns and the API behavior.

buildconfig/stubs/pygame/display.pyi (2)

425-442: Stub signature and docstring LGTM

Return type list[Rect] matches runtime objects. The docstring mirrors the new API semantics and version tag is present.


55-55: No Union usage detected – import removal is safe

  • buildconfig/stubs/pygame/display.pyi: scanned for “Union” with ripgrep; no occurrences found.
src_c/display.c (1)

3179-3181: Method registration LGTM

Correctly registered with METH_NOARGS and linked to the doc macro.

@robertpfeiffer
Copy link
Contributor

This could be used to implement "Do what I mean" window sizing when passing in (0,0) for windowed mode. I always hated how windowed mode overlaps with the task bar in that case!

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Member

@MyreMylar MyreMylar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, LGTM 👍 Seems like it could be useful for application positioning especially with things like no frame windows.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
test/display_test.py (1)

700-710: LGTM! Consider optional enhancements for more comprehensive coverage.

The test correctly validates the return type, length consistency with get_desktop_sizes(), and basic dimension constraints.

For enhanced test coverage, you may optionally consider:

  • Validating that bounds have non-negative coordinates (x, y >= 0)
  • Checking that bounds have positive dimensions (w > 0, h > 0)
  • Verifying bounds are positioned within desktop boundaries (x+w <= size[0], y+h <= size[1])

Example enhancement:

     def test_get_desktop_usable_bounds(self):
         bounds = pygame.display.get_desktop_usable_bounds()
         sizes = pygame.display.get_desktop_sizes()
         self.assertIsInstance(bounds, list)
         self.assertEqual(len(bounds), len(sizes))
         for i, bound in enumerate(bounds):
             self.assertIsInstance(bound, pygame.Rect)
             size = sizes[i]
             self.assertLessEqual(bound.w, size[0])
             self.assertLessEqual(bound.h, size[1])
+            # Optional: validate coordinates and dimensions
+            self.assertGreaterEqual(bound.x, 0)
+            self.assertGreaterEqual(bound.y, 0)
+            self.assertGreater(bound.w, 0)
+            self.assertGreater(bound.h, 0)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0cc2c49 and 4831155.

📒 Files selected for processing (1)
  • test/display_test.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
test/display_test.py (2)
buildconfig/stubs/pygame/display.pyi (2)
  • get_desktop_usable_bounds (425-441)
  • get_desktop_sizes (406-423)
buildconfig/stubs/pygame/rect.pyi (1)
  • Rect (303-303)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: AMD64
  • GitHub Check: x86
  • GitHub Check: aarch64
  • GitHub Check: x86_64
  • GitHub Check: debug_coverage (ubuntu-24.04, 3.9.23)
  • GitHub Check: i686
  • GitHub Check: debug_coverage (ubuntu-24.04, 3.14.0rc1)
  • GitHub Check: debug_coverage (ubuntu-24.04, 3.13.5)
  • GitHub Check: Debian (Bookworm - 12) [ppc64le]
  • GitHub Check: msys2 (clang64, clang-x86_64)
  • GitHub Check: Debian (Bookworm - 12) [armv7]
  • GitHub Check: msys2 (ucrt64, ucrt-x86_64)
  • GitHub Check: Debian (Bookworm - 12) [armv6]
  • GitHub Check: Debian (Bookworm - 12) [s390x]
  • GitHub Check: build (windows-latest)
  • GitHub Check: msys2 (mingw64, x86_64)
  • GitHub Check: build (ubuntu-24.04)
  • GitHub Check: build (macos-14)
  • GitHub Check: build (ubuntu-22.04)
  • GitHub Check: dev-check

@damusss
Copy link
Member Author

damusss commented Oct 1, 2025

This could be used to implement "Do what I mean" window sizing when passing in (0,0) for windowed mode. I always hated how windowed mode overlaps with the task bar in that case!

Yeah I agree, useful to manually maximize a window with it being aware of the taskbar or other things occupying space.

Also thanks myre for pushing the memory fixes, have been out of the contributing loop during summer (github even signed me out...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
display pygame.display New API This pull request may need extra debate as it adds a new class or function to pygame
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add pygame.display.get_desktop_workarea() to retrieve usable display bounds (excluding taskbar / Dock)
4 participants