Skip to content

Add interface for _sdl2.video classes #3317

New issue

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

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

Already on GitHub? # to your account

Merged
merged 12 commits into from
Feb 18, 2025

Conversation

MightyJosip
Copy link
Member

@MightyJosip MightyJosip commented Jan 28, 2025

I imagined this as the first PR towards the goal of rewriting _sdl2.video stuff in C code. Since those classes are used in combination with each other, I wanted to add them all, so then for example when I will write the implementation for Renderer, I can do it without any compilation errors where I am missing the declaration for Texture. At this moment, this code itself doesn't do anything, you can write

import pygame
import pygame._render

window = pygame.Window()
renderer = pygame._render.Renderer(window)
texture = pygame._render.Texture(renderer)
image = pygame._render.Image(texture)

print(renderer)
print(texture)
print(image)

for example, and you get

<pygame._render.Renderer object at 0x0000026FE34668E0>
<pygame._render.Texture object at 0x0000026FE3467FC0>
<pygame._render.Image object at 0x0000026FE34966F0>

as the output. But that is everything that has been implemented. Also, I didn't include _render in pygame.init, so you need to manually import it, otherwise it won't work, so at the moment, this module is basically hidden, just so it doesn't break anything

@MightyJosip MightyJosip requested a review from a team as a code owner January 28, 2025 11:51
@MightyJosip MightyJosip mentioned this pull request Jan 28, 2025
60 tasks
@Starbuck5
Copy link
Member

Starbuck5 commented Jan 29, 2025

Hey, good to see you moving this forward, this is important for us to do!

Some organizational things:
I'm surprised to see the 3 classes all getting their own module. I would think they would all be part of one module, as they are currently. (You could still have separate files for implementation and have them in one module).

Also, I would think these new files would live in the _sdl2 folder. I believe when Window was ported it went there first.

@damusss
Copy link
Member

damusss commented Jan 29, 2025

I agree that they should live in one module. a good name would be pygame.renderer (privatized at the moment) in my opinion. Just like pygame.gpu should hold all gpu features in the future. it makes so things that are directly related to each other and that depend on each other under the same name (you can't use a texture without a renderer, but you can use a window with regular surfaces, you get the idea)

@MightyJosip
Copy link
Member Author

MightyJosip commented Jan 29, 2025

Yea you are correct about the multiple modules, I will add it in a single module. Is there an example of that in the repo at this moment, I can probably do that with the multiple header files, but if we already have more elegant solution I would like to use it, probably call it pygame._renderer like damusss said. (EDIT: I guess I can do the same as it is done for geometry module)

About _sdl2, I am pretty sure Window was added outside of it in the first commit https://github.com/pygame-community/pygame-ce/pull/2114/files#diff-3279c6977adb71e8d262e5974889267e57e33c5ef0d5c76a0146999fba0b40c6

@Starbuck5
Copy link
Member

The code looks fine reading through, but I disagree with damus, this should be called pygame._render.

"Render" is what SDL2 and SDL3 both call it. https://wiki.libsdl.org/SDL2/CategoryRender, https://wiki.libsdl.org/SDL3/CategoryRender

@Starbuck5 Starbuck5 mentioned this pull request Feb 12, 2025
32 tasks
@MightyJosip
Copy link
Member Author

The code looks fine reading through, but I disagree with damus, this should be called pygame._render.

Yea makes sense. Done

Copy link
Member

@Starbuck5 Starbuck5 left a comment

Choose a reason for hiding this comment

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

OK! looks good.

Copy link
Member

@damusss damusss left a comment

Choose a reason for hiding this comment

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

I thought this was super big, instead just the interface (who could have guessed, reading the title?). LGTM, Thanks!

@damusss damusss added the render/_sdl2 pygame._render or former pygame._sdl2 label Feb 18, 2025
@damusss damusss added this to the 2.5.4 milestone Feb 18, 2025
@damusss damusss merged commit efeb3ff into pygame-community:main Feb 18, 2025
25 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
render/_sdl2 pygame._render or former pygame._sdl2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants