Fix laplacian dst format and simplify 24bit writes

[Starbuck5]

This is a follow-up to #3790

CodeRabbit said the bugfix in that PR was lacking from laplacian, and it was wrong about that. However, going through laplacian found that it also has a bug with writing pixel data-- it is using the wrong surface's information about what to write.

For laplacian--
Previously, the color gathered in acolor was mapped using the source surface's information, and then written into the destination surface. Since destination surfaces are restricted to the same depth, this did work most of the time, but would not work if two surfaces had the same depth but different pixel layouts. Except for in the 24 bit path, which would map RGB, then unmap with source format, then remap into channels in destination format.

Simplified 24 bit writes--
These are using mapped colors, they can be stored directly into the pixel buffer. There is no need to "unmap" them using the pixelformat and store them in the red, green, and blue channels separately.
(This is related because simplifying the 24 bit writes broke the laplacian test, because of the aforementioned bug)

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR refactors 24-bpp pixel write logic to use fixed byte positions based on system byte order instead of per-format channel shifts, and corrects the laplacian function to map colors using the destination surface's format and palette. A regression test validates laplacian correctness when source and destination have different channel masks.

Changes

24-bpp pixel write and laplacian cross-format fix

Layer / File(s)	Summary
24-bpp pixel write refactoring src_c/surface.c, src_c/transform.c	The 24-bpp branches in surf_set_at (surface.c) and transform SURF_SET_AT macros (little-endian and big-endian paths) replace per-format Rshift/Gshift/Bshift channel offset calculations with direct byte_buf[0..2] assignments derived from SDL_BYTEORDER and bit shifts of the mapped color value.
Laplacian color mapping and destination write src_c/transform.c	The laplacian() function retrieves the destination surface's palette and uses it with the destination format to map output colors via PG_MapRGBA(destformat, destsurf_palette, ...) instead of using source format. The 3-bpp destination write path uses the refactored byte_buf assignments with the laplacian-computed color.
Regression test test/transform_test.py	New test_laplacian__cross_format validates that pygame.transform.laplacian produces correct RGB output when transforming between surfaces with matching bytes-per-pixel but different channel layouts (RGBA source, BGRA destination).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• pygame-community/pygame-ce#3314: Both PRs modify transform.c's per-pixel color handling—including 24-bpp byte-order writes and laplacian color-mapping—to use surface-format-aware routines/palettes.

Suggested labels

Surface

Suggested reviewers

• ankith26
• itzpr3d4t0r

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely summarizes the two main changes: fixing the laplacian destination format bug and simplifying 24-bit pixel write logic.
Description check	✅ Passed	The description clearly explains both the laplacian format bug and the 24-bit write simplification, with detailed context about why these changes are necessary.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

test/transform_test.py (1)

1272-1304: ⚡ Quick win

Consider asserting the surfaces have different channel masks before the laplacian call.

The test validates that laplacian works when source and destination have different channel layouts, but doesn't explicitly verify that the layouts are actually different before running the transform. Adding a precondition assertion would make the test more self-documenting and catch potential regressions in the test setup itself.

🔍 Suggested precondition check

         s2 = pygame.Surface(
             (SIZE, SIZE),
             0,
             32,
             masks=(0xFF0000, 0xFF00, 0xFF, 0xFF000000),
         )
+        # Verify the surfaces actually have different channel layouts
+        self.assertNotEqual(s1.get_masks(), s2.get_masks())
         pygame.transform.laplacian(s1, s2)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/transform_test.py` around lines 1272 - 1304, Add a precondition
assertion in test_laplacian__cross_format to ensure the source and destination
actually have different channel layouts: check s1.get_masks() != s2.get_masks()
(or compare masks tuples created when constructing s1/s2) before calling
pygame.transform.laplacian so the test fails early if the surfaces end up with
the same channel masks; place this assertion right after s2 is created and
before the laplacian call.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@test/transform_test.py`:
- Around line 1272-1304: Add a precondition assertion in
test_laplacian__cross_format to ensure the source and destination actually have
different channel layouts: check s1.get_masks() != s2.get_masks() (or compare
masks tuples created when constructing s1/s2) before calling
pygame.transform.laplacian so the test fails early if the surfaces end up with
the same channel masks; place this assertion right after s2 is created and
before the laplacian call.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4c334018-a822-46b3-8ac0-a2a91dea887c

📥 Commits

Reviewing files that changed from the base of the PR and between 747df97 and 1315c94.

📒 Files selected for processing (3)

• src_c/surface.c
• src_c/transform.c
• test/transform_test.py