Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strip the leading SDL_ from the pixel format name #2477

Merged
merged 8 commits into from
Oct 7, 2023

Conversation

MyreMylar
Copy link
Member

Fixes issue #1741.

I'm not sure this is the best way to strip 4 characters from a C string, so please let me know.

@MyreMylar MyreMylar requested a review from a team as a code owner September 30, 2023 13:01
Copy link
Member

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

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

A one-line change to achieve this could be passing pixel_format_name + 4 directly instead of the copying to new memory. This of course, assumes that pixel_format_name is a non-null pointer and the string has atleast 4 chars (which should be checked, either way)

@ankith26
Copy link
Member

if (!strncmp(pixel_format_name, "SDL_", 4)) {
    pixel_format_name += 4
}

(untested code)
This should do maybe?

@zoldalma999
Copy link
Member

zoldalma999 commented Sep 30, 2023

The vidinfo type also has a getter, defined a bit above the str function. This currently does not even return a value for pixel format, so I think this could be an opportunity to fix that (and of course, strip "SDL_" off there as well).

ps: SDL_GetPixelFormatName will always return a string starting with "SDL_"

@ankith26
Copy link
Member

ps: SDL_GetPixelFormatName will always return a string starting with "SDL_"

Yeah I realised that too as I made my comment, but then it's safer (more future-proof) this way and does no harm.

@ankith26 ankith26 added this to the 2.4.0 milestone Sep 30, 2023
@ankith26 ankith26 added the display pygame.display label Sep 30, 2023
@ankith26 ankith26 linked an issue Sep 30, 2023 that may be closed by this pull request
@MyreMylar
Copy link
Member Author

The vidinfo type also has a getter, defined a bit above the str function. This currently does not even return a value for pixel format, so I think this could be an opportunity to fix that (and of course, strip "SDL_" off there as well).

Should work now:

import pygame

pygame.init()
print(pygame.display.Info().pixel_format)

Output:

pygame-ce 2.4.0.dev1 (SDL 2.26.5, Python 3.11.1)
PIXELFORMAT_RGB888

Copy link
Member

@zoldalma999 zoldalma999 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@MyreMylar MyreMylar requested a review from ankith26 October 5, 2023 19:54
@MyreMylar
Copy link
Member Author

Documented the pixel_format attribute and fixed that attribute documentation block while I was in there. Before it was formatted as python code and had an unnecessarily messy horizontal scrollbar.

👍

Copy link
Member

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the PR 🎉

@ankith26 ankith26 merged commit 01c6f22 into main Oct 7, 2023
27 of 29 checks passed
@ankith26 ankith26 deleted the strip_sdl_from_pixelformat branch October 7, 2023 05:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
display pygame.display
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VideoInfo.pixel_format: remove SDL_ prefix (3526)
3 participants