Skip to content

Rect attribute kwargs for Surface.blit (3202) #1607

Open
@GalacticEmperor1

Description

@GalacticEmperor1

Issue №3202 opened by Starbuck5 at 2022-05-29 08:43:16

Description

In pygame, as we know, blitted surfaces are placed onto the target surface by the position of their top left corners.

It's very common to want a "centered" blit for example, so it's a common pattern to do

pos_rect = surf.get_rect(center=pos)
screen.blit(surf, pos_rect)

We could modify the blit function to support this need directly.

screen.blit(surf, center=pos)

This would change the current dest argument into an optional argument, along with a host of optional kwargs center, midtop, bottomright, etc.

If none or multiple of the position specifiers are used, that would be a ValueError.

Concerns
Performance? we don't want to burden one of the most common pygame functions with additional calling overhead.


Comments

# # MyreMylar commented at 2022-05-30 06:12:09

If performance is a concern, you could always create a new, more flexible version of blit (with a more modern name?) that includes a range of common surface manipulation options as arguments and then keep blit around as the rawest speed function.

i.e.

my_surf.paste(other_surf, center=pos, crop=crop_rect, scale=2.0, rotation=45)

something like that.


# # Starbuck5 commented at 2022-05-30 06:45:14

Thanks for putting the idea out.

It's kinda funny you say that, since I've thought in the past about making a version of blit that is more performant, by using METH_FASTCALL. Python doesn't expose a good way to deal with kwargs and fastcall though, so this would need to be positional only. Most people also don't use the returned bounding box, but I'm not sure how much performance benefit it would be to get rid of it in a hypothetical faster blit function.

One question your idea raises is "if the rect centering stuff can be inlined into a blit function, why not other things?" Like you bring up with rotation and scale. Other things might want to be more lines of code external to represent the extra work being done, since rotating and scaling Surfaces are both expensive operations.


# # novialriptide commented at 2022-05-31 21:47:30

We discussed in the Discord server that I'll be working on this issue. I'm putting this list of the new kwargs I plan on introducing here as a self note.

  • topleft
  • topright
  • bottomleft
  • bottomright
  • midtop
  • midleft
  • midbottom
  • midright
  • center

# # Starbuck5 commented at 2022-05-31 23:05:05

The new kwargs are not out of nowhere, they should be the same set as all rect attributes that accept a pair of numbers.

But the list looks good, everything matches up.


# # novialriptide commented at 2022-06-02 04:58:16

The new kwargs are not out of nowhere, they should be the same set as all rect attributes that accept a pair of numbers.

But the list looks good, everything matches up.

Oh yeah, I took them from this list here:
https://www.pygame.org/docs/ref/rect.html

x,y
top, left, bottom, right
topleft, bottomleft, topright, bottomright
midtop, midleft, midbottom, midright
center, centerx, centery
size, width, height
w,h

# # ankith26 commented at 2022-06-05 09:37:14

I think we should investigate more on the API here, and get feedback from more people.

I'm not a fan of how we are having to add so many more keywords. I propose the new function signature to look like:

blit(source, dest, area=None, special_flags=0, relative_to="topleft")

This way we are only adding one more keyword argument, and also maintain full backwards compatibility (just like the API previously suggested does too)

This can be an enum (please no more plain constants, enums are the way forward in python 3), or a string (in this case we would type it in the stubs as Literal so that editors can catch typos)

strings are the least verbose to type, but can be a bit more expensive to handle than constants (although I imagine the overhead is tiny and of the order of a few extra microseconds, but considering blit is usually called many times it might be worth benchmarking and making sure the performance is not too bad, if we end up going this way)

If we are going with enums, we can make a member for each position, or we could also do some more funky stuff

 topleft = EnumName.TOP | EnumName.LEFT
 topright = EnumName.TOP | EnumName.RIGHT
 bottomleft = EnumName.BOTTOM | EnumName.LEFT
 bottomright = EnumName.BOTTOM | EnumName.RIGHT
 midleft = EnumName.MID | EnumName.LEFT
 midright = EnumName.MID | EnumName.RIGHT
 midbottom = EnumName.MID | EnumName.BOTTOM
 midright = EnumName.MID | EnumName.BOTTOM
 center = EnumName.CENTRE #  this could also be an alias of EnumName.MID since they kinda mean the same thing)

this way we end up with lesser constants, although this seems to be more verbose


# # Starbuck5 commented at 2022-06-05 09:56:23

Interesting.
I'm not opposed to your idea Ankith.


# # novialriptide commented at 2022-06-05 16:20:41

I'm on board with this idea.


# # ankith26 commented at 2022-06-06 17:01:19

Well, there might still be reasons to stick with kwargs, one of them being that the API will stay consistent with already existing stuff like get_rect.

So personally I'd be fine with anything, I just put some ideas up for consideration.


# # novialriptide commented at 2022-06-07 04:36:37

Should we put a poll in the Discord server to finalize the idea?


# # Matiiss commented at 2022-07-02 13:29:48

blit(source, dest, area=None, special_flags=0, relative_to="topleft")

I agree with using this, but would propose anchor as the parameter name as I have seen it being used in tkinter and it is shorter. I also like the usage of strings (another thing seen in tkinter but also like pygame uses them as an alternative to colors), however, adding enums as an another method could turn out to be beneficial, although I wouldn't see any reason to use them if string literals were available.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions