fix(indexed): stop transparent pixels reverting to opaque black#24
Open
Fuitad wants to merge 6 commits into
Open
fix(indexed): stop transparent pixels reverting to opaque black#24Fuitad wants to merge 6 commits into
Fuitad wants to merge 6 commits into
Conversation
draw_pixels wrote to cel.image via Image:putPixel using sprite coordinates, but putPixel expects cel-LOCAL coordinates. Once a layer's cel is trimmed to a non-(0,0) origin (e.g. after a shape tool runs), every pixel was offset by the cel's position, and pixels falling outside the cel's bounds were dropped entirely. The shape tools are unaffected because they use app.useTool, which operates in sprite space. Normalize the target cel to a full-canvas image at (0,0) before writing (compositing any existing content first), so sprite coordinates equal image coordinates. This also gives a freshly created cel a valid image, fixing draw_pixels on newly added layers. Aseprite re-trims the cel on save, so canvas dimensions and file size are unchanged. Tests: - pkg/aseprite/lua_test.go: unit assertion that the generated script normalizes the cel to full-canvas at (0,0). Runs without Aseprite, so it guards the fix in CI, where the integration tests do not run. - pkg/tools: integration tests for the offset-after-shape case and for an asymmetric cel origin with a pixel outside the shape's bounds (the lost-pixel case). Both fail on stock and pass with this change.
…ette color Palette:resize silently clamps Sprite.transparentColor into the new palette's valid index range. After set_palette, quantize_palette, add_palette_color, or apply_auto_shading resize an indexed sprite's palette, transparentColor can land on the same index as a real, in-use color (typically the palette's last entry). Once that happens, painting "transparent" (fill_area with a fully transparent color) writes the colliding index, so background fills read back as an opaque real color instead of transparent, and pixels intentionally drawn in that color can be auto-trimmed away as if they were transparent - the same class of bug fixed for index 0 in b5175b8, reintroduced at index N-1. Fix: re-assert transparentColor = 255 after every palette resize on an indexed sprite via a shared ReassertIndexedTransparentColor() helper, keeping it out of the palette's real, addressable range. Adds a regression test reproducing the exact collision and verifying both the drawn color and the transparent fill read back correctly.
Root cause: on an indexed sprite, img:putPixel and app.useTool both expect a palette index, not an arbitrary RGBA value. Passing a Color object directly resolves correctly when the sprite's palette was set earlier in the SAME Aseprite process, but pixel-mcp runs each tool call as its own `aseprite --batch` invocation: the palette is set in one process and drawing happens in the next. In that (normal, for every real tool call) cross-process scenario, Aseprite's own Color-to-index resolution for img:putPixel/app.useTool does not reliably match the sprite's actual, correctly-loaded palette (verified by reading spr.palettes[1] back at the point of failure) and silently resolves to an unrelated index instead, corrupting the drawn color with no error. Affects draw_pixels, draw_line, draw_contour, draw_rectangle, draw_circle, and fill_area whenever use_palette is false (the default) on an indexed sprite. Fix: resolveExactPaletteColor (pkg/aseprite/lua_core.go) resolves the color itself by reading the sprite's actual active palette at Lua runtime, bypassing Aseprite's unreliable resolution entirely. - Fully-transparent colors (alpha 0, e.g. a "paint transparent" fill_area call) resolve directly to spr.transparentColor, since transparency is a designated index, not a color expected to exist in the palette. - Any other color requires an exact match in the active palette and errors if none exists, so a caller drawing an approximate/non-locked color silently gets the wrong pixel instead of an error telling them to pass use_palette=true to snap to the nearest palette color. Wired into DrawPixels (FormatColorWithPalette) and DrawLine/ DrawContour/DrawRectangle/DrawCircle/FillArea (FormatColorWithPaletteForTool). Testing: - go test ./...: 628 passed - go test -tags=integration ./...: 805 passed - Added TestIntegration_IndexedMode_ExactColorResolution (pkg/tools/exact_palette_color_indexed_mode_bug_test.go): exact match via draw_rectangle, exact match via draw_pixels, and error on no match. Verified RED without the fix (all three fail/misbehave exactly as described above) and GREEN with it. - Updated two pre-existing tests that asserted the old, buggy behavior: TestFormatColorWithPalette_NoPalette and TestLuaGenerator_DrawPixels now expect resolveExactPaletteColor(...) instead of a raw Color(...) literal. - Fixed TestIntegration_SetPalette_ThenApplyShading, which drew an approximate "mid-tone" color with use_palette=false even though that color was never one of the locked palette's 16 exact entries; it now correctly passes use_palette=true to snap it.
MedianCutQuantization split each bucket at the pixel-array midpoint (mid := len(pixels) / 2), not at a boundary between distinct colors. Pixel art typically has wildly imbalanced color frequency (a large solid fill vastly outnumbering a 1-2px accent detail), so splitting at the raw pixel-count median lands the cut inside the dominant color's own run instead of between colors. The dominant color got needlessly re-subdivided into near-duplicate buckets while rare accent colors never got isolated into their own bucket, and were instead averaged into a blended, off-palette hex value - even when target_colors comfortably exceeded the number of unique colors actually present, which should guarantee lossless recovery. On a 32x32 sprite with a ~200px tunic fill, ~90px skin fill, a thin outline, and 2px highlight/buckle accents, quantizing to target_colors=32 (13 unique colors present) returned colors like #373130 that did not exist anywhere in the source image. Fix: reduce to a frequency-weighted histogram of unique colors before bucketing, so a split always falls between distinct colors. The split point still favors common colors over rare ones by cumulative pixel count, preserving normal median-cut reduction behavior when target_colors is genuinely smaller than the number of unique colors. I initially suspected a second bug in ApplyQuantizedPalette/ ChangePixelFormat (pixels resolving to the wrong palette index, transparency lost on conversion to indexed mode). Verified via aseprite --batch scripts and full end-to-end integration tests (Go clustering -> Lua apply -> get_pixels) that ChangePixelFormat correctly honors a palette set immediately before it runs, in both the lossless (target_colors >= unique colors) and genuine lossy reduction cases. The apparent scrambling in my initial hand-written repro script was caused by omitting the existing ReassertIndexedTransparentColor() call, not a new bug - so no change was needed there. Testing: - go test ./...: 633 passed - go test -tags=integration ./...: unchanged pre-existing suite passed, plus the new regression test - Added TestMedianCutQuantization_ImbalancedFrequencyPreservesExactColors (pkg/aseprite/quantization_test.go): pure-Go unit test, no Aseprite needed, guards the fix in CI - Added TestIntegration_QuantizePalette_ImbalancedFrequencyPreservesExactColors (pkg/tools/quantize_palette_imbalanced_frequency_bug_test.go): reproduces the exact hero-sprite shape end to end through the real quantize_palette pipeline (export -> Go quantize -> Lua apply -> get_pixels), checking both exact colors and transparency survive conversion to indexed mode. Verified RED without the fix (blended colors and wrong pixels, matching the originally observed corruption) and GREEN with it.
On indexed sprites CreateCanvas sets transparentColor = 255 so palette index 0 can hold a real color, which makes index 0 an opaque color. Two paths left transparent areas showing that opaque index-0 background: - draw_pixels normalizes the target cel into a full-canvas Image() (filled with index 0) and composites the existing content with the default NORMAL blend, which skips the source's transparent (index 255) pixels. Any pixel a previous call had made transparent but the current call does not rewrite flipped to opaque black. Fixed by clearing the full-canvas image to spr.transparentColor before compositing/drawing, in both the existing-cel and new-cel paths. - create_canvas left the initial cel filled with index 0, so a freshly created indexed canvas started with an opaque background. Fixed by clearing the initial cel to spr.transparentColor right after creation. RGB and grayscale behavior is unchanged (both guarded to indexed mode). Adds behavioral integration tests that reproduce both cases and CI-safe string checks.
Author
|
Revalidated on 2026-07-20 against Aseprite 1.3.17.2.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On indexed sprites
CreateCanvassetsSprite.transparentColor = 255so palette index 0 can hold a real color. That makes index 0 an opaque color, and two code paths left transparent areas showing that opaque index-0 background (black on a DawnBringer 32 palette):draw_pixels(reported bug). The tool normalizes the target cel into a full-canvasImage(spr.width, spr.height, spr.colorMode), which Aseprite fills with index 0. Compositing the existing content with the defaultNORMALblend (full:drawImage(cel.image, cel.position)) skips the source's transparent (index 255) pixels, so every pixel a previousdraw_pixelscall made transparent, but the current call does not rewrite, silently flips to opaque black. In practice this corrupts already-transparent regions whenever a sprite is drawn across multiple calls.Fixed by clearing the full-canvas image to
spr.transparentColorbefore compositing/drawing, in both the existing-cel and new-cel paths.create_canvas(related). The initial cel of a new indexed sprite is filled with index 0, so a freshly created indexed canvas starts with an opaque background instead of transparent. Fixed by clearing the initial cel tospr.transparentColorright after creation.RGB and grayscale behavior is unchanged; both fixes are guarded to indexed mode.
Root cause
Image()initializes indexed pixels to index 0, but on these sprites index 255 is the transparent index. Anywhere the code builds a fresh full-canvas image and treats it as empty/transparent, the background is actually opaque index 0, andNORMAL-blenddrawImagethen preserves that opaque background wherever the source is transparent.Tests
draw_pixelscall preserving previously-transparent pixels, and a fresh indexed canvas reading back transparent. Both fail on the pre-fix code (transparent pixels read#000000FF) and pass after.Note on base
This builds on the
draw_pixelscel-normalization work (#20 / #21): the full-canvas compositing that this corrects is not onmainyet, so until those merge this PR's diff shows those commits stacked underneath. The net new change here is the twospr.transparentColorclears plus their tests.