Implement pitch argument for image.tobytes()

[whydoubt]

The pitch argument was added a while back to image.{fromstring,frombytes,frombuffer}.
This PR adds the equivalent argument to image.{tostring,tobytes}.

At least one place this comes in handy:

• By default, glTexImage2D expects rows to be 4-byte aligned.
• RGB data, when image width is not a multiple of 4, results in rows that are not 4-byte aligned.
• In this case, a pitch argument in tobytes allows for 4-byte alignment.

example: imgbytes = pygame.image.tobytes(surf, 'RGB', pitch=surf.get_width() * 3 + 3 & ~3)

[Starbuck5]

Hello, thank you for your pull request. I think adding a pitch here is a good idea.

Since you're a new contributor to this repo, I had to start up your CI runs manually, that's why they just started now. I see your PR fails the format check. You can resolve that by installing black and clang-format and running python setup.py format.

[whydoubt]

I made updates to fix the formatting warnings.

[ankith26]

Idea to simplify implementation and reduce the diff in this PR: We can just do surf->pitch directly to access the pitch more easily. There is no need for calculating byte_width manually.

[whydoubt]

Idea to simplify implementation and reduce the diff in this PR: We can just do surf->pitch directly to access the pitch more easily. There is no need for calculating byte_width manually.

• surf->pitch is the pitch of the surface being input to the conversion.
• pitch is the pitch of data being output from the conversion.
• byte_width and padding are components of that output pitch.

Even if the bytes-per-pixel of the input and output matched, the surface pitch may still not match either the bytes of pixel data per row, or the output pitch.

[ankith26]

Oh, you are right, I just got confused

[Starbuck5]

Sorry for the long wait on reviews.

I gave this another look today. I'm on board with your approach, however I'm concerned about hascolorkey. It seems like there could be cases for RGBA or ARGB where hascolorkey is not consistent before vs after this PR, which seems like it could cause the output to differ.

Also on a very minor note the versionchanged should be updated for 2.5.0, since that's the development version now.

[whydoubt]

@Starbuck5
As far as I can tell, this is how hascolorkey works out for each of the formats:

before: initialized to SDL_GetColorKey()
after: initialized to 0

 format         before value        after value         value used?
  P              SDL_GetColorKey()   0                   No
  RGB            SDL_GetColorKey()   0                   No
  RGBA           SDL_GetColorKey()   SDL_GetColorKey()   YES
  RGBX           0                   0                   YES
  ARGB           0                   0                   YES
  BGRA           0                   0                   No
  RGBA_PREMULT   SDL_GetColorKey()   0                   No
  ARGB_PREMULT   SDL_GetColorKey()   0                   No

So for the three formats that use hascolorkey, the value will be the same.
FWIW, this also causes SDL_GetColorKey() to only be called when necessary.

[Starbuck5]

@whydoubt You're right!

I was getting confused at

if (strcmp(format, "RGBA"))
    hascolorkey = 0;

Which on a first look appears to be "if RGBA" but is actually the opposite.

[Starbuck5]

I updated the versionchanged notice in the docs for this.

Thanks, this looks good to me!

Wrote and ran a test script locally to be hopefully this sure this doesn't break anything:

import random
import hashlib

import pygame

random.seed(36)

test_formats = [
    "P",
    "RGB",
    "BGR",
    "RGBX",
    "RGBA",
    "ARGB",
    "BGRA",
    "RGBA_PREMULT",
    "ARGB_PREMULT",
]

expected_hashes = [
    "65e513220c81266943a63aa85a5c72dcccdc5bbfb2707c40244bdd1aaf65986d",
    "40a378c728f45074aff9f4f718032e8b4702ec38edb54bb3e71c1f8d423709df",
    "c915b8a3468fac17f1249988269bdb7812bf126530f15a0275a1986dfec4d9ce",
    "7c81301d6c4d6b40c795f198aa399a83f3cbfe6efb1478bcf886c031f5e82509",
    "f302be1ce578b73239651e6f306bd6080ee79217b16bece31fda4113a59a4e11",
    "45749db9a01cea1a4ebd7efa019ca4f46bcda8f64ad33a8ee4451b6818739bbc",
    "99b515eb67741954ba436e31230b988fb469d0b3ce76daf852112245df4666f0",
    "57116be36243be03a4dabf3294b7a775f08eaac81a772e2fbdda2baf9949654f",
    "d67940bc7fd2fa3995b3c388d95721347907b9b2dbd4e780b6e5c0213b2eb034",
    "71f6c50ec54e977552e3c4550b81b70e9a0a163016ebc753cdb352f626c8bb95",
    "eb9791edd251a9017e2b37197eeea77c8cd11225746da9bd74b32945ae845f33",
    "ffbf983d1fb723b1596415949349920459f835f0efbbb75b3396fb202719fe3a",
    "cd338f8ac411ba67fa63bac5d21a2bdbc8250f0607fb937757f1986124be0225",
    "9e88d9d5153f94752fbeb370845b7a183e9b7779661fac463a7975de5aabffae",
    "46576577d2a7c3802d00da54153a0cb6cc84e5ece7b2c922eb97ef5cb7b06eae",
    "ca6f528193b2fd112c67bba2535a3eb4a08e6031495df69e2421d3ae13d9e9ab",
    "e665660cdf5b9459c165fa1f414c47040a302f4ab0be3c7251c93bd05566724c",
    "301cb58200c769b17ce5b6eed778a00b1f0c7c27df529e172b3099a6046031ac",
    "9022d4ce04c89e191f4926c6ce040fbac8aec8fa6fbaee672d775039ebda68ae",
    "e75d9334d70c9f0d27a124fae66698668000924d2a50318f354989491a755041",
    "5c073c6762b98e6214b4c91239bd462ac68c7ff81298328af7fefbb23d87b94b",
    "e4759dae451157c71822ce0ff17598bcc96560531189a5e0753b758a61b74267",
    "0120f2576929009953f994245d5e858abb699223ff65b1b2b004c879a87236b5",
    "883a53ce799e600122c32dd81f91ab5c9fb6e25fd2643a86d024c17545bfa93a",
    "9dba697cd2815f29e970777ca86c71bd1c148ea3eccfe4658c8eaad65446d82f",
    "a307447060848499510eab9b394cf006845d847ef6600f4285ef37768fea8979",
    "ab872716ed62ab49bd2b18f74d8b64a3ccfe197781278c5e5676f9cc38aa5a39",
    "5dae0b2fbbae03f7a1a5bff7975354075cdafe9f05d9571a02578a363efcb620",
]

surf_size = (20, 10)
offset = (3, 7)


def populate_surf(surf):
    for y in range(surf.get_height()):
        for x in range(surf.get_width()):
            surf.set_at(
                (x, y),
                (
                    random.randint(0, 255),
                    random.randint(0, 255),
                    random.randint(0, 255),
                    random.randint(0, 255),
                ),
            )


surf_creations = [
    lambda: pygame.Surface(surf_size, pygame.SRCALPHA, depth=32),
    lambda: pygame.Surface(surf_size, depth=32),
    lambda: pygame.Surface(surf_size, depth=24),
    lambda: pygame.Surface(surf_size, depth=16),
    lambda: pygame.Surface(surf_size, depth=8),
]

hashes = []

for format in test_formats:
    for surf_create in surf_creations:
        surf = surf_create()
        populate_surf(surf)

        try:
            output = pygame.image.tobytes(surf, format)
        except ValueError:
            continue  # some unsupported combos

        sha256 = hashlib.sha256()
        sha256.update(output)
        digest = sha256.hexdigest()

        assert digest == expected_hashes.pop(0)

        hashes.append(digest)
        print(digest)

print(hashes)

Also played with pitch with various surface types in a REPL.

[Starbuck5]

The python lint failure is not this PRs fault, black's default style changed, nothing to worry about. Will be resolved by #2697.

[itzpr3d4t0r]

I mean aside from a minor nitpick LGTM! Thanks for the PR!

[MyreMylar]

Looks like this could also use a quick merge with main to clear up that lint error in math_test, but I think this would be safe to merge anyway.