From ec42689b820cc025e685e4f2bc3f230a76bb1e89 Mon Sep 17 00:00:00 2001 From: David Capello Date: Wed, 11 Sep 2024 12:35:21 -0300 Subject: [PATCH] Avoid setting an invalid mask color (-1) in doc::Image (fix #4651) This regression came from 09bb5cc3d3676e88048d5cd845b075ea30ca8e66 as now we don't Sprite::setTransparentColor() on each undo/redo and only when needed. This brought a new kind of error where the mask color for images was set to -1 after convert_pixel_format(). This also fixes a conversion from Indexed -> RGB where the transparent color was not set back to 0. And the transparent color is always set when we are in indexed mode to avoid any assert in debug mode. --- src/app/cmd/set_pixel_format.cpp | 22 ++++++++++++++++++++-- src/app/util/clipboard.cpp | 16 ++++------------ src/app/util/clipboard.h | 3 ++- src/app/util/clipboard_native.cpp | 5 +++-- src/doc/image_spec.h | 16 +++++++++++++--- src/doc/sprite.cpp | 7 +++++-- src/render/quantization.cpp | 17 ++++++++++------- 7 files changed, 57 insertions(+), 29 deletions(-) diff --git a/src/app/cmd/set_pixel_format.cpp b/src/app/cmd/set_pixel_format.cpp index f8f945e03..48254def7 100644 --- a/src/app/cmd/set_pixel_format.cpp +++ b/src/app/cmd/set_pixel_format.cpp @@ -139,20 +139,38 @@ SetPixelFormat::SetPixelFormat(Sprite* sprite, } } - // Set all cels opacity to 100% if we are converting to indexed. + // By default, when converting to RGB or grayscale, the mask color + // is always 0. + int newMaskIndex = 0; if (newFormat == IMAGE_INDEXED) { + // Set all cels opacity to 100% if we are converting to indexed. // TODO remove this (?) for (Cel* cel : sprite->uniqueCels()) { if (cel->opacity() < 255) m_pre.add(new cmd::SetCelOpacity(cel, 255)); } - int newMaskIndex = sprite->palette(0)->findMaskColor(); + // When converting to indexed mode the mask color depends if the + // palette includes a fully transparent entry. + newMaskIndex = sprite->palette(0)->findMaskColor(); if (newMaskIndex < 0) newMaskIndex = 0; + + // We change the transparent color after (m_post) changing the + // color mode (when we are already in indexed mode). if (newMaskIndex != sprite->transparentColor()) m_post.add(new cmd::SetTransparentColor(sprite, newMaskIndex)); } + else if (m_oldFormat == IMAGE_INDEXED) { + // We change the transparent color before (m_pre) changing the + // color mode (when we are still in indexed mode). + if (newMaskIndex != sprite->transparentColor()) + m_pre.add(new cmd::SetTransparentColor(sprite, newMaskIndex)); + } + else { + // RGB <-> Grayscale + ASSERT(sprite->transparentColor() == 0); + } // When we are converting to grayscale color mode, we've to destroy // all palettes and put only one grayscaled-palette at the first diff --git a/src/app/util/clipboard.cpp b/src/app/util/clipboard.cpp index 52bc803ef..397bafe80 100644 --- a/src/app/util/clipboard.cpp +++ b/src/app/util/clipboard.cpp @@ -230,21 +230,13 @@ void Clipboard::setData(Image* image, // Copy tilemap to the native clipboard if (isTilemap) { ASSERT(tileset); - setNativeBitmap(image, mask, palette, tileset); + setNativeBitmap(image, mask, palette, tileset, -1); } // Copy non-tilemap images to the native clipboard else { - color_t oldMask = 0; - if (image) { - oldMask = image->maskColor(); - if (!image_source_is_transparent) - image->setMaskColor(-1); - } - - setNativeBitmap(image, mask, palette); - - if (image && !image_source_is_transparent) - image->setMaskColor(oldMask); + setNativeBitmap( + image, mask, palette, nullptr, + image_source_is_transparent ? image->maskColor(): -1); } } } diff --git a/src/app/util/clipboard.h b/src/app/util/clipboard.h index 18c96aeea..24cbbbbcf 100644 --- a/src/app/util/clipboard.h +++ b/src/app/util/clipboard.h @@ -108,7 +108,8 @@ namespace app { bool setNativeBitmap(const doc::Image* image, const doc::Mask* mask, const doc::Palette* palette, - const doc::Tileset* tileset = nullptr); + const doc::Tileset* tileset, + const doc::color_t indexMaskColor); bool getNativeBitmap(doc::Image** image, doc::Mask** mask, doc::Palette** palette, diff --git a/src/app/util/clipboard_native.cpp b/src/app/util/clipboard_native.cpp index 801c72954..22ccad337 100644 --- a/src/app/util/clipboard_native.cpp +++ b/src/app/util/clipboard_native.cpp @@ -95,7 +95,8 @@ bool Clipboard::hasNativeBitmap() const bool Clipboard::setNativeBitmap(const doc::Image* image, const doc::Mask* mask, const doc::Palette* palette, - const doc::Tileset* tileset) + const doc::Tileset* tileset, + const doc::color_t indexMaskColor) { clip::lock l(native_window_handle()); if (!l.locked()) @@ -180,7 +181,7 @@ bool Clipboard::setNativeBitmap(const doc::Image* image, doc::color_t c = palette->getEntry(*it); // Use alpha=0 for mask color - if (*it == image->maskColor()) + if (*it == indexMaskColor) c &= doc::rgba_rgb_mask; *(dst++) = c; diff --git a/src/doc/image_spec.h b/src/doc/image_spec.h index d77ba67db..ab269dbf0 100644 --- a/src/doc/image_spec.h +++ b/src/doc/image_spec.h @@ -1,5 +1,5 @@ // Aseprite Document Library -// Copyright (C) 2018-2020 Igara Studio S.A. +// Copyright (C) 2018-2024 Igara Studio S.A. // Copyright (c) 2016 David Capello // // This file is released under the terms of the MIT license. @@ -54,8 +54,18 @@ namespace doc { void setColorMode(const ColorMode colorMode) { m_colorMode = colorMode; } void setWidth(const int width) { m_size.w = width; } void setHeight(const int height) { m_size.h = height; } - void setMaskColor(const color_t color) { m_maskColor = color; } - void setColorSpace(const gfx::ColorSpaceRef& cs) { m_colorSpace = cs; } + + void setMaskColor(const color_t color) { +#if 0 // Sometimes, mask color = -1 is temporarily used to paint an + // opaque indexed image in PixelsMovement. + ASSERT(color != -1); +#endif + m_maskColor = color; + } + + void setColorSpace(const gfx::ColorSpaceRef& cs) { + m_colorSpace = cs; + } void setSize(const int width, const int height) { diff --git a/src/doc/sprite.cpp b/src/doc/sprite.cpp index c11a56a26..23d37207a 100644 --- a/src/doc/sprite.cpp +++ b/src/doc/sprite.cpp @@ -250,8 +250,11 @@ bool Sprite::supportAlpha() const void Sprite::setTransparentColor(color_t color) { #if _DEBUG - if (colorMode() != ColorMode::INDEXED) { - ASSERT(color == 0); + if (colorMode() == ColorMode::INDEXED) { + ASSERT(color != -1); // Setting mask = -1 is a logic error + } + else { + ASSERT(color == 0); // Always 0 for non-indexed color modes } #endif // _DEBUG diff --git a/src/render/quantization.cpp b/src/render/quantization.cpp index df2070a7b..0aac52b01 100644 --- a/src/render/quantization.cpp +++ b/src/render/quantization.cpp @@ -142,18 +142,21 @@ Palette* create_palette_from_sprite( Image* convert_pixel_format( const Image* image, Image* new_image, - PixelFormat pixelFormat, + const PixelFormat pixelFormat, const Dithering& dithering, const RgbMap* rgbmap, const Palette* palette, - bool is_background, - color_t new_mask_color, + const bool is_background, + const color_t new_mask_color, rgba_to_graya_func toGray, TaskDelegate* delegate) { if (!new_image) new_image = Image::create(pixelFormat, image->width(), image->height()); - new_image->setMaskColor(new_mask_color); + + // Don't set the image mask color to -1 + const color_t new_mask_color0 = (new_mask_color == -1 ? 0 : new_mask_color); + new_image->setMaskColor(new_mask_color0); // RGB -> Indexed with ordered dithering if (image->pixelFormat() == IMAGE_RGB && @@ -236,7 +239,7 @@ Image* convert_pixel_format( a = rgba_geta(c); if (a == 0) - *dst_it = (new_mask_color == -1? 0 : new_mask_color); + *dst_it = new_mask_color0; else if (rgbmap) *dst_it = rgbmap->mapColor(c); else @@ -293,7 +296,7 @@ Image* convert_pixel_format( c = graya_getv(c); if (a == 0) - *dst_it = (new_mask_color == -1? 0 : new_mask_color); + *dst_it = new_mask_color0; else if (rgbmap) *dst_it = rgbmap->mapColor(c, c, c, a); else @@ -372,7 +375,7 @@ Image* convert_pixel_format( c = *src_it; if (!is_background && c == image->maskColor()) - *dst_it = new_mask_color; + *dst_it = new_mask_color0; else { c = palette->getEntry(c); r = rgba_getr(c);