diff --git a/esphome/components/animation/animation.cpp b/esphome/components/animation/animation.cpp index c2ae3b2f76..2f59a7fa5a 100644 --- a/esphome/components/animation/animation.cpp +++ b/esphome/components/animation/animation.cpp @@ -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(this->width_) * this->height_; + } this->data_start_ = this->animation_data_start_ + image_size * this->current_frame_; } diff --git a/esphome/components/image/__init__.py b/esphome/components/image/__init__.py index 8375ab91d3..365554f7d2 100644 --- a/esphome/components/image/__init__.py +++ b/esphome/components/image/__init__.py @@ -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) diff --git a/tests/component_tests/image/test_init.py b/tests/component_tests/image/test_init.py index 6f73888c7d..f7f60a1f4d 100644 --- a/tests/component_tests/image/test_init.py +++ b/tests/component_tests/image/test_init.py @@ -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}" + )