pygame-ce icon indicating copy to clipboard operation
pygame-ce copied to clipboard

Add SurfaceWindow subclass, remove borrowed window logic

Open ankith26 opened this issue 1 year ago • 7 comments
trafficstars

This is a draft PR to get feedback on the API and idea, so I have not updated docs/tests/types

Related to issue #2603

How to test this PR?

Use the example written by @Starbuck5 in https://github.com/pygame-community/pygame-ce/pull/2626, but have to just replace the two lines that construct Window and instead use SurfaceWindow

ankith26 avatar Apr 28 '24 10:04 ankith26

~~hmm. There is no abstract Window class in this code. Doesn't is defeat the whole purpose?~~

If you remove the borrowing logic, you have to remove from_window from Renderer as well.

robertpfeiffer avatar Apr 28 '24 10:04 robertpfeiffer

If you remove the borrowing logic, you have to remove from_window from Renderer as well.

Yeah I though of doing it but didn't because I didn't understand its purpose well enough. Thanks for letting me know I will remove it

ankith26 avatar Apr 28 '24 10:04 ankith26

So @ankith26 you first brought up the "window subclass" idea a few months ago and I've been tossing it around in my head since. You brought it up back in https://github.com/pygame-community/pygame-ce/pull/2632 when thinking about a .surface property.

I've been doing some thinking on it, and they are very interesting ideas, but I don't think we should implement either of them at this time.

Reasoning:

  • People have already started using the API, big changes now are hard. We should focus on gaining confidence in what we have and releasing it out.
  • When in doubt, follow SDL. SDL2 has one Window type, SDL doesn't have a type level way to stop people from mixing software and hardware rendering (and they could, even in C).
  • SDL3 adds a new windowing concept with SDL_CreatePopupWindow. Us saying that the rendering type is part of the window object closes the door having "pop up window vs normal" be the type, or something else entirely.
  • We can add this later as a backwards compatible change, maybe when SDL3 is out, and we can be more confident about doing something new with the API

Also if we ever implement this I think it would be good to do the subclasses in Python, rather than C. Makes it easier to write and maintain. (I think blubber brought this up at some point too, to give him some credit for this idea as well)

Starbuck5 avatar Apr 29 '24 05:04 Starbuck5

I have a few things to say about Starbuck's comment, about the reasoning

  1. as far as I remember Window is an experimental API, people are aware it might get modified in the future, it's not on us. plus they would really just change the class name. plus plus the more we wait the more are gonna have to change in the future. I think it's ok to forward this to pygame 3.
  2. I don't think it's that bad to do things slightly different than SDL, pygame users aren't aware of the underlying sdl code and aren't supposed to (plus pygame is not just a wrapper)

About python subclassing, that's up to whoever implements it but for now it doesn't seem like it's too messy implemented in C. That's just my opinion, have a nice day :)

damusss avatar Apr 29 '24 06:04 damusss

as far as I remember Window is an experimental API, people are aware it might get modified in the future, it's not on us. plus they would really just change the class name. plus plus the more we wait the more are gonna have to change in the future. I think it's ok to forward this to pygame 3.

People are aware in theory, but I've already seen complaining about tiny things changing in between releases with _sdl2. But the users are not my only concern, we want to push the Window API sooner rather than later, and a big change to the API like this shouldn't be happening at the stage I would like us to be in its development.

I don't think it's that bad to do things slightly different than SDL, pygame users aren't aware of the underlying sdl code and aren't supposed to (plus pygame is not just a wrapper)

Yes, I agree that pygame-ce is not just a wrapper, as people sometimes say. However, if we're not confident about doing something differently, it's not a good idea to do so from a maintenance perspective. Sticking with SDL concepts and SDL API design is a good insurance policy to make sure our stuff keeps working the way we expect.

Starbuck5 avatar Apr 29 '24 06:04 Starbuck5

@Starbuck5 To conclude this, I basically agree and i think we should keep developing this and eventually release it with pygame 3.0 among other OOP related ideas like python event types.

damusss avatar Apr 29 '24 07:04 damusss

I basically agree with Starbuck. I don't think doing it like this would be that bad, but it probably will be worse than what we have.

robertpfeiffer avatar Apr 29 '24 07:04 robertpfeiffer

I have put this PR up for the 3.0.0 milestone. We don't have a consensus to do a change like this at this point so I'm fine with this PR being stalled for the foreseeable future, though I will leave this PR open so we can decide in the long term. For now, let's not halt Window progress. This plan can be re-introduced in a backwards compatible fashion if and when needed

ankith26 avatar May 18 '24 08:05 ankith26