From 4a91d150af97e70e588e82d698dc8c5e0db8e6fe Mon Sep 17 00:00:00 2001 From: Gaspar Capello Date: Thu, 22 Aug 2024 11:07:54 -0300 Subject: [PATCH] Fix transparent color is possible on opaque sprites (fix #4370) To reproduce the error before this fix on RGBA/Grayscale Color Mode: - New 100x100 RGBA/Grayscale opaque sprite (white background). - Draw something with some gray color in the palette. - Keep the selected gray color as primary color. - Configure as secondary color the mask color (#000000 alpha=0). - Pick 'eraser' tool and erase over the gray color with right click. - Result: The sprite doesn't look more opaque, which is wrong. Also, if we export this sprite, the transparent parts will turn black. A similar problem occurs in Indexed Color Mode, but getting a transparent color in a Background sprite is inevitable if the color of a palette entry is transparent or semi-transparent, since the index must be set as is. This could be fixed in the future at the render stage, however, this could lead to other perceived inconsistencies. For now it'll be left as is. Original issue description: Downloaded PNG in RGB mode fails to support transparency: erase uses secondary color and export PNG replaces transparent color with black Added tests for 'eraser' in 'Replace Color Mode' To make the eraser work in 'Replace Color Mode' within the tests, was implemented the possibility of using the right button in the creation of the point vector. During testing with UI available it was observed that the 'bg' color was copied from the 'fg'. Changed this to be compatible with the way the default value of 'fg' is assigned when it is not specified. This last modification resulted in errors during 'tilemap.lua' due to incompatibility of the type of 'bg' color. This was corrected considering the color type of 'fg' color. Furthermore, it was found that the command 'app.range.tiles = { 1 }' did not finish assigning the tile picks to the activeSite, then 'assert(1, #app.range.tiles)' was failing. This was fixed too. --- src/app/app.cpp | 33 +++++- src/app/script/app_object.cpp | 14 ++- src/app/tools/ink_processing.h | 8 +- src/app/ui_context.cpp | 10 +- tests/scripts/inks.lua | 183 +++++++++++++++++++++++++++++++++ 5 files changed, 242 insertions(+), 6 deletions(-) diff --git a/src/app/app.cpp b/src/app/app.cpp index e0f480e3a..ab273c9c8 100644 --- a/src/app/app.cpp +++ b/src/app/app.cpp @@ -859,8 +859,37 @@ int app_get_color_to_clear_layer(Layer* layer) if (layer->isBackground()) { if (auto* colorBar = ColorBar::instance()) color = colorBar->getBgColor(); - else - color = app::Color::fromRgb(0, 0, 0); // TODO get background color color from doc::Settings + else { + auto c = Preferences::instance().colorBar.bgColor(); + if (layer->sprite()->pixelFormat() == IMAGE_INDEXED) { + if (c.getType() == Color::IndexType) { + color = + (c.getIndex() < layer->sprite()->palette(0)->size() ? + c : + app::Color::fromIndex(0)); + } + else { + Palette* pal = layer->sprite()->palette(0); + color_t bg; + if (c.getType() == Color::RgbType) { + bg = rgba(c.getRed(), c.getGreen(), c.getBlue(), 255); + bg = pal->findBestfit(rgba_getr(bg), + rgba_getg(bg), + rgba_getb(bg), 255, -1); + } + else { + ASSERT(c.getType() == Color::GrayType); + bg = graya(c.getGray(), 255); + bg = pal->findBestfit(graya_getv(bg), + graya_getv(bg), + graya_getv(bg), 255, -1); + } + color = app::Color::fromIndex(bg); + } + } + else + color = app::Color::fromRgb(0, 0, 0); + } } else { // All transparent layers are cleared with the mask color color = app::Color::fromMask(); diff --git a/src/app/script/app_object.cpp b/src/app/script/app_object.cpp index 26c514f6a..20c01b56d 100644 --- a/src/app/script/app_object.cpp +++ b/src/app/script/app_object.cpp @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2018-2023 Igara Studio S.A. +// Copyright (C) 2018-2024 Igara Studio S.A. // Copyright (C) 2015-2018 David Capello // // This program is distributed under the terms of @@ -361,6 +361,9 @@ int App_useTool(lua_State* L) type = lua_getfield(L, 1, "bgColor"); if (type != LUA_TNIL) params.bg = convert_args_into_color(L, -1); + else if (params.fg.getType() == + Preferences::instance().colorBar.bgColor().getType()) + params.bg = Preferences::instance().colorBar.bgColor(); else params.bg = params.fg; lua_pop(L, 1); @@ -470,6 +473,13 @@ int App_useTool(lua_State* L) bool first = true; lua_pushnil(L); + tools::ToolBox* toolbox = App::instance()->toolBox(); + const bool isSelectionInk = + (params.ink == toolbox->getInkById(tools::WellKnownInks::Selection)); + const tools::Pointer::Button button = + (!isSelectionInk ? (buttonIdx == 0 ? tools::Pointer::Button::Left : + tools::Pointer::Button::Right) : + tools::Pointer::Button::Left); while (lua_next(L, -2) != 0) { gfx::Point pt = convert_args_into_point(L, -1); @@ -477,7 +487,7 @@ int App_useTool(lua_State* L) pt, // TODO configurable params tools::Vec2(0.0f, 0.0f), - tools::Pointer::Button::Left, + button, tools::Pointer::Type::Unknown, 0.0f); if (first) { diff --git a/src/app/tools/ink_processing.h b/src/app/tools/ink_processing.h index 8a49d4370..44b63ddc6 100644 --- a/src/app/tools/ink_processing.h +++ b/src/app/tools/ink_processing.h @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2019-2023 Igara Studio S.A. +// Copyright (C) 2019-2024 Igara Studio S.A. // Copyright (C) 2001-2018 David Capello // // This program is distributed under the terms of @@ -570,6 +570,12 @@ public: ReplaceInkProcessing(ToolLoop* loop) { m_color1 = loop->getPrimaryColor(); m_color2 = loop->getSecondaryColor(); + if (loop->getLayer()->isBackground()) { + switch (loop->sprite()->pixelFormat()) { + case IMAGE_RGB: m_color2 |= rgba_a_mask; break; + case IMAGE_GRAYSCALE: m_color2 |= graya_a_mask; break; + } + } m_opacity = loop->getOpacity(); } diff --git a/src/app/ui_context.cpp b/src/app/ui_context.cpp index 018ce8259..888c02f93 100644 --- a/src/app/ui_context.cpp +++ b/src/app/ui_context.cpp @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2019-2023 Igara Studio S.A. +// Copyright (C) 2019-2024 Igara Studio S.A. // Copyright (C) 2001-2018 David Capello // // This program is distributed under the terms of @@ -366,6 +366,14 @@ void UIContext::onGetActiveSite(Site* site) const colorBar->getPaletteView()->getSelectedEntries(picks); site->selectedColors(picks); } + else if (colorBar && + colorBar->getTilesView()->getSelectedEntriesCount() > 0) { + site->focus(Site::InColorBar); + + doc::PalettePicks picks; + colorBar->getTilesView()->getSelectedEntries(picks); + site->selectedTiles(picks); + } else { site->focus(Site::InEditor); } diff --git a/tests/scripts/inks.lua b/tests/scripts/inks.lua index 9cf3e9ca0..673f472e9 100644 --- a/tests/scripts/inks.lua +++ b/tests/scripts/inks.lua @@ -331,4 +331,187 @@ do { 1, 0, 1, 0, 2, 0, 1, 0, 1}) +end + +---------------------------------------------------------------------- +-- Tests for Eraser Tool, normal erase and replace color erasing +---------------------------------------------------------------------- +do + -- Indexed image + background layer + + -- the mask color is in the palette and it's defined as + -- transparent color + local s = Sprite(3, 3, ColorMode.INDEXED) + local p = s.palettes[1] + p:setColor(0, Color{ r=0 , g=0 , b=0 , a=255 }) + p:setColor(1, Color{ r=255, g=0 , b=0 , a=255 }) + p:setColor(2, Color{ r=0 , g=255, b=0 , a=255 }) + p:setColor(3, Color{ r=0 , g=0 , b=255, a=255 }) + p:setColor(4, Color{ r=255, g=255, b=0 , a=255 }) + p:setColor(5, Color{ r=255, g=255, b=255, a=255 }) + p:setColor(6, Color{ r=0 , g=0 , b=0 , a=0 }) + p:resize(7) + + app.fgColor = 0 + app.bgColor = 0 + app.command.BackgroundFromLayer() + expect_img(app.activeImage, + { 0, 0, 0, + 0, 0, 0, + 0, 0, 0 }) + + s.transparentColor = 6 -- mask color as transparent color + app.fgColor = 1 + app.bgColor = 3 + app.useTool{ tool='pencil', + points={ Point(0, 0), Point(2, 0)} } + app.useTool{ tool='pencil', + color=2, + points={ Point(0, 1), Point(2, 1)} } + app.useTool{ tool='pencil', + button=MouseButton.RIGHT, + points={ Point(0, 2), Point(2, 2)} } + expect_img(app.activeImage, + { 1, 1, 1, + 2, 2, 2, + 3, 3, 3 }) + + app.fgColor = 2 + app.bgColor = 6 -- (mask color) + app.useTool{ tool='eraser', + button=MouseButton.LEFT, + points={ Point(0, 0), Point(0, 2) } } + expect_img(app.activeImage, + { 6, 1, 1, + 6, 2, 2, + 6, 3, 3 }) + + app.useTool{ tool='eraser', + button=MouseButton.RIGHT, + points={ Point(1, 0), Point(1, 2) } } + expect_img(app.activeImage, + { 6, 1, 1, + 6, 6, 2, + 6, 3, 3 }) +end + +-- Tests for Eraser Tool with RGBA image + background layer +do + local s = Sprite(3, 3, ColorMode.RGB) + local p = s.palettes[1] + local c0 = app.pixelColor.rgba(0 , 0, 0, 255) + local c1 = app.pixelColor.rgba(255, 0, 0, 255) + local c2 = app.pixelColor.rgba(0 , 255, 0, 255) + local c3 = app.pixelColor.rgba(0 , 0, 255, 255) + local c4 = app.pixelColor.rgba(255, 255, 0, 255) + local c5 = app.pixelColor.rgba(255, 255, 255, 255) + local c6 = app.pixelColor.rgba(0 , 0, 0, 0) + p:setColor(0, c0) + p:setColor(1, c1) + p:setColor(2, c2) + p:setColor(3, c3) + p:setColor(4, c4) + p:setColor(5, c5) + p:setColor(6, c6) + p:resize(7) + + app.fgColor = c0 + app.bgColor = c0 + app.command.BackgroundFromLayer() + expect_img(app.activeImage, + { c0, c0, c0, + c0, c0, c0, + c0, c0, c0 }) + + app.fgColor = c1 + app.bgColor = c3 + app.useTool{ tool='pencil', + points={ Point(0, 0), Point(2, 0)} } + app.useTool{ tool='pencil', + color=c2, + points={ Point(0, 1), Point(2, 1)} } + app.useTool{ tool='pencil', + button=MouseButton.RIGHT, + points={ Point(0, 2), Point(2, 2)} } + expect_img(app.activeImage, + { c1, c1, c1, + c2, c2, c2, + c3, c3, c3 }) + + app.fgColor = c2 + app.bgColor = c6 + app.useTool{ tool='eraser', + button=MouseButton.LEFT, + points={ Point(0, 0), Point(0, 2) } } + expect_img(app.activeImage, + { c0, c1, c1, + c0, c2, c2, + c0, c3, c3 }) + + app.useTool{ tool='eraser', + button=MouseButton.RIGHT, + points={ Point(1, 0), Point(1, 2) } } + expect_img(app.activeImage, + { c0, c1, c1, + c0, c0, c2, + c0, c3, c3 }) +end + +-- Tests for Eraser Tool with GRAYSCALE image + background layer +do + local s = Sprite(3, 3, ColorMode.GRAYSCALE) + local p = s.palettes[1] + local c0 = app.pixelColor.graya( 0, 255) + local c1 = app.pixelColor.graya( 63, 255) + local c2 = app.pixelColor.graya(127, 255) + local c3 = app.pixelColor.graya(191, 255) + local c4 = app.pixelColor.graya(255, 255) + local c5 = app.pixelColor.graya( 0, 0) + p:setColor(0, c0) + p:setColor(1, c1) + p:setColor(2, c2) + p:setColor(3, c3) + p:setColor(4, c4) + p:setColor(5, c5) + p:resize(6) + + app.fgColor = c0 + app.bgColor = c0 + app.command.BackgroundFromLayer() + expect_img(app.activeImage, + { c0, c0, c0, + c0, c0, c0, + c0, c0, c0 }) + + app.fgColor = c1 + app.bgColor = c3 + app.useTool{ tool='pencil', + points={ Point(0, 0), Point(2, 0)} } + app.useTool{ tool='pencil', + color=c2, + points={ Point(0, 1), Point(2, 1)} } + app.useTool{ tool='pencil', + button=MouseButton.RIGHT, + points={ Point(0, 2), Point(2, 2)} } + expect_img(app.activeImage, + { c1, c1, c1, + c2, c2, c2, + c3, c3, c3 }) + + app.fgColor = c2 + app.bgColor = c5 + app.useTool{ tool='eraser', + button=MouseButton.LEFT, + points={ Point(0, 0), Point(0, 2) } } + expect_img(app.activeImage, + { c0, c1, c1, + c0, c2, c2, + c0, c3, c3 }) + + app.useTool{ tool='eraser', + button=MouseButton.RIGHT, + points={ Point(1, 0), Point(1, 2) } } + expect_img(app.activeImage, + { c0, c1, c1, + c0, c0, c2, + c0, c3, c3 }) end \ No newline at end of file