Add pygame.transform.bloom function

[damusss]

I implemented the bloom algorithm to add it to pygame.transform.
I also added documentation and stubs.

I think this is the best implementation. This is the steps, if someone needs them to review:

• Create a bright pass filter surface
• Iterate the source surface and keep only the pixels with the luminance within the threshold
• Run either the box or blur algorithm
• On the lines where the blurred color is applied, also blit it to the original surface with additive blend

Please tell me if there are any C improvements, structure improvements or performance improvements.

[itzpr3d4t0r]

personally, I'd prefer gaussian_blur to be default since it's more precise. I believe we should be as precise as possible by default and if the user wants less precision they can change it.

[damusss]

personally, I'd prefer gaussian_blur to be default since it's more precise. I believe we should be as precise as possible by default and if the user wants less precision they can change it.

I set box as default because gassian blur is 5 or 6 times slower, but I guess the default should be the best looking. I think on a real usage, you would use box_blur for realtime effects (on not-too-big surfaces) while gaussian_blur for cached results. the gaussian bloom of realtime big surfaces is out of question, but the same logic applies to gaussian_blur without bloom

[itzpr3d4t0r]

And on this note you could add a note about all this in the docs.

[damusss]

And on this note you could add a note about all this in the docs.

Wonderful idea

[itzpr3d4t0r]

The only mandatory argument is the surface to bloom, I think either just blur_radius or/and intensity should be mandatory as well.
And I think the argumets "order" should be changed a bit and ordered by importance, in case one doesn't use kwargs.
I'd do something like this:
"surface" -> "blur_radius" (mandatory) -> "intensity" (mandatory) -> "luminance_threshold" (optional) -> "blur_type" (optional default="gaussian") -> "dest_surface" (optional).

[itzpr3d4t0r]

The reordering isn't a strong requirement.

[damusss]

Yeah I like this 👍
@itzpr3d4t0r I'll fix this 2 comments once I know if I should keep the buffer or keep the macros (I left a message on the discord server)

[robertpfeiffer]

I see no reason not to merge this, but I think this might be more flexible as three separate operations: Threshold, blur, and blit. At least, I think this could be more useful as a function that computes the bloom overlay with a black or transparent background.

[damusss]

I see no reason not to merge this, but I think this might be more flexible as three separate operations: Threshold, blur, and blit. At least, I think this could be more useful as a function that computes the bloom overlay with a black or transparent background.

Maybe it would be more flexible but not more performant as the blit is implemented within the blur using only 2 loops. I'd like for @itzpr3d4t0r to have a word about what you mentioned. I can always modify it as requested.

[damusss]

To add to what I just said, what i think would be best is to keep bloom as a function and maybe add the luminance filter as another function that can reuse the same code this pull request is using.

[MyreMylar]

After this section I think some details on the expected parameter ranges would be useful, particularly on the valid values for blur_type, which I believe are gaussian and box. Without some guidance it is hard to know if values like 'intensity` should normally be in the range of 0-1000, or 0.0 to1.0.

[MyreMylar]

I gave this the full walt test:

with this test program:

import pygame


display_surf = pygame.display.set_mode((1280, 600))
display_surf.fill((255, 255, 255))

walt = pygame.image.load("images/walt.png").convert_alpha()

walt_2 = pygame.transform.bloom(
    walt, blur_radius=2, intensity=1.5, luminance_threshold=0.6
)
walt_3 = pygame.transform.bloom(
    walt, blur_radius=15, intensity=4, luminance_threshold=0.4, blur_type="box"
)

pygame.init()

clock = pygame.time.Clock()

running = True

while running:
    for event in pygame.event.get():
        if event.type == pygame.QUIT:
            running = False

    display_surf.fill((255, 255, 255))
    display_surf.blit(walt, (0, 100))
    display_surf.blit(walt_2, (417, 100))
    display_surf.blit(walt_3, (834, 100))

    pygame.display.flip()

    clock.tick(60)

I think it is a useful effect that developers will use. They'll probably use it, and all of the blurs more if we can eventually get them SIMD accelerated in some manner, but I have no problem adding this among them. Especially as it is reusing a lot of the code from the blurs.

My main criticism is that the input parameters are quite loosely defined right now in the documentation with no expected ranges, and not much description of what they effect in the output.

[damusss]

My main criticism is that the input parameters are quite loosely defined right now in the documentation with no expected ranges, and not much description of what they effect in the output.

you have a point. I'll improve the documentation and explain what each argument does

[MyreMylar]

Nice docs, LGTM 👍