[image] Fix RGB565+alpha rendering for multi-frame animations (#16017)

Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
J. Nick Koston
2026-04-27 19:57:42 -05:00
committed by GitHub
parent a34836c290
commit 39a69385fb
3 changed files with 89 additions and 8 deletions

View File

@@ -62,7 +62,12 @@ void Animation::set_frame(int frame) {
}
void Animation::update_data_start_() {
const uint32_t image_size = this->get_width_stride() * this->height_;
uint32_t image_size = this->get_width_stride() * this->height_;
// RGB565 with an alpha channel stores the alpha plane immediately after the RGB
// plane within each frame, so the per-frame stride includes the alpha bytes.
if (this->type_ == image::IMAGE_TYPE_RGB565 && this->transparency_ == image::TRANSPARENCY_ALPHA_CHANNEL) {
image_size += static_cast<uint32_t>(this->width_) * this->height_;
}
this->data_start_ = this->animation_data_start_ + image_size * this->current_frame_;
}

View File

@@ -744,21 +744,28 @@ async def write_image(config, all_frames=False):
if frame_count <= 1:
_LOGGER.warning("Image file %s has no animation frames", path)
total_rows = height * frame_count
encoder = IMAGE_TYPE[type](width, total_rows, transparency, dither, invert_alpha)
if byte_order := config.get(CONF_BYTE_ORDER):
# Check for valid type has already been done in validate_settings
encoder.set_big_endian(byte_order == "BIG_ENDIAN")
# Encode each frame with its own encoder and concatenate. This keeps every
# frame self-contained on disk (e.g. RGB565+alpha emits [RGB plane | alpha plane]
# per frame) so animation frame stepping in image.cpp / animation.cpp stays
# correct without needing to know the total frame count.
byte_order = config.get(CONF_BYTE_ORDER)
combined_data: list[int] = []
encoder: ImageEncoder | None = None
for frame_index in range(frame_count):
image.seek(frame_index)
encoder = IMAGE_TYPE[type](width, height, transparency, dither, invert_alpha)
if byte_order is not None:
# Check for valid type has already been done in validate_settings
encoder.set_big_endian(byte_order == "BIG_ENDIAN")
pixels = encoder.convert(image.resize((width, height)), path).getdata()
for row in range(height):
for col in range(width):
encoder.encode(pixels[row * width + col])
encoder.end_row()
encoder.end_image()
encoder.end_image()
combined_data.extend(encoder.data)
rhs = [HexInt(x) for x in encoder.data]
rhs = [HexInt(x) for x in combined_data]
prog_arr = cg.progmem_array(config[CONF_RAW_DATA_ID], rhs)
image_type = get_image_type_enum(type)
trans_value = get_transparency_enum(encoder.transparency)

View File

@@ -7,10 +7,12 @@ from pathlib import Path
from typing import Any
from unittest.mock import MagicMock, patch
from PIL import Image as PILImage
import pytest
from esphome import config_validation as cv
from esphome.components.image import (
CONF_ALPHA_CHANNEL,
CONF_INVERT_ALPHA,
CONF_OPAQUE,
CONF_TRANSPARENCY,
@@ -411,3 +413,70 @@ async def test_svg_with_mm_dimensions_succeeds(
assert 30 < height < 50, (
f"Height should be around 39 pixels for 10mm at 100dpi, got {height}"
)
@pytest.mark.asyncio
async def test_rgb565_alpha_animation_layout_per_frame(
tmp_path: Path,
mock_progmem_array: MagicMock,
) -> None:
"""RGB565+alpha animations must store each frame as a self-contained
[RGB plane | alpha plane] block. Animation::update_data_start_ steps frames
with a single per-frame stride, so any cross-frame layout (all RGB then all
alpha) makes the C++ alpha read land in the next frame's RGB bytes — that
was the regression behind issue #15999.
"""
# Build a 2-frame APNG where each frame is a solid color with a known
# alpha. APNG preserves full RGBA per pixel (GIF only has 1-bit alpha so
# round-tripping mid-range alpha values does not work). Frame 0 is fully
# opaque red, frame 1 is fully transparent blue.
width = 4
height = 3
frame0 = PILImage.new("RGBA", (width, height), (255, 0, 0, 0xFF))
frame1 = PILImage.new("RGBA", (width, height), (0, 0, 255, 0x00))
apng_path = tmp_path / "anim.png"
frame0.save(
apng_path,
format="PNG",
save_all=True,
append_images=[frame1],
duration=100,
loop=0,
)
config = {
CONF_FILE: str(apng_path),
CONF_TYPE: "RGB565",
CONF_TRANSPARENCY: CONF_ALPHA_CHANNEL,
CONF_DITHER: "NONE",
CONF_INVERT_ALPHA: False,
CONF_RAW_DATA_ID: "test_raw_data_id",
}
_, _, _, _, _, frame_count = await write_image(config, all_frames=True)
assert frame_count == 2
# Recover the bytes handed to progmem_array. Signature is (id_, rhs).
_, raw_data = mock_progmem_array.call_args.args
data = [int(x) for x in raw_data]
rgb_size = width * height * 2
alpha_size = width * height
frame_size = rgb_size + alpha_size
assert len(data) == frame_size * frame_count, (
"RGB565+alpha animation buffer must be (RGB + alpha) per frame, not "
"all RGB followed by all alpha"
)
# Frame 0: RGB plane is red, alpha plane is 0xFF. Frame 1: alpha plane is
# 0x00. If the layout regresses to [all RGB | all alpha], the alpha bytes
# would all land at the tail of the buffer and the per-frame slices below
# would point at RGB565 noise instead.
frame0_alpha = data[rgb_size : rgb_size + alpha_size]
frame1_alpha = data[frame_size + rgb_size : frame_size + rgb_size + alpha_size]
assert all(a == 0xFF for a in frame0_alpha), (
f"Frame 0 alpha plane should be opaque, got {frame0_alpha}"
)
assert all(a == 0x00 for a in frame1_alpha), (
f"Frame 1 alpha plane should be transparent, got {frame1_alpha}"
)