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

Remove shuffle masks in AVX fillers #2642

Merged

Conversation

itzpr3d4t0r
Copy link
Member

This PR gets rid of some masks we used for shuffling since we can achieve the same result with simpler code all at the same performance.

new:

Flag: BLEND_SUB
fill: 0.023698640009388327
blit: 0.027505460020620376
--------------------
Flag: BLEND_ADD
fill: 0.02056832000380382
blit: 0.02708268000278622
--------------------
Flag: BLEND_MULT
fill: 0.03269677999196574
blit: 0.04088069999124855
--------------------
Flag: BLEND_MIN
fill: 0.01716678000520915
blit: 0.02253773999400437
--------------------
Flag: BLEND_MAX
fill: 0.01713769999332726
blit: 0.022413880005478858
--------------------

old:

Flag: BLEND_SUB
fill: 0.02369291998911649
blit: 0.0274960600072518
--------------------
Flag: BLEND_ADD
fill: 0.020125439984258264
blit: 0.027089380007237197
--------------------
Flag: BLEND_MULT
fill: 0.032841119973454624
blit: 0.04257354000583291
--------------------
Flag: BLEND_MIN
fill: 0.01882048000115901
blit: 0.022260580002330242
--------------------
Flag: BLEND_MAX
fill: 0.01668819998158142
blit: 0.022541640000417827
--------------------

Test Program:

from timeit import repeat

import pygame

pygame.init()

surf = pygame.Surface((500, 500))
surf.fill((132, 33, 200))

color = pygame.Surface((500, 500))
color.fill((24, 24, 24))

flags = [
    "BLEND_SUB",
    "BLEND_ADD",
    "BLEND_MULT",
    "BLEND_MIN",
    "BLEND_MAX",
]

G = globals()

for flag in flags:
    print(f"Flag: {flag}")
    teststr = "surf.fill((24, 24, 24), None, pygame." + flag + ")"
    l = [min(repeat(teststr, globals=G, number=1000, repeat=10)) for _ in range(5)]
    print(f"fill: {sum(l) / len(l)}")

    teststr = "surf.blit(color, (0, 0), None, pygame." + flag + ")"
    l = [min(repeat(teststr, globals=G, number=1000, repeat=10)) for _ in range(5)]
    print(f"blit: {sum(l) / len(l)}")
    print("-" * 20)

@itzpr3d4t0r itzpr3d4t0r added Code quality/robustness Code quality and resilience to changes SIMD labels Dec 31, 2023
@itzpr3d4t0r itzpr3d4t0r requested a review from a team as a code owner December 31, 2023 12:15
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 - straightforward instruction swaps that seem equivalent to me.

@itzpr3d4t0r itzpr3d4t0r changed the title Simplified AVX fillers Remove shuffe masks in AVX fillers Jan 15, 2024
@Starbuck5 Starbuck5 changed the title Remove shuffe masks in AVX fillers Remove shuffle masks in AVX fillers Jan 17, 2024
@Starbuck5
Copy link
Member

Tested correctness with a variant of my correctness program from #2656 (comment)

import random
import hashlib

import pygame

random.seed(36)

test_flags = [
    pygame.BLEND_RGB_ADD,
    pygame.BLEND_RGBA_ADD,
    pygame.BLEND_RGB_SUB,
    pygame.BLEND_RGBA_SUB,
    pygame.BLEND_RGB_MAX,
    pygame.BLEND_RGBA_MAX,
    pygame.BLEND_RGB_MIN,
    pygame.BLEND_RGBA_MIN,
    pygame.BLEND_RGB_MULT,
    pygame.BLEND_RGBA_MULT,
]

expected_hashes = [
    "89bd2ab15e86cda268b39b1732b67e28ebd2f2ce306004d523458d066a33e820",
    "e36896ac1233ba1c546f129d32f4f867969349107c56cb74b06330c8831b4275",
    "61faa3d0bf86ca7657d4bec572b0fd97affb52110f4770125ec2d14efdf81088",
    "8713d221902c99f727566c2c921db682a04e718c0aca6baaefdfd638cc7d8b94",
    "0cdc97df7a809de2cfd93ef80177d41b4b26d43463fe553ae9d3c957929513aa",
    "488881e2a425502d0a3809584d81450e0f0e1de9a729269ecd52dbdda7d91ef4",
    "f475c0bd7fc4fb15d6757fa9926c63da4a0606fe4ebbf9ef7cdb745c330f45d8",
    "6bbf0fb2cb67931146657149ebd3cfcf5979f7c2e8e89013c08467cbbbb9c834",
    "03669121a5b75f974d6567cb7b11bb803d373d93366c37e93ad59862e4850d2b",
    "f7e0ef07ff29eddaa0c2a902edeb446a001b073ea4fb9cf1420e7cc79db28a44",
]

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),
                ),
            )


hashes = []

rects = [pygame.Rect(2, 2, 2, 2), pygame.Rect(0, 0, 19, 10), pygame.Rect(10, 5, 4, 4)]

for flag in test_flags:
    dest = pygame.Surface(surf_size, pygame.SRCALPHA)

    populate_surf(dest)

    for rect in rects:
        dest.fill(
            (
                random.randint(0, 255),
                random.randint(0, 255),
                random.randint(0, 255),
                random.randint(0, 255),
            ),
            rect,
            special_flags=flag,
        )

    sha256 = hashlib.sha256()
    sha256.update(pygame.image.tobytes(dest, "RGBA"))
    digest = sha256.hexdigest()

    assert digest == expected_hashes.pop(0)

    hashes.append(digest)
    print(digest)

print(hashes)

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. Thanks for the improvement.

@Starbuck5 Starbuck5 added this to the 2.5.0 milestone Jan 17, 2024
@Starbuck5 Starbuck5 merged commit dddc7bf into pygame-community:main Jan 17, 2024
30 checks passed
@itzpr3d4t0r itzpr3d4t0r deleted the fillers_simplifications branch January 17, 2024 11:40
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 SIMD
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants