From 907138d5dd5d20a36ad92340e4ac4f121e06d472 Mon Sep 17 00:00:00 2001 From: Gaspar Capello Date: Tue, 2 Sep 2025 17:27:33 -0300 Subject: [PATCH 1/9] Fix wrong color space in multiple UI elements This fix solves color space missmatch in the following UI elements: - Timeline thumbnails - Palette entries - Tileset entries - Color Spectrum and Wheels - Color Popup Gradients - Color Button --- laf | 2 +- src/app/modules/gfx.cpp | 4 +-- src/app/script/canvas_widget.cpp | 17 +++++++++-- src/app/thumbnails.cpp | 4 ++- src/app/ui/palette_view.cpp | 4 ++- src/app/ui/tabs.cpp | 4 ++- src/ui/graphics.cpp | 52 +++++++++++++++++++++++++++++--- src/ui/widget.cpp | 3 +- 8 files changed, 75 insertions(+), 15 deletions(-) diff --git a/laf b/laf index 3f1f86cc7..5667a0334 160000 --- a/laf +++ b/laf @@ -1 +1 @@ -Subproject commit 3f1f86cc734443ba5c72d25c72cdf41ca208e9fe +Subproject commit 5667a0334d0d04eb50aa935e7dafe1d4e0fa025b diff --git a/src/app/modules/gfx.cpp b/src/app/modules/gfx.cpp index c808b181d..095fc0793 100644 --- a/src/app/modules/gfx.cpp +++ b/src/app/modules/gfx.cpp @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2018-2024 Igara Studio S.A. +// Copyright (C) 2018-2025 Igara Studio S.A. // Copyright (C) 2001-2018 David Capello // // This program is distributed under the terms of @@ -219,7 +219,7 @@ void draw_tile(ui::Graphics* g, const Rect& rc, const Site& site, doc::tile_t ti int w = tileImage->width(); int h = tileImage->height(); - os::SurfaceRef surface = os::System::instance()->makeRgbaSurface(w, h); + os::SurfaceRef surface = os::System::instance()->makeRgbaSurface(w, h, get_current_color_space()); convert_image_to_surface(tileImage.get(), get_current_palette(), surface.get(), 0, 0, 0, 0, w, h); ui::Paint paint; diff --git a/src/app/script/canvas_widget.cpp b/src/app/script/canvas_widget.cpp index 7d27075dd..2640a6258 100644 --- a/src/app/script/canvas_widget.cpp +++ b/src/app/script/canvas_widget.cpp @@ -1,11 +1,12 @@ // Aseprite -// Copyright (c) 2022-2024 Igara Studio S.A. +// Copyright (c) 2022-2025 Igara Studio S.A. // // This program is distributed under the terms of // the End-User License Agreement for Aseprite. #include "app/script/canvas_widget.h" +#include "app/color_spaces.h" #include "app/script/graphics_context.h" #include "app/ui/skin/skin_theme.h" #include "os/system.h" @@ -15,6 +16,10 @@ #include "ui/size_hint_event.h" #include "ui/system.h" +#ifdef LAF_SKIA + #include "os/skia/skia_color_space.h" +#endif + namespace app { namespace script { // static @@ -45,8 +50,14 @@ void Canvas::callPaint() return; os::Paint p; +#ifdef LAF_SKIA + if (m_surface && m_surface->colorSpace()) + p.color(bgColor(), m_surface->colorSpace().get()); + else +#else p.color(bgColor()); - m_surface->drawRect(m_surface->bounds(), p); +#endif + m_surface->drawRect(m_surface->bounds(), p); // Draw only on resize (onPaint we draw the cached m_surface) GraphicsContext gc(m_surface, m_autoScaling ? ui::guiscale() : 1); @@ -189,7 +200,7 @@ void Canvas::onResize(ui::ResizeEvent& ev) } if (!m_surface || m_surface->width() != w || m_surface->height() != h) { - m_surface = system->makeSurface(w, h); + m_surface = system->makeSurface(w, h, get_current_color_space()); callPaint(); } } diff --git a/src/app/thumbnails.cpp b/src/app/thumbnails.cpp index c2e436b63..6765e1acd 100644 --- a/src/app/thumbnails.cpp +++ b/src/app/thumbnails.cpp @@ -10,6 +10,7 @@ #include "config.h" #endif +#include "app/color_spaces.h" #include "app/util/conversion_to_surface.h" #include "doc/blend_mode.h" #include "doc/cel.h" @@ -55,7 +56,8 @@ os::SurfaceRef get_cel_thumbnail(const doc::Cel* cel, if (os::SurfaceRef thumbnail = os::System::instance()->makeRgbaSurface( thumbnailImage->width(), - thumbnailImage->height())) { + thumbnailImage->height(), + get_current_color_space())) { convert_image_to_surface(thumbnailImage.get(), palette, thumbnail.get(), diff --git a/src/app/ui/palette_view.cpp b/src/app/ui/palette_view.cpp index ee4013670..2dfcaa176 100644 --- a/src/app/ui/palette_view.cpp +++ b/src/app/ui/palette_view.cpp @@ -13,6 +13,7 @@ #include "app/app.h" #include "app/color.h" +#include "app/color_spaces.h" #include "app/color_utils.h" #include "app/commands/commands.h" #include "app/modules/gfx.h" @@ -307,7 +308,8 @@ public: if (tileImage) { int w = tileImage->width(); int h = tileImage->height(); - os::SurfaceRef surface = os::System::instance()->makeRgbaSurface(w, h); + os::SurfaceRef surface = + os::System::instance()->makeRgbaSurface(w, h, get_current_color_space()); convert_image_to_surface(tileImage.get(), get_current_palette(), surface.get(), diff --git a/src/app/ui/tabs.cpp b/src/app/ui/tabs.cpp index 3b0b3f1d8..e5ac6534e 100644 --- a/src/app/ui/tabs.cpp +++ b/src/app/ui/tabs.cpp @@ -11,6 +11,7 @@ #include "app/ui/tabs.h" +#include "app/color_spaces.h" #include "app/color_utils.h" #include "app/modules/gfx.h" #include "app/modules/gui.h" @@ -969,7 +970,8 @@ void Tabs::createFloatingUILayer(Tab* tab) ASSERT(!m_floatingUILayer); ui::Display* display = this->display(); - os::SurfaceRef surface = os::System::instance()->makeRgbaSurface(tab->width, m_tabsHeight); + os::SurfaceRef surface = + os::System::instance()->makeRgbaSurface(tab->width, m_tabsHeight, get_current_color_space()); // Fill the surface with pink color { diff --git a/src/ui/graphics.cpp b/src/ui/graphics.cpp index cc52eaaf8..cf5b8924d 100644 --- a/src/ui/graphics.cpp +++ b/src/ui/graphics.cpp @@ -155,7 +155,12 @@ void Graphics::drawHLine(gfx::Color color, int x, int y, int w) os::SurfaceLock lock(m_surface.get()); os::Paint paint; - paint.color(color); +#ifdef LAF_SKIA + if (m_surface && m_surface->colorSpace()) + paint.color(color, m_surface->colorSpace().get()); + else +#endif + paint.color(color); m_surface->drawRect(gfx::Rect(m_dx + x, m_dy + y, w, 1), paint); } @@ -173,7 +178,12 @@ void Graphics::drawVLine(gfx::Color color, int x, int y, int h) os::SurfaceLock lock(m_surface.get()); os::Paint paint; - paint.color(color); +#ifdef LAF_SKIA + if (m_surface && m_surface->colorSpace()) + paint.color(color, m_surface->colorSpace().get()); + else +#endif + paint.color(color); m_surface->drawRect(gfx::Rect(m_dx + x, m_dy + y, 1, h), paint); } @@ -185,7 +195,12 @@ void Graphics::drawLine(gfx::Color color, const gfx::Point& _a, const gfx::Point os::SurfaceLock lock(m_surface.get()); os::Paint paint; - paint.color(color); +#ifdef LAF_SKIA + if (m_surface && m_surface->colorSpace()) + paint.color(color, m_surface->colorSpace().get()); + else +#endif + paint.color(color); m_surface->drawLine(a, b, paint); } @@ -242,7 +257,12 @@ void Graphics::drawRect(gfx::Color color, const gfx::Rect& rcOrig) os::SurfaceLock lock(m_surface.get()); os::Paint paint; - paint.color(color); +#ifdef LAF_SKIA + if (m_surface && m_surface->colorSpace()) + paint.color(color, m_surface->colorSpace().get()); + else +#endif + paint.color(color); paint.style(os::Paint::Stroke); m_surface->drawRect(rc, paint); } @@ -255,7 +275,12 @@ void Graphics::fillRect(gfx::Color color, const gfx::Rect& rcOrig) os::SurfaceLock lock(m_surface.get()); os::Paint paint; - paint.color(color); +#ifdef LAF_SKIA + if (m_surface && m_surface->colorSpace()) + paint.color(color, m_surface->colorSpace().get()); + else +#endif + paint.color(color); paint.style(os::Paint::Fill); m_surface->drawRect(rc, paint); } @@ -443,12 +468,21 @@ void Graphics::drawUIText(const std::string& str, auto textBlob = text::TextBlob::MakeWithShaper(fontMgr, m_font, str); Paint paint; +#ifdef LAF_SKIA + if (gfx::geta(bg) > 0) { // Paint background + paint.color(bg, m_surface->colorSpace().get()); + paint.style(os::Paint::Fill); + drawRect(gfx::RectF(textBlob->bounds()).offset(pt), paint); + } + paint.color(fg, m_surface->colorSpace().get()); +#else if (gfx::geta(bg) > 0) { // Paint background paint.color(bg); paint.style(os::Paint::Fill); drawRect(gfx::RectF(textBlob->bounds()).offset(pt), paint); } paint.color(fg); +#endif drawTextBlob(textBlob, gfx::PointF(pt), paint); @@ -602,11 +636,19 @@ gfx::Size Graphics::doUIStringAlgorithm(const std::string& str, Paint paint; paint.style(os::Paint::Fill); +#ifdef LAF_SKIA + if (!gfx::is_transparent(bg)) { + paint.color(bg, m_surface->colorSpace().get()); + drawRect(gfx::RectF(xout, pt.y, rc.w, lineSize.h), paint); + } + paint.color(fg, m_surface->colorSpace().get()); +#else if (!gfx::is_transparent(bg)) { paint.color(bg); drawRect(gfx::RectF(xout, pt.y, rc.w, lineSize.h), paint); } paint.color(fg); +#endif float baselineDelta = -metrics.ascent - lineBlob->baseline(); drawTextBlob(lineBlob, gfx::PointF(xout, pt.y + baselineDelta), paint); diff --git a/src/ui/widget.cpp b/src/ui/widget.cpp index 2f62f7490..ea2412416 100644 --- a/src/ui/widget.cpp +++ b/src/ui/widget.cpp @@ -1391,7 +1391,8 @@ GraphicsPtr Widget::getGraphics(const gfx::Rect& clip) // In case of double-buffering, we need to create the temporary // buffer only if the default surface is the screen. if (isDoubleBuffered() && dstSurface->isDirectToScreen()) { - os::SurfaceRef surface = os::System::instance()->makeSurface(clip.w, clip.h); + os::SurfaceRef surface = + os::System::instance()->makeSurface(clip.w, clip.h, dstSurface->colorSpace()); graphics.reset(new Graphics(display, surface, -clip.x, -clip.y), DeleteGraphicsAndSurface(clip, surface, dstSurface)); } From 689e1ff524998f5a92c1b9613fa53cde1f9bee8c Mon Sep 17 00:00:00 2001 From: David Capello Date: Thu, 11 Sep 2025 17:59:13 -0300 Subject: [PATCH 2/9] Remove trailing whitespace from feature_request.md This was the only file failing a pre-commit run --all-files --- .github/ISSUE_TEMPLATE/feature_request.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md index 9acbb5ce1..a193bb4a3 100644 --- a/.github/ISSUE_TEMPLATE/feature_request.md +++ b/.github/ISSUE_TEMPLATE/feature_request.md @@ -14,7 +14,7 @@ assignees: '' > - GitHub issues: https://github.com/aseprite/aseprite/issues?q=label%3Afeature > - Community site: https://community.aseprite.org/c/features/7 > - Steam community: https://steamcommunity.com/app/431730/discussions/1/ - > In case you find a similar feature request, making a comment there will be useful to give some traction and show interest in the feature. + > In case you find a similar feature request, making a comment there will be useful to give some traction and show interest in the feature. **Is your feature request related to a problem? Please describe.** A clear and concise description of what the problem is. Ex. I'm always frustrated when [...] From fae42dbe12f2c8812ae31c1364c4f400b0686e33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thorbj=C3=B8rn=20Lindeijer?= Date: Fri, 12 Sep 2025 11:36:17 +0200 Subject: [PATCH 3/9] CMake: Use find_package(PkgConfig) instead of direct include This gets rid of the following warning: ``` CMake Warning (dev) at /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:441 (message): The package name passed to `find_package_handle_standard_args` (PkgConfig) does not match the name of the calling package (HarfBuzz). This can lead to problems in calling code that expects `find_package` result variables (e.g., `_FOUND`) to follow a certain pattern. Call Stack (most recent call first): /usr/share/cmake/Modules/FindPkgConfig.cmake:114 (find_package_handle_standard_args) aseprite/cmake/FindHarfBuzz.cmake:33 (include) CMakeLists.txt:173 (find_package) This warning is for project developers. Use -Wno-dev to suppress it. ``` By applying the following change: https://github.com/WebKit/WebKit/commit/407fa892d9dfa41738fd985f274485be63d26cfb The file could use further updates based on the upstream version. --- cmake/FindHarfBuzz.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/FindHarfBuzz.cmake b/cmake/FindHarfBuzz.cmake index 562b4bd21..6ad84e01b 100644 --- a/cmake/FindHarfBuzz.cmake +++ b/cmake/FindHarfBuzz.cmake @@ -30,7 +30,7 @@ # HARFBUZZ_INCLUDE_DIRS - containg the HarfBuzz headers # HARFBUZZ_LIBRARIES - containg the HarfBuzz library -include(FindPkgConfig) +find_package(PkgConfig QUIET) pkg_check_modules(PC_HARFBUZZ harfbuzz>=0.9.7) From d7b07a517351ee645e2f328a5475f76e87beaf3b Mon Sep 17 00:00:00 2001 From: David Capello Date: Thu, 11 Sep 2025 17:41:07 -0300 Subject: [PATCH 4/9] Now Paint::color() can receive a color space w/any laf backend --- laf | 2 +- src/app/script/canvas_widget.cpp | 14 ++------ src/ui/graphics.cpp | 60 +++++--------------------------- src/ui/graphics.h | 2 ++ 4 files changed, 14 insertions(+), 64 deletions(-) diff --git a/laf b/laf index 5667a0334..ff3146071 160000 --- a/laf +++ b/laf @@ -1 +1 @@ -Subproject commit 5667a0334d0d04eb50aa935e7dafe1d4e0fa025b +Subproject commit ff3146071e86db059af6eef64fd24096554fa15e diff --git a/src/app/script/canvas_widget.cpp b/src/app/script/canvas_widget.cpp index 2640a6258..228865995 100644 --- a/src/app/script/canvas_widget.cpp +++ b/src/app/script/canvas_widget.cpp @@ -16,10 +16,6 @@ #include "ui/size_hint_event.h" #include "ui/system.h" -#ifdef LAF_SKIA - #include "os/skia/skia_color_space.h" -#endif - namespace app { namespace script { // static @@ -50,14 +46,8 @@ void Canvas::callPaint() return; os::Paint p; -#ifdef LAF_SKIA - if (m_surface && m_surface->colorSpace()) - p.color(bgColor(), m_surface->colorSpace().get()); - else -#else - p.color(bgColor()); -#endif - m_surface->drawRect(m_surface->bounds(), p); + p.color(bgColor(), m_surface->colorSpace().get()); + m_surface->drawRect(m_surface->bounds(), p); // Draw only on resize (onPaint we draw the cached m_surface) GraphicsContext gc(m_surface, m_autoScaling ? ui::guiscale() : 1); diff --git a/src/ui/graphics.cpp b/src/ui/graphics.cpp index cf5b8924d..6ca437947 100644 --- a/src/ui/graphics.cpp +++ b/src/ui/graphics.cpp @@ -155,12 +155,7 @@ void Graphics::drawHLine(gfx::Color color, int x, int y, int w) os::SurfaceLock lock(m_surface.get()); os::Paint paint; -#ifdef LAF_SKIA - if (m_surface && m_surface->colorSpace()) - paint.color(color, m_surface->colorSpace().get()); - else -#endif - paint.color(color); + paint.color(color, colorSpace()); m_surface->drawRect(gfx::Rect(m_dx + x, m_dy + y, w, 1), paint); } @@ -178,12 +173,7 @@ void Graphics::drawVLine(gfx::Color color, int x, int y, int h) os::SurfaceLock lock(m_surface.get()); os::Paint paint; -#ifdef LAF_SKIA - if (m_surface && m_surface->colorSpace()) - paint.color(color, m_surface->colorSpace().get()); - else -#endif - paint.color(color); + paint.color(color, colorSpace()); m_surface->drawRect(gfx::Rect(m_dx + x, m_dy + y, 1, h), paint); } @@ -195,12 +185,7 @@ void Graphics::drawLine(gfx::Color color, const gfx::Point& _a, const gfx::Point os::SurfaceLock lock(m_surface.get()); os::Paint paint; -#ifdef LAF_SKIA - if (m_surface && m_surface->colorSpace()) - paint.color(color, m_surface->colorSpace().get()); - else -#endif - paint.color(color); + paint.color(color, colorSpace()); m_surface->drawLine(a, b, paint); } @@ -257,12 +242,7 @@ void Graphics::drawRect(gfx::Color color, const gfx::Rect& rcOrig) os::SurfaceLock lock(m_surface.get()); os::Paint paint; -#ifdef LAF_SKIA - if (m_surface && m_surface->colorSpace()) - paint.color(color, m_surface->colorSpace().get()); - else -#endif - paint.color(color); + paint.color(color, colorSpace()); paint.style(os::Paint::Stroke); m_surface->drawRect(rc, paint); } @@ -275,12 +255,7 @@ void Graphics::fillRect(gfx::Color color, const gfx::Rect& rcOrig) os::SurfaceLock lock(m_surface.get()); os::Paint paint; -#ifdef LAF_SKIA - if (m_surface && m_surface->colorSpace()) - paint.color(color, m_surface->colorSpace().get()); - else -#endif - paint.color(color); + paint.color(color, colorSpace()); paint.style(os::Paint::Fill); m_surface->drawRect(rc, paint); } @@ -468,21 +443,12 @@ void Graphics::drawUIText(const std::string& str, auto textBlob = text::TextBlob::MakeWithShaper(fontMgr, m_font, str); Paint paint; -#ifdef LAF_SKIA if (gfx::geta(bg) > 0) { // Paint background - paint.color(bg, m_surface->colorSpace().get()); + paint.color(bg, colorSpace()); paint.style(os::Paint::Fill); drawRect(gfx::RectF(textBlob->bounds()).offset(pt), paint); } - paint.color(fg, m_surface->colorSpace().get()); -#else - if (gfx::geta(bg) > 0) { // Paint background - paint.color(bg); - paint.style(os::Paint::Fill); - drawRect(gfx::RectF(textBlob->bounds()).offset(pt), paint); - } - paint.color(fg); -#endif + paint.color(fg, colorSpace()); drawTextBlob(textBlob, gfx::PointF(pt), paint); @@ -636,19 +602,11 @@ gfx::Size Graphics::doUIStringAlgorithm(const std::string& str, Paint paint; paint.style(os::Paint::Fill); -#ifdef LAF_SKIA if (!gfx::is_transparent(bg)) { - paint.color(bg, m_surface->colorSpace().get()); + paint.color(bg, colorSpace()); drawRect(gfx::RectF(xout, pt.y, rc.w, lineSize.h), paint); } - paint.color(fg, m_surface->colorSpace().get()); -#else - if (!gfx::is_transparent(bg)) { - paint.color(bg); - drawRect(gfx::RectF(xout, pt.y, rc.w, lineSize.h), paint); - } - paint.color(fg); -#endif + paint.color(fg, colorSpace()); float baselineDelta = -metrics.ascent - lineBlob->baseline(); drawTextBlob(lineBlob, gfx::PointF(xout, pt.y + baselineDelta), paint); diff --git a/src/ui/graphics.h b/src/ui/graphics.h index 8b08b717a..c7c5be53d 100644 --- a/src/ui/graphics.h +++ b/src/ui/graphics.h @@ -48,6 +48,8 @@ public: int height() const; os::Surface* getInternalSurface() { return m_surface.get(); } + os::ColorSpace* colorSpace() { return m_surface->colorSpace().get(); } + int getInternalDeltaX() { return m_dx; } int getInternalDeltaY() { return m_dy; } From 85793a9e5f3e9422826b906b18155a2f1a43ba69 Mon Sep 17 00:00:00 2001 From: David Capello Date: Tue, 2 Sep 2025 16:25:26 -0300 Subject: [PATCH 5/9] Add AppShortcut to include app::KeySource + ui::Shortcut We need a list of shorcuts (AppShortcuts) similar to ui::Shortcuts but including the key source to compare later where the shortcut was defined and give more priority to the user-defined shortcuts. This logic is not yet added, this patch is only a refactor. --- src/app/app_menus.cpp | 2 +- src/app/commands/cmd_keyboard_shortcuts.cpp | 10 +-- src/app/ui/key.cpp | 67 +++++++++++---------- src/app/ui/key.h | 42 ++++++++++--- src/app/ui/keyboard_shortcuts.cpp | 14 ++--- src/ui/shortcut.cpp | 23 ------- src/ui/shortcut.h | 40 +++++++++--- 7 files changed, 111 insertions(+), 87 deletions(-) diff --git a/src/app/app_menus.cpp b/src/app/app_menus.cpp index cc3b2470c..fdec58e9a 100644 --- a/src/app/app_menus.cpp +++ b/src/app/app_menus.cpp @@ -285,7 +285,7 @@ void destroy_menu_item(ui::Widget* item) os::Shortcut get_os_shortcut_from_key(const Key* key) { if (key && !key->shortcuts().empty()) { - const ui::Shortcut& shortcut = key->shortcuts().front(); + const ui::Shortcut& shortcut = key->shortcuts().front().shortcut; #if LAF_MACOS // Shortcuts with spacebar as modifier do not work well in macOS diff --git a/src/app/commands/cmd_keyboard_shortcuts.cpp b/src/app/commands/cmd_keyboard_shortcuts.cpp index 2cab3c4a2..bf0b93fd6 100644 --- a/src/app/commands/cmd_keyboard_shortcuts.cpp +++ b/src/app/commands/cmd_keyboard_shortcuts.cpp @@ -207,7 +207,7 @@ private: void onChangeShortcut(int index) { LockButtons lock(this); - Shortcut origShortcut = m_key->shortcuts()[index]; + Shortcut origShortcut = m_key->shortcuts()[index].shortcut; SelectShortcut window(origShortcut, m_key->keycontext(), m_keys); window.openWindowInForeground(); @@ -225,7 +225,7 @@ private: LockButtons lock(this); // We need to create a copy of the shortcut because // Key::disableShortcut() will modify the shortcuts() collection itself. - ui::Shortcut shortcut = m_key->shortcuts()[index]; + Shortcut shortcut = m_key->shortcuts()[index].shortcut; if (ui::Alert::show(Strings::alerts_delete_shortcut(shortcut.toString())) != 1) return; @@ -329,7 +329,7 @@ private: gfx::Rect(keyXPos, y, contextXPos - keyXPos, dh * m_key->shortcuts().size())); if (clip) { int i = 0; - for (const Shortcut& shortcut : m_key->shortcuts()) { + for (const AppShortcut& shortcut : m_key->shortcuts()) { if (i != m_hotShortcut || !m_changeButton) { g->drawText(getShortcutText(shortcut), fg, bg, gfx::Point(keyXPos, y)); } @@ -362,7 +362,7 @@ private: gfx::Rect bounds = this->bounds(); MouseMessage* mouseMsg = static_cast(msg); - const Shortcuts* shortcuts = (m_key ? &m_key->shortcuts() : NULL); + const AppShortcuts* shortcuts = (m_key ? &m_key->shortcuts() : nullptr); int y = bounds.y; int dh = textSize().h + 4 * guiscale(); int maxi = (shortcuts && shortcuts->size() > 1 ? shortcuts->size() : 1); @@ -457,7 +457,7 @@ private: m_hotShortcut = -1; } - std::string getShortcutText(const Shortcut& shortcut) const + std::string getShortcutText(const AppShortcut& shortcut) const { if (m_key && m_key->type() == KeyType::WheelAction && shortcut.isEmpty()) { return Strings::keyboard_shortcuts_default_action(); diff --git a/src/app/ui/key.cpp b/src/app/ui/key.cpp index 257236c89..3e0206d95 100644 --- a/src/app/ui/key.cpp +++ b/src/app/ui/key.cpp @@ -202,27 +202,25 @@ std::string get_user_friendly_string_for_wheelaction(app::WheelAction wheelActio return {}; } -void erase_shortcut(app::KeySourceShortcutList& kvs, +void erase_shortcut(app::AppShortcuts& kvs, const app::KeySource source, const ui::Shortcut& shortcut) { for (auto it = kvs.begin(); it != kvs.end();) { auto& kv = *it; - if (kv.first == source && kv.second == shortcut) { + if (kv.source == source && kv.shortcut == shortcut) it = kvs.erase(it); - } else ++it; } } -void erase_shortcuts(app::KeySourceShortcutList& kvs, const app::KeySource source) +void erase_shortcuts(app::AppShortcuts& kvs, const app::KeySource source) { for (auto it = kvs.begin(); it != kvs.end();) { auto& kv = *it; - if (kv.first == source) { + if (kv.source == source) it = kvs.erase(it); - } else ++it; } @@ -389,38 +387,38 @@ KeyPtr Key::MakeDragAction(WheelAction dragAction) return k; } -const ui::Shortcuts& Key::shortcuts() const +const AppShortcuts& Key::shortcuts() const { if (!m_shortcuts) { - m_shortcuts = std::make_unique(); + m_shortcuts = std::make_unique(); // Add default keys for (const auto& kv : m_adds) { - if (kv.first == KeySource::Original) - m_shortcuts->add(kv.second); + if (kv.source == KeySource::Original) + m_shortcuts->add(kv); } // Delete/add extension-defined keys for (const auto& kv : m_dels) { - if (kv.first == KeySource::ExtensionDefined) - m_shortcuts->remove(kv.second); + if (kv.source == KeySource::ExtensionDefined) + m_shortcuts->remove(kv); else { - ASSERT(kv.first != KeySource::Original); + ASSERT(kv.source != KeySource::Original); } } for (const auto& kv : m_adds) { - if (kv.first == KeySource::ExtensionDefined) - m_shortcuts->add(kv.second); + if (kv.source == KeySource::ExtensionDefined) + m_shortcuts->add(kv); } // Delete/add user-defined keys for (const auto& kv : m_dels) { - if (kv.first == KeySource::UserDefined) - m_shortcuts->remove(kv.second); + if (kv.source == KeySource::UserDefined) + m_shortcuts->remove(kv); } for (const auto& kv : m_adds) { - if (kv.first == KeySource::UserDefined) - m_shortcuts->add(kv.second); + if (kv.source == KeySource::UserDefined) + m_shortcuts->add(kv); } } return *m_shortcuts; @@ -428,7 +426,7 @@ const ui::Shortcuts& Key::shortcuts() const void Key::add(const ui::Shortcut& shortcut, const KeySource source, KeyboardShortcuts& globalKeys) { - m_adds.emplace_back(source, shortcut); + m_adds.push_back(AppShortcut(source, shortcut)); m_shortcuts.reset(); // Remove the shortcut from other commands @@ -439,19 +437,21 @@ void Key::add(const ui::Shortcut& shortcut, const KeySource source, KeyboardShor } } -const ui::Shortcut* Key::isPressed(const Message* msg, const KeyContext keyContext) const +const AppShortcut* Key::isPressed(const Message* msg, const KeyContext keyContext) const { if (const auto* keyMsg = dynamic_cast(msg)) { - for (const Shortcut& shortcut : shortcuts()) { - if (shortcut.isPressed(keyMsg->modifiers(), keyMsg->scancode(), keyMsg->unicodeChar()) && + for (const AppShortcut& shortcut : shortcuts()) { + if (shortcut.shortcut.isPressed(keyMsg->modifiers(), + keyMsg->scancode(), + keyMsg->unicodeChar()) && (m_keycontext == KeyContext::Any || match_key_context(m_keycontext, keyContext))) { return &shortcut; } } } else if (const auto* mouseMsg = dynamic_cast(msg)) { - for (const Shortcut& shortcut : shortcuts()) { - if ((shortcut.modifiers() == mouseMsg->modifiers()) && + for (const AppShortcut& shortcut : shortcuts()) { + if ((shortcut.shortcut.modifiers() == mouseMsg->modifiers()) && (m_keycontext == KeyContext::Any || // TODO we could have multiple mouse wheel key-context, // like "sprite editor" context, or "timeline" context, @@ -464,7 +464,7 @@ const ui::Shortcut* Key::isPressed(const Message* msg, const KeyContext keyConte return nullptr; } -const ui::Shortcut* Key::isPressed(const Message* msg) const +const AppShortcut* Key::isPressed(const Message* msg) const { return isPressed(msg, KeyboardShortcuts::getCurrentKeyContext()); } @@ -472,7 +472,7 @@ const ui::Shortcut* Key::isPressed(const Message* msg) const bool Key::isPressed() const { const auto& ss = this->shortcuts(); - return std::any_of(ss.begin(), ss.end(), [](const Shortcut& shortcut) { + return std::any_of(ss.begin(), ss.end(), [](const AppShortcut& shortcut) { return shortcut.isPressed(); }); } @@ -480,7 +480,7 @@ bool Key::isPressed() const bool Key::isLooselyPressed() const { const auto& ss = this->shortcuts(); - return std::any_of(ss.begin(), ss.end(), [](const Shortcut& shortcut) { + return std::any_of(ss.begin(), ss.end(), [](const AppShortcut& shortcut) { return shortcut.isLooselyPressed(); }); } @@ -492,13 +492,16 @@ bool Key::isCommandListed() const bool Key::hasShortcut(const ui::Shortcut& shortcut) const { - return shortcuts().has(shortcut); + return shortcuts().has(AppShortcut( + // KeySource is not used in has() + KeySource::Original, + shortcut)); } bool Key::hasUserDefinedShortcuts() const { return std::any_of(m_adds.begin(), m_adds.end(), [](const auto& kv) { - return (kv.first == KeySource::UserDefined); + return (kv.source == KeySource::UserDefined); }); } @@ -511,7 +514,7 @@ void Key::disableShortcut(const ui::Shortcut& shortcut, const KeySource source) erase_shortcut(m_adds, source, shortcut); erase_shortcut(m_dels, source, shortcut); - m_dels.emplace_back(source, shortcut); + m_dels.push_back(AppShortcut(source, shortcut)); m_shortcuts.reset(); } @@ -531,7 +534,7 @@ void Key::copyOriginalToUser() // Then copy all original & extension-defined keys as user-defined auto copy = m_adds; for (const auto& kv : copy) - m_adds.emplace_back(KeySource::UserDefined, kv.second); + m_adds.push_back(AppShortcut(KeySource::UserDefined, kv.shortcut)); m_shortcuts.reset(); } diff --git a/src/app/ui/key.h b/src/app/ui/key.h index dc3a0b0ec..b2d98e922 100644 --- a/src/app/ui/key.h +++ b/src/app/ui/key.h @@ -103,10 +103,34 @@ inline KeyAction operator&(KeyAction a, KeyAction b) return KeyAction(int(a) & int(b)); } +struct AppShortcut { + KeySource source; + ui::Shortcut shortcut; + + AppShortcut(const KeySource source, const ui::Shortcut& shortcut) + : source(source) + , shortcut(shortcut) + { + } + + bool isEmpty() const { return shortcut.isEmpty(); } + std::string toString() const { return shortcut.toString(); } + bool isPressed() const { return shortcut.isPressed(); } + bool isLooselyPressed() const { return shortcut.isLooselyPressed(); } + + bool operator==(const AppShortcut& other) const { return shortcut.operator==(other.shortcut); } + bool operator!=(const AppShortcut& other) const { return shortcut.operator!=(other.shortcut); } + + ui::KeyModifiers modifiers() const { return shortcut.modifiers(); } + ui::KeyScancode scancode() const { return shortcut.scancode(); } + int unicodeChar() const { return shortcut.unicodeChar(); } +}; + +using AppShortcuts = ui::ShortcutsT; + class Key; using KeyPtr = std::shared_ptr; using Keys = std::vector; -using KeySourceShortcutList = std::vector>; using DragVector = base::Vector2d; class Key { @@ -119,13 +143,13 @@ public: static KeyPtr MakeDragAction(WheelAction dragAction); KeyType type() const { return m_type; } - const ui::Shortcuts& shortcuts() const; - const KeySourceShortcutList& addsKeys() const { return m_adds; } - const KeySourceShortcutList& delsKeys() const { return m_dels; } + const AppShortcuts& shortcuts() const; + const AppShortcuts& addsKeys() const { return m_adds; } + const AppShortcuts& delsKeys() const { return m_dels; } void add(const ui::Shortcut& shortcut, KeySource source, KeyboardShortcuts& globalKeys); - const ui::Shortcut* isPressed(const ui::Message* msg, KeyContext keyContext) const; - const ui::Shortcut* isPressed(const ui::Message* msg) const; + const AppShortcut* isPressed(const ui::Message* msg, KeyContext keyContext) const; + const AppShortcut* isPressed(const ui::Message* msg) const; bool isPressed() const; bool isLooselyPressed() const; bool isCommandListed() const; @@ -161,11 +185,11 @@ public: private: KeyType m_type; - KeySourceShortcutList m_adds; - KeySourceShortcutList m_dels; + AppShortcuts m_adds; + AppShortcuts m_dels; // Final list of shortcuts after processing the // addition/deletion of extension-defined & user-defined keys. - mutable std::unique_ptr m_shortcuts; + mutable std::unique_ptr m_shortcuts; KeyContext m_keycontext; // for KeyType::Command diff --git a/src/app/ui/keyboard_shortcuts.cpp b/src/app/ui/keyboard_shortcuts.cpp index 3405a2ebc..fa28bfbca 100644 --- a/src/app/ui/keyboard_shortcuts.cpp +++ b/src/app/ui/keyboard_shortcuts.cpp @@ -367,12 +367,12 @@ void KeyboardShortcuts::exportKeys(XMLElement* parent, KeyType type) continue; for (const auto& kv : key->delsKeys()) - if (kv.first == KeySource::UserDefined) - exportShortcut(parent, key.get(), kv.second, true); + if (kv.source == KeySource::UserDefined) + exportShortcut(parent, key.get(), kv.shortcut, true); for (const auto& kv : key->addsKeys()) - if (kv.first == KeySource::UserDefined) - exportShortcut(parent, key.get(), kv.second, false); + if (kv.source == KeySource::UserDefined) + exportShortcut(parent, key.get(), kv.shortcut, false); } } @@ -634,10 +634,10 @@ WheelAction KeyboardShortcuts::getWheelActionFromMouseMessage(const KeyContext c const ui::Message* msg) { WheelAction wheelAction = WheelAction::None; - const ui::Shortcut* bestShortcut = nullptr; + const AppShortcut* bestShortcut = nullptr; for (const KeyPtr& key : m_keys) { if (key->type() == KeyType::WheelAction && key->keycontext() == context) { - const ui::Shortcut* shortcut = key->isPressed(msg); + const AppShortcut* shortcut = key->isPressed(msg); if ((shortcut) && (!bestShortcut || bestShortcut->modifiers() < shortcut->modifiers())) { bestShortcut = shortcut; wheelAction = key->wheelAction(); @@ -653,7 +653,7 @@ Keys KeyboardShortcuts::getDragActionsFromKeyMessage(const ui::Message* msg) Keys keys; for (const KeyPtr& key : m_keys) { if (key->type() == KeyType::DragAction) { - const ui::Shortcut* shortcut = key->isPressed(msg); + const AppShortcut* shortcut = key->isPressed(msg); if (shortcut) { keys.push_back(key); } diff --git a/src/ui/shortcut.cpp b/src/ui/shortcut.cpp index 914ce3893..d48a49698 100644 --- a/src/ui/shortcut.cpp +++ b/src/ui/shortcut.cpp @@ -22,8 +22,6 @@ #include #include -#include - namespace ui { #ifdef _WIN32 @@ -377,25 +375,4 @@ bool Shortcut::isLooselyPressed() const return false; } -////////////////////////////////////////////////////////////////////// -// Shortcuts - -bool Shortcuts::has(const Shortcut& shortcut) const -{ - return (std::find(begin(), end(), shortcut) != end()); -} - -void Shortcuts::add(const Shortcut& shortcut) -{ - if (!has(shortcut)) - m_list.push_back(shortcut); -} - -void Shortcuts::remove(const Shortcut& shortcut) -{ - auto it = std::find(begin(), end(), shortcut); - if (it != end()) - m_list.erase(it); -} - } // namespace ui diff --git a/src/ui/shortcut.h b/src/ui/shortcut.h index 7f7e08b03..58c7c98a1 100644 --- a/src/ui/shortcut.h +++ b/src/ui/shortcut.h @@ -9,6 +9,7 @@ #define UI_SHORTCUT_H_INCLUDED #pragma once +#include #include #include @@ -51,11 +52,12 @@ private: int m_unicodeChar; }; -class Shortcuts { +template +class ShortcutsT { public: - typedef std::vector List; - typedef List::iterator iterator; - typedef List::const_iterator const_iterator; + using List = std::vector; + using iterator = typename List::iterator; + using const_iterator = typename List::const_iterator; iterator begin() { return m_list.begin(); } iterator end() { return m_list.end(); } @@ -65,21 +67,39 @@ public: bool empty() const { return m_list.empty(); } std::size_t size() const { return m_list.size(); } - const ui::Shortcut& front() const { return m_list.front(); } + const T& front() const { return m_list.front(); } - const ui::Shortcut& operator[](int index) const { return m_list[index]; } + const T& operator[](int index) const { return m_list[index]; } - ui::Shortcut& operator[](int index) { return m_list[index]; } + T& operator[](int index) { return m_list[index]; } void clear() { m_list.clear(); } - bool has(const Shortcut& shortcut) const; - void add(const Shortcut& shortcut); - void remove(const Shortcut& shortcut); + + bool has(const T& shortcut) const { return (std::find(begin(), end(), shortcut) != end()); } + + void push_back(const T& shortcut) { m_list.push_back(shortcut); } + + void add(const T& shortcut) + { + if (!has(shortcut)) + m_list.push_back(shortcut); + } + + void remove(const T& shortcut) + { + auto it = std::find(begin(), end(), shortcut); + if (it != end()) + m_list.erase(it); + } + + iterator erase(const iterator& it) { return m_list.erase(it); } private: List m_list; }; +using Shortcuts = ShortcutsT; + } // namespace ui #endif From df5dcdc1d943a1e63e317472f325ad3ca7fce452 Mon Sep 17 00:00:00 2001 From: David Capello Date: Tue, 2 Sep 2025 19:19:59 -0300 Subject: [PATCH 6/9] Add some KeyboardShortcuts basic tests --- src/CMakeLists.txt | 1 + src/app/ui/keyboard_shortcuts.cpp | 13 +-- src/app/ui/keyboard_shortcuts.h | 8 +- src/app/ui/keyboard_shortcuts_tests.cpp | 107 ++++++++++++++++++++++++ 4 files changed, 123 insertions(+), 6 deletions(-) create mode 100644 src/app/ui/keyboard_shortcuts_tests.cpp diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index ae600d500..ebcc34eb3 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -220,6 +220,7 @@ if(ENABLE_TESTS) find_tests(ui ui-lib) find_tests(app/cli app-lib) find_tests(app/file app-lib) + find_tests(app/ui app-lib) find_tests(app app-lib) find_tests(. app-lib) endif() diff --git a/src/app/ui/keyboard_shortcuts.cpp b/src/app/ui/keyboard_shortcuts.cpp index fa28bfbca..c1cfefb05 100644 --- a/src/app/ui/keyboard_shortcuts.cpp +++ b/src/app/ui/keyboard_shortcuts.cpp @@ -89,8 +89,10 @@ void KeyboardShortcuts::destroyInstance() KeyboardShortcuts::KeyboardShortcuts() { - ASSERT(Strings::instance()); - Strings::instance()->LanguageChange.connect([] { reset_key_tables_that_depends_on_language(); }); + // Strings instance can be nullptr in tests. + if (auto* strings = Strings::instance()) { + strings->LanguageChange.connect([] { reset_key_tables_that_depends_on_language(); }); + } } KeyboardShortcuts::~KeyboardShortcuts() @@ -574,11 +576,12 @@ KeyContext KeyboardShortcuts::getCurrentKeyContext() return KeyContext::Normal; } -bool KeyboardShortcuts::getCommandFromKeyMessage(const Message* msg, +bool KeyboardShortcuts::getCommandFromKeyMessage(const ui::Message* msg, Command** command, - Params* params) + Params* params, + KeyContext currentKeyContext) { - const KeyContext contexts[] = { getCurrentKeyContext(), KeyContext::Normal }; + const KeyContext contexts[] = { currentKeyContext, KeyContext::Normal }; int n = (contexts[0] != contexts[1] ? 2 : 1); for (int i = 0; i < n; ++i) { for (KeyPtr& key : m_keys) { diff --git a/src/app/ui/keyboard_shortcuts.h b/src/app/ui/keyboard_shortcuts.h index bce7db4a5..0fbf93593 100644 --- a/src/app/ui/keyboard_shortcuts.h +++ b/src/app/ui/keyboard_shortcuts.h @@ -59,7 +59,13 @@ public: const Key* newKey); static KeyContext getCurrentKeyContext(); - bool getCommandFromKeyMessage(const ui::Message* msg, Command** command, Params* params); + + bool getCommandFromKeyMessage( + const ui::Message* msg, + Command** command, + Params* params, + KeyContext currentKeyContext = KeyboardShortcuts::getCurrentKeyContext()); + tools::Tool* getCurrentQuicktool(tools::Tool* currentTool); KeyAction getCurrentActionModifiers(KeyContext context); WheelAction getWheelActionFromMouseMessage(KeyContext context, const ui::Message* msg); diff --git a/src/app/ui/keyboard_shortcuts_tests.cpp b/src/app/ui/keyboard_shortcuts_tests.cpp new file mode 100644 index 000000000..413fdd07e --- /dev/null +++ b/src/app/ui/keyboard_shortcuts_tests.cpp @@ -0,0 +1,107 @@ +// Aseprite +// Copyright (C) 2025 Igara Studio S.A. +// +// This program is distributed under the terms of +// the End-User License Agreement for Aseprite. + +#ifdef HAVE_CONFIG_H + #include "config.h" +#endif + +#include + +#include "app/ui/keyboard_shortcuts.h" + +#include "app/app.h" +#include "app/cli/app_options.h" +#include "app/commands/command.h" +#include "app/commands/command_ids.h" +#include "os/system.h" +#include "ui/app_state.h" +#include "ui/message.h" + +using namespace app; +using namespace ui; + +static KeyboardShortcuts* ks = nullptr; + +#define DEFINE_KEY(commandId, scancode, keycontext) \ + { \ + KeyPtr k = ks->command(CommandId::commandId(), {}, keycontext); \ + k->add(ui::Shortcut(kKeyNoneModifier, scancode, 0), KeySource::Original, *ks); \ + } + +#define DEFINE_USER_KEY(commandId, scancode, keycontext) \ + { \ + KeyPtr k = ks->command(CommandId::commandId(), {}, keycontext); \ + k->add(ui::Shortcut(kKeyNoneModifier, scancode, 0), KeySource::UserDefined, *ks); \ + } + +#define EXPECT_COMMAND_FOR_KEY(commandId, scancode, keycontext) \ + { \ + KeyMessage msg(kKeyDownMessage, scancode, kKeyNoneModifier, 0, 0); \ + Command* cmd = nullptr; \ + Params params; \ + EXPECT_TRUE(ks->getCommandFromKeyMessage(&msg, &cmd, ¶ms, keycontext)); \ + ASSERT_TRUE(cmd != nullptr) << "command not found for key"; \ + EXPECT_EQ(CommandId::commandId(), cmd->id()) << "other command found: " << cmd->id(); \ + } + +TEST(KeyboardShortcuts, Basic) +{ + ks->clear(); + + KeyPtr k = ks->command(CommandId::Undo(), {}, KeyContext::Any); + + // Simulate a key press and check that the command is not yet + // associated to the 'Z' key. + { + KeyMessage msg(kKeyDownMessage, kKeyZ, kKeyNoneModifier, 0, 0); + Command* cmd = nullptr; + Params params; + EXPECT_FALSE(ks->getCommandFromKeyMessage(&msg, &cmd, ¶ms)); + } + + // Associate the command to the 'Z' key and check. + k->add(ui::Shortcut(kKeyNoneModifier, kKeyZ, 0), KeySource::Original, *ks); + EXPECT_COMMAND_FOR_KEY(Undo, kKeyZ, KeyContext::Normal); +} + +TEST(KeyboardShortcuts, KeyContexts) +{ + ks->clear(); + + DEFINE_KEY(Cancel, kKeyEsc, KeyContext::Any); + DEFINE_KEY(GotoPreviousFrame, kKeyLeft, KeyContext::Normal); + DEFINE_KEY(MoveMask, kKeyLeft, KeyContext::SelectionTool); + + EXPECT_COMMAND_FOR_KEY(Cancel, kKeyEsc, KeyContext::Normal); + EXPECT_COMMAND_FOR_KEY(GotoPreviousFrame, kKeyLeft, KeyContext::Normal); + EXPECT_COMMAND_FOR_KEY(MoveMask, kKeyLeft, KeyContext::SelectionTool); +} + +TEST(KeyboardShortcuts, UserDefined) +{ + ks->clear(); + + DEFINE_KEY(Undo, kKeyZ, KeyContext::Normal); + EXPECT_COMMAND_FOR_KEY(Undo, kKeyZ, KeyContext::Normal); + + DEFINE_USER_KEY(Redo, kKeyZ, KeyContext::Normal); + EXPECT_COMMAND_FOR_KEY(Redo, kKeyZ, KeyContext::Normal); +} + +int app_main(int argc, char* argv[]) +{ + os::SystemRef system = os::System::make(); + const char* argv2[] = { argv[0] }; + app::AppOptions options(sizeof(argv2) / sizeof(argv2[0]), argv2); + app::App app; + app.initialize(options); + app.run(false); + + ks = KeyboardShortcuts::instance(); + + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} From 71e680e05aeb9d4b72c8e2e7458650dceb29ab33 Mon Sep 17 00:00:00 2001 From: David Capello Date: Wed, 3 Sep 2025 18:17:32 -0300 Subject: [PATCH 7/9] Refactor key context matching (fix #5390) * Added AppShortcut::fitsBetterThan() to compare shortcuts and decide which one is better depending on the current key context and the ones where shortcuts are defined * Added Key::fitsContext() to known if this key definition makes sense for a given current key context, e.g. Normal context can be used in SelectionTool context, and SelectionTool can be used in Transformation * Added KeyboardShortcuts::findBestKeyFromMessage() to centralize the keyboard shortcuts matching moving some code from CustomizedGuiManager::processKey() * Added more keyboard shortcuts tests (including some for #5390) --- src/app/modules/gui.cpp | 18 +----- src/app/ui/key.cpp | 83 ++++++++++++++++++++----- src/app/ui/key.h | 13 ++++ src/app/ui/key_context.h | 47 +++++++++++--- src/app/ui/keyboard_shortcuts.cpp | 47 ++++++++++---- src/app/ui/keyboard_shortcuts.h | 7 +++ src/app/ui/keyboard_shortcuts_tests.cpp | 83 ++++++++++++++++++++++++- 7 files changed, 245 insertions(+), 53 deletions(-) diff --git a/src/app/modules/gui.cpp b/src/app/modules/gui.cpp index d0b034743..8455a2d61 100644 --- a/src/app/modules/gui.cpp +++ b/src/app/modules/gui.cpp @@ -680,27 +680,13 @@ void CustomizedGuiManager::onNewDisplayConfiguration(Display* display) bool CustomizedGuiManager::processKey(Message* msg) { - App* app = App::instance(); - const KeyContext currentCtx = KeyboardShortcuts::getCurrentKeyContext(); const KeyboardShortcuts* keys = KeyboardShortcuts::instance(); - const KeyContext contexts[] = { currentCtx, KeyContext::Normal }; - int n = (contexts[0] != contexts[1] ? 2 : 1); - - // Find best match (prefer the shortcut that matches the context first) - KeyPtr key = nullptr; - for (int i = 0; i < n; ++i) { - for (const KeyPtr& k : *keys) { - if (k->isPressed(msg, contexts[i]) && - (!key || - (key->keycontext() != currentCtx && match_key_context(k->keycontext(), currentCtx)))) { - key = k; - } - } - } + const KeyPtr key = keys->findBestKeyFromMessage(msg); if (!key) return false; // Cancel menu-bar loops (to close any popup menu) + App* app = App::instance(); app->mainWindow()->getMenuBar()->cancelMenuLoop(); switch (key->type()) { diff --git a/src/app/ui/key.cpp b/src/app/ui/key.cpp index 3e0206d95..4684515a1 100644 --- a/src/app/ui/key.cpp +++ b/src/app/ui/key.cpp @@ -295,6 +295,32 @@ namespace app { using namespace ui; +bool AppShortcut::fitsBetterThan(const KeyContext currentContext, + const KeyContext thisShortcutContext, + const KeyContext otherShortcutContext, + const AppShortcut& otherShortcut) const +{ + // Better context in the same source + if (otherShortcut.source == this->source && otherShortcutContext != currentContext && + thisShortcutContext == currentContext) + return true; + + // Better key source/level: User-defined > Extension-defined > App-defined + if (int(this->source) > int(otherShortcut.source) && (thisShortcutContext == currentContext || + // User-defined "Any" context overwrites all + // app-defined context + thisShortcutContext == KeyContext::Any)) + return true; + + // Normal > SelectionTool > Transformation + if ((currentContext == KeyContext::Transformation && + otherShortcutContext != KeyContext::Transformation && + thisShortcutContext == KeyContext::SelectionTool)) + return true; + + return false; +} + Key::Key(const Key& k) : m_type(k.m_type) , m_adds(k.m_adds) @@ -437,31 +463,58 @@ void Key::add(const ui::Shortcut& shortcut, const KeySource source, KeyboardShor } } +bool Key::fitsContext(const KeyContext keyContext) const +{ + // This key is for any context + if (m_keycontext == KeyContext::Any) + return true; + + // This key is for the same context + if (m_keycontext == keyContext) + return true; + + // Use Normal or SelectionTool keys in Transformation context + if (keyContext == KeyContext::Transformation && + (m_keycontext == KeyContext::SelectionTool || m_keycontext == KeyContext::Normal)) + return true; + + // Use Normal keys in SelectionTool or FramesSelection contexts + if ((keyContext == KeyContext::SelectionTool || keyContext == KeyContext::FramesSelection) && + (m_keycontext == KeyContext::Normal)) + return true; + + return false; +} + const AppShortcut* Key::isPressed(const Message* msg, const KeyContext keyContext) const { + const AppShortcut* best = nullptr; + if (const auto* keyMsg = dynamic_cast(msg)) { - for (const AppShortcut& shortcut : shortcuts()) { - if (shortcut.shortcut.isPressed(keyMsg->modifiers(), - keyMsg->scancode(), - keyMsg->unicodeChar()) && - (m_keycontext == KeyContext::Any || match_key_context(m_keycontext, keyContext))) { - return &shortcut; + if (fitsContext(keyContext)) { + for (const AppShortcut& shortcut : shortcuts()) { + if (shortcut.shortcut.isPressed(keyMsg->modifiers(), + keyMsg->scancode(), + keyMsg->unicodeChar()) && + (!best || shortcut.fitsBetterThan(keyContext, keycontext(), keycontext(), *best))) { + best = &shortcut; + } } } } else if (const auto* mouseMsg = dynamic_cast(msg)) { - for (const AppShortcut& shortcut : shortcuts()) { - if ((shortcut.shortcut.modifiers() == mouseMsg->modifiers()) && - (m_keycontext == KeyContext::Any || - // TODO we could have multiple mouse wheel key-context, - // like "sprite editor" context, or "timeline" context, - // etc. - m_keycontext == KeyContext::MouseWheel)) { - return &shortcut; + if (m_keycontext == KeyContext::Any || + // TODO we could have multiple mouse wheel key-context, + // like "sprite editor" context, or "timeline" context, + // etc. + m_keycontext == KeyContext::MouseWheel) { + for (const AppShortcut& shortcut : shortcuts()) { + if (shortcut.shortcut.modifiers() == mouseMsg->modifiers()) + return &shortcut; } } } - return nullptr; + return best; } const AppShortcut* Key::isPressed(const Message* msg) const diff --git a/src/app/ui/key.h b/src/app/ui/key.h index b2d98e922..aed0eb5d9 100644 --- a/src/app/ui/key.h +++ b/src/app/ui/key.h @@ -103,6 +103,8 @@ inline KeyAction operator&(KeyAction a, KeyAction b) return KeyAction(int(a) & int(b)); } +// This is a ui::Shortcut wrapper (just one key shortcut) defined by +// the app, an extension, or the user (KeySource). struct AppShortcut { KeySource source; ui::Shortcut shortcut; @@ -124,6 +126,13 @@ struct AppShortcut { ui::KeyModifiers modifiers() const { return shortcut.modifiers(); } ui::KeyScancode scancode() const { return shortcut.scancode(); } int unicodeChar() const { return shortcut.unicodeChar(); } + + // Returns true if this AppShortcut is better for the current + // context, compared to another shortcut. + bool fitsBetterThan(KeyContext currentContext, + KeyContext thisShortcutContext, + KeyContext otherShortcutContext, + const AppShortcut& otherShortcut) const; }; using AppShortcuts = ui::ShortcutsT; @@ -133,6 +142,8 @@ using KeyPtr = std::shared_ptr; using Keys = std::vector; using DragVector = base::Vector2d; +// A set of key shortcuts (AppShortcuts) associated to one command, +// tool, or specific action. class Key { public: Key(const Key& key); @@ -148,6 +159,8 @@ public: const AppShortcuts& delsKeys() const { return m_dels; } void add(const ui::Shortcut& shortcut, KeySource source, KeyboardShortcuts& globalKeys); + + bool fitsContext(KeyContext keyContext) const; const AppShortcut* isPressed(const ui::Message* msg, KeyContext keyContext) const; const AppShortcut* isPressed(const ui::Message* msg) const; bool isPressed() const; diff --git a/src/app/ui/key_context.h b/src/app/ui/key_context.h index 205773dbc..6b8f11067 100644 --- a/src/app/ui/key_context.h +++ b/src/app/ui/key_context.h @@ -11,28 +11,61 @@ namespace app { +// Specifies a possible App state/context where the user can press a +// key and associate different actions/commands in that specific +// context. Useful to associate the same key to different actions +// depending on the current context. +// +// This list should be sorted in such a way that more specific +// contexts are below: +// +// Normal > SelectionTool > Transformation +// +// This means that keys defined in SelectionTool are still valid in +// Transformation context. +// +// Other key context are just temporary in the Editor, like a mouse +// drag operation when we are transforming the +// selection. E.g. TranslatingSelection, ScalingSelection, etc. enum class KeyContext { + + // Special context to define keys. This is not used as a "current + // key context", but just to define keys for the whole range of key + // contexts. Any, + + // Regular context in a standby user state, the user is not dragging + // or clicking the mouse. Normal, + + // The user has a selection on the canvas and a selection-like tool + // (selection ink) selected. SelectionTool, + + // Special context to modify a specific action when the mouse is + // above the handles to translate/scale/rotate or is already + // translating/scaling/rotating the selection. TranslatingSelection, ScalingSelection, RotatingSelection, + + // Special context when the user has a specific tool selected to + // modify the tool behavior with a key: MoveTool, FreehandTool, ShapeTool, + + // When the mouse wheel is used above the editor. MouseWheel, + + // The user has a range of frames/cels selected in the timeline. FramesSelection, + + // The user has moved the selection and has yet to drop the pixels + // (the user can drop or undo the transformation). Transformation, }; -inline bool match_key_context(const KeyContext a, const KeyContext b) -{ - return (a == b) || (a == KeyContext::Any || b == KeyContext::Any) || - ((a == KeyContext::SelectionTool && b == KeyContext::Transformation) || - (a == KeyContext::Transformation && b == KeyContext::SelectionTool)); -} - } // namespace app #endif diff --git a/src/app/ui/keyboard_shortcuts.cpp b/src/app/ui/keyboard_shortcuts.cpp index c1cfefb05..de5e9194d 100644 --- a/src/app/ui/keyboard_shortcuts.cpp +++ b/src/app/ui/keyboard_shortcuts.cpp @@ -576,23 +576,46 @@ KeyContext KeyboardShortcuts::getCurrentKeyContext() return KeyContext::Normal; } +KeyPtr KeyboardShortcuts::findBestKeyFromMessage(const ui::Message* msg, + KeyContext currentKeyContext, + std::optional filterByType) const +{ + const KeyContext contexts[] = { currentKeyContext, KeyContext::Normal }; + int n = (contexts[0] != contexts[1] ? 2 : 1); + KeyPtr bestKey = nullptr; + const AppShortcut* bestShortcut = nullptr; + for (int i = 0; i < n; ++i) { + for (const KeyPtr& key : m_keys) { + // Skip keys that are not for the specific KeyType (e.g. only for commands). + if (filterByType.has_value() && key->type() != *filterByType) + continue; + + const AppShortcut* shortcut = key->isPressed(msg, contexts[i]); + if (shortcut && (!bestKey || shortcut->fitsBetterThan(currentKeyContext, + key->keycontext(), + bestKey->keycontext(), + *bestShortcut))) { + bestKey = key; + bestShortcut = shortcut; + } + } + } + return bestKey; +} + bool KeyboardShortcuts::getCommandFromKeyMessage(const ui::Message* msg, Command** command, Params* params, KeyContext currentKeyContext) { - const KeyContext contexts[] = { currentKeyContext, KeyContext::Normal }; - int n = (contexts[0] != contexts[1] ? 2 : 1); - for (int i = 0; i < n; ++i) { - for (KeyPtr& key : m_keys) { - if (key->type() == KeyType::Command && key->isPressed(msg, contexts[i])) { - if (command) - *command = key->command(); - if (params) - *params = key->params(); - return true; - } - } + KeyPtr key = findBestKeyFromMessage(msg, currentKeyContext, std::make_optional(KeyType::Command)); + if (key) { + ASSERT(key->type() == KeyType::Command); + if (command) + *command = key->command(); + if (params) + *params = key->params(); + return true; } return false; } diff --git a/src/app/ui/keyboard_shortcuts.h b/src/app/ui/keyboard_shortcuts.h index 0fbf93593..791bdb06c 100644 --- a/src/app/ui/keyboard_shortcuts.h +++ b/src/app/ui/keyboard_shortcuts.h @@ -12,6 +12,8 @@ #include "app/ui/key.h" #include "obs/signal.h" +#include + namespace tinyxml2 { class XMLElement; } @@ -60,6 +62,11 @@ public: static KeyContext getCurrentKeyContext(); + KeyPtr findBestKeyFromMessage( + const ui::Message* msg, + KeyContext currentKeyContext = KeyboardShortcuts::getCurrentKeyContext(), + std::optional filterByType = std::nullopt) const; + bool getCommandFromKeyMessage( const ui::Message* msg, Command** command, diff --git a/src/app/ui/keyboard_shortcuts_tests.cpp b/src/app/ui/keyboard_shortcuts_tests.cpp index 413fdd07e..1848fdd21 100644 --- a/src/app/ui/keyboard_shortcuts_tests.cpp +++ b/src/app/ui/keyboard_shortcuts_tests.cpp @@ -41,12 +41,18 @@ static KeyboardShortcuts* ks = nullptr; { \ KeyMessage msg(kKeyDownMessage, scancode, kKeyNoneModifier, 0, 0); \ Command* cmd = nullptr; \ - Params params; \ - EXPECT_TRUE(ks->getCommandFromKeyMessage(&msg, &cmd, ¶ms, keycontext)); \ + EXPECT_TRUE(ks->getCommandFromKeyMessage(&msg, &cmd, nullptr, keycontext)); \ ASSERT_TRUE(cmd != nullptr) << "command not found for key"; \ EXPECT_EQ(CommandId::commandId(), cmd->id()) << "other command found: " << cmd->id(); \ } +#define NO_COMMAND_FOR_KEY(scancode, keycontext) \ + { \ + KeyMessage msg(kKeyDownMessage, scancode, kKeyNoneModifier, 0, 0); \ + Command* cmd = nullptr; \ + EXPECT_FALSE(ks->getCommandFromKeyMessage(&msg, &cmd, nullptr, keycontext)); \ + ASSERT_TRUE(cmd == nullptr) << "command found for key: " << cmd->id(); \ + } TEST(KeyboardShortcuts, Basic) { ks->clear(); @@ -73,14 +79,28 @@ TEST(KeyboardShortcuts, KeyContexts) DEFINE_KEY(Cancel, kKeyEsc, KeyContext::Any); DEFINE_KEY(GotoPreviousFrame, kKeyLeft, KeyContext::Normal); + DEFINE_KEY(PlayAnimation, kKeyEnter, KeyContext::Normal); + DEFINE_KEY(Clear, kKeyBackspace, KeyContext::Normal); DEFINE_KEY(MoveMask, kKeyLeft, KeyContext::SelectionTool); + DEFINE_KEY(Apply, kKeyEnter, KeyContext::Transformation); + DEFINE_KEY(Cut, kKeyX, KeyContext::SelectionTool); EXPECT_COMMAND_FOR_KEY(Cancel, kKeyEsc, KeyContext::Normal); EXPECT_COMMAND_FOR_KEY(GotoPreviousFrame, kKeyLeft, KeyContext::Normal); EXPECT_COMMAND_FOR_KEY(MoveMask, kKeyLeft, KeyContext::SelectionTool); + EXPECT_COMMAND_FOR_KEY(PlayAnimation, kKeyEnter, KeyContext::Normal); + EXPECT_COMMAND_FOR_KEY(PlayAnimation, kKeyEnter, KeyContext::SelectionTool); + EXPECT_COMMAND_FOR_KEY(Apply, kKeyEnter, KeyContext::Transformation); + EXPECT_COMMAND_FOR_KEY(Clear, kKeyBackspace, KeyContext::Normal); + EXPECT_COMMAND_FOR_KEY(Clear, kKeyBackspace, KeyContext::SelectionTool); + EXPECT_COMMAND_FOR_KEY(Clear, kKeyBackspace, KeyContext::Transformation); + + NO_COMMAND_FOR_KEY(kKeyX, KeyContext::Normal); + EXPECT_COMMAND_FOR_KEY(Cut, kKeyX, KeyContext::SelectionTool); + EXPECT_COMMAND_FOR_KEY(Cut, kKeyX, KeyContext::Transformation); } -TEST(KeyboardShortcuts, UserDefined) +TEST(KeyboardShortcuts, UserDefinedPriority) { ks->clear(); @@ -89,6 +109,63 @@ TEST(KeyboardShortcuts, UserDefined) DEFINE_USER_KEY(Redo, kKeyZ, KeyContext::Normal); EXPECT_COMMAND_FOR_KEY(Redo, kKeyZ, KeyContext::Normal); + + DEFINE_KEY(MoveMask, kKeyLeft, KeyContext::SelectionTool); + DEFINE_USER_KEY(GotoPreviousFrame, kKeyLeft, KeyContext::Normal); + EXPECT_COMMAND_FOR_KEY(MoveMask, kKeyLeft, KeyContext::SelectionTool); + + DEFINE_USER_KEY(GotoPreviousFrame, kKeyLeft, KeyContext::Any); + EXPECT_COMMAND_FOR_KEY(GotoPreviousFrame, kKeyLeft, KeyContext::SelectionTool); +} + +TEST(KeyboardShortcuts, SpecificContextHasMorePriorityButNotIfItsUserDefined) +{ + ks->clear(); + DEFINE_KEY(Cancel, kKeyEsc, KeyContext::Any); + DEFINE_KEY(Undo, kKeyEsc, KeyContext::Transformation); + + // Pressing "Esc" in "Transformation" context should run "Undo", + // although "Cancel" is defined for "Any" context, a more specific + // context should have more priority. + EXPECT_COMMAND_FOR_KEY(Undo, kKeyEsc, KeyContext::Transformation); + + // But an user-defined key, even for Any context, will overwrite the + // app-defined shortcut in all contexts. + DEFINE_USER_KEY(Zoom, kKeyEsc, KeyContext::Any); + EXPECT_COMMAND_FOR_KEY(Zoom, kKeyEsc, KeyContext::Transformation); +} + +// Test that we can configure the Left key to always Undo when the +// default configuration says that the Left key does other actions in +// different contexts. +// +// Related issue: https://github.com/aseprite/aseprite/issues/5390 +TEST(KeyboardShortcuts, UndoWithLeftAndRight) +{ + ks->clear(); + + DEFINE_KEY(GotoPreviousFrame, kKeyLeft, KeyContext::Normal); + DEFINE_KEY(MoveMask, kKeyLeft, KeyContext::SelectionTool); + EXPECT_COMMAND_FOR_KEY(GotoPreviousFrame, kKeyLeft, KeyContext::Normal); + EXPECT_COMMAND_FOR_KEY(MoveMask, kKeyLeft, KeyContext::SelectionTool); + // "Transformation" is a sub-context of "Selection" context + EXPECT_COMMAND_FOR_KEY(MoveMask, kKeyLeft, KeyContext::Transformation); + + // Now we try defining the "Left" key for "Any" context overwriting it in all contexts. + DEFINE_USER_KEY(Undo, kKeyLeft, KeyContext::Any); + EXPECT_COMMAND_FOR_KEY(Undo, kKeyLeft, KeyContext::Normal); + EXPECT_COMMAND_FOR_KEY(Undo, kKeyLeft, KeyContext::SelectionTool); + EXPECT_COMMAND_FOR_KEY(Undo, kKeyLeft, KeyContext::Transformation); +} + +TEST(KeyboardShortcuts, FramesSelection) +{ + ks->clear(); + + DEFINE_KEY(LayerProperties, kKeyF2, KeyContext::Normal); + DEFINE_KEY(SetLoopSection, kKeyF2, KeyContext::FramesSelection); + EXPECT_COMMAND_FOR_KEY(LayerProperties, kKeyF2, KeyContext::Normal); + EXPECT_COMMAND_FOR_KEY(SetLoopSection, kKeyF2, KeyContext::FramesSelection); } int app_main(int argc, char* argv[]) From 5d5f3ec234f37760d1da06e7286ed7b43207a857 Mon Sep 17 00:00:00 2001 From: David Capello Date: Fri, 12 Sep 2025 15:48:38 -0300 Subject: [PATCH 8/9] Convert AppShortcut in a ui::Shortcut subclass --- src/app/app_menus.cpp | 2 +- src/app/commands/cmd_keyboard_shortcuts.cpp | 4 +-- src/app/ui/key.cpp | 36 ++++++++++----------- src/app/ui/key.h | 27 +++++++--------- src/app/ui/keyboard_shortcuts.cpp | 8 ++--- 5 files changed, 35 insertions(+), 42 deletions(-) diff --git a/src/app/app_menus.cpp b/src/app/app_menus.cpp index fdec58e9a..cc3b2470c 100644 --- a/src/app/app_menus.cpp +++ b/src/app/app_menus.cpp @@ -285,7 +285,7 @@ void destroy_menu_item(ui::Widget* item) os::Shortcut get_os_shortcut_from_key(const Key* key) { if (key && !key->shortcuts().empty()) { - const ui::Shortcut& shortcut = key->shortcuts().front().shortcut; + const ui::Shortcut& shortcut = key->shortcuts().front(); #if LAF_MACOS // Shortcuts with spacebar as modifier do not work well in macOS diff --git a/src/app/commands/cmd_keyboard_shortcuts.cpp b/src/app/commands/cmd_keyboard_shortcuts.cpp index bf0b93fd6..06cc3e723 100644 --- a/src/app/commands/cmd_keyboard_shortcuts.cpp +++ b/src/app/commands/cmd_keyboard_shortcuts.cpp @@ -207,7 +207,7 @@ private: void onChangeShortcut(int index) { LockButtons lock(this); - Shortcut origShortcut = m_key->shortcuts()[index].shortcut; + Shortcut origShortcut = m_key->shortcuts()[index]; SelectShortcut window(origShortcut, m_key->keycontext(), m_keys); window.openWindowInForeground(); @@ -225,7 +225,7 @@ private: LockButtons lock(this); // We need to create a copy of the shortcut because // Key::disableShortcut() will modify the shortcuts() collection itself. - Shortcut shortcut = m_key->shortcuts()[index].shortcut; + Shortcut shortcut = m_key->shortcuts()[index]; if (ui::Alert::show(Strings::alerts_delete_shortcut(shortcut.toString())) != 1) return; diff --git a/src/app/ui/key.cpp b/src/app/ui/key.cpp index 4684515a1..e6df4cd2d 100644 --- a/src/app/ui/key.cpp +++ b/src/app/ui/key.cpp @@ -208,7 +208,7 @@ void erase_shortcut(app::AppShortcuts& kvs, { for (auto it = kvs.begin(); it != kvs.end();) { auto& kv = *it; - if (kv.source == source && kv.shortcut == shortcut) + if (kv.source() == source && kv.shortcut() == shortcut) it = kvs.erase(it); else ++it; @@ -219,7 +219,7 @@ void erase_shortcuts(app::AppShortcuts& kvs, const app::KeySource source) { for (auto it = kvs.begin(); it != kvs.end();) { auto& kv = *it; - if (kv.source == source) + if (kv.source() == source) it = kvs.erase(it); else ++it; @@ -301,15 +301,15 @@ bool AppShortcut::fitsBetterThan(const KeyContext currentContext, const AppShortcut& otherShortcut) const { // Better context in the same source - if (otherShortcut.source == this->source && otherShortcutContext != currentContext && + if (otherShortcut.source() == this->source() && otherShortcutContext != currentContext && thisShortcutContext == currentContext) return true; // Better key source/level: User-defined > Extension-defined > App-defined - if (int(this->source) > int(otherShortcut.source) && (thisShortcutContext == currentContext || - // User-defined "Any" context overwrites all - // app-defined context - thisShortcutContext == KeyContext::Any)) + if (int(source()) > int(otherShortcut.source()) && (thisShortcutContext == currentContext || + // User-defined "Any" context overwrites all + // app-defined context + thisShortcutContext == KeyContext::Any)) return true; // Normal > SelectionTool > Transformation @@ -420,30 +420,30 @@ const AppShortcuts& Key::shortcuts() const // Add default keys for (const auto& kv : m_adds) { - if (kv.source == KeySource::Original) + if (kv.source() == KeySource::Original) m_shortcuts->add(kv); } // Delete/add extension-defined keys for (const auto& kv : m_dels) { - if (kv.source == KeySource::ExtensionDefined) + if (kv.source() == KeySource::ExtensionDefined) m_shortcuts->remove(kv); else { - ASSERT(kv.source != KeySource::Original); + ASSERT(kv.source() != KeySource::Original); } } for (const auto& kv : m_adds) { - if (kv.source == KeySource::ExtensionDefined) + if (kv.source() == KeySource::ExtensionDefined) m_shortcuts->add(kv); } // Delete/add user-defined keys for (const auto& kv : m_dels) { - if (kv.source == KeySource::UserDefined) + if (kv.source() == KeySource::UserDefined) m_shortcuts->remove(kv); } for (const auto& kv : m_adds) { - if (kv.source == KeySource::UserDefined) + if (kv.source() == KeySource::UserDefined) m_shortcuts->add(kv); } } @@ -493,9 +493,7 @@ const AppShortcut* Key::isPressed(const Message* msg, const KeyContext keyContex if (const auto* keyMsg = dynamic_cast(msg)) { if (fitsContext(keyContext)) { for (const AppShortcut& shortcut : shortcuts()) { - if (shortcut.shortcut.isPressed(keyMsg->modifiers(), - keyMsg->scancode(), - keyMsg->unicodeChar()) && + if (shortcut.isPressed(keyMsg->modifiers(), keyMsg->scancode(), keyMsg->unicodeChar()) && (!best || shortcut.fitsBetterThan(keyContext, keycontext(), keycontext(), *best))) { best = &shortcut; } @@ -509,7 +507,7 @@ const AppShortcut* Key::isPressed(const Message* msg, const KeyContext keyContex // etc. m_keycontext == KeyContext::MouseWheel) { for (const AppShortcut& shortcut : shortcuts()) { - if (shortcut.shortcut.modifiers() == mouseMsg->modifiers()) + if (shortcut.modifiers() == mouseMsg->modifiers()) return &shortcut; } } @@ -554,7 +552,7 @@ bool Key::hasShortcut(const ui::Shortcut& shortcut) const bool Key::hasUserDefinedShortcuts() const { return std::any_of(m_adds.begin(), m_adds.end(), [](const auto& kv) { - return (kv.source == KeySource::UserDefined); + return (kv.source() == KeySource::UserDefined); }); } @@ -587,7 +585,7 @@ void Key::copyOriginalToUser() // Then copy all original & extension-defined keys as user-defined auto copy = m_adds; for (const auto& kv : copy) - m_adds.push_back(AppShortcut(KeySource::UserDefined, kv.shortcut)); + m_adds.push_back(AppShortcut(KeySource::UserDefined, kv)); m_shortcuts.reset(); } diff --git a/src/app/ui/key.h b/src/app/ui/key.h index aed0eb5d9..9f5ec834e 100644 --- a/src/app/ui/key.h +++ b/src/app/ui/key.h @@ -105,27 +105,19 @@ inline KeyAction operator&(KeyAction a, KeyAction b) // This is a ui::Shortcut wrapper (just one key shortcut) defined by // the app, an extension, or the user (KeySource). -struct AppShortcut { - KeySource source; - ui::Shortcut shortcut; - +class AppShortcut : public ui::Shortcut { +public: AppShortcut(const KeySource source, const ui::Shortcut& shortcut) - : source(source) - , shortcut(shortcut) + : Shortcut(shortcut) + , m_source(source) { } - bool isEmpty() const { return shortcut.isEmpty(); } - std::string toString() const { return shortcut.toString(); } - bool isPressed() const { return shortcut.isPressed(); } - bool isLooselyPressed() const { return shortcut.isLooselyPressed(); } + KeySource source() const { return m_source; } + const ui::Shortcut& shortcut() const { return *this; } - bool operator==(const AppShortcut& other) const { return shortcut.operator==(other.shortcut); } - bool operator!=(const AppShortcut& other) const { return shortcut.operator!=(other.shortcut); } - - ui::KeyModifiers modifiers() const { return shortcut.modifiers(); } - ui::KeyScancode scancode() const { return shortcut.scancode(); } - int unicodeChar() const { return shortcut.unicodeChar(); } + // bool operator==(const AppShortcut& other) const { return shortcut.operator==(other.shortcut); } + // bool operator!=(const AppShortcut& other) const { return shortcut.operator!=(other.shortcut); } // Returns true if this AppShortcut is better for the current // context, compared to another shortcut. @@ -133,6 +125,9 @@ struct AppShortcut { KeyContext thisShortcutContext, KeyContext otherShortcutContext, const AppShortcut& otherShortcut) const; + +private: + KeySource m_source; }; using AppShortcuts = ui::ShortcutsT; diff --git a/src/app/ui/keyboard_shortcuts.cpp b/src/app/ui/keyboard_shortcuts.cpp index de5e9194d..ee8fa689b 100644 --- a/src/app/ui/keyboard_shortcuts.cpp +++ b/src/app/ui/keyboard_shortcuts.cpp @@ -369,12 +369,12 @@ void KeyboardShortcuts::exportKeys(XMLElement* parent, KeyType type) continue; for (const auto& kv : key->delsKeys()) - if (kv.source == KeySource::UserDefined) - exportShortcut(parent, key.get(), kv.shortcut, true); + if (kv.source() == KeySource::UserDefined) + exportShortcut(parent, key.get(), kv, true); for (const auto& kv : key->addsKeys()) - if (kv.source == KeySource::UserDefined) - exportShortcut(parent, key.get(), kv.shortcut, false); + if (kv.source() == KeySource::UserDefined) + exportShortcut(parent, key.get(), kv, false); } } From c444b566e11eb8415cdeee92d37469d557455d8e Mon Sep 17 00:00:00 2001 From: David Capello Date: Mon, 15 Sep 2025 15:49:09 -0300 Subject: [PATCH 9/9] Fix painting UI theme colors as they are specified in sRGB color space Fixes a regression found after merging #5414: https://github.com/aseprite/aseprite/pull/5414#issuecomment-3286339563 We always expected a sRGB color in ui::Graphics API and we can specify a color in another color space using the ui::Paint version of its member functions. Several functions related to color spaces are now using a ui::Display to receive the specific display where we're going to paint, instead of using os::System::instance()->defaultWindow()->colorSpace(). --- src/app/color_spaces.cpp | 31 +++++++++++++++---------------- src/app/color_spaces.h | 16 ++++++++++------ src/app/modules/gfx.cpp | 15 +++++++++------ src/app/script/canvas_widget.cpp | 4 +++- src/app/thumbnails.cpp | 5 +++-- src/app/thumbnails.h | 4 +++- src/app/ui/color_selector.cpp | 16 ++++++++-------- src/app/ui/color_sliders.cpp | 10 ++++++---- src/app/ui/color_wheel.cpp | 13 +++++++++---- src/app/ui/palette_view.cpp | 2 +- src/app/ui/tabs.cpp | 6 ++++-- src/app/ui/timeline/timeline.cpp | 5 +++-- src/ui/display.h | 3 ++- src/ui/graphics.cpp | 18 +++++++++--------- src/ui/graphics.h | 6 ++++++ 15 files changed, 91 insertions(+), 63 deletions(-) diff --git a/src/app/color_spaces.cpp b/src/app/color_spaces.cpp index ed591d3ea..5818eac00 100644 --- a/src/app/color_spaces.cpp +++ b/src/app/color_spaces.cpp @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2018-2024 Igara Studio S.A. +// Copyright (C) 2018-2025 Igara Studio S.A. // // This program is distributed under the terms of // the End-User License Agreement for Aseprite. @@ -15,6 +15,7 @@ #include "app/ui/editor/editor.h" #include "os/system.h" #include "os/window.h" +#include "ui/display.h" namespace app { @@ -29,17 +30,15 @@ void initialize_color_spaces(Preferences& pref) pref.color.manage.AfterChange.connect([](bool manage) { g_manage = manage; }); } -os::ColorSpaceRef get_screen_color_space() +os::ColorSpaceRef get_current_color_space(ui::Display* display, Doc* doc) { - return os::System::instance()->defaultWindow()->colorSpace(); -} - -os::ColorSpaceRef get_current_color_space() -{ - if (auto* editor = Editor::activeEditor()) - return editor->document()->osColorSpace(); - else - return get_screen_color_space(); + if (!doc) { + if (auto* editor = Editor::activeEditor()) + doc = editor->document(); + } + if (doc) + return doc->osColorSpace(); + return display->colorSpace(); } gfx::ColorSpaceRef get_working_rgb_space_from_preferences() @@ -62,11 +61,11 @@ gfx::ColorSpaceRef get_working_rgb_space_from_preferences() ////////////////////////////////////////////////////////////////////// // Color conversion -ConvertCS::ConvertCS() +ConvertCS::ConvertCS(ui::Display* display, Doc* doc) { if (g_manage) { - auto srcCS = get_current_color_space(); - auto dstCS = get_screen_color_space(); + auto srcCS = get_current_color_space(display, doc); + auto dstCS = display->colorSpace(); if (srcCS && dstCS) m_conversion = os::System::instance()->convertBetweenColorSpace(srcCS, dstCS); } @@ -95,9 +94,9 @@ gfx::Color ConvertCS::operator()(const gfx::Color c) } } -ConvertCS convert_from_current_to_screen_color_space() +ConvertCS convert_from_current_to_display_color_space(ui::Display* display) { - return ConvertCS(); + return ConvertCS(display); } ConvertCS convert_from_custom_to_srgb(const os::ColorSpaceRef& from) diff --git a/src/app/color_spaces.h b/src/app/color_spaces.h index d0f994f21..567dbf1c5 100644 --- a/src/app/color_spaces.h +++ b/src/app/color_spaces.h @@ -1,5 +1,5 @@ // Aseprite -// Copyright (c) 2018-2020 Igara Studio S.A. +// Copyright (c) 2018-2025 Igara Studio S.A. // // This program is distributed under the terms of // the End-User License Agreement for Aseprite. @@ -16,21 +16,25 @@ namespace doc { class Sprite; } +namespace ui { +class Display; +} + namespace app { +class Doc; class Preferences; void initialize_color_spaces(Preferences& pref); -os::ColorSpaceRef get_screen_color_space(); - // Returns the color space of the current document. -os::ColorSpaceRef get_current_color_space(); +os::ColorSpaceRef get_current_color_space(ui::Display* display, Doc* doc = nullptr); gfx::ColorSpaceRef get_working_rgb_space_from_preferences(); class ConvertCS { public: - ConvertCS(); + ConvertCS() = delete; + ConvertCS(ui::Display* display, Doc* doc = nullptr); ConvertCS(const os::ColorSpaceRef& srcCS, const os::ColorSpaceRef& dstCS); ConvertCS(ConvertCS&&); ConvertCS& operator=(const ConvertCS&) = delete; @@ -40,7 +44,7 @@ private: os::Ref m_conversion; }; -ConvertCS convert_from_current_to_screen_color_space(); +ConvertCS convert_from_current_to_display_color_space(ui::Display* display); ConvertCS convert_from_custom_to_srgb(const os::ColorSpaceRef& from); } // namespace app diff --git a/src/app/modules/gfx.cpp b/src/app/modules/gfx.cpp index 095fc0793..251b11a71 100644 --- a/src/app/modules/gfx.cpp +++ b/src/app/modules/gfx.cpp @@ -30,6 +30,7 @@ #include "os/surface.h" #include "os/system.h" #include "ui/intern.h" +#include "ui/paint.h" #include "ui/system.h" #include "ui/theme.h" @@ -118,9 +119,6 @@ void draw_color(ui::Graphics* g, app::Color color = _color; const int alpha = color.getAlpha(); - // Color space conversion - auto convertColor = convert_from_current_to_screen_color_space(); - if (alpha < 255) { if (rc.w == rc.h) draw_checkered_grid(g, rc, gfx::Size(rc.w / 2, rc.h / 2)); @@ -133,11 +131,15 @@ void draw_color(ui::Graphics* g, color = app::Color::fromGray(color.getGray(), color.getAlpha()); } + // The color is in the current sprite color space. + ui::Paint paint; + paint.color(color_utils::color_for_ui(color), get_current_color_space(g->display()).get()); + if (color.getType() == app::Color::IndexType) { int index = color.getIndex(); if (index >= 0 && index < get_current_palette()->size()) { - g->fillRect(convertColor(color_utils::color_for_ui(color)), rc); + g->drawRect(rc, paint); } else { g->fillRect(gfx::rgba(0, 0, 0), rc); @@ -147,7 +149,7 @@ void draw_color(ui::Graphics* g, } } else { - g->fillRect(convertColor(color_utils::color_for_ui(color)), rc); + g->drawRect(rc, paint); } } } @@ -219,7 +221,8 @@ void draw_tile(ui::Graphics* g, const Rect& rc, const Site& site, doc::tile_t ti int w = tileImage->width(); int h = tileImage->height(); - os::SurfaceRef surface = os::System::instance()->makeRgbaSurface(w, h, get_current_color_space()); + os::SurfaceRef surface = + os::System::instance()->makeRgbaSurface(w, h, get_current_color_space(g->display())); convert_image_to_surface(tileImage.get(), get_current_palette(), surface.get(), 0, 0, 0, 0, w, h); ui::Paint paint; diff --git a/src/app/script/canvas_widget.cpp b/src/app/script/canvas_widget.cpp index 228865995..b5f5c12b4 100644 --- a/src/app/script/canvas_widget.cpp +++ b/src/app/script/canvas_widget.cpp @@ -190,7 +190,9 @@ void Canvas::onResize(ui::ResizeEvent& ev) } if (!m_surface || m_surface->width() != w || m_surface->height() != h) { - m_surface = system->makeSurface(w, h, get_current_color_space()); + ui::Display* display = this->display(); + os::ColorSpaceRef cs = (display ? display->colorSpace() : nullptr); + m_surface = system->makeSurface(w, h, cs); callPaint(); } } diff --git a/src/app/thumbnails.cpp b/src/app/thumbnails.cpp index 6765e1acd..5668ee353 100644 --- a/src/app/thumbnails.cpp +++ b/src/app/thumbnails.cpp @@ -22,7 +22,8 @@ namespace app { namespace thumb { -os::SurfaceRef get_cel_thumbnail(const doc::Cel* cel, +os::SurfaceRef get_cel_thumbnail(ui::Display* display, + const doc::Cel* cel, const bool scaleUpToFit, const gfx::Size& fitInSize) { @@ -57,7 +58,7 @@ os::SurfaceRef get_cel_thumbnail(const doc::Cel* cel, if (os::SurfaceRef thumbnail = os::System::instance()->makeRgbaSurface( thumbnailImage->width(), thumbnailImage->height(), - get_current_color_space())) { + get_current_color_space(display))) { convert_image_to_surface(thumbnailImage.get(), palette, thumbnail.get(), diff --git a/src/app/thumbnails.h b/src/app/thumbnails.h index 2218655e6..8ab84cb21 100644 --- a/src/app/thumbnails.h +++ b/src/app/thumbnails.h @@ -11,6 +11,7 @@ #include "gfx/size.h" #include "os/surface.h" +#include "ui/display.h" namespace doc { class Cel; @@ -22,7 +23,8 @@ class Surface; namespace app { namespace thumb { -os::SurfaceRef get_cel_thumbnail(const doc::Cel* cel, +os::SurfaceRef get_cel_thumbnail(ui::Display* display, + const doc::Cel* cel, const bool scaleUpToFit, const gfx::Size& fitInSize); diff --git a/src/app/ui/color_selector.cpp b/src/app/ui/color_selector.cpp index 6fa6002af..5bd91ac76 100644 --- a/src/app/ui/color_selector.cpp +++ b/src/app/ui/color_selector.cpp @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2018-2024 Igara Studio S.A. +// Copyright (C) 2018-2025 Igara Studio S.A. // Copyright (C) 2016-2018 David Capello // // This program is distributed under the terms of @@ -107,11 +107,11 @@ public: } } - os::Surface* getCanvas(int w, int h, gfx::Color bgColor) + os::Surface* getCanvas(Display* display, int w, int h, gfx::Color bgColor) { assert_ui_thread(); - auto activeCS = get_current_color_space(); + auto activeCS = get_current_color_space(display); if (!m_canvas || m_canvas->width() != w || m_canvas->height() != h || m_canvas->colorSpace() != activeCS) { @@ -432,9 +432,9 @@ void ColorSelector::onPaint(ui::PaintEvent& ev) SkCanvas* canvas; bool isSRGB; // TODO compare both color spaces - if ((!get_current_color_space() || get_current_color_space()->isSRGB()) && - (!g->getInternalSurface()->colorSpace() || - g->getInternalSurface()->colorSpace()->isSRGB())) { + auto displayCs = get_current_color_space(display()); + auto gCs = g->getInternalSurface()->colorSpace(); + if ((!displayCs || displayCs->isSRGB()) && (!gCs || gCs->isSRGB())) { // We can render directly in the ui::Graphics surface canvas = &static_cast(g->getInternalSurface())->canvas(); isSRGB = true; @@ -442,7 +442,7 @@ void ColorSelector::onPaint(ui::PaintEvent& ev) else { // We'll paint in the ColorSelector::Painter canvas, and so we // can convert color spaces. - painterSurface = painter.getCanvas(rc.w, rc.h, theme->colors.workspace()); + painterSurface = painter.getCanvas(display(), rc.w, rc.h, theme->colors.workspace()); canvas = &static_cast(painterSurface)->canvas(); isSRGB = false; } @@ -497,7 +497,7 @@ void ColorSelector::onPaint(ui::PaintEvent& ev) else #endif // SK_ENABLE_SKSL { - painterSurface = painter.getCanvas(rc.w, rc.h, theme->colors.workspace()); + painterSurface = painter.getCanvas(display(), rc.w, rc.h, theme->colors.workspace()); } if (painterSurface) diff --git a/src/app/ui/color_sliders.cpp b/src/app/ui/color_sliders.cpp index 5af153f9f..c1477072b 100644 --- a/src/app/ui/color_sliders.cpp +++ b/src/app/ui/color_sliders.cpp @@ -58,9 +58,7 @@ public: return; } - // Color space conversion - auto convertColor = convert_from_current_to_screen_color_space(); - + Paint paint; gfx::Color color = gfx::ColorNone; int w = std::max(rc.w - 1, 1); @@ -110,7 +108,11 @@ public: color = color_utils::color_for_ui(app::Color::fromGray(255 * x / w)); break; } - g->drawVLine(convertColor(color), rc.x + x, rc.y, rc.h); + + // Color space conversion + paint.color(color, get_current_color_space(slider->display()).get()); + + g->drawVLine(rc.x + x, rc.y, rc.h, paint); } } diff --git a/src/app/ui/color_wheel.cpp b/src/app/ui/color_wheel.cpp index bef37b325..73b89f854 100644 --- a/src/app/ui/color_wheel.cpp +++ b/src/app/ui/color_wheel.cpp @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2020-2022 Igara Studio S.A. +// Copyright (C) 2020-2025 Igara Studio S.A. // Copyright (C) 2001-2018 David Capello // // This program is distributed under the terms of @@ -11,6 +11,7 @@ #include "app/ui/color_wheel.h" +#include "app/color_spaces.h" #include "app/color_utils.h" #include "app/i18n/strings.h" #include "app/pref/preferences.h" @@ -358,6 +359,9 @@ void ColorWheel::onPaintMainArea(ui::Graphics* g, const gfx::Rect& rc) int n = getHarmonies(); int boxsize = std::min(rc.w / 10, rc.h / 10); + ui::Paint paint; + auto cs = get_current_color_space(g->display()); + for (int i = 0; i < n; ++i) { app::Color color = getColorInHarmony(i); double angle = color.getHsvHue() - 30.0; @@ -374,9 +378,10 @@ void ColorWheel::onPaintMainArea(ui::Graphics* g, const gfx::Rect& rc) paintColorIndicator(g, pos, color.getHsvValue() < 0.5); - g->fillRect( - gfx::rgba(color.getRed(), color.getGreen(), color.getBlue(), 255), - gfx::Rect(rc.x + rc.w - (n - i) * boxsize, rc.y + rc.h - boxsize, boxsize, boxsize)); + paint.color(gfx::rgba(color.getRed(), color.getGreen(), color.getBlue(), 255), cs.get()); + g->drawRect( + gfx::Rect(rc.x + rc.w - (n - i) * boxsize, rc.y + rc.h - boxsize, boxsize, boxsize), + paint); } } } diff --git a/src/app/ui/palette_view.cpp b/src/app/ui/palette_view.cpp index 2dfcaa176..7f402fb2b 100644 --- a/src/app/ui/palette_view.cpp +++ b/src/app/ui/palette_view.cpp @@ -309,7 +309,7 @@ public: int w = tileImage->width(); int h = tileImage->height(); os::SurfaceRef surface = - os::System::instance()->makeRgbaSurface(w, h, get_current_color_space()); + os::System::instance()->makeRgbaSurface(w, h, get_current_color_space(g->display())); convert_image_to_surface(tileImage.get(), get_current_palette(), surface.get(), diff --git a/src/app/ui/tabs.cpp b/src/app/ui/tabs.cpp index e5ac6534e..176020422 100644 --- a/src/app/ui/tabs.cpp +++ b/src/app/ui/tabs.cpp @@ -970,8 +970,10 @@ void Tabs::createFloatingUILayer(Tab* tab) ASSERT(!m_floatingUILayer); ui::Display* display = this->display(); - os::SurfaceRef surface = - os::System::instance()->makeRgbaSurface(tab->width, m_tabsHeight, get_current_color_space()); + os::SurfaceRef surface = os::System::instance()->makeRgbaSurface( + tab->width, + m_tabsHeight, + get_current_color_space(display)); // Fill the surface with pink color { diff --git a/src/app/ui/timeline/timeline.cpp b/src/app/ui/timeline/timeline.cpp index 23dcaef11..c2a53aae5 100644 --- a/src/app/ui/timeline/timeline.cpp +++ b/src/app/ui/timeline/timeline.cpp @@ -2526,7 +2526,7 @@ void Timeline::drawCel(ui::Graphics* g, if (!thumb_bounds.isEmpty()) { if (os::SurfaceRef surface = - thumb::get_cel_thumbnail(cel, m_scaleUpToFit, thumb_bounds.size())) { + thumb::get_cel_thumbnail(g->display(), cel, m_scaleUpToFit, thumb_bounds.size())) { const int t = std::clamp(thumb_bounds.w / 8, 4, 16); draw_checkered_grid(g, thumb_bounds, gfx::Size(t, t), docPref()); @@ -2618,7 +2618,8 @@ void Timeline::drawCelOverlay(ui::Graphics* g) return; gfx::Rect rc = m_sprite->bounds().fitIn(gfx::Rect(m_thumbnailsOverlayBounds).shrink(1)); - if (os::SurfaceRef surface = thumb::get_cel_thumbnail(cel, m_scaleUpToFit, rc.size())) { + if (os::SurfaceRef surface = + thumb::get_cel_thumbnail(g->display(), cel, m_scaleUpToFit, rc.size())) { draw_checkered_grid(g, rc, gfx::Size(8, 8) * ui::guiscale(), docPref()); g->drawRgbaSurface(surface.get(), diff --git a/src/ui/display.h b/src/ui/display.h index 8fc9ba516..39f275a90 100644 --- a/src/ui/display.h +++ b/src/ui/display.h @@ -1,5 +1,5 @@ // Aseprite UI Library -// Copyright (C) 2019-2024 Igara Studio S.A. +// Copyright (C) 2019-2025 Igara Studio S.A. // // This file is released under the terms of the MIT license. // Read LICENSE.txt for more information. @@ -31,6 +31,7 @@ public: Display* parentDisplay() { return m_parentDisplay; } os::Window* nativeWindow() const { return m_nativeWindow.get(); } os::SurfaceRef nativeSurface() const; + os::ColorSpaceRef colorSpace() const { return m_nativeWindow->colorSpace(); } UILayers layers() { return m_layers; } UILayerRef backLayer() { return m_layers.front(); } diff --git a/src/ui/graphics.cpp b/src/ui/graphics.cpp index 6ca437947..cc52eaaf8 100644 --- a/src/ui/graphics.cpp +++ b/src/ui/graphics.cpp @@ -155,7 +155,7 @@ void Graphics::drawHLine(gfx::Color color, int x, int y, int w) os::SurfaceLock lock(m_surface.get()); os::Paint paint; - paint.color(color, colorSpace()); + paint.color(color); m_surface->drawRect(gfx::Rect(m_dx + x, m_dy + y, w, 1), paint); } @@ -173,7 +173,7 @@ void Graphics::drawVLine(gfx::Color color, int x, int y, int h) os::SurfaceLock lock(m_surface.get()); os::Paint paint; - paint.color(color, colorSpace()); + paint.color(color); m_surface->drawRect(gfx::Rect(m_dx + x, m_dy + y, 1, h), paint); } @@ -185,7 +185,7 @@ void Graphics::drawLine(gfx::Color color, const gfx::Point& _a, const gfx::Point os::SurfaceLock lock(m_surface.get()); os::Paint paint; - paint.color(color, colorSpace()); + paint.color(color); m_surface->drawLine(a, b, paint); } @@ -242,7 +242,7 @@ void Graphics::drawRect(gfx::Color color, const gfx::Rect& rcOrig) os::SurfaceLock lock(m_surface.get()); os::Paint paint; - paint.color(color, colorSpace()); + paint.color(color); paint.style(os::Paint::Stroke); m_surface->drawRect(rc, paint); } @@ -255,7 +255,7 @@ void Graphics::fillRect(gfx::Color color, const gfx::Rect& rcOrig) os::SurfaceLock lock(m_surface.get()); os::Paint paint; - paint.color(color, colorSpace()); + paint.color(color); paint.style(os::Paint::Fill); m_surface->drawRect(rc, paint); } @@ -444,11 +444,11 @@ void Graphics::drawUIText(const std::string& str, Paint paint; if (gfx::geta(bg) > 0) { // Paint background - paint.color(bg, colorSpace()); + paint.color(bg); paint.style(os::Paint::Fill); drawRect(gfx::RectF(textBlob->bounds()).offset(pt), paint); } - paint.color(fg, colorSpace()); + paint.color(fg); drawTextBlob(textBlob, gfx::PointF(pt), paint); @@ -603,10 +603,10 @@ gfx::Size Graphics::doUIStringAlgorithm(const std::string& str, Paint paint; paint.style(os::Paint::Fill); if (!gfx::is_transparent(bg)) { - paint.color(bg, colorSpace()); + paint.color(bg); drawRect(gfx::RectF(xout, pt.y, rc.w, lineSize.h), paint); } - paint.color(fg, colorSpace()); + paint.color(fg); float baselineDelta = -metrics.ascent - lineBlob->baseline(); drawTextBlob(lineBlob, gfx::PointF(xout, pt.y + baselineDelta), paint); diff --git a/src/ui/graphics.h b/src/ui/graphics.h index c7c5be53d..420d5413d 100644 --- a/src/ui/graphics.h +++ b/src/ui/graphics.h @@ -39,6 +39,11 @@ namespace ui { class Display; // Class to render a widget in the screen. +// +// The gfx::Color parameter is a color in the sRGB color space +// (e.g. used to paint theme elements on widgets). If you want to +// paint a color from other color space, use the Paint version of each +// function. class Graphics { public: Graphics(Display* display, const os::SurfaceRef& surface, int dx, int dy); @@ -47,6 +52,7 @@ public: int width() const; int height() const; + Display* display() const { return m_display; } os::Surface* getInternalSurface() { return m_surface.get(); } os::ColorSpace* colorSpace() { return m_surface->colorSpace().get(); }