From 0d5075ff9363c3d7d7c1918458b5aea3bc876100 Mon Sep 17 00:00:00 2001 From: David Capello Date: Wed, 21 Feb 2024 12:47:27 -0300 Subject: [PATCH 01/39] Add -noinapp option to disable Steam "in game" visibility (fix #4314) Some minor changes in this commit includes the usage of std::unique_ptr for the Pimpl-idiom in steam::SteamAPI class and renaming the SteamAPI::initialized() to SteamAPI::isInitialized() to avoid confusion reading the code. Forum post: https://steamcommunity.com/app/431730/discussions/2/7260435303111061192/ --- src/app/CMakeLists.txt | 5 ++++- src/app/app.cpp | 22 ++++++++++++++++++---- src/app/app.h | 5 ++++- src/app/cli/app_options.cpp | 12 +++++++++++- src/app/cli/app_options.h | 8 +++++++- src/steam/steam.cpp | 10 ++++------ src/steam/steam.h | 8 +++++--- 7 files changed, 53 insertions(+), 17 deletions(-) diff --git a/src/app/CMakeLists.txt b/src/app/CMakeLists.txt index 7d48cddd6..979ea91b7 100644 --- a/src/app/CMakeLists.txt +++ b/src/app/CMakeLists.txt @@ -775,7 +775,10 @@ if(ENABLE_UPDATER) endif() if(ENABLE_STEAM) - add_definitions(-DENABLE_STEAM) + # We need the ENABLE_STEAM flag in main module too so AppOptions are + # equal in both modules, app-lib and main (that's why this flag is + # marked as PUBLIC). + target_compile_definitions(app-lib PUBLIC -DENABLE_STEAM) target_link_libraries(app-lib steam-lib) endif() diff --git a/src/app/app.cpp b/src/app/app.cpp index 9c6413629..9979fc6a9 100644 --- a/src/app/app.cpp +++ b/src/app/app.cpp @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2018-2023 Igara Studio S.A. +// Copyright (C) 2018-2024 Igara Studio S.A. // Copyright (C) 2001-2018 David Capello // // This program is distributed under the terms of @@ -328,6 +328,11 @@ int App::initialize(const AppOptions& options) app_configure_drm(); #endif +#ifdef ENABLE_STEAM + if (options.noInApp()) + m_inAppSteam = false; +#endif + // Load modules m_modules = std::make_unique(createLogInDesktop, preferences()); m_legacy = std::make_unique(isGui() ? REQUIRE_INTERFACE: 0); @@ -510,9 +515,18 @@ void App::run() // Initialize Steam API #ifdef ENABLE_STEAM - steam::SteamAPI steam; - if (steam.initialized()) - os::instance()->activateApp(); + std::unique_ptr steam; + if (m_inAppSteam) { + steam = std::make_unique(); + if (steam->isInitialized()) + os::instance()->activateApp(); + } + else { + // We tried to load the Steam SDK without calling + // SteamAPI_InitSafe() to check if we could run the program + // without "in game" indication but still capturing screenshots + // on Steam, and that wasn't the case. + } #endif #if defined(_DEBUG) || defined(ENABLE_DEVMODE) diff --git a/src/app/app.h b/src/app/app.h index 24426f9a5..b55c79dc0 100644 --- a/src/app/app.h +++ b/src/app/app.h @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2018-2023 Igara Studio S.A. +// Copyright (C) 2018-2024 Igara Studio S.A. // Copyright (C) 2001-2018 David Capello // // This program is distributed under the terms of @@ -142,6 +142,9 @@ namespace app { std::unique_ptr m_legacy; bool m_isGui; bool m_isShell; +#ifdef ENABLE_STEAM + bool m_inAppSteam = true; +#endif std::unique_ptr m_mainWindow; base::paths m_files; #ifdef ENABLE_UI diff --git a/src/app/cli/app_options.cpp b/src/app/cli/app_options.cpp index 86767337e..741863173 100644 --- a/src/app/cli/app_options.cpp +++ b/src/app/cli/app_options.cpp @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2018-2022 Igara Studio S.A. +// Copyright (C) 2018-2024 Igara Studio S.A. // Copyright (C) 2001-2017 David Capello // // This program is distributed under the terms of @@ -79,6 +79,9 @@ AppOptions::AppOptions(int argc, const char* argv[]) , m_exportTileset(m_po.add("export-tileset").description("Export only tilesets from visible tilemap layers")) , m_verbose(m_po.add("verbose").mnemonic('v').description("Explain what is being done")) , m_debug(m_po.add("debug").description("Extreme verbose mode and\ncopy log to desktop")) +#ifdef ENABLE_STEAM + , m_noInApp(m_po.add("noinapp").description("Disable \"in game\" visibility on Steam\nDoesn't count playtime")) +#endif #ifdef _WIN32 , m_disableWintab(m_po.add("disable-wintab").description("Don't load wintab32.dll library")) #endif @@ -121,6 +124,13 @@ bool AppOptions::hasExporterParams() const m_po.enabled(m_sheet); } +#ifdef ENABLE_STEAM +bool AppOptions::noInApp() const +{ + return m_po.enabled(m_noInApp); +} +#endif + #ifdef _WIN32 bool AppOptions::disableWintab() const { diff --git a/src/app/cli/app_options.h b/src/app/cli/app_options.h index 172752e82..5d3cbde43 100644 --- a/src/app/cli/app_options.h +++ b/src/app/cli/app_options.h @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2018-2022 Igara Studio S.A. +// Copyright (C) 2018-2024 Igara Studio S.A. // Copyright (C) 2001-2017 David Capello // // This program is distributed under the terms of @@ -95,6 +95,9 @@ public: const Option& exportTileset() const { return m_exportTileset; } bool hasExporterParams() const; +#ifdef ENABLE_STEAM + bool noInApp() const; +#endif #ifdef _WIN32 bool disableWintab() const; #endif @@ -166,6 +169,9 @@ private: Option& m_verbose; Option& m_debug; +#ifdef ENABLE_STEAM + Option& m_noInApp; +#endif #ifdef _WIN32 Option& m_disableWintab; #endif diff --git a/src/steam/steam.cpp b/src/steam/steam.cpp index e99d3ed11..ab27228d2 100644 --- a/src/steam/steam.cpp +++ b/src/steam/steam.cpp @@ -134,7 +134,7 @@ public: unloadLib(); } - bool initialized() const { + bool isInitialized() const { return m_initialized; } @@ -239,7 +239,7 @@ SteamAPI* SteamAPI::instance() } SteamAPI::SteamAPI() - : m_impl(new Impl) + : m_impl(std::make_unique()) { ASSERT(g_instance == nullptr); g_instance = this; @@ -247,15 +247,13 @@ SteamAPI::SteamAPI() SteamAPI::~SteamAPI() { - delete m_impl; - ASSERT(g_instance == this); g_instance = nullptr; } -bool SteamAPI::initialized() const +bool SteamAPI::isInitialized() const { - return m_impl->initialized(); + return m_impl->isInitialized(); } void SteamAPI::runCallbacks() diff --git a/src/steam/steam.h b/src/steam/steam.h index 4f3ba4748..1ac7e7105 100644 --- a/src/steam/steam.h +++ b/src/steam/steam.h @@ -1,5 +1,5 @@ // Aseprite Steam Wrapper -// Copyright (c) 2020 Igara Studio S.A. +// Copyright (c) 2020-2024 Igara Studio S.A. // Copyright (c) 2016 David Capello // // This file is released under the terms of the MIT license. @@ -9,6 +9,8 @@ #define STEAM_STEAM_H_INCLUDED #pragma once +#include + namespace steam { class SteamAPI { @@ -18,7 +20,7 @@ public: SteamAPI(); ~SteamAPI(); - bool initialized() const; + bool isInitialized() const; void runCallbacks(); bool writeScreenshot(void* rgbBuffer, @@ -27,7 +29,7 @@ public: private: class Impl; - Impl* m_impl; + std::unique_ptr m_impl; }; } // namespace steam From 10dda30a15a58d09e561a59594f348b4db3a4405 Mon Sep 17 00:00:00 2001 From: David Capello Date: Thu, 22 Feb 2024 19:43:37 -0300 Subject: [PATCH 02/39] Don't write color2 chunk for files with more than 256 colors (fix #4322) We were incorrectly saving a wrong number of entries for palettes with more than 256 colors in color2 chunk, anyway it doesn't make sense to use this chunks as it doesn't support more than 256 colors. So we removed it for this case. We've also removed the palette chunk for cases where it's not required at all, e.g. when we have less than 256 colors and doesn't have alpha channel, it makes sense to use the color2 chunk as it's smaller in the output file. --- docs/ase-file-specs.md | 9 ++++++--- src/app/file/ase_format.cpp | 14 +++++++++----- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/docs/ase-file-specs.md b/docs/ase-file-specs.md index 8442ca9bb..18ff5b247 100644 --- a/docs/ase-file-specs.md +++ b/docs/ase-file-specs.md @@ -126,9 +126,12 @@ at least. ### Old palette chunk (0x0004) -Ignore this chunk if you find the new palette chunk (0x2019) Aseprite -v1.1 saves both chunks 0x0004 and 0x2019 just for backward -compatibility. +Ignore this chunk if you find the new palette chunk (0x2019). Aseprite +v1.1 saves both chunks (0x0004 and 0x2019) just for backward +compatibility. Aseprite v1.3.5 writes this chunk if the palette +doesn't have alpha channel and contains 256 colors or less (because +this chunk is smaller), in other case the new palette chunk (0x2019) +will be used (and the old one is not saved anymore). WORD Number of packets + For each packet diff --git a/src/app/file/ase_format.cpp b/src/app/file/ase_format.cpp index 8ca37ff26..20fd02588 100644 --- a/src/app/file/ase_format.cpp +++ b/src/app/file/ase_format.cpp @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2018-2023 Igara Studio S.A. +// Copyright (C) 2018-2024 Igara Studio S.A. // Copyright (C) 2001-2018 David Capello // // This program is distributed under the terms of @@ -355,7 +355,7 @@ bool AseFormat::onSave(FileOp* fop) bool require_new_palette_chunk = false; for (Palette* pal : sprite->getPalettes()) { - if (pal->size() != 256 || pal->hasAlpha()) { + if (pal->size() > 256 || pal->hasAlpha()) { require_new_palette_chunk = true; break; } @@ -393,9 +393,12 @@ bool AseFormat::onSave(FileOp* fop) ase_file_write_palette_chunk(f, &frame_header, pal, palFrom, palTo); } - - // Write color chunk for backward compatibility only - ase_file_write_color2_chunk(f, &frame_header, pal); + else { + // Use old color chunk only when the palette has 256 or less + // colors, and we don't need the alpha channel (as this chunk + // is smaller than the new palette chunk). + ase_file_write_color2_chunk(f, &frame_header, pal); + } } // Write extra chunks in the first frame @@ -676,6 +679,7 @@ static void ase_file_write_color2_chunk(FILE* f, dio::AsepriteFrameHeader* frame // First packet fputc(0, f); // skip 0 colors + ASSERT(pal->size() <= 256); // For >256 we use the palette chunk fputc(pal->size() == 256 ? 0: pal->size(), f); // number of colors for (c=0; csize(); c++) { color = pal->getEntry(c); From 6ab2731fadc6f235c209fe49239b3dc80a57cc89 Mon Sep 17 00:00:00 2001 From: David Capello Date: Fri, 23 Feb 2024 20:00:01 -0300 Subject: [PATCH 03/39] Remove verbosity in .exe file description on Windows (fix #4333) --- src/main/resources_win32.rc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/resources_win32.rc b/src/main/resources_win32.rc index 3758d2c27..8e295ea99 100644 --- a/src/main/resources_win32.rc +++ b/src/main/resources_win32.rc @@ -21,12 +21,12 @@ BEGIN BEGIN VALUE "Comments", "https://www.aseprite.org/" VALUE "CompanyName", "Igara Studio S.A." - VALUE "FileDescription", "Aseprite - Animated sprites editor & pixel art tool" + VALUE "FileDescription", "Aseprite" VALUE "FileVersion", "1,3,0,0" VALUE "InternalName", "aseprite" VALUE "LegalCopyright", "Copyright (C) 2001-2024 Igara Studio S.A." VALUE "OriginalFilename", "aseprite.exe" - VALUE "ProductName", "ASEPRITE" + VALUE "ProductName", "Aseprite" VALUE "ProductVersion", "1,3,0,0" END END From 078dac28d7064b00e6a167f11b71b43e8012005c Mon Sep 17 00:00:00 2001 From: Gaspar Capello Date: Tue, 6 Feb 2024 14:29:29 -0300 Subject: [PATCH 04/39] Drop selection if we hide the layer (fix #4179) --- src/app/ui/editor/pixels_movement.cpp | 36 +++++++++++++++++++++------ src/app/ui/editor/pixels_movement.h | 8 +++++- 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/src/app/ui/editor/pixels_movement.cpp b/src/app/ui/editor/pixels_movement.cpp index 982f919bb..3ab2680f9 100644 --- a/src/app/ui/editor/pixels_movement.cpp +++ b/src/app/ui/editor/pixels_movement.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 @@ -44,7 +44,6 @@ #include "doc/mask.h" #include "doc/sprite.h" #include "gfx/region.h" -#include "render/render.h" #include @@ -1066,10 +1065,8 @@ void PixelsMovement::drawImage( if (renderOriginalLayer) { render::Render render; - render.renderLayer( - dst, m_site.layer(), m_site.frame(), - gfx::Clip(bounds.x-pt.x, bounds.y-pt.y, bounds), - BlendMode::SRC); + renderLayer(&render, dst, m_site.layer(), m_site.frame(), + gfx::Clip(bounds.x-pt.x, bounds.y-pt.y, bounds)); } color_t maskColor = m_maskColor; @@ -1310,8 +1307,11 @@ CelList PixelsMovement::getEditableCels() // TODO This case is used in paste too, where the cel() can be // nullptr (e.g. we paste the clipboard image into an empty // cel). + // Note: m_site.layer()->canEditPixels() is not used since + // PixelsMovement can modify hidden layers. if (m_site.layer() && - m_site.layer()->canEditPixels()) { + m_site.layer()->isEditable() && + !m_site.layer()->isReference()) { cels.push_back(m_site.cel()); } return cels; @@ -1320,7 +1320,8 @@ CelList PixelsMovement::getEditableCels() // Current cel (m_site.cel()) can be nullptr when we paste in an // empty cel (Ctrl+V) and cut (Ctrl+X) the floating pixels. if (m_site.cel() && - m_site.cel()->layer()->canEditPixels()) { + m_site.cel()->layer()->isEditable() && + !m_site.cel()->layer()->isReference()) { CelList::iterator it; // If we are in a linked cel, remove the cel that matches the @@ -1440,6 +1441,25 @@ void PixelsMovement::reproduceAllTransformationsWithInnerCmds() updateDocumentMask(); } +void PixelsMovement::renderLayer( + render::Render* render, + Image* dstImage, + const Layer* layer, + frame_t frame, + const gfx::Clip& area) +{ + // On PixelsMovement is posible to modify the pixels on + // the current layer, to do so we need to include the layer + // on the renderPlan (inside renderLayer), which only includes + // visible layers, that is why we turn ON the layer visibility + // temporarily. + const bool layerIsVisible = m_site.layer()->isVisible(); + m_site.layer()->setVisible(true); + render->renderLayer(dstImage, layer, frame, area, BlendMode::SRC); + if (!layerIsVisible) + m_site.layer()->setVisible(false); +} + #if _DEBUG void PixelsMovement::dumpInnerCmds() { diff --git a/src/app/ui/editor/pixels_movement.h b/src/app/ui/editor/pixels_movement.h index b5b13d1d8..db6c03c84 100644 --- a/src/app/ui/editor/pixels_movement.h +++ b/src/app/ui/editor/pixels_movement.h @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2019-2021 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 @@ -20,6 +20,7 @@ #include "doc/image_ref.h" #include "gfx/size.h" #include "obs/connection.h" +#include "render/render.h" #include @@ -159,6 +160,11 @@ namespace app { const double angle); CelList getEditableCels(); void reproduceAllTransformationsWithInnerCmds(); + void renderLayer(render::Render* render, + Image* dstImage, + const Layer* layer, + frame_t frame, + const gfx::Clip& area); #if _DEBUG void dumpInnerCmds(); #endif From f22603caea379f0247e5596f4f0edd111594f00a Mon Sep 17 00:00:00 2001 From: Gaspar Capello Date: Wed, 21 Feb 2024 14:44:40 -0300 Subject: [PATCH 05/39] Fix crash when trying to access a property of a Style which is nullptr (fix #4015) Before this fix, an incomplete custom theme or an outdated official theme could cause a crash during Aseprite startup. This fix does not alert the artist the problem of the theme. Simply avoid the crash. --- src/app/ui/skin/skin_theme.h | 4 ++-- src/ui/grid.cpp | 5 ++++- src/ui/theme.cpp | 5 ++++- src/ui/theme.h | 6 +++++- src/ui/widget.cpp | 6 ++++-- 5 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/app/ui/skin/skin_theme.h b/src/app/ui/skin/skin_theme.h index 2791d8193..53eae6db5 100644 --- a/src/app/ui/skin/skin_theme.h +++ b/src/app/ui/skin/skin_theme.h @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2020-2023 Igara Studio S.A. +// Copyright (C) 2020-2024 Igara Studio S.A. // Copyright (C) 2001-2017 David Capello // // This program is distributed under the terms of @@ -108,7 +108,7 @@ namespace app { if (it != m_styles.end()) return it->second; else - return nullptr; + return getDefaultStyle(); } SkinPartPtr getPartById(const std::string& id) const { diff --git a/src/ui/grid.cpp b/src/ui/grid.cpp index b380da915..6ac7824a3 100644 --- a/src/ui/grid.cpp +++ b/src/ui/grid.cpp @@ -1,5 +1,5 @@ // Aseprite UI Library -// Copyright (C) 2018-2023 Igara Studio S.A. +// Copyright (C) 2018-2024 Igara Studio S.A. // Copyright (C) 2001-2017 David Capello // // This file is released under the terms of the MIT license. @@ -125,6 +125,9 @@ Grid::Info Grid::getChildInfo(Widget* child) void Grid::setStyle(Style* style) { + ASSERT(style); + if (!style) + style = Theme::getDefaultStyle(); Widget::setStyle(style); setGap(style->gap()); } diff --git a/src/ui/theme.cpp b/src/ui/theme.cpp index 9af6140eb..5a9487e0a 100644 --- a/src/ui/theme.cpp +++ b/src/ui/theme.cpp @@ -1,5 +1,5 @@ // Aseprite UI Library -// Copyright (C) 2019-2023 Igara Studio S.A. +// Copyright (C) 2019-2024 Igara Studio S.A. // Copyright (C) 2001-2018 David Capello // // This file is released under the terms of the MIT license. @@ -142,6 +142,9 @@ Theme::~Theme() set_theme(nullptr, guiscale()); } +// static +ui::Style Theme::m_defaultStyle(nullptr); + void Theme::regenerateTheme() { set_mouse_cursor(kNoCursor); diff --git a/src/ui/theme.h b/src/ui/theme.h index f3a2aeb7d..8c29fbd15 100644 --- a/src/ui/theme.h +++ b/src/ui/theme.h @@ -1,5 +1,5 @@ // Aseprite UI Library -// Copyright (C) 2020-2022 Igara Studio S.A. +// Copyright (Cg) 2020-2024 Igara Studio S.A. // Copyright (C) 2001-2017 David Capello // // This file is released under the terms of the MIT license. @@ -141,6 +141,8 @@ namespace ui { static void drawTextBox(Graphics* g, const Widget* textbox, int* w, int* h, gfx::Color bg, gfx::Color fg); + static ui::Style* getDefaultStyle() { return &m_defaultStyle; } + protected: virtual void onRegenerateTheme() = 0; @@ -165,6 +167,8 @@ namespace ui { gfx::Size& sizeHint, gfx::Border& borderHint, gfx::Rect& textHint, int& textAlign); + + static ui::Style m_defaultStyle; }; } // namespace ui diff --git a/src/ui/widget.cpp b/src/ui/widget.cpp index 7095f1db0..599c09504 100644 --- a/src/ui/widget.cpp +++ b/src/ui/widget.cpp @@ -1,5 +1,5 @@ // Aseprite UI Library -// Copyright (C) 2018-2023 Igara Studio S.A. +// Copyright (C) 2018-2024 Igara Studio S.A. // Copyright (C) 2001-2018 David Capello // // This file is released under the terms of the MIT license. @@ -199,7 +199,9 @@ void Widget::setTheme(Theme* theme) void Widget::setStyle(Style* style) { assert_ui_thread(); - + ASSERT(style); + if (!style) + style = Theme::getDefaultStyle(); m_style = style; m_border = m_theme->calcBorder(this, style); m_bgColor = m_theme->calcBgColor(this, style); From 5dbaa295c54001ced8615622af03e841ebb87d16 Mon Sep 17 00:00:00 2001 From: David Capello Date: Mon, 26 Feb 2024 10:13:07 -0300 Subject: [PATCH 06/39] Revert "Drop selection if we hide the layer (fix #4179)" This reverts commit 078dac28d7064b00e6a167f11b71b43e8012005c. --- src/app/ui/editor/pixels_movement.cpp | 36 ++++++--------------------- src/app/ui/editor/pixels_movement.h | 8 +----- 2 files changed, 9 insertions(+), 35 deletions(-) diff --git a/src/app/ui/editor/pixels_movement.cpp b/src/app/ui/editor/pixels_movement.cpp index 3ab2680f9..982f919bb 100644 --- a/src/app/ui/editor/pixels_movement.cpp +++ b/src/app/ui/editor/pixels_movement.cpp @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2019-2024 Igara Studio S.A. +// Copyright (C) 2019-2023 Igara Studio S.A. // Copyright (C) 2001-2018 David Capello // // This program is distributed under the terms of @@ -44,6 +44,7 @@ #include "doc/mask.h" #include "doc/sprite.h" #include "gfx/region.h" +#include "render/render.h" #include @@ -1065,8 +1066,10 @@ void PixelsMovement::drawImage( if (renderOriginalLayer) { render::Render render; - renderLayer(&render, dst, m_site.layer(), m_site.frame(), - gfx::Clip(bounds.x-pt.x, bounds.y-pt.y, bounds)); + render.renderLayer( + dst, m_site.layer(), m_site.frame(), + gfx::Clip(bounds.x-pt.x, bounds.y-pt.y, bounds), + BlendMode::SRC); } color_t maskColor = m_maskColor; @@ -1307,11 +1310,8 @@ CelList PixelsMovement::getEditableCels() // TODO This case is used in paste too, where the cel() can be // nullptr (e.g. we paste the clipboard image into an empty // cel). - // Note: m_site.layer()->canEditPixels() is not used since - // PixelsMovement can modify hidden layers. if (m_site.layer() && - m_site.layer()->isEditable() && - !m_site.layer()->isReference()) { + m_site.layer()->canEditPixels()) { cels.push_back(m_site.cel()); } return cels; @@ -1320,8 +1320,7 @@ CelList PixelsMovement::getEditableCels() // Current cel (m_site.cel()) can be nullptr when we paste in an // empty cel (Ctrl+V) and cut (Ctrl+X) the floating pixels. if (m_site.cel() && - m_site.cel()->layer()->isEditable() && - !m_site.cel()->layer()->isReference()) { + m_site.cel()->layer()->canEditPixels()) { CelList::iterator it; // If we are in a linked cel, remove the cel that matches the @@ -1441,25 +1440,6 @@ void PixelsMovement::reproduceAllTransformationsWithInnerCmds() updateDocumentMask(); } -void PixelsMovement::renderLayer( - render::Render* render, - Image* dstImage, - const Layer* layer, - frame_t frame, - const gfx::Clip& area) -{ - // On PixelsMovement is posible to modify the pixels on - // the current layer, to do so we need to include the layer - // on the renderPlan (inside renderLayer), which only includes - // visible layers, that is why we turn ON the layer visibility - // temporarily. - const bool layerIsVisible = m_site.layer()->isVisible(); - m_site.layer()->setVisible(true); - render->renderLayer(dstImage, layer, frame, area, BlendMode::SRC); - if (!layerIsVisible) - m_site.layer()->setVisible(false); -} - #if _DEBUG void PixelsMovement::dumpInnerCmds() { diff --git a/src/app/ui/editor/pixels_movement.h b/src/app/ui/editor/pixels_movement.h index db6c03c84..b5b13d1d8 100644 --- a/src/app/ui/editor/pixels_movement.h +++ b/src/app/ui/editor/pixels_movement.h @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2019-2024 Igara Studio S.A. +// Copyright (C) 2019-2021 Igara Studio S.A. // Copyright (C) 2001-2018 David Capello // // This program is distributed under the terms of @@ -20,7 +20,6 @@ #include "doc/image_ref.h" #include "gfx/size.h" #include "obs/connection.h" -#include "render/render.h" #include @@ -160,11 +159,6 @@ namespace app { const double angle); CelList getEditableCels(); void reproduceAllTransformationsWithInnerCmds(); - void renderLayer(render::Render* render, - Image* dstImage, - const Layer* layer, - frame_t frame, - const gfx::Clip& area); #if _DEBUG void dumpInnerCmds(); #endif From a2b294b0fe3757f039b3cedf93291dee1d019602 Mon Sep 17 00:00:00 2001 From: David Capello Date: Mon, 26 Feb 2024 15:32:40 -0300 Subject: [PATCH 07/39] Add final modifier to ToolLoopImpl to avoid clang-tidy warning The warning is about using a virtual member function in the destructor where it can be overridden by a derived class so in this case the derived version wouldn't be called. Just testing clang-tidy to see if we can add something in the CI. --- src/app/ui/editor/tool_loop_impl.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/app/ui/editor/tool_loop_impl.cpp b/src/app/ui/editor/tool_loop_impl.cpp index 123c55c9a..a82e3f880 100644 --- a/src/app/ui/editor/tool_loop_impl.cpp +++ b/src/app/ui/editor/tool_loop_impl.cpp @@ -452,8 +452,8 @@ protected: ////////////////////////////////////////////////////////////////////// // For drawing -class ToolLoopImpl : public ToolLoopBase, - public EditorObserver { +class ToolLoopImpl final : public ToolLoopBase, + public EditorObserver { Context* m_context; bool m_filled; bool m_previewFilled; @@ -592,6 +592,9 @@ public: m_editor->remove_observer(this); #endif + // getSrcImage() is a virtual member function but ToolLoopImpl is + // marked as final to avoid not calling a derived version from + // this destructor. if (m_floodfillSrcImage != getSrcImage()) delete m_floodfillSrcImage; } @@ -954,7 +957,7 @@ tools::ToolLoop* create_tool_loop_for_script( #ifdef ENABLE_UI -class PreviewToolLoopImpl : public ToolLoopBase { +class PreviewToolLoopImpl final : public ToolLoopBase { Image* m_image; public: From e949a5401dd2496d9aa11a26a98ade950524f88b Mon Sep 17 00:00:00 2001 From: David Capello Date: Mon, 26 Feb 2024 13:11:26 -0300 Subject: [PATCH 08/39] Drop selection when we hide a layer that is being transformed (fix #4179, fix #3254) This fixes several problems in MovingPixelsState where hidden layers were transformed anyway when we switched the visibility of a layer in this state. Other fix was tried before in #3254 but we needed the onBefore/After layer visibility change notifications to make this work properly (i.e. drop pixels when the visiblity of a layer is changed). The only drawback at this moment is that changing the visibility of the non-active layer when we are transforming multiple cels/timeline range can be confused because we don't have #2144/#2865 implemented yet. This bug was originally reported here: https://community.aseprite.org/t/20621 --- src/app/commands/cmd_layer_visibility.cpp | 15 ++++---- src/app/doc.cpp | 26 ++++++++++++- src/app/doc.h | 12 +++++- src/app/doc_observer.h | 6 ++- src/app/script/layer_class.cpp | 6 ++- src/app/ui/doc_view.cpp | 28 +++++++++++++- src/app/ui/doc_view.h | 9 ++++- src/app/ui/editor/editor.cpp | 8 +++- src/app/ui/editor/editor.h | 3 +- src/app/ui/editor/editor_state.h | 8 +++- src/app/ui/editor/moving_pixels_state.cpp | 21 ++++++++++- src/app/ui/editor/moving_pixels_state.h | 46 ++++++++++++----------- src/app/ui/editor/pixels_movement.cpp | 7 ++-- src/app/ui/editor/pixels_movement.h | 4 +- src/app/ui/timeline/timeline.cpp | 30 ++++++++------- src/app/ui/timeline/timeline.h | 3 +- src/app/util/layer_utils.cpp | 28 ++++++++++++-- 17 files changed, 197 insertions(+), 63 deletions(-) diff --git a/src/app/commands/cmd_layer_visibility.cpp b/src/app/commands/cmd_layer_visibility.cpp index aa95920ff..49d423934 100644 --- a/src/app/commands/cmd_layer_visibility.cpp +++ b/src/app/commands/cmd_layer_visibility.cpp @@ -1,4 +1,5 @@ // Aseprite +// Copyright (C) 2024 Igara Studio S.A. // Copyright (C) 2001-2017 David Capello // // This program is distributed under the terms of @@ -8,7 +9,6 @@ #include "config.h" #endif -#include "app/app.h" #include "app/commands/command.h" #include "app/context_access.h" #include "app/modules/gui.h" @@ -49,7 +49,7 @@ bool LayerVisibilityCommand::onChecked(Context* context) return false; SelectedLayers selLayers; - auto range = App::instance()->timeline()->range(); + DocRange range = context->activeSite().range(); if (range.enabled()) { selLayers = range.selectedLayers(); } @@ -67,24 +67,25 @@ bool LayerVisibilityCommand::onChecked(Context* context) void LayerVisibilityCommand::onExecute(Context* context) { ContextWriter writer(context); + Doc* doc = writer.document(); SelectedLayers selLayers; - auto range = App::instance()->timeline()->range(); + DocRange range = context->activeSite().range(); if (range.enabled()) { selLayers = range.selectedLayers(); } else { selLayers.insert(writer.layer()); } + bool anyVisible = false; for (auto layer : selLayers) { if (layer->isVisible()) anyVisible = true; } - for (auto layer : selLayers) { - layer->setVisible(!anyVisible); - } - update_screen_for_document(writer.document()); + const bool newState = !anyVisible; + for (auto layer : selLayers) + doc->setLayerVisibilityWithNotifications(layer, newState); } Command* CommandFactory::createLayerVisibilityCommand() diff --git a/src/app/doc.cpp b/src/app/doc.cpp index f18685e6b..006b9c00e 100644 --- a/src/app/doc.cpp +++ b/src/app/doc.cpp @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2018-2023 Igara Studio S.A. +// Copyright (C) 2018-2024 Igara Studio S.A. // Copyright (C) 2001-2018 David Capello // // This program is distributed under the terms of @@ -193,6 +193,16 @@ color_t Doc::bgColor(Layer* layer) const return layer->sprite()->transparentColor(); } +////////////////////////////////////////////////////////////////////// +// Modifications with notifications + +void Doc::setLayerVisibilityWithNotifications(Layer* layer, const bool visible) +{ + notifyBeforeLayerVisibilityChange(layer, visible); + layer->setVisible(visible); + notifyAfterLayerVisibilityChange(layer); +} + ////////////////////////////////////////////////////////////////////// // Notifications @@ -244,6 +254,20 @@ void Doc::notifyLayerMergedDown(Layer* srcLayer, Layer* targetLayer) notify_observers(&DocObserver::onLayerMergedDown, ev); } +void Doc::notifyBeforeLayerVisibilityChange(Layer* layer, bool newState) +{ + DocEvent ev(this); + ev.layer(layer); + notify_observers(&DocObserver::onBeforeLayerVisibilityChange, ev, newState); +} + +void Doc::notifyAfterLayerVisibilityChange(Layer* layer) +{ + DocEvent ev(this); + ev.layer(layer); + notify_observers(&DocObserver::onAfterLayerVisibilityChange, ev); +} + void Doc::notifyCelMoved(Layer* fromLayer, frame_t fromFrame, Layer* toLayer, frame_t toFrame) { DocEvent ev(this); diff --git a/src/app/doc.h b/src/app/doc.h index 2a8118444..41f2b47ee 100644 --- a/src/app/doc.h +++ b/src/app/doc.h @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2018-2023 Igara Studio S.A. +// Copyright (C) 2018-2024 Igara Studio S.A. // Copyright (C) 2001-2018 David Capello // // This program is distributed under the terms of @@ -107,6 +107,14 @@ namespace app { os::ColorSpaceRef osColorSpace() const { return m_osColorSpace; } + ////////////////////////////////////////////////////////////////////// + // Modifications with notifications + + // Use this function to change the layer visibility and notify all + // DocObservers about this change (e.g. so the Editor can be + // invalidated/redrawn, MovingPixelsState can drop pixels, etc.) + void setLayerVisibilityWithNotifications(Layer* layer, const bool visible); + ////////////////////////////////////////////////////////////////////// // Notifications @@ -116,6 +124,8 @@ namespace app { void notifySpritePixelsModified(Sprite* sprite, const gfx::Region& region, frame_t frame); void notifyExposeSpritePixels(Sprite* sprite, const gfx::Region& region); void notifyLayerMergedDown(Layer* srcLayer, Layer* targetLayer); + void notifyBeforeLayerVisibilityChange(Layer* layer, bool newState); + void notifyAfterLayerVisibilityChange(Layer* layer); void notifyCelMoved(Layer* fromLayer, frame_t fromFrame, Layer* toLayer, frame_t toFrame); void notifyCelCopied(Layer* fromLayer, frame_t fromFrame, Layer* toLayer, frame_t toFrame); void notifySelectionChanged(); diff --git a/src/app/doc_observer.h b/src/app/doc_observer.h index a400fb08c..07e072063 100644 --- a/src/app/doc_observer.h +++ b/src/app/doc_observer.h @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2018-2023 Igara Studio S.A. +// Copyright (C) 2018-2024 Igara Studio S.A. // Copyright (C) 2001-2018 David Capello // // This program is distributed under the terms of @@ -97,6 +97,10 @@ namespace app { // The collapsed/expanded flag of a specific layer changed. virtual void onLayerCollapsedChanged(DocEvent& ev) { } + // The visibility flag of a specific layer is going to change/changed. + virtual void onBeforeLayerVisibilityChange(DocEvent& ev, bool newState) { } + virtual void onAfterLayerVisibilityChange(DocEvent& ev) { } + // The tileset was remapped (e.g. when tiles are re-ordered). virtual void onRemapTileset(DocEvent& ev, const doc::Remap& remap) { } diff --git a/src/app/script/layer_class.cpp b/src/app/script/layer_class.cpp index 68054d1a3..f320a68a9 100644 --- a/src/app/script/layer_class.cpp +++ b/src/app/script/layer_class.cpp @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2018-2023 Igara Studio S.A. +// Copyright (C) 2018-2024 Igara Studio S.A. // Copyright (C) 2018 David Capello // // This program is distributed under the terms of @@ -329,7 +329,9 @@ int Layer_set_isEditable(lua_State* L) int Layer_set_isVisible(lua_State* L) { auto layer = get_docobj(L, 1); - layer->setVisible(lua_toboolean(L, 2)); + const bool newState = lua_toboolean(L, 2); + Doc* doc = static_cast(layer->sprite()->document()); + doc->setLayerVisibilityWithNotifications(layer, newState); return 0; } diff --git a/src/app/ui/doc_view.cpp b/src/app/ui/doc_view.cpp index 43532d24e..fcf95d3fb 100644 --- a/src/app/ui/doc_view.cpp +++ b/src/app/ui/doc_view.cpp @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2018-2023 Igara Studio S.A. +// Copyright (C) 2018-2024 Igara Studio S.A. // Copyright (C) 2001-2018 David Capello // // This program is distributed under the terms of @@ -478,7 +478,16 @@ void DocView::onTotalFramesChanged(DocEvent& ev) void DocView::onLayerRestacked(DocEvent& ev) { - m_editor->invalidate(); + if (hasContentInActiveFrame(ev.layer())) + m_editor->invalidate(); +} + +void DocView::onAfterLayerVisibilityChange(DocEvent& ev) +{ + // If there is no cel for this layer in the current frame, there is + // no need to redraw the editor + if (hasContentInActiveFrame(ev.layer())) + m_editor->invalidate(); } void DocView::onTilesetChanged(DocEvent& ev) @@ -653,4 +662,19 @@ void DocView::onCancel(Context* ctx) } } +bool DocView::hasContentInActiveFrame(const doc::Layer* layer) const +{ + if (!layer) + return false; + else if (layer->cel(m_editor->frame())) + return true; + else if (layer->isGroup()) { + for (const doc::Layer* child : static_cast(layer)->layers()) { + if (hasContentInActiveFrame(child)) + return true; + } + } + return false; +} + } // namespace app diff --git a/src/app/ui/doc_view.h b/src/app/ui/doc_view.h index 4b703d049..696965f75 100644 --- a/src/app/ui/doc_view.h +++ b/src/app/ui/doc_view.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 @@ -15,6 +15,10 @@ #include "app/ui/workspace_view.h" #include "ui/box.h" +namespace doc { + class Layer; +} + namespace ui { class View; } @@ -86,6 +90,7 @@ namespace app { void onAfterRemoveCel(DocEvent& ev) override; void onTotalFramesChanged(DocEvent& ev) override; void onLayerRestacked(DocEvent& ev) override; + void onAfterLayerVisibilityChange(DocEvent& ev) override; void onTilesetChanged(DocEvent& ev) override; // InputChainElement impl @@ -105,6 +110,8 @@ namespace app { bool onProcessMessage(ui::Message* msg) override; private: + bool hasContentInActiveFrame(const doc::Layer* layer) const; + Type m_type; Doc* m_document; ui::View* m_view; diff --git a/src/app/ui/editor/editor.cpp b/src/app/ui/editor/editor.cpp index 8e152b115..c7882bbd1 100644 --- a/src/app/ui/editor/editor.cpp +++ b/src/app/ui/editor/editor.cpp @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2018-2023 Igara Studio S.A. +// Copyright (C) 2018-2024 Igara Studio S.A. // Copyright (C) 2001-2018 David Capello // // This program is distributed under the terms of @@ -2438,6 +2438,12 @@ void Editor::onRemoveSlice(DocEvent& ev) } } +void Editor::onBeforeLayerVisibilityChange(DocEvent& ev, bool newState) +{ + if (m_state) + m_state->onBeforeLayerVisibilityChange(this, ev.layer(), newState); +} + void Editor::setCursor(const gfx::Point& mouseDisplayPos) { Rect vp = View::getView(this)->viewportBounds(); diff --git a/src/app/ui/editor/editor.h b/src/app/ui/editor/editor.h index 35c37bff7..5a160a9cf 100644 --- a/src/app/ui/editor/editor.h +++ b/src/app/ui/editor/editor.h @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2018-2023 Igara Studio S.A. +// Copyright (C) 2018-2024 Igara Studio S.A. // Copyright (C) 2001-2018 David Capello // // This program is distributed under the terms of @@ -351,6 +351,7 @@ namespace app { void onAddTag(DocEvent& ev) override; void onRemoveTag(DocEvent& ev) override; void onRemoveSlice(DocEvent& ev) override; + void onBeforeLayerVisibilityChange(DocEvent& ev, bool newState) override; // ActiveToolObserver impl void onActiveToolChange(tools::Tool* tool) override; diff --git a/src/app/ui/editor/editor_state.h b/src/app/ui/editor/editor_state.h index 631922a0f..b2118befd 100644 --- a/src/app/ui/editor/editor_state.h +++ b/src/app/ui/editor/editor_state.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-2016 David Capello // // This program is distributed under the terms of @@ -26,6 +26,7 @@ namespace ui { } namespace doc { + class Layer; class Tag; } @@ -145,6 +146,11 @@ namespace app { // collection. virtual void onBeforeRemoveLayer(Editor* editor) { } + // Called when the visibility of a specific layer is changed. + virtual void onBeforeLayerVisibilityChange(Editor* editor, + doc::Layer* layer, + bool newState) { } + private: DISABLE_COPYING(EditorState); }; diff --git a/src/app/ui/editor/moving_pixels_state.cpp b/src/app/ui/editor/moving_pixels_state.cpp index 70c1c9323..a76985b21 100644 --- a/src/app/ui/editor/moving_pixels_state.cpp +++ b/src/app/ui/editor/moving_pixels_state.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 @@ -578,6 +578,25 @@ bool MovingPixelsState::acceptQuickTool(tools::Tool* tool) tool->getInk(0)->isZoom()); } +void MovingPixelsState::onBeforeLayerVisibilityChange(Editor* editor, + doc::Layer* layer, + bool newState) +{ + if (!isActiveDocument()) + return; + + // If the layer visibility of any selected layer changes, we just + // drop the pixels (it's the easiest way to avoid modifying hidden + // pixels). + if (m_pixelsMovement) { + const Site& site = m_pixelsMovement->site(); + if (site.layer() == layer || + site.range().contains(layer)) { + dropPixels(); + } + } +} + // Before executing any command, we drop the pixels (go back to standby). void MovingPixelsState::onBeforeCommandExecution(CommandExecutionEvent& ev) { diff --git a/src/app/ui/editor/moving_pixels_state.h b/src/app/ui/editor/moving_pixels_state.h index 78c247165..b7e9827b5 100644 --- a/src/app/ui/editor/moving_pixels_state.h +++ b/src/app/ui/editor/moving_pixels_state.h @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2019-2022 Igara Studio S.A. +// Copyright (C) 2019-2024 Igara Studio S.A. // Copyright (C) 2001-2017 David Capello // // This program is distributed under the terms of @@ -21,7 +21,7 @@ #include "ui/timer.h" namespace doc { - class Image; + class Layer; } namespace app { @@ -54,35 +54,37 @@ namespace app { void updateTransformation(const Transformation& t); // EditorState - virtual void onEnterState(Editor* editor) override; - virtual void onEditorGotFocus(Editor* editor) override; - virtual LeaveAction onLeaveState(Editor* editor, EditorState* newState) override; - virtual void onActiveToolChange(Editor* editor, tools::Tool* tool) override; - virtual bool onMouseDown(Editor* editor, ui::MouseMessage* msg) override; - virtual bool onMouseUp(Editor* editor, ui::MouseMessage* msg) override; - virtual bool onMouseMove(Editor* editor, ui::MouseMessage* msg) override; - virtual bool onSetCursor(Editor* editor, const gfx::Point& mouseScreenPos) override; - virtual bool onKeyDown(Editor* editor, ui::KeyMessage* msg) override; - virtual bool onKeyUp(Editor* editor, ui::KeyMessage* msg) override; - virtual bool onUpdateStatusBar(Editor* editor) override; - virtual bool acceptQuickTool(tools::Tool* tool) override; - virtual bool requireBrushPreview() override { return false; } + void onEnterState(Editor* editor) override; + void onEditorGotFocus(Editor* editor) override; + LeaveAction onLeaveState(Editor* editor, EditorState* newState) override; + void onActiveToolChange(Editor* editor, tools::Tool* tool) override; + bool onMouseDown(Editor* editor, ui::MouseMessage* msg) override; + bool onMouseUp(Editor* editor, ui::MouseMessage* msg) override; + bool onMouseMove(Editor* editor, ui::MouseMessage* msg) override; + bool onSetCursor(Editor* editor, const gfx::Point& mouseScreenPos) override; + bool onKeyDown(Editor* editor, ui::KeyMessage* msg) override; + bool onKeyUp(Editor* editor, ui::KeyMessage* msg) override; + bool onUpdateStatusBar(Editor* editor) override; + bool acceptQuickTool(tools::Tool* tool) override; + bool requireBrushPreview() override { return false; } + void onBeforeLayerVisibilityChange(Editor* editor, doc::Layer* layer, bool newState) override; + // EditorObserver - virtual void onDestroyEditor(Editor* editor) override; - virtual void onBeforeFrameChanged(Editor* editor) override; - virtual void onBeforeLayerChanged(Editor* editor) override; + void onDestroyEditor(Editor* editor) override; + void onBeforeFrameChanged(Editor* editor) override; + void onBeforeLayerChanged(Editor* editor) override; // TimelineObserver - virtual void onBeforeRangeChanged(Timeline* timeline) override; + void onBeforeRangeChanged(Timeline* timeline) override; // ContextBarObserver - virtual void onDropPixels(ContextBarObserver::DropAction action) override; + void onDropPixels(ContextBarObserver::DropAction action) override; // PixelsMovementDelegate - virtual void onPivotChange() override; + void onPivotChange() override; - virtual Transformation getTransformation(Editor* editor) override; + Transformation getTransformation(Editor* editor) override; private: // DelayedMouseMoveDelegate impl diff --git a/src/app/ui/editor/pixels_movement.cpp b/src/app/ui/editor/pixels_movement.cpp index 982f919bb..f0e71b9bd 100644 --- a/src/app/ui/editor/pixels_movement.cpp +++ b/src/app/ui/editor/pixels_movement.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 @@ -787,9 +787,10 @@ void PixelsMovement::stampImage(bool finalStamp) cels.push_back(currentCel); } - if (currentCel && currentCel->layer() && + if (currentCel && + currentCel->layer() && currentCel->layer()->isImage() && - !currentCel->layer()->isEditableHierarchy()) { + !currentCel->layer()->canEditPixels()) { Transformation initialCelPos(gfx::Rect(m_initialMask0->bounds()), m_currentData.cornerThick()); redrawExtraImage(&initialCelPos); stampExtraCelImage(); diff --git a/src/app/ui/editor/pixels_movement.h b/src/app/ui/editor/pixels_movement.h index b5b13d1d8..4a73313fa 100644 --- a/src/app/ui/editor/pixels_movement.h +++ b/src/app/ui/editor/pixels_movement.h @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2019-2021 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 @@ -75,6 +75,8 @@ namespace app { const Mask* mask, const char* operationName); + const Site& site() { return m_site; } + HandleType handle() const { return m_handle; } bool canHandleFrameChange() const { return m_canHandleFrameChange; } diff --git a/src/app/ui/timeline/timeline.cpp b/src/app/ui/timeline/timeline.cpp index 4b9736b38..bf0b2f98c 100644 --- a/src/app/ui/timeline/timeline.cpp +++ b/src/app/ui/timeline/timeline.cpp @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2018-2023 Igara Studio S.A. +// Copyright (C) 2018-2024 Igara Studio S.A. // Copyright (C) 2001-2018 David Capello // // This program is distributed under the terms of @@ -696,7 +696,9 @@ bool Timeline::onProcessMessage(Message* msg) bool newVisibleState = !allLayersVisible(); for (Layer* topLayer : m_sprite->root()->layers()) { if (topLayer->isVisible() != newVisibleState) { - topLayer->setVisible(newVisibleState); + m_document->setLayerVisibilityWithNotifications( + topLayer, newVisibleState); + if (topLayer->isGroup()) regenRows = true; } @@ -825,11 +827,11 @@ bool Timeline::onProcessMessage(Message* msg) for (Row& row : m_rows) { Layer* l = row.layer(); if (l->hasFlags(LayerFlags::Internal_WasVisible)) { - l->setVisible(true); + m_document->setLayerVisibilityWithNotifications(l, true); l->switchFlags(LayerFlags::Internal_WasVisible, false); } else { - l->setVisible(false); + m_document->setLayerVisibilityWithNotifications(l, false); } } } @@ -838,7 +840,7 @@ bool Timeline::onProcessMessage(Message* msg) for (Row& row : m_rows) { Layer* l = row.layer(); l->switchFlags(LayerFlags::Internal_WasVisible, l->isVisible()); - l->setVisible(false); + m_document->setLayerVisibilityWithNotifications(l, false); } } @@ -2016,6 +2018,14 @@ void Timeline::onLayerCollapsedChanged(DocEvent& ev) invalidate(); } +void Timeline::onAfterLayerVisibilityChange(DocEvent& ev) +{ + layer_t layerIdx = getLayerIndex(ev.layer()); + if (layerIdx >= 0) + invalidateRect(getPartBounds(Hit(PART_ROW_EYE_ICON, layerIdx)) + .offset(origin())); +} + void Timeline::onStateChanged(Editor* editor) { m_aniControls.updateUsingEditor(editor); @@ -4475,12 +4485,10 @@ void Timeline::setLayerVisibleFlag(const layer_t l, const bool state) if (!layer) return; - bool redrawEditors = false; bool regenRows = false; if (layer->isVisible() != state) { - layer->setVisible(state); - redrawEditors = true; + m_document->setLayerVisibilityWithNotifications(layer, state); // Regenerate rows because might change the flag of the children // (the flag is propagated to the children in m_inheritedFlags). @@ -4493,9 +4501,8 @@ void Timeline::setLayerVisibleFlag(const layer_t l, const bool state) layer = layer->parent(); while (layer) { if (!layer->isVisible()) { - layer->setVisible(true); + m_document->setLayerVisibilityWithNotifications(layer, true); regenRows = true; - redrawEditors = true; } layer = layer->parent(); } @@ -4506,9 +4513,6 @@ void Timeline::setLayerVisibleFlag(const layer_t l, const bool state) regenerateRows(); invalidate(); } - - if (redrawEditors) - m_document->notifyGeneralUpdate(); } void Timeline::setLayerEditableFlag(const layer_t l, const bool state) diff --git a/src/app/ui/timeline/timeline.h b/src/app/ui/timeline/timeline.h index 5961f5a93..4e815652f 100644 --- a/src/app/ui/timeline/timeline.h +++ b/src/app/ui/timeline/timeline.h @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2018-2023 Igara Studio S.A. +// Copyright (C) 2018-2024 Igara Studio S.A. // Copyright (C) 2001-2018 David Capello // // This program is distributed under the terms of @@ -163,6 +163,7 @@ namespace app { void onTagChange(DocEvent& ev) override; void onTagRename(DocEvent& ev) override; void onLayerCollapsedChanged(DocEvent& ev) override; + void onAfterLayerVisibilityChange(DocEvent& ev) override; // app::Context slots. void onBeforeCommandExecution(CommandExecutionEvent& ev); diff --git a/src/app/util/layer_utils.cpp b/src/app/util/layer_utils.cpp index fa39281f6..874ba819f 100644 --- a/src/app/util/layer_utils.cpp +++ b/src/app/util/layer_utils.cpp @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2020 Igara Studio S.A. +// Copyright (C) 2020-2024 Igara Studio S.A. // // This program is distributed under the terms of // the End-User License Agreement for Aseprite. @@ -45,14 +45,34 @@ Layer* candidate_if_layer_is_deleted( bool layer_is_locked(Editor* editor) { Layer* layer = editor->layer(); - if (layer && !layer->isEditableHierarchy()) { + if (!layer) + return false; + #ifdef ENABLE_UI - if (auto statusBar = StatusBar::instance()) + auto statusBar = StatusBar::instance(); +#endif + + if (!layer->isVisibleHierarchy()) { +#ifdef ENABLE_UI + if (statusBar) { statusBar->showTip( - 1000, fmt::format(Strings::statusbar_tips_layer_locked(), layer->name())); + 1000, fmt::format(Strings::statusbar_tips_layer_x_is_hidden(), + layer->name())); + } #endif return true; } + + if (!layer->isEditableHierarchy()) { +#ifdef ENABLE_UI + if (statusBar) { + statusBar->showTip( + 1000, fmt::format(Strings::statusbar_tips_layer_locked(), layer->name())); + } +#endif + return true; + } + return false; } From 9f75260d259aa71148f7dc5ad75997e2bb15640f Mon Sep 17 00:00:00 2001 From: David Capello Date: Mon, 26 Feb 2024 17:12:11 -0300 Subject: [PATCH 09/39] Change userdata_codec.lua test to check pre-saved user data properties --- tests/scripts/userdata_codec.lua | 27 ++++++++++++++---------- tests/sprites/README.md | 5 +++-- tests/sprites/file-tests-props.aseprite | Bin 1838 -> 2300 bytes 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/tests/scripts/userdata_codec.lua b/tests/scripts/userdata_codec.lua index 4733a8dac..d52e12c83 100644 --- a/tests/scripts/userdata_codec.lua +++ b/tests/scripts/userdata_codec.lua @@ -1,4 +1,4 @@ --- Copyright (C) 2023 Igara Studio S.A. +-- Copyright (C) 2023-2024 Igara Studio S.A. -- -- This file is released under the terms of the MIT license. -- Read LICENSE.txt for more information. @@ -8,7 +8,11 @@ -- dofile('./test_utils.lua') -do +-- You can use this code to fill file-tests-props.aseprite properties, +-- but we did this just one time in the past, now we want to check +-- that we can read the properties correctly from the file, e.g. to +-- check the correct endianness of the platform. +if false then local spr = Sprite{ fromFile="sprites/file-tests-props.aseprite" } -- Set sprite custom properties @@ -16,6 +20,7 @@ do spr.properties.b = 1 spr.properties.c = "hi" spr.properties.d = 2.3 + spr.properties.e = 8.123456e-12 spr.properties("ext").a = {"one", "two", "three"} -- Set layer custom properties spr.layers[2].properties.a = "i'm the layer 3" @@ -24,11 +29,11 @@ do spr.layers[1].tileset.properties.a = "i'm a tilemap" spr.layers[1].tileset.properties.b = 11 spr.layers[1].tileset.properties("ext").a = "text from extension" - -- Create some tiles with properties - local tile = spr:newTile(spr.layers[1].tileset, 1) + -- Set tiles custom properties + local tile = spr.layers[1].tileset:tile(1) tile.properties.a = 320 tile.properties.b = 330 - tile = spr:newTile(spr.layers[1].tileset, 2) + tile = spr.layers[1].tileset:tile(2) tile.properties.a = 640 tile.properties.b = 650 -- Set tags custom properties @@ -42,19 +47,19 @@ do spr.slices[1].properties = {a=Point(3,4), b=Size(10,20)} spr.slices[1].properties("ext", {a=Rectangle(10,20,30,40)}) - spr:saveAs("_test_userdata_codec_1.aseprite") + spr:saveAs("sprites/file-tests-props.aseprite") spr:close() +end - local origSpr = Sprite{ fromFile="sprites/file-tests-props.aseprite" } - spr = Sprite{ fromFile="_test_userdata_codec_1.aseprite" } - assert_sprites_eq(origSpr, spr) - origSpr:close() - assert(#spr.properties == 4) +do + local spr = Sprite{ fromFile="sprites/file-tests-props.aseprite" } + assert(#spr.properties == 5) assert(#spr.properties("ext") == 1) assert(spr.properties.a == true) assert(spr.properties.b == 1) assert(spr.properties.c == "hi") assert(spr.properties.d == 2.3) + assert(spr.properties.e == 8.123456e-12) assert(spr.properties("ext").a[1] == "one") assert(spr.properties("ext").a[2] == "two") assert(spr.properties("ext").a[3] == "three") diff --git a/tests/sprites/README.md b/tests/sprites/README.md index 04274775c..6f8aecdab 100644 --- a/tests/sprites/README.md +++ b/tests/sprites/README.md @@ -28,10 +28,11 @@ * `4f-index-4x4.aseprite`: Indexed, 4 frames, 1 layer, mask color set to index 0. * `file-tests-props.aseprite`: Indexed, 64x64, 6 frames, 4 layers (one - of them is a tilemap), 13 cels, 1 tag. + of them is a tilemap), 13 cels, 1 tag, pre-defined user data + properties of all kinds in several sprite elements. * `slices.aseprite`: Indexed, 4x4, background layer, 2 slices. * `slices-moving.aseprite`: Indexed, 4x4, 1 linked cel in 4 frames, background layer, 1 slice with 4 keyframes (each keyframe with a different position/size). * `2x2tilemap2x2tile.aseprite`: RGB, 6x6, 2x2 tilemap layer, 5 tiles tileset, -2x2 tile size, 1 frame. + 2x2 tile size, 1 frame. diff --git a/tests/sprites/file-tests-props.aseprite b/tests/sprites/file-tests-props.aseprite index b72b36f3acc15f989443d76dcb7cda72bfe9ad63..dd26e60a3a2e9ba65f4b2fe0fabf4bd180f19f6b 100644 GIT binary patch literal 2300 zcmd5-TS!z<6y103Of!|EW>FKBowUrqoXOsr(>qiu%#09eX71d{8JwBYp$9=lM3EFp zM$rQ*q`nIL3!(zEFsMH2CoCcbgMPBdPfxh5eePVH`Y4cs_GO>vKKrb-_c`Z&cMegn3>_9h3J+2)zJDPu2hrCL6XmWGu|Xl!CFGXr?YM1Aw^h=2r`XK-lL^np zpmru8&?pe4tBT7ai@Yc5mb}_g)O#s1*nRHX$N6VpZ;0I~S#T;*6`fO4HPctNsG=fO z5;$0&=-190TOSWReDU2djPuuT9NKpz8jEeItJ%4{^_tJuyIFg-_vVQ+m95E=I5rT% zj#7hK(7*$`2NjN2$i}7S#$;nup>P&Oq}col=~>h@cvYMH@^U#dGc`DlRCL@ZlL=j= z)UKq1QjN{J-h$ymfr3Us*@!d2tA3U>=+Tz zpM06>dG0;d)0yAB=grO9g{cuFBnBGR0U3pDgZOEj4nBfX-aV%g&O7ck;9ZBD%A$J9m9YS97Kq*55+IcvLzG~Mkr7E zkbLcu4fe{S{jpoA_6N)S$gP@Vo;KpPDQx7V>fj!|S*G02 zX8Aq=d`b^mUq7hw9psgN7tuU;bb#$7Bx)E@9u)vl`{{^Zed;ObAf;r*w(yZj-hPFS z-d0wPgvLop0|yK>Gyl6ZMuO;*MD(u$I}$`X5Scf(J{=`gGFo^s%pU>%A2tr7xD@l`uSKEx8TZ5bf4~E*VDSGX;{QYR0}Iko=Kufz delta 430 zcmew(xQ>rWk9{IjUHyD!28LfB1sGBo7#IYASPY2S6&QgeGPnigNGd2mh2)W?Km}Z)cE*-o>5R>g81+M Date: Mon, 26 Feb 2024 17:44:56 -0300 Subject: [PATCH 10/39] Keep testing loading/saving properties as they are in userdata_codec.lua --- tests/scripts/userdata_codec.lua | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/scripts/userdata_codec.lua b/tests/scripts/userdata_codec.lua index d52e12c83..e20020408 100644 --- a/tests/scripts/userdata_codec.lua +++ b/tests/scripts/userdata_codec.lua @@ -52,8 +52,14 @@ if false then end do + -- Test load/save file (and keep the properties intact) local spr = Sprite{ fromFile="sprites/file-tests-props.aseprite" } assert(#spr.properties == 5) + spr:saveAs("_test_userdata_codec_1.aseprite") + spr:close() + + local spr = Sprite{ fromFile="_test_userdata_codec_1.aseprite" } + assert(#spr.properties == 5) assert(#spr.properties("ext") == 1) assert(spr.properties.a == true) assert(spr.properties.b == 1) From 7b9594f4e00cdfe373e9f390b49fd646af0377f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Capello?= Date: Mon, 26 Feb 2024 12:43:48 -0300 Subject: [PATCH 11/39] Avoid moving the playback cue when decrementing a ping-pong tag's repeat field that has only one frame (fix #4336) --- src/doc/playback.cpp | 3 ++- src/doc/playback_tests.cpp | 43 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/src/doc/playback.cpp b/src/doc/playback.cpp index cfb9063b4..2eaa9b287 100644 --- a/src/doc/playback.cpp +++ b/src/doc/playback.cpp @@ -435,7 +435,8 @@ bool Playback::decrementRepeat(const frame_t frameDelta) PLAY_TRACE(" Repeat tag", tag->name(), " frame=", m_frame, "repeat=", m_playing.back()->repeat, "forward=", m_playing.back()->forward); - return true; + // Tag has only 1 frame, then don't move the playback cue. + return tag->frames() > 1; } else { // Remove tag from played diff --git a/src/doc/playback_tests.cpp b/src/doc/playback_tests.cpp index aa68abfa1..957097483 100644 --- a/src/doc/playback_tests.cpp +++ b/src/doc/playback_tests.cpp @@ -282,6 +282,48 @@ TEST(Playback, SimplePingPong3) EXPECT_FALSE(play.isStopped()); } +TEST(Playback, SimplePingPong4) +{ + // A + // <> + // 0 + + Tag* a = make_tag("A", 0, 0, AniDir::PING_PONG, 1); + auto sprite = make_sprite(1, { a }); + + Playback play(sprite.get(), 0, Playback::Mode::PlayAll); + expect_frames(play, {0,0}); + EXPECT_TRUE(play.isStopped()); +} + +TEST(Playback, SimplePingPong5) +{ + // A + // <> + // 0 + + Tag* a = make_tag("A", 0, 0, AniDir::PING_PONG, 3); + auto sprite = make_sprite(1, { a }); + + Playback play(sprite.get(), 0, Playback::Mode::PlayAll); + expect_frames(play, {0,0,0,0}); + EXPECT_TRUE(play.isStopped()); +} + +TEST(Playback, SimplePingPong6) +{ + // A + // <> + // 0 + + Tag* a = make_tag("A", 0, 0, AniDir::PING_PONG, 0); + auto sprite = make_sprite(1, { a }); + + Playback play(sprite.get(), 0, Playback::Mode::PlayAll); + expect_frames(play, {0,0,0}); + EXPECT_TRUE(play.isStopped()); +} + TEST(Playback, SimplePingPong3Repeats) { // A @@ -297,6 +339,7 @@ TEST(Playback, SimplePingPong3Repeats) EXPECT_FALSE(play.isStopped()); } + TEST(Playback, TagOneFrame) { // A From 9e2728992d5aa1c971afd64cde8d303fff6b3a6c Mon Sep 17 00:00:00 2001 From: Gaspar Capello Date: Thu, 8 Feb 2024 12:42:38 -0300 Subject: [PATCH 12/39] Fix slice tool doesn't work correctly in a tilemap layer when we are in Manual mode (fix #4290) --- src/app/ui/editor/drawing_state.cpp | 7 ++++--- src/app/ui/editor/tool_loop_impl.cpp | 17 ++++++++++------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/app/ui/editor/drawing_state.cpp b/src/app/ui/editor/drawing_state.cpp index 1902cc290..0fcdc1f6b 100644 --- a/src/app/ui/editor/drawing_state.cpp +++ b/src/app/ui/editor/drawing_state.cpp @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2018-2022 Igara Studio S.A. +// Copyright (C) 2018-2024 Igara Studio S.A. // Copyright (C) 2001-2018 David Capello // // This program is distributed under the terms of @@ -96,8 +96,9 @@ void DrawingState::initToolLoop(Editor* editor, // For selection inks we don't use a "the selected layer" for // preview purposes, because we want the selection feedback to be at // the top of all layers. - Layer* previewLayer = (m_toolLoop->getInk()->isSelection() ? nullptr: - m_toolLoop->getLayer()); + Layer* previewLayer = (m_toolLoop->getInk()->isSelection() || + m_toolLoop->getInk()->isSlice() ? + nullptr : m_toolLoop->getLayer()); // Prepare preview image (the destination image will be our preview // in the tool-loop time, so we can see what we are drawing) diff --git a/src/app/ui/editor/tool_loop_impl.cpp b/src/app/ui/editor/tool_loop_impl.cpp index a82e3f880..80b7ab552 100644 --- a/src/app/ui/editor/tool_loop_impl.cpp +++ b/src/app/ui/editor/tool_loop_impl.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 @@ -517,6 +517,9 @@ public: } } + // 'isSelectionPreview = true' if the intention is to show a preview + // of Selection tools or Slice tool. + const bool isSelectionPreview = m_ink->isSelection() || m_ink->isSlice(); m_expandCelCanvas.reset(new ExpandCelCanvas( site, m_layer, m_docPref.tiled.mode(), @@ -530,10 +533,10 @@ public: (m_layer->isTilemap() && site.tilemapMode() == TilemapMode::Pixels && site.tilesetMode() == TilesetMode::Manual && - !m_ink->isSelection() ? ExpandCelCanvas::TilesetPreview: - ExpandCelCanvas::None) | - (m_ink->isSelection() ? ExpandCelCanvas::SelectionPreview: - ExpandCelCanvas::None)))); + (!isSelectionPreview ? ExpandCelCanvas::TilesetPreview: + ExpandCelCanvas::None)) | + (isSelectionPreview ? ExpandCelCanvas::SelectionPreview: + ExpandCelCanvas::None)))); if (!m_floodfillSrcImage) m_floodfillSrcImage = const_cast(getSrcImage()); @@ -555,7 +558,7 @@ public: m_sprayWidth = m_toolPref.spray.width(); m_spraySpeed = m_toolPref.spray.speed(); - if (m_ink->isSelection()) { + if (isSelectionPreview) { m_useMask = false; } else { @@ -563,7 +566,7 @@ public: } // Start with an empty mask if the user is selecting with "default selection mode" - if (m_ink->isSelection() && + if (isSelectionPreview && (!m_document->isMaskVisible() || (int(getModifiers()) & int(tools::ToolLoopModifiers::kReplaceSelection)))) { Mask emptyMask; From b813c1ad5f84162a1f562a0eabb103ff21781d45 Mon Sep 17 00:00:00 2001 From: David Capello Date: Tue, 27 Feb 2024 10:45:18 -0300 Subject: [PATCH 13/39] Fix regression introduced in 9e2728992d5aa1c971afd64cde8d303fff6b3a6c Some info here: https://github.com/aseprite/aseprite/pull/4299#discussion_r1503955328 --- src/app/ui/editor/tool_loop_impl.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/ui/editor/tool_loop_impl.cpp b/src/app/ui/editor/tool_loop_impl.cpp index 80b7ab552..a16e6934f 100644 --- a/src/app/ui/editor/tool_loop_impl.cpp +++ b/src/app/ui/editor/tool_loop_impl.cpp @@ -533,8 +533,8 @@ public: (m_layer->isTilemap() && site.tilemapMode() == TilemapMode::Pixels && site.tilesetMode() == TilesetMode::Manual && - (!isSelectionPreview ? ExpandCelCanvas::TilesetPreview: - ExpandCelCanvas::None)) | + !isSelectionPreview ? ExpandCelCanvas::TilesetPreview: + ExpandCelCanvas::None) | (isSelectionPreview ? ExpandCelCanvas::SelectionPreview: ExpandCelCanvas::None)))); From 908677edbb5782581e826f4fa196af7494298f99 Mon Sep 17 00:00:00 2001 From: David Capello Date: Tue, 27 Feb 2024 11:04:03 -0300 Subject: [PATCH 14/39] Update clang-tidy checks --- .clang-tidy | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/.clang-tidy b/.clang-tidy index 7f469847b..a69e80dd0 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,4 +1,57 @@ +# +# Disabled or configured checks: +# +# bugprone-easily-swappable-parameters: We have a lot of functions +# with (int, int) or (string, string) so it does't make sense to enable +# this option. +# +# readability-braces-around-statements: We use a lot of: +# if (cond) +# stmt; +# else +# stmt; +# and there is no way to allow this with this check. +# +# readability-function-cognitive-complexity: We have this disabled +# temporarily, but it'd be nice to enable this with a high threshold +# in the future. +# +# readability-identifier-length: We use a lot of short names like x, +# y, w, h so we prefer to remove this. +# +# readability-magic-numbers: We use a lot of magic numbers like 8, 16, +# 24 for masks like 0xFF00, etc. +# +# readability-isolate-declaration: We use multiple declarations +# several times (e.g. int x, y, etc.) +# +# readability-uppercase-literal-suffix: We use a lot of 0.0f, but in a +# future we might enable this. +# +# misc-non-private-member-variables-in-classes: We use structs with +# all public members in some cases. +# --- -Checks: 'clang-analyzer-*' +Checks: > + -*, + bugprone-*, + clang-analyzer-*, + concurrency-*, + misc-*, + performance-*, + portability-*, + readability-*, + -bugprone-easily-swappable-parameters, + -readability-braces-around-statements, + -readability-function-cognitive-complexity, + -readability-identifier-length, + -readability-isolate-declaration, + -readability-magic-numbers, + -readability-uppercase-literal-suffix WarningsAsErrors: '' +CheckOptions: + - key: readability-implicit-bool-conversion.AllowPointerConditions + value: true + - key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic + value: true ... From 924eaddf75a954a72c3b9563de53b65347d83d7c Mon Sep 17 00:00:00 2001 From: David Capello Date: Tue, 27 Feb 2024 11:50:07 -0300 Subject: [PATCH 15/39] Add clang tidy workflow action --- .github/workflows/clang_tidy.yml | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 .github/workflows/clang_tidy.yml diff --git a/.github/workflows/clang_tidy.yml b/.github/workflows/clang_tidy.yml new file mode 100644 index 000000000..d629b257d --- /dev/null +++ b/.github/workflows/clang_tidy.yml @@ -0,0 +1,31 @@ +name: Clang Tidy Diff +on: + pull_request_target: + paths: + - '**.cpp' + - '**.h' + - '.github/workflows/clang-tidy.yml' +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + submodules: 'recursive' + - uses: ilammy/msvc-dev-cmd@v1 + if: runner.os == 'Windows' + - uses: turtlesec-no/get-ninja@main + - uses: ZedThree/clang-tidy-review@v0.17.1 + id: review + with: + token: ${{ secrets.CLANG_TIDY_TOKEN }} + apt_packages: | + libc++-dev, libc++abi-dev, libpixman-1-dev, + libfreetype6-dev, libharfbuzz-dev, zlib1g-dev, libx11-dev, + libxcursor-dev, libxi-dev, libgl1-mesa-dev, ninja-build + cmake_command: | + cmake . -G Ninja -DCMAKE_BUILD_TYPE=Debug -DLAF_BACKEND=none -DCMAKE_EXPORT_COMPILE_COMMANDS=on + - uses: ZedThree/clang-tidy-review/upload@v0.17.1 + id: upload-review + - if: steps.review.outputs.total_comments > 0 + run: exit 1 From 1d1b25d425447d79c533d11be84401a3e0c7d3c5 Mon Sep 17 00:00:00 2001 From: David Capello Date: Tue, 27 Feb 2024 11:54:11 -0300 Subject: [PATCH 16/39] Update laf module --- laf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/laf b/laf index 58723a95b..2f4d2cb10 160000 --- a/laf +++ b/laf @@ -1 +1 @@ -Subproject commit 58723a95b3a8809bdd4621e5dea88632b0a4f5bf +Subproject commit 2f4d2cb1088cfe9ad9a85dbd5637935e36cd444b From a86b0a7d0c0264c52aa27f71402a18415d4116e6 Mon Sep 17 00:00:00 2001 From: David Capello Date: Tue, 27 Feb 2024 12:07:19 -0300 Subject: [PATCH 17/39] Fix clang_tidy.yml path --- .github/workflows/clang_tidy.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/clang_tidy.yml b/.github/workflows/clang_tidy.yml index d629b257d..d7ec9514b 100644 --- a/.github/workflows/clang_tidy.yml +++ b/.github/workflows/clang_tidy.yml @@ -4,7 +4,7 @@ on: paths: - '**.cpp' - '**.h' - - '.github/workflows/clang-tidy.yml' + - '.github/workflows/clang_tidy.yml' jobs: build: runs-on: ubuntu-latest From 104f80b334cd2fac1bde8d437097cfdd16ff78e8 Mon Sep 17 00:00:00 2001 From: David Capello Date: Tue, 27 Feb 2024 12:19:32 -0300 Subject: [PATCH 18/39] Change job name of clang tidy to avoid confusion with the build process --- .github/workflows/clang_tidy.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/clang_tidy.yml b/.github/workflows/clang_tidy.yml index d7ec9514b..6a4fabf47 100644 --- a/.github/workflows/clang_tidy.yml +++ b/.github/workflows/clang_tidy.yml @@ -6,7 +6,7 @@ on: - '**.h' - '.github/workflows/clang_tidy.yml' jobs: - build: + review: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 From 52afc9e9a7e0c445aca842d5ccae47d3361d4232 Mon Sep 17 00:00:00 2001 From: David Capello Date: Tue, 27 Feb 2024 12:05:38 -0300 Subject: [PATCH 19/39] Use build directory for cmake command on clang tidy It's required by freetype library as it looks like it doesn't allow to build in the same source directory. --- .github/workflows/clang_tidy.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/clang_tidy.yml b/.github/workflows/clang_tidy.yml index 6a4fabf47..b92a4da8f 100644 --- a/.github/workflows/clang_tidy.yml +++ b/.github/workflows/clang_tidy.yml @@ -19,12 +19,13 @@ jobs: id: review with: token: ${{ secrets.CLANG_TIDY_TOKEN }} + build_dir: build apt_packages: | libc++-dev, libc++abi-dev, libpixman-1-dev, libfreetype6-dev, libharfbuzz-dev, zlib1g-dev, libx11-dev, libxcursor-dev, libxi-dev, libgl1-mesa-dev, ninja-build cmake_command: | - cmake . -G Ninja -DCMAKE_BUILD_TYPE=Debug -DLAF_BACKEND=none -DCMAKE_EXPORT_COMPILE_COMMANDS=on + cmake -S . B build -G Ninja -DCMAKE_BUILD_TYPE=Debug -DLAF_BACKEND=none -DCMAKE_EXPORT_COMPILE_COMMANDS=on - uses: ZedThree/clang-tidy-review/upload@v0.17.1 id: upload-review - if: steps.review.outputs.total_comments > 0 From 7aca017a5836d2aee0f062feaf238305e28d2afe Mon Sep 17 00:00:00 2001 From: David Capello Date: Tue, 27 Feb 2024 13:20:13 -0300 Subject: [PATCH 20/39] Fix use-after free in ase_ungroup_all() Anyway this code is not used anymore, it was for v1.1 when v1.1 and v1.2 branches were developed at the same time (layer groups feature was added in v1.2, so v1.1 just moved groups children to the root). --- src/app/file/ase_format.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/app/file/ase_format.cpp b/src/app/file/ase_format.cpp index 20fd02588..ad19b5cdc 100644 --- a/src/app/file/ase_format.cpp +++ b/src/app/file/ase_format.cpp @@ -1696,8 +1696,12 @@ static void ase_ungroup_all(LayerGroup* group) for (Layer* child : list) { if (child->isGroup()) { - ase_ungroup_all(static_cast(child)); - group->removeLayer(child); + auto* childGroup = static_cast(child); + ase_ungroup_all(childGroup); + group->removeLayer(childGroup); + + ASSERT(childGroup->layersCount() == 0); + delete childGroup; } else if (group != root) { // Create a new name adding all group layer names @@ -1715,11 +1719,6 @@ static void ase_ungroup_all(LayerGroup* group) root->addLayer(child); } } - - if (group != root) { - ASSERT(group->layersCount() == 0); - delete group; - } } } // namespace app From f19215ea9d04f40179521f2992edbd0e2361f9e7 Mon Sep 17 00:00:00 2001 From: David Capello Date: Tue, 27 Feb 2024 13:23:04 -0300 Subject: [PATCH 21/39] Update laf module --- laf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/laf b/laf index 2f4d2cb10..a5561b2ed 160000 --- a/laf +++ b/laf @@ -1 +1 @@ -Subproject commit 2f4d2cb1088cfe9ad9a85dbd5637935e36cd444b +Subproject commit a5561b2ed9adac89ebf353fb86753229eafea9e0 From be891230dad34d12fc21cf88fc26d765384da00e Mon Sep 17 00:00:00 2001 From: David Capello Date: Tue, 27 Feb 2024 13:49:32 -0300 Subject: [PATCH 22/39] Remove Ninja from clang tidy action --- .github/workflows/clang_tidy.yml | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/.github/workflows/clang_tidy.yml b/.github/workflows/clang_tidy.yml index b92a4da8f..3abf28b86 100644 --- a/.github/workflows/clang_tidy.yml +++ b/.github/workflows/clang_tidy.yml @@ -12,9 +12,6 @@ jobs: - uses: actions/checkout@v4 with: submodules: 'recursive' - - uses: ilammy/msvc-dev-cmd@v1 - if: runner.os == 'Windows' - - uses: turtlesec-no/get-ninja@main - uses: ZedThree/clang-tidy-review@v0.17.1 id: review with: @@ -23,9 +20,9 @@ jobs: apt_packages: | libc++-dev, libc++abi-dev, libpixman-1-dev, libfreetype6-dev, libharfbuzz-dev, zlib1g-dev, libx11-dev, - libxcursor-dev, libxi-dev, libgl1-mesa-dev, ninja-build + libxcursor-dev, libxi-dev, libgl1-mesa-dev cmake_command: | - cmake -S . B build -G Ninja -DCMAKE_BUILD_TYPE=Debug -DLAF_BACKEND=none -DCMAKE_EXPORT_COMPILE_COMMANDS=on + cmake -S . B build -DCMAKE_BUILD_TYPE=Debug -DLAF_BACKEND=none -DCMAKE_EXPORT_COMPILE_COMMANDS=on - uses: ZedThree/clang-tidy-review/upload@v0.17.1 id: upload-review - if: steps.review.outputs.total_comments > 0 From 351d9c863f1a454acc2b7cd1d2d438d08f97e17f Mon Sep 17 00:00:00 2001 From: David Capello Date: Tue, 27 Feb 2024 13:55:43 -0300 Subject: [PATCH 23/39] Add config_file parameter to clang tidy action --- .github/workflows/clang_tidy.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/clang_tidy.yml b/.github/workflows/clang_tidy.yml index 3abf28b86..88ff1008d 100644 --- a/.github/workflows/clang_tidy.yml +++ b/.github/workflows/clang_tidy.yml @@ -17,6 +17,7 @@ jobs: with: token: ${{ secrets.CLANG_TIDY_TOKEN }} build_dir: build + config_file: .clang-tidy apt_packages: | libc++-dev, libc++abi-dev, libpixman-1-dev, libfreetype6-dev, libharfbuzz-dev, zlib1g-dev, libx11-dev, From 03403953f36853f0fb03b193b8021f307b35399a Mon Sep 17 00:00:00 2001 From: David Capello Date: Tue, 27 Feb 2024 14:11:02 -0300 Subject: [PATCH 24/39] Fix cmake -B param in clang tidy action --- .github/workflows/clang_tidy.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/clang_tidy.yml b/.github/workflows/clang_tidy.yml index 88ff1008d..6e6ff380a 100644 --- a/.github/workflows/clang_tidy.yml +++ b/.github/workflows/clang_tidy.yml @@ -23,7 +23,7 @@ jobs: libfreetype6-dev, libharfbuzz-dev, zlib1g-dev, libx11-dev, libxcursor-dev, libxi-dev, libgl1-mesa-dev cmake_command: | - cmake -S . B build -DCMAKE_BUILD_TYPE=Debug -DLAF_BACKEND=none -DCMAKE_EXPORT_COMPILE_COMMANDS=on + cmake -S . -B build -DCMAKE_BUILD_TYPE=Debug -DLAF_BACKEND=none -DCMAKE_EXPORT_COMPILE_COMMANDS=on - uses: ZedThree/clang-tidy-review/upload@v0.17.1 id: upload-review - if: steps.review.outputs.total_comments > 0 From 619b5cbada309ea765f8d5a2230fcda12a2d303a Mon Sep 17 00:00:00 2001 From: David Capello Date: Tue, 27 Feb 2024 15:41:07 -0300 Subject: [PATCH 25/39] Run clang tidy in the context of the merge commit --- .github/workflows/clang_tidy.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/clang_tidy.yml b/.github/workflows/clang_tidy.yml index 6e6ff380a..60fbee5c5 100644 --- a/.github/workflows/clang_tidy.yml +++ b/.github/workflows/clang_tidy.yml @@ -1,6 +1,6 @@ name: Clang Tidy Diff on: - pull_request_target: + pull_request: paths: - '**.cpp' - '**.h' From 8fc759206671f5ba12c937009b53b26af1816aec Mon Sep 17 00:00:00 2001 From: David Capello Date: Tue, 27 Feb 2024 19:07:53 -0300 Subject: [PATCH 26/39] Make clang tidy action work in PRs from forks using split_workflow --- .github/workflows/clang_tidy.yml | 12 +++++++----- .github/workflows/clang_tidy_post.yml | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) create mode 100644 .github/workflows/clang_tidy_post.yml diff --git a/.github/workflows/clang_tidy.yml b/.github/workflows/clang_tidy.yml index 60fbee5c5..38114e47e 100644 --- a/.github/workflows/clang_tidy.yml +++ b/.github/workflows/clang_tidy.yml @@ -5,6 +5,10 @@ on: - '**.cpp' - '**.h' - '.github/workflows/clang_tidy.yml' + +permissions: + contents: read + jobs: review: runs-on: ubuntu-latest @@ -12,19 +16,17 @@ jobs: - uses: actions/checkout@v4 with: submodules: 'recursive' - - uses: ZedThree/clang-tidy-review@v0.17.1 + - uses: ZedThree/clang-tidy-review@v0.14.0 id: review with: - token: ${{ secrets.CLANG_TIDY_TOKEN }} build_dir: build config_file: .clang-tidy + split_workflow: true apt_packages: | libc++-dev, libc++abi-dev, libpixman-1-dev, libfreetype6-dev, libharfbuzz-dev, zlib1g-dev, libx11-dev, libxcursor-dev, libxi-dev, libgl1-mesa-dev cmake_command: | cmake -S . -B build -DCMAKE_BUILD_TYPE=Debug -DLAF_BACKEND=none -DCMAKE_EXPORT_COMPILE_COMMANDS=on - - uses: ZedThree/clang-tidy-review/upload@v0.17.1 + - uses: ZedThree/clang-tidy-review/upload@v0.14.0 id: upload-review - - if: steps.review.outputs.total_comments > 0 - run: exit 1 diff --git a/.github/workflows/clang_tidy_post.yml b/.github/workflows/clang_tidy_post.yml new file mode 100644 index 000000000..4ccf7845b --- /dev/null +++ b/.github/workflows/clang_tidy_post.yml @@ -0,0 +1,18 @@ +name: Post Clang Tidy Comments +on: + workflow_run: + workflows: ["Clang Tidy Diff"] + types: + - completed + +permissions: + checks: write + pull-requests: write + +jobs: + post-comments: + runs-on: ubuntu-latest + steps: + - uses: ZedThree/clang-tidy-review/post@v0.14.0 + with: + token: ${{ secrets.CLANG_TIDY_TOKEN }} From b5288d85e77cf3b2a03f51fc86ffe2fb37c4eb64 Mon Sep 17 00:00:00 2001 From: David Capello Date: Tue, 27 Feb 2024 23:17:32 -0300 Subject: [PATCH 27/39] Update some clang tidy checks --- .clang-tidy | 8 ++++++++ laf | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/.clang-tidy b/.clang-tidy index a69e80dd0..f261b2ee8 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -28,6 +28,12 @@ # readability-uppercase-literal-suffix: We use a lot of 0.0f, but in a # future we might enable this. # +# readability-named-parameter: We prefer misc-unused-parameters to +# remove a parameter name that is not used. +# +# misc-use-anonymous-namespace: We use anonymous namespaces or static +# functions indifferently. +# # misc-non-private-member-variables-in-classes: We use structs with # all public members in some cases. # @@ -42,11 +48,13 @@ Checks: > portability-*, readability-*, -bugprone-easily-swappable-parameters, + -misc-use-anonymous-namespace, -readability-braces-around-statements, -readability-function-cognitive-complexity, -readability-identifier-length, -readability-isolate-declaration, -readability-magic-numbers, + -readability-named-parameter, -readability-uppercase-literal-suffix WarningsAsErrors: '' CheckOptions: diff --git a/laf b/laf index a5561b2ed..de1aeaca3 160000 --- a/laf +++ b/laf @@ -1 +1 @@ -Subproject commit a5561b2ed9adac89ebf353fb86753229eafea9e0 +Subproject commit de1aeaca3552e9ce4a7681fc54ec8424a5eac910 From 1a067bb223ff787161a66c21667aafc5cd7395cd Mon Sep 17 00:00:00 2001 From: David Capello Date: Wed, 28 Feb 2024 08:30:23 -0300 Subject: [PATCH 28/39] Update json11 module --- third_party/json11 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/third_party/json11 b/third_party/json11 index e5868fff1..92ba6ce0f 160000 --- a/third_party/json11 +++ b/third_party/json11 @@ -1 +1 @@ -Subproject commit e5868fff1fc5128077ed7699bdaa20ae47c47eca +Subproject commit 92ba6ce0fa1f1c8fd8783b6930b52539b3861888 From edc248d4183b26876c4ee1f2b86e66e6e0f24efe Mon Sep 17 00:00:00 2001 From: David Capello Date: Wed, 28 Feb 2024 10:16:11 -0300 Subject: [PATCH 29/39] Removing some unnecessary #includes --- src/doc/tag.h | 1 - src/ui/paint.h | 1 - src/updater/user_agent.h | 4 +--- 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/doc/tag.h b/src/doc/tag.h index 43f56e688..048969fa5 100644 --- a/src/doc/tag.h +++ b/src/doc/tag.h @@ -9,7 +9,6 @@ #define DOC_TAG_H_INCLUDED #pragma once -#include "base/disable_copying.h" #include "doc/anidir.h" #include "doc/frame.h" #include "doc/object.h" diff --git a/src/ui/paint.h b/src/ui/paint.h index 9889acfe6..f010f21e7 100644 --- a/src/ui/paint.h +++ b/src/ui/paint.h @@ -8,7 +8,6 @@ #define UI_PAINT_H_INCLUDED #pragma once -#include "base/disable_copying.h" #include "gfx/color.h" #include "os/paint.h" diff --git a/src/updater/user_agent.h b/src/updater/user_agent.h index 929769dea..0ba55b1bb 100644 --- a/src/updater/user_agent.h +++ b/src/updater/user_agent.h @@ -8,9 +8,7 @@ #define UPDATER_USER_AGENT_H_INCLUDED #pragma once -#include "base/disable_copying.h" - -#include +#include namespace updater { From 32a1b327a6c52fcaefde92c554dee4387d5d7d09 Mon Sep 17 00:00:00 2001 From: David Capello Date: Wed, 28 Feb 2024 10:16:36 -0300 Subject: [PATCH 30/39] Fix typo --- src/ui/theme.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/theme.h b/src/ui/theme.h index 8c29fbd15..9141add7a 100644 --- a/src/ui/theme.h +++ b/src/ui/theme.h @@ -1,5 +1,5 @@ // Aseprite UI Library -// Copyright (Cg) 2020-2024 Igara Studio S.A. +// Copyright (C) 2020-2024 Igara Studio S.A. // Copyright (C) 2001-2017 David Capello // // This file is released under the terms of the MIT license. From 166124526f6dcbcada228d44d7dea89e154bd15d Mon Sep 17 00:00:00 2001 From: David Capello Date: Wed, 28 Feb 2024 10:16:53 -0300 Subject: [PATCH 31/39] Update laf --- laf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/laf b/laf index de1aeaca3..4e601f4b9 160000 --- a/laf +++ b/laf @@ -1 +1 @@ -Subproject commit de1aeaca3552e9ce4a7681fc54ec8424a5eac910 +Subproject commit 4e601f4b9fa5b47c1ececca765398c67c6137039 From d37bfa6c43e119b1b565adfee454492f8b703561 Mon Sep 17 00:00:00 2001 From: David Capello Date: Wed, 28 Feb 2024 10:45:52 -0300 Subject: [PATCH 32/39] Fix compilation of tests when LAF_BACKEND=none --- laf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/laf b/laf index 4e601f4b9..3119ce537 160000 --- a/laf +++ b/laf @@ -1 +1 @@ -Subproject commit 4e601f4b9fa5b47c1ececca765398c67c6137039 +Subproject commit 3119ce537721992c5cbc3760ea6db7fa742702bb From 83d83cd8c6b8e4e62d00d3ef9a87775ac8cc295b Mon Sep 17 00:00:00 2001 From: David Capello Date: Wed, 28 Feb 2024 12:31:38 -0300 Subject: [PATCH 33/39] Add const to lock_guards --- laf | 2 +- src/app/commands/filters/filter_worker.cpp | 10 +++++----- src/app/file/file.cpp | 14 +++++++------- src/app/task.cpp | 2 +- src/app/task.h | 8 ++++---- src/app/thumbnail_generator.cpp | 22 +++++++++++----------- src/doc/object.cpp | 6 +++--- 7 files changed, 32 insertions(+), 32 deletions(-) diff --git a/laf b/laf index 3119ce537..12b411862 160000 --- a/laf +++ b/laf @@ -1 +1 @@ -Subproject commit 3119ce537721992c5cbc3760ea6db7fa742702bb +Subproject commit 12b411862df1655fac1f9c32847f1801c7205e2b diff --git a/src/app/commands/filters/filter_worker.cpp b/src/app/commands/filters/filter_worker.cpp index 450346a72..ab298d0cf 100644 --- a/src/app/commands/filters/filter_worker.cpp +++ b/src/app/commands/filters/filter_worker.cpp @@ -151,7 +151,7 @@ void FilterWorker::run() } { - std::lock_guard lock(m_mutex); + const std::lock_guard lock(m_mutex); if (m_done && m_filterMgr->isTransaction()) m_filterMgr->commitTransaction(); else @@ -180,7 +180,7 @@ void FilterWorker::run() // void FilterWorker::reportProgress(float progress) { - std::lock_guard lock(m_mutex); + const std::lock_guard lock(m_mutex); m_pos = progress; } @@ -192,7 +192,7 @@ bool FilterWorker::isCancelled() { bool cancelled; - std::lock_guard lock(m_mutex); + const std::lock_guard lock(m_mutex); cancelled = (m_cancelled || m_abort); return cancelled; @@ -209,7 +209,7 @@ void FilterWorker::applyFilterInBackground() m_filterMgr->applyToTarget(); // Mark the work as 'done'. - std::lock_guard lock(m_mutex); + const std::lock_guard lock(m_mutex); m_done = true; } catch (std::exception& e) { @@ -224,7 +224,7 @@ void FilterWorker::applyFilterInBackground() // every 100 milliseconds). void FilterWorker::onMonitoringTick() { - std::lock_guard lock(m_mutex); + const std::lock_guard lock(m_mutex); if (m_alert) { m_alert->setProgress(m_pos); diff --git a/src/app/file/file.cpp b/src/app/file/file.cpp index 12f20466b..2f5243b5a 100644 --- a/src/app/file/file.cpp +++ b/src/app/file/file.cpp @@ -1122,13 +1122,13 @@ void FileOp::operate(IFileOpProgress* progress) void FileOp::done() { // Finally done. - std::lock_guard lock(m_mutex); + const std::lock_guard lock(m_mutex); m_done = true; } void FileOp::stop() { - std::lock_guard lock(m_mutex); + const std::lock_guard lock(m_mutex); if (!m_done) m_stop = true; } @@ -1443,7 +1443,7 @@ void FileOp::setError(const char *format, ...) // Concatenate the new error { - std::lock_guard lock(m_mutex); + const std::lock_guard lock(m_mutex); // Add a newline char automatically if it's needed if (!m_error.empty() && m_error.back() != '\n') m_error.push_back('\n'); @@ -1455,7 +1455,7 @@ void FileOp::setIncompatibilityError(const std::string& msg) { // Concatenate the new error { - std::lock_guard lock(m_mutex); + const std::lock_guard lock(m_mutex); // Add a newline char automatically if it's needed if (!m_incompatibilityError.empty() && m_incompatibilityError.back() != '\n') m_incompatibilityError.push_back('\n'); @@ -1465,7 +1465,7 @@ void FileOp::setIncompatibilityError(const std::string& msg) void FileOp::setProgress(double progress) { - std::lock_guard lock(m_mutex); + const std::lock_guard lock(m_mutex); if (isSequence()) { m_progress = @@ -1494,7 +1494,7 @@ double FileOp::progress() const { double progress; { - std::lock_guard lock(m_mutex); + const std::lock_guard lock(m_mutex); progress = m_progress; } return progress; @@ -1506,7 +1506,7 @@ bool FileOp::isDone() const { bool done; { - std::lock_guard lock(m_mutex); + const std::lock_guard lock(m_mutex); done = m_done; } return done; diff --git a/src/app/task.cpp b/src/app/task.cpp index ee421fdaa..531da7430 100644 --- a/src/app/task.cpp +++ b/src/app/task.cpp @@ -29,7 +29,7 @@ Task::~Task() void Task::run(base::task::func_t&& func) { - std::lock_guard lock(m_token_mutex); + const std::lock_guard lock(m_token_mutex); m_task.on_execute(std::move(func)); m_token = &m_task.start(tasks_pool); } diff --git a/src/app/task.h b/src/app/task.h index 14b502afd..e762f0a62 100644 --- a/src/app/task.h +++ b/src/app/task.h @@ -34,7 +34,7 @@ namespace app { } bool canceled() const { - std::lock_guard lock(m_token_mutex); + const std::lock_guard lock(m_token_mutex); if (m_token) return m_token->canceled(); else @@ -42,7 +42,7 @@ namespace app { } float progress() const { - std::lock_guard lock(m_token_mutex); + const std::lock_guard lock(m_token_mutex); if (m_token) return m_token->progress(); else @@ -50,13 +50,13 @@ namespace app { } void cancel() { - std::lock_guard lock(m_token_mutex); + const std::lock_guard lock(m_token_mutex); if (m_token) m_token->cancel(); } void set_progress(float progress) { - std::lock_guard lock(m_token_mutex); + const std::lock_guard lock(m_token_mutex); if (m_token) m_token->set_progress(progress); } diff --git a/src/app/thumbnail_generator.cpp b/src/app/thumbnail_generator.cpp index 05d732cae..8f9751d2a 100644 --- a/src/app/thumbnail_generator.cpp +++ b/src/app/thumbnail_generator.cpp @@ -49,7 +49,7 @@ public: ~Worker() { { - std::lock_guard lock(m_mutex); + const std::lock_guard lock(m_mutex); if (m_fop) m_fop->stop(); } @@ -57,7 +57,7 @@ public: } void stop() const { - std::lock_guard lock(m_mutex); + const std::lock_guard lock(m_mutex); if (m_fop) m_fop->stop(); } @@ -67,7 +67,7 @@ public: } void updateProgress() { - std::lock_guard lock(m_mutex); + const std::lock_guard lock(m_mutex); if (m_item.fileitem && m_item.fop) { double progress = m_item.fop->progress(); if (progress > m_item.fileitem->getThumbnailProgress()) @@ -80,7 +80,7 @@ private: ASSERT(!m_fop); try { { - std::lock_guard lock(m_mutex); + const std::lock_guard lock(m_mutex); m_fop = m_item.fop; ASSERT(m_fop); } @@ -167,7 +167,7 @@ private: 0, 0, 0, 0, thumbnailImage->width(), thumbnailImage->height()); { - std::lock_guard lock(m_mutex); + const std::lock_guard lock(m_mutex); m_item.fileitem->setThumbnail(thumbnail); } } @@ -191,13 +191,13 @@ private: // Reset the m_item (first the fileitem so this worker is not // associated to this fileitem anymore, and then the FileOp). { - std::lock_guard lock(m_mutex); + const std::lock_guard lock(m_mutex); m_item.fileitem = nullptr; } m_fop->done(); { - std::lock_guard lock(m_mutex); + const std::lock_guard lock(m_mutex); m_item.fop = nullptr; delete m_fop; m_fop = nullptr; @@ -212,7 +212,7 @@ private: bool success = true; while (success) { { - std::lock_guard lock(m_mutex); // To access m_item + const std::lock_guard lock(m_mutex); // To access m_item success = m_queue.try_pop(m_item); } if (success) @@ -253,7 +253,7 @@ ThumbnailGenerator::ThumbnailGenerator() bool ThumbnailGenerator::checkWorkers() { - std::lock_guard lock(m_workersAccess); + const std::lock_guard lock(m_workersAccess); bool doingWork = (!m_workers.empty()); for (WorkerList::iterator @@ -339,14 +339,14 @@ void ThumbnailGenerator::stopAllWorkers() } } - std::lock_guard lock(m_workersAccess); + const std::lock_guard lock(m_workersAccess); for (const auto& worker : m_workers) worker->stop(); } void ThumbnailGenerator::startWorker() { - std::lock_guard lock(m_workersAccess); + const std::lock_guard lock(m_workersAccess); if (m_workers.size() < m_maxWorkers) { m_workers.push_back(std::make_unique(m_remainingItems)); } diff --git a/src/doc/object.cpp b/src/doc/object.cpp index 2bfb6d467..97aed7726 100644 --- a/src/doc/object.cpp +++ b/src/doc/object.cpp @@ -53,7 +53,7 @@ const ObjectId Object::id() const // The first time the ID is request, we store the object in the // "objects" hash table. if (!m_id) { - std::lock_guard lock(g_mutex); + const std::lock_guard lock(g_mutex); m_id = ++newId; objects.insert(std::make_pair(m_id, const_cast(this))); } @@ -62,7 +62,7 @@ const ObjectId Object::id() const void Object::setId(ObjectId id) { - std::lock_guard lock(g_mutex); + const std::lock_guard lock(g_mutex); if (m_id) { auto it = objects.find(m_id); @@ -101,7 +101,7 @@ void Object::setVersion(ObjectVersion version) Object* get_object(ObjectId id) { - std::lock_guard lock(g_mutex); + const std::lock_guard lock(g_mutex); auto it = objects.find(id); if (it != objects.end()) return it->second; From 4ea0390623b337a1c30bd704fa73f6f32f5edb2c Mon Sep 17 00:00:00 2001 From: David Capello Date: Wed, 28 Feb 2024 21:09:23 -0300 Subject: [PATCH 34/39] Add [[nodiscard]] to Tx as we have to create an instance of Tx when we use it --- src/app/tx.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/tx.h b/src/app/tx.h index 08a7c586b..9b287d1c0 100644 --- a/src/app/tx.h +++ b/src/app/tx.h @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2019-2023 Igara Studio S.A. +// Copyright (C) 2019-2024 Igara Studio S.A. // Copyright (C) 2018 David Capello // // This program is distributed under the terms of @@ -23,7 +23,7 @@ namespace app { // Wrapper to create a new transaction or get the current // transaction in the context. - class Tx { + class [[nodiscard]] Tx { public: enum LockAction { DocIsLocked, // The doc is locked to be written From 32bdb3a695c33aaa74a360b3c83d61fa1313075e Mon Sep 17 00:00:00 2001 From: David Capello Date: Wed, 28 Feb 2024 21:10:07 -0300 Subject: [PATCH 35/39] Minor rename of header guard In this way we avoid problems with clang-tidy and defined identifiers that starts with __ --- src/config.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/config.h b/src/config.h index 900148c05..e2e873786 100644 --- a/src/config.h +++ b/src/config.h @@ -1,15 +1,15 @@ // Aseprite -// Copyright (C) 2018-2020 Igara Studio S.A. +// Copyright (C) 2018-2024 Igara Studio S.A. // Copyright (C) 2001-2018 David Capello // // This program is distributed under the terms of // the End-User License Agreement for Aseprite. -#ifdef __ASEPRITE_CONFIG_H +#ifdef ASEPRITE_CONFIG_H_INCLUDED #error You cannot use config.h two times #endif -#define __ASEPRITE_CONFIG_H +#define ASEPRITE_CONFIG_H_INCLUDED // In MSVC #ifdef _MSC_VER From 667335be628e9c71ebbda3a9868f6db54b8dbd35 Mon Sep 17 00:00:00 2001 From: David Capello Date: Wed, 28 Feb 2024 21:10:57 -0300 Subject: [PATCH 36/39] Update laf --- laf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/laf b/laf index 12b411862..bf90f6778 160000 --- a/laf +++ b/laf @@ -1 +1 @@ -Subproject commit 12b411862df1655fac1f9c32847f1801c7205e2b +Subproject commit bf90f6778801bc89190f71527d50e9b1de34c3ed From cfb663f820cfe48fb0c0247559d1c31481d8d2bf Mon Sep 17 00:00:00 2001 From: David Capello Date: Thu, 29 Feb 2024 10:26:47 -0300 Subject: [PATCH 37/39] Some fixes to readability-else-after-return --- src/app/app.cpp | 12 ++++-------- src/app/commands/params.h | 3 +-- src/app/task.h | 6 ++---- src/doc/brush.h | 3 +-- src/doc/image_traits.h | 22 +++++++++------------- src/doc/keyframes.h | 17 ++++++----------- src/doc/palette.h | 3 +-- src/doc/remap.h | 3 +-- src/doc/tileset.h | 6 ++---- src/doc/tilesets.h | 3 +-- src/fixmath/fixmath.h | 34 ++++++++++++---------------------- src/render/dithering_matrix.h | 7 +++---- src/render/zoom.h | 6 ++---- src/ui/widget.h | 3 +-- 14 files changed, 46 insertions(+), 82 deletions(-) diff --git a/src/app/app.cpp b/src/app/app.cpp index 9979fc6a9..e5b6d6ff8 100644 --- a/src/app/app.cpp +++ b/src/app/app.cpp @@ -720,24 +720,21 @@ Workspace* App::workspace() const { if (m_mainWindow) return m_mainWindow->getWorkspace(); - else - return nullptr; + return nullptr; } ContextBar* App::contextBar() const { if (m_mainWindow) return m_mainWindow->getContextBar(); - else - return nullptr; + return nullptr; } Timeline* App::timeline() const { if (m_mainWindow) return m_mainWindow->getTimeline(); - else - return nullptr; + return nullptr; } Preferences& App::preferences() const @@ -866,8 +863,7 @@ PixelFormat app_get_current_pixel_format() Doc* doc = ctx->activeDocument(); if (doc) return doc->sprite()->pixelFormat(); - else - return IMAGE_RGB; + return IMAGE_RGB; } int app_get_color_to_clear_layer(Layer* layer) diff --git a/src/app/commands/params.h b/src/app/commands/params.h index 8a4a6452d..fe6600441 100644 --- a/src/app/commands/params.h +++ b/src/app/commands/params.h @@ -48,8 +48,7 @@ namespace app { auto it = m_params.find(name); if (it != m_params.end()) return it->second; - else - return std::string(); + return std::string(); } void operator|=(const Params& params) { diff --git a/src/app/task.h b/src/app/task.h index e762f0a62..46d65abcf 100644 --- a/src/app/task.h +++ b/src/app/task.h @@ -37,16 +37,14 @@ namespace app { const std::lock_guard lock(m_token_mutex); if (m_token) return m_token->canceled(); - else - return false; + return false; } float progress() const { const std::lock_guard lock(m_token_mutex); if (m_token) return m_token->progress(); - else - return 0.0f; + return 0.0f; } void cancel() { diff --git a/src/doc/brush.h b/src/doc/brush.h index 1a2d0e0aa..47d94de2a 100644 --- a/src/doc/brush.h +++ b/src/doc/brush.h @@ -75,8 +75,7 @@ namespace doc { Image* originalImage() const { if (m_backupImage) return m_backupImage.get(); - else - return m_image.get(); + return m_image.get(); } private: diff --git a/src/doc/image_traits.h b/src/doc/image_traits.h index 1691337fd..cdc546f7e 100644 --- a/src/doc/image_traits.h +++ b/src/doc/image_traits.h @@ -1,5 +1,5 @@ // Aseprite Document Library -// Copyright (c) 2018-2023 Igara Studio S.A. +// Copyright (c) 2018-2024 Igara Studio S.A. // Copyright (c) 2001-2015 David Capello // // This file is released under the terms of the MIT license. @@ -53,13 +53,11 @@ namespace doc { if (rgba_geta(a) == 0) { if (rgba_geta(b) == 0) return true; - else - return false; - } - else if (rgba_geta(b) == 0) return false; - else - return a == b; + } + if (rgba_geta(b) == 0) + return false; + return a == b; } }; @@ -98,13 +96,11 @@ namespace doc { if (graya_geta(a) == 0) { if (graya_geta(b) == 0) return true; - else - return false; - } - else if (graya_geta(b) == 0) return false; - else - return a == b; + } + if (graya_geta(b) == 0) + return false; + return a == b; } }; diff --git a/src/doc/keyframes.h b/src/doc/keyframes.h index 8196c3d93..0dcc39166 100644 --- a/src/doc/keyframes.h +++ b/src/doc/keyframes.h @@ -1,5 +1,5 @@ // Aseprite Document Library -// Copyright (c) 2022 Igara Studio S.A. +// Copyright (c) 2022-2024 Igara Studio S.A. // Copyright (c) 2017 David Capello // // This file is released under the terms of the MIT license. @@ -72,14 +72,12 @@ namespace doc { T* operator*() { if (m_it != m_end) return m_it->value(); - else - return nullptr; + return nullptr; } T* operator->() { if (m_it != m_end) return m_it->value(); - else - return nullptr; + return nullptr; } private: iterator m_it, m_next; @@ -154,8 +152,7 @@ namespace doc { it->value() && frame >= it->frame()) return it->value(); - else - return nullptr; + return nullptr; } iterator begin() { return m_keys.begin(); } @@ -185,15 +182,13 @@ namespace doc { frame_t fromFrame() const { if (!m_keys.empty()) return m_keys.front().frame(); - else - return -1; + return -1; } frame_t toFrame() const { if (!m_keys.empty()) return m_keys.back().frame(); - else - return -1; + return -1; } Range range(const frame_t from, diff --git a/src/doc/palette.h b/src/doc/palette.h index 0cfbc94f5..1ca5579e3 100644 --- a/src/doc/palette.h +++ b/src/doc/palette.h @@ -73,8 +73,7 @@ namespace doc { ASSERT(i >= 0); if (i >= 0 && i < size()) return m_colors[i]; - else - return 0; + return 0; } color_t getEntry(int i) const { return entry(i); diff --git a/src/doc/remap.h b/src/doc/remap.h index feb484b66..b5a8cc824 100644 --- a/src/doc/remap.h +++ b/src/doc/remap.h @@ -54,8 +54,7 @@ namespace doc { //ASSERT(index >= 0 && index < size()); if (index >= 0 && index < size()) return m_map[index]; - else - return index; // No remap + return index; // No remap } void merge(const Remap& other); diff --git a/src/doc/tileset.h b/src/doc/tileset.h index 865a3e232..85235f480 100644 --- a/src/doc/tileset.h +++ b/src/doc/tileset.h @@ -84,8 +84,7 @@ namespace doc { ImageRef get(const tile_index ti) const { if (ti >= 0 && ti < size()) return m_tiles[ti].image; - else - return ImageRef(nullptr); + return ImageRef(nullptr); } void set(const tile_index ti, const ImageRef& image); @@ -93,8 +92,7 @@ namespace doc { UserData& getTileData(const tile_index ti) const { if (ti >= 0 && ti < size()) return const_cast(m_tiles[ti].data); - else - return kNoUserData; + return kNoUserData; } void setTileData(const tile_index ti, const UserData& userData); diff --git a/src/doc/tilesets.h b/src/doc/tilesets.h index a93fd5023..558d64a0b 100644 --- a/src/doc/tilesets.h +++ b/src/doc/tilesets.h @@ -38,8 +38,7 @@ namespace doc { Tileset* get(const tileset_index tsi) const { if (tsi < size()) return m_tilesets[tsi]; - else - return nullptr; + return nullptr; } tileset_index getIndex(const Tileset *tileset) { diff --git a/src/fixmath/fixmath.h b/src/fixmath/fixmath.h index 664906973..20509bd8e 100644 --- a/src/fixmath/fixmath.h +++ b/src/fixmath/fixmath.h @@ -52,17 +52,13 @@ namespace fixmath { errno = ERANGE; return -0x7FFFFFFF; } - else - return result; + return result; } - else { - if ((x > 0) && (y > 0)) { - errno = ERANGE; - return 0x7FFFFFFF; - } - else - return result; + if ((x > 0) && (y > 0)) { + errno = ERANGE; + return 0x7FFFFFFF; } + return result; } inline fixed fixsub(fixed x, fixed y) { @@ -73,17 +69,13 @@ namespace fixmath { errno = ERANGE; return -0x7FFFFFFF; } - else - return result; + return result; } - else { - if ((x > 0) && (y < 0)) { - errno = ERANGE; - return 0x7FFFFFFF; - } - else - return result; + if ((x > 0) && (y < 0)) { + errno = ERANGE; + return 0x7FFFFFFF; } + return result; } inline fixed fixmul(fixed x, fixed y) { @@ -95,16 +87,14 @@ namespace fixmath { errno = ERANGE; return (x < 0) ? -0x7FFFFFFF : 0x7FFFFFFF; } - else - return ftofix(fixtof(x) / fixtof(y)); + return ftofix(fixtof(x) / fixtof(y)); } inline int fixfloor(fixed x) { /* (x >> 16) is not portable */ if (x >= 0) return (x >> 16); - else - return ~((~x) >> 16); + return ~((~x) >> 16); } inline int fixceil(fixed x) { diff --git a/src/render/dithering_matrix.h b/src/render/dithering_matrix.h index 4730ea0ef..dfbabdade 100644 --- a/src/render/dithering_matrix.h +++ b/src/render/dithering_matrix.h @@ -72,10 +72,9 @@ namespace render { if (n == 2) return D2[i*2 + j]; - else - return - + 4*Dn(i%(n/2), j%(n/2), n/2) - + Dn(i/(n/2), j/(n/2), 2); + return + + 4*Dn(i%(n/2), j%(n/2), n/2) + + Dn(i/(n/2), j/(n/2), 2); } }; diff --git a/src/render/zoom.h b/src/render/zoom.h index 1cac0b65b..b55e91dc9 100644 --- a/src/render/zoom.h +++ b/src/render/zoom.h @@ -76,16 +76,14 @@ namespace render { inline int Zoom::remove(int x) const { if (x < 0) return (x * m_den / m_num) - 1; - else - return (x * m_den / m_num); + return (x * m_den / m_num); } inline int Zoom::removeCeiling(int x) const { int v = x * m_den; if (x < 0) return (v / m_num); - else - return (v / m_num) + (v % m_num != 0); + return (v / m_num) + (v % m_num != 0); } inline gfx::Rect Zoom::apply(const gfx::Rect& r) const { diff --git a/src/ui/widget.h b/src/ui/widget.h index d20fa3ed9..ba5ff9080 100644 --- a/src/ui/widget.h +++ b/src/ui/widget.h @@ -142,8 +142,7 @@ namespace ui { gfx::Color bgColor() const { if (gfx::geta(m_bgColor) == 0 && m_parent) return m_parent->bgColor(); - else - return m_bgColor; + return m_bgColor; } // Sets the background color of the widget From 73fe8099d96479364f92ff6317a5687498d43d36 Mon Sep 17 00:00:00 2001 From: David Capello Date: Thu, 29 Feb 2024 23:20:44 -0300 Subject: [PATCH 38/39] [win] Add 20x20 and 28x28 versions of Aseprite icon (fix #3034) This is to fix the appearance of the icon when Windows is configured with 125% DPI or 175% DPI. In the past we've done something similar for 150% DPI (713a2eac80c76f0e53cfcbc3efdb6de41a9a0fc8) with a 24x24 icon. --- data/icons/ase.ico | Bin 57358 -> 58478 bytes data/icons/ase20.png | Bin 0 -> 207 bytes data/icons/ase28.png | Bin 0 -> 263 bytes 3 files changed, 0 insertions(+), 0 deletions(-) create mode 100644 data/icons/ase20.png create mode 100644 data/icons/ase28.png diff --git a/data/icons/ase.ico b/data/icons/ase.ico index 6a60b76b5bf13227d2b3d1466577daaff4692fd2..f68bb5d2cd6ddff5743cc64c14546f294366f899 100644 GIT binary patch delta 874 zcmcIiyGjE=6g~SO_yE}rNlX!C-9<gxK2ywFLL|bR zDdIat8>+%w0aq7}xG>~pw{|q0=3wwQMh_X%UxV-QefHroH3-`TJFd*3aUIgQ7E;Qp zh)r0jhZcsh*|u8lyPNXB4b~Bdx_2S;p&wcfy+pow0plbVqD50>D7$~t^2J-FisjUH jX1kx!nE6}CL3gb;rEz`1Q^@H}F*17Ne*x=f{khgRW~ZKe delta 138 zcmaENg1PSib3FqCBLh2wfPer40|N_#1|tJQ8IYx*0O7x2VqlmD6f-b@@H5yL7}{7E z7#tj+d@crtcR<|G0Oe~iFocN$4Pb!s7cemFvt?kAkbv-C0L@an$G`wo23Dc4aen`O E0Q6N882|tP diff --git a/data/icons/ase20.png b/data/icons/ase20.png new file mode 100644 index 0000000000000000000000000000000000000000..a4bbf8237cdc0e1235b5db7734e51bb1f022e504 GIT binary patch literal 207 zcmeAS@N?(olHy`uVBq!ia0vp^A|TAc1|)ksWqE-VV{wqX6T`Z5GB1IgPEQxd5R2aA zgat+kF3+B%O*Q>f@psoe5oQY)S7~+s*I5T9oH(nM?ZNB&$6pIX&YvCHmZ@ObrYOy2 z+}k2fvQ1)k<8@)(9ClXOt0#l`B;yv-4SAnxChqK6mj`|tn3{aFXJ&qC=ge*}|6H8y{E3Z;3^g88S~C+p3#Ly}_TSWcg=Mi= zUzCSYf})$?!b1&H4ki6ye|g||;@u}dc|gE-XQzlHk1@L}&mR6nhUwgztjZb}58RT| z=aXNT4s_Cl4O^F Date: Mon, 4 Mar 2024 14:57:19 -0300 Subject: [PATCH 39/39] Update laf --- laf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/laf b/laf index bf90f6778..3be4aa115 160000 --- a/laf +++ b/laf @@ -1 +1 @@ -Subproject commit bf90f6778801bc89190f71527d50e9b1de34c3ed +Subproject commit 3be4aa115bfc7a009a49a57a09c4b7dd509ebfcd