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

Set default convert format when Window.get_surface() is called #2575

Merged
merged 5 commits into from
Dec 16, 2023

Conversation

yunline
Copy link
Contributor

@yunline yunline commented Nov 20, 2023

Test code (should not raise exception):

import pygame
from pygame._sdl2 import Window

win = Window()

sf1 = win.get_surface()

sf2 = pygame.Surface((10,10)).convert()

while 1:
    event = pygame.event.wait()
    if event.type == pygame.QUIT:
        break

If convert() is called but no convert format have been set, it should raise:

pygame.error: No convert format has been set, try display.set_mode() 

After the Window API published, it should raise:

pygame.error: No convert format has been set, try display.set_mode() or Window.get_surface()

@yunline yunline added the Surface pygame.Surface label Nov 20, 2023
@yunline yunline requested a review from a team as a code owner November 20, 2023 02:03
@yunline yunline changed the title Set default convert format when WIndow.get_surface() is called Set default convert format when Window.get_surface() is called Nov 20, 2023
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.

LGTM 👍

src_c/surface.c Outdated Show resolved Hide resolved
src_c/surface.c Outdated Show resolved Hide resolved
src_c/base.c Show resolved Hide resolved
@Starbuck5
Copy link
Member

Hey @yunline, can you rebase this against main?

It will make it easy to test with a full Window.flip() example, and it'll fix the circleci change.

I see you didn't respond to my long rambling paragraph feedback, I'd like to try and add that as a commit to this branch myself, and then if it looks good to you we can move forward.

"Window will be published next release" shouldn't go in the error message! That was information for you, not for the user :)

@yunline
Copy link
Contributor Author

yunline commented Dec 14, 2023

"Window will be published next release" shouldn't go in the error message! That was information for you, not for the user :)

Oops, I'll fix it soon.

I'm worried that if we use a reference to a pixelformat struct from a surface, the pixelformat might be deallocated later by SDL. This makes the code more complex but it ensures that the module always has a handle on a real and not-deallocated pixelformat.
@Starbuck5
Copy link
Member

@yunline I added a new commit to this branch, which makes the code more but also safer.

I was worried about what would happen if the SDL_PixelFormat struct we hold was deallocated by SDL, making it allocate a SDL_PixelFormat specifically to hold on to will prevent that from happening.

Please let me know if this looks okay to you

@Starbuck5 Starbuck5 added the window pygame.Window label Dec 15, 2023
@Starbuck5 Starbuck5 added this to the 2.4.0 milestone Dec 15, 2023
src_c/surface.c Outdated Show resolved Hide resolved
src_c/window.c Show resolved Hide resolved
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.

Looks good to me.

@MyreMylar MyreMylar merged commit 24a3674 into pygame-community:main Dec 16, 2023
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Surface pygame.Surface window pygame.Window
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants