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

Port video.Window to C #2114

Merged
merged 40 commits into from
Jun 4, 2023
Merged

Port video.Window to C #2114

merged 40 commits into from
Jun 4, 2023

Conversation

yunline
Copy link
Contributor

@yunline yunline commented Apr 12, 2023

Split from #2070

  • Port video.Window to C
  • _sdl2.video.Window (old) -> _sdl2.video._Window and _sdl2.window.Window

commit d8f7d21
Author: yunline <yunline@foxmail.com>
Date:   Thu Apr 6 21:02:16 2023 +0800

    fix warning

commit c66552b
Author: yunline <yunline@foxmail.com>
Date:   Thu Apr 6 20:56:37 2023 +0800

    Fix compatibility with video module

commit d0e11c9
Author: yunline <yunline@foxmail.com>
Date:   Thu Apr 6 18:48:15 2023 +0800

    fix typo

commit 58b75cb
Author: yunline <yunline@foxmail.com>
Date:   Wed Apr 5 14:01:50 2023 +0800

    fix bug

commit 50ddba4
Author: yunline <yunline@foxmail.com>
Date:   Wed Apr 5 13:58:04 2023 +0800

    Add support for DPI aware applications pygame-community#2081

commit 65fac03
Author: yunline <yunline@foxmail.com>
Date:   Wed Apr 5 10:23:53 2023 +0800

    compatible with _sdl2.video.Window

commit 8fe04eb
Author: yunline <yunline@foxmail.com>
Date:   Tue Apr 4 22:03:00 2023 +0800

    fix warning

commit 6cd35a3
Author: yunline <yunline@foxmail.com>
Date:   Tue Apr 4 21:42:46 2023 +0800

    fix warning

commit d87fe5e
Author: yunline <yunline@foxmail.com>
Date:   Tue Apr 4 21:21:23 2023 +0800

    update docs

commit 043cfa3
Author: yunline <yunline@foxmail.com>
Date:   Tue Apr 4 21:13:55 2023 +0800

    support surface.convert

commit 398ade9
Author: yunline <yunline@foxmail.com>
Date:   Tue Apr 4 11:50:52 2023 +0800

    add docs

commit 78b6882
Author: yunline <yunline@foxmail.com>
Date:   Tue Apr 4 11:06:20 2023 +0800

    fix error

commit 0e5ca30
Author: yunline <yunline@foxmail.com>
Date:   Tue Apr 4 10:59:54 2023 +0800

    port docs to window

commit 817eef9
Author: yunline <yunline@foxmail.com>
Date:   Tue Apr 4 10:27:09 2023 +0800

    revert _sdl2 changes and port the docs to window

commit 6c957aa
Author: Mega-JC <65417594+Mega-JC@users.noreply.github.com>
Date:   Mon Apr 3 00:40:00 2023 +0200

    Rewrite pygame._sdl2.video docs
    This commit effectively rewrites all the documentation of the  module, by either adding new information for undocumented content, or extracting information from existing docstrings in the codebase.
    Along with the documentation changes, some function signatures were modified to adhere to the snake case naming convention described in PEP 8.

commit 7c8a3fe
Author: yunline <yunline@foxmail.com>
Date:   Mon Apr 3 16:08:38 2023 +0800

    add get_foreign()

commit 524470b
Author: yunline <yunline@foxmail.com>
Date:   Sun Apr 2 19:47:03 2023 +0800

    add docs

commit 848d1e7
Author: yunline <yunline@foxmail.com>
Date:   Sun Apr 2 16:28:42 2023 +0800

    add docs

commit 09ebc61
Author: yunline <yunline@foxmail.com>
Date:   Sun Apr 2 16:23:53 2023 +0800

    add docs

commit 2b96b95
Author: yunline <yunline@foxmail.com>
Date:   Sun Apr 2 15:43:32 2023 +0800

    add test_init_flags

commit d336009
Author: yunline <yunline@foxmail.com>
Date:   Sun Apr 2 14:56:50 2023 +0800

    fix warnings

commit 3ad335c
Author: yunline <yunline@foxmail.com>
Date:   Sun Apr 2 14:50:44 2023 +0800

    move “fullscreen” into flags

commit 0c99d92
Author: yunline <yunline@foxmail.com>
Date:   Sun Apr 2 14:42:17 2023 +0800

    fix error

commit 1c19229
Author: yunline <yunline@foxmail.com>
Date:   Sun Apr 2 14:24:29 2023 +0800

    add version check

commit 5ca1972
Merge: ff8d75d bcfab19
Author: yunline <yunline@foxmail.com>
Date:   Sun Apr 2 13:57:12 2023 +0800

    Merge branch 'main' into c-window-module

commit ff8d75d
Author: yunline <yunline@foxmail.com>
Date:   Sun Apr 2 13:55:31 2023 +0800

    fix segfault

commit b215750
Author: yunline <yunline@foxmail.com>
Date:   Sun Apr 2 12:10:04 2023 +0800

    fix warning in pypy

commit 2db4048
Author: yunline <yunline@foxmail.com>
Date:   Sun Apr 2 11:54:31 2023 +0800

    fix warning

commit 849082d
Author: yunline <yunline@foxmail.com>
Date:   Sun Apr 2 11:09:04 2023 +0800

    add **flags

commit f84fc2c
Author: yunline <yunline@foxmail.com>
Date:   Sun Apr 2 10:21:14 2023 +0800

    "pygame" -> "pygame window"

commit 98e42bf
Author: yunline <yunline@foxmail.com>
Date:   Sun Apr 2 08:54:43 2023 +0800

    add warning

commit 7d6dd24
Author: yunline <yunline@foxmail.com>
Date:   Sun Apr 2 08:47:53 2023 +0800

    add version check for flash() and flash_cancel()

commit d9d549a
Author: yunline <yunline@foxmail.com>
Date:   Sun Apr 2 08:43:21 2023 +0800

    add flash() and flash_cancel()

commit dd4ea5f
Author: yunline <yunline@foxmail.com>
Date:   Sat Apr 1 13:00:34 2023 +0800

    fix bug

commit 587c628
Author: yunline <yunline@foxmail.com>
Date:   Sat Apr 1 12:40:00 2023 +0800

    check size when init & add tests

commit ecfba92
Author: yunline <yunline@foxmail.com>
Date:   Sat Apr 1 12:11:32 2023 +0800

    move constants to constants.c

commit 78282bb
Author: yunline <yunline@foxmail.com>
Date:   Fri Mar 31 21:10:28 2023 +0800

    add get_wm_info()

commit 7ced741
Author: yunline <yunline@foxmail.com>
Date:   Fri Mar 31 20:37:49 2023 +0800

    skip some test when SDL_VIDEODRIVER is dummy

commit 5b6e171
Author: yunline <yunline@foxmail.com>
Date:   Fri Mar 31 19:02:25 2023 +0800

    rename variable

commit b5ce697
Author: yunline <yunline@foxmail.com>
Date:   Fri Mar 31 18:52:54 2023 +0800

    format and fix warning

commit 25ceeae
Author: yunline <yunline@foxmail.com>
Date:   Fri Mar 31 17:33:36 2023 +0800

    add tests for window module

commit 2282c52
Author: yunline <yunline@foxmail.com>
Date:   Fri Mar 31 17:11:10 2023 +0800

    raise exception when size<=0

commit bf14dff
Author: yunline <yunline@foxmail.com>
Date:   Fri Mar 31 16:48:17 2023 +0800

    quit display module when destroying

commit e224a07
Merge: 088953c 8b73bae
Author: yunline <yunline@foxmail.com>
Date:   Fri Mar 31 16:20:50 2023 +0800

    Merge branch 'main' into c-window-module

commit 088953c
Author: yunline <yunline@foxmail.com>
Date:   Fri Mar 31 16:13:06 2023 +0800

    fix segfault

commit ce5cd31
Author: yunline <yunline@foxmail.com>
Date:   Fri Mar 31 15:43:24 2023 +0800

    fix warning

commit a72b476
Author: yunline <yunline@foxmail.com>
Date:   Fri Mar 31 15:29:18 2023 +0800

    auto resize the surface when resizing the window

commit 74e125e
Author: yunline <yunline@foxmail.com>
Date:   Fri Mar 31 07:33:21 2023 +0800

    fix bug

commit 1aa97c5
Author: yunline <yunline@foxmail.com>
Date:   Thu Mar 30 21:43:57 2023 +0800

    add from_existing_window()

commit 70b14e7
Author: yunline <yunline@foxmail.com>
Date:   Thu Mar 30 17:31:01 2023 +0800

    add get_always_on_top() and set_always_on_top()

commit fbe0716
Merge: 2f776fd fdad67a
Author: yunline <yunline@foxmail.com>
Date:   Thu Mar 30 17:11:54 2023 +0800

    Merge branch 'main' into c-window-module

commit 2f776fd
Author: yunline <yunline@foxmail.com>
Date:   Thu Mar 30 16:56:22 2023 +0800

    fix warning

commit efd11ae
Author: yunline <yunline@foxmail.com>
Date:   Thu Mar 30 13:09:59 2023 +0800

    fix memory leak

commit d5fc2c5
Author: yunline <yunline@foxmail.com>
Date:   Thu Mar 30 12:55:25 2023 +0800

    add from_display_module()

commit 1344595
Author: yunline <yunline@foxmail.com>
Date:   Thu Mar 30 09:38:41 2023 +0800

    modified window.c

commit d4f9abc
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 22:48:52 2023 +0800

    fix warning

commit 8b9d2e7
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 22:21:47 2023 +0800

    add get_surface() and update_from_surface()

commit 19054f3
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 20:50:17 2023 +0800

    fix error

commit 07d1f6d
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 20:47:30 2023 +0800

    fix error

commit 5af9737
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 20:34:45 2023 +0800

    fix error

commit 59eadd2
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 20:23:33 2023 +0800

    fix error

commit a1be68d
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 20:18:36 2023 +0800

    fix error

commit 59658b2
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 20:12:45 2023 +0800

    fix bug

commit 42d33a1
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 20:03:49 2023 +0800

    fix error

commit 751c94b
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 19:57:08 2023 +0800

    add set_modal_for()

commit 1c621e5
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 19:43:09 2023 +0800

    METH_FASTCALL -> METH_O

commit dd8bc3b
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 19:30:04 2023 +0800

    set up c api for window module

commit ed12441
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 18:14:51 2023 +0800

    add get_display_index()

commit f4a0562
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 18:05:14 2023 +0800

    fix warning

commit 37e4b20
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 17:31:30 2023 +0800

    add set_opacity() and get_opacity()

commit a5190a4
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 17:19:32 2023 +0800

    Modified set_position()

commit d7ccbe0
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 17:15:19 2023 +0800

    add get/set_size() and get/set_position()

commit 47ca75c
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 16:50:13 2023 +0800

    add get_window_id()

commit 59a9917
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 16:37:21 2023 +0800

    add set_borderless() and get_borderless()

commit 587bc4e
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 16:31:12 2023 +0800

    add set_resizable() and get_resizable()

commit 1973665
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 16:19:59 2023 +0800

    add restore() maximize() and minimize()

commit cec834c
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 16:11:04 2023 +0800

    fix warning

commit caabb2b
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 16:09:01 2023 +0800

    add focus() hide() and show()

commit 648e049
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 13:39:23 2023 +0800

    add set_title() and get_title()

commit 6226bc0
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 13:16:32 2023 +0800

    add set_windowed() and set_fullscreen()

commit bff91b8
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 12:55:51 2023 +0800

    fix stub and format

commit bc9c4dd
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 09:59:04 2023 +0800

    add constants and stub

commit 00510af
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 09:35:39 2023 +0800

    add get_grabbed_window()

commit 7b1dc5f
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 09:21:11 2023 +0800

    add get_grab() and set_grab()

commit 517785c
Author: yunline <yunline@foxmail.com>
Date:   Wed Mar 29 09:05:03 2023 +0800

    Modify get_windows()

commit d0b3256
Author: yunline <yunline@foxmail.com>
Date:   Tue Mar 28 22:53:25 2023 +0800

    Add get_windows()

commit 0b62138
Author: yunline <yunline@foxmail.com>
Date:   Tue Mar 28 22:05:08 2023 +0800

    fix compatibility

commit 8a7a80d
Author: yunline <yunline@foxmail.com>
Date:   Tue Mar 28 21:55:40 2023 +0800

    fix warning

commit c1a47f4
Author: yunline <yunline@foxmail.com>
Date:   Tue Mar 28 20:43:17 2023 +0800

    create new module `pygame.window`
@yunline yunline requested a review from a team as a code owner April 12, 2023 01:55
@yunline yunline marked this pull request as draft April 13, 2023 14:56
@yunline yunline marked this pull request as ready for review April 30, 2023 09:01
src_c/window.c Outdated Show resolved Hide resolved
src_c/window.c Outdated Show resolved Hide resolved
src_c/window.c Outdated Show resolved Hide resolved
src_c/window.c Outdated Show resolved Hide resolved
src_c/window.c Outdated Show resolved Hide resolved
@Starbuck5
Copy link
Member

Starbuck5 commented May 7, 2023

One thing I'd like to address besides the code is the namespace considerations.

Right now

_sdl2.video.Window -> _sdl2.video._Window
*** -> window.Window

I would like this new window to be in _sdl2 for now as well, until we decide it's ready for a public release (do a lot more integration work, add surface rendering functionality)

It also breaks every _sdl2 program to get rid of the Window class being where it is.

So instead, I suggest

_sdl2.video.Window -> _sdl2.video._Window
*** -> _sdl2.window.Window
    |  _sdl2.video.Window

I suggest aliasing a _sdl2.window.Window to _sdl2.video.Window. This way we can use existing _sdl2 programs and users to test the new Window implementation. Since the old window is still around (for the moment), we can also switch to that at runtime to test differences.


Also if you're looking for test code, https://mcpalmer1980.itch.io/flyboy is a complete game in _sdl2.

@Starbuck5 Starbuck5 added the _sdl2 pygame._sdl2 label May 7, 2023
@Starbuck5 Starbuck5 mentioned this pull request May 11, 2023
43 tasks
_sdl2.window.Window as an alias of _sdl2.video._Window
_sdl2.video.Window  as an alias of _window.Window
@Starbuck5
Copy link
Member

Tests are failing because of the relative mouse mode not being in the type stub?

src_py/__init__.py Outdated Show resolved Hide resolved
src_c/window.c Outdated Show resolved Hide resolved
@Starbuck5 Starbuck5 added the Code quality/robustness Code quality and resilience to changes label May 29, 2023
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.

This is a very important PR to get the Window API up and running, thank you so much for tackling it!

I think it looks good now.

Roadmap in my head:

  • Two window APIs coexist for a release (for back testing purposes)
  • Window tests!
  • Old _sdl2.window cython code is removed
  • New window features: (display interop, software rendering, etc)
  • Release window API publicly

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.

Things I've noticed:

  1. No snake icon in the title bar top left corner of the new C window versus the other ways to create a window.
  2. When expanding a resizable new window to the bottom right some white lines are left behind.
  3. If you .destroy() a new window with .grab = True your mouse is still grabbed to the now destroyed window's area.
  4. Going directly from minimised to maximised with the functions does not expand the black area of the window in the same way it does if you go, for example: minimise() -> restore() ->maximise()
  5. calling .show() on a hidden window turns the background white.

All this is tested on a Windows 11 PC, results may vary on other platforms as we are dealing with window management here. Some of these issues may be pre-exisiting in the old cython window.

@Starbuck5 Starbuck5 added this to the 2.3 milestone Jun 1, 2023
@Starbuck5
Copy link
Member

Starbuck5 commented Jun 1, 2023

@MyreMylar Thanks for testing this out.

Number 1 is consistent with the cython implementation. Number 2, number 4, and number 5 are also consistent with cython, and seem like the product of not clearing the screen with a Renderer. Even display surfaces have some weird artifacts when you don't fill them every frame (when resized).

I want this PR to be consistent with the cython implementation. It's "port window," not "improve window."

I can't replicate number 4.

@MyreMylar
Copy link
Member

MyreMylar commented Jun 1, 2023

@MyreMylar Thanks for testing this out.

Number 1 is consistent with the cython implementation. Number 2, number 4, and number 5 are also consistent with cython, and seem like the product of not clearing the screen with a Renderer. Even display surfaces have some weird artifacts when you don't fill them every frame (when resized).

I want this PR to be consistent with the cython implementation. It's "port window," not "improve window."

I can't replicate number 4.

For me the snake icon does appear on the old cython window but never on the new C window.

image

This is my test program:

import pygame
from pygame._sdl2.window import Window as OldWindow
from pygame._sdl2.video import Window as NewWindow

pygame.init()

old_window = OldWindow(title="Old Window", position=(20, 100))
new_window = NewWindow(title="New Window", position=(1000, 100))


running = True
while running:
    for event in pygame.event.get():
        if event.type == pygame.QUIT or event.type == pygame.KEYDOWN and event.key == pygame.K_ESCAPE:
            running = False

I also figured out that it is the first window created that gets the black background, any subsequent windows get a white background so that explains those differences somewhat. This is consistent between cython window and C window. Not really an issue for this PR.

Anyway it basically works being the important thing 👍

@yunline
Copy link
Contributor Author

yunline commented Jun 3, 2023

I copied some code from display.c to set the default icon.
I think copying is not a good choice, so I opened #2222, made a new API to get resource from pkgdata

@Starbuck5
Copy link
Member

Starbuck5 commented Jun 3, 2023

I’m unconvinced that the old cython code did this.

Also if I understand the imports correctly, Myre’s test code is testing the new window twice. Haven’t fully verified yet, just how it looks.
Edit: I get it now, I think. It's strange to have this new interface for the old window. I thought it would just be available as _sdl2.video._Window. I haven't tested either way, all of my testing has involved switching back and forth versions.

@yunline
Copy link
Contributor Author

yunline commented Jun 3, 2023

I’m unconvinced that the old cython code did this.

import pygame.pkgdata
surf = pygame.image.load(pygame.pkgdata.getResource(
'pygame_icon.bmp'))
surf.set_colorkey(0)
self.set_icon(surf)

It really does.

@Starbuck5
Copy link
Member

Ok, it's because my test code had the window go fullscreen first, which leads SDL to lose track of the icon and the resulting non fullscreen window not having any icon.

Copy link
Member

@oddbookworm oddbookworm left a comment

Choose a reason for hiding this comment

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

I forgot to actually leave a review when I looked at it a couple days ago, LGTM!

@Starbuck5 Starbuck5 merged commit 091f89c into pygame-community:main Jun 4, 2023
29 checks passed
@yunline yunline deleted the port-window branch June 5, 2023 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code quality/robustness Code quality and resilience to changes _sdl2 pygame._sdl2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants