From ba81d36666c2e6bfb2970e75fd29cbe2ded1f570 Mon Sep 17 00:00:00 2001 From: Daniel Brooks Date: Tue, 23 Apr 2024 08:44:56 -0700 Subject: [PATCH 1/5] factor out color conversion to ImVec4 --- src/cata_imgui.cpp | 44 ++++++++++++++++++++++++++------------------ src/cata_imgui.h | 3 +++ src/color.cpp | 8 ++++++-- src/color.h | 4 ++++ 4 files changed, 39 insertions(+), 20 deletions(-) diff --git a/src/cata_imgui.cpp b/src/cata_imgui.cpp index e4ed52b3de74b..d5c7c940aee26 100644 --- a/src/cata_imgui.cpp +++ b/src/cata_imgui.cpp @@ -36,6 +36,22 @@ struct pairs { std::array::COLOR_NAMES_COUNT> rgbPalette; std::array colorpairs; //storage for pair'ed colored +ImVec4 cataimgui::imvec4_from_color( nc_color &color ) +{ + int pair_id = color.get_index(); + pairs &pair = colorpairs[pair_id]; + + int palette_index = pair.FG != 0 ? pair.FG : pair.BG; + if( color.is_bold() ) { + palette_index += color_loader::COLOR_NAMES_COUNT / 2; + } + RGBTuple &rgbCol = rgbPalette[palette_index]; + return { static_cast( rgbCol.Red / 255. ), + static_cast( rgbCol.Green / 255. ), + static_cast( rgbCol.Blue / 255. ), + static_cast( 255. ) }; +} + std::vector> imtui_events; static int GetFallbackStrWidth( const char *s_begin, const char *s_end, @@ -189,6 +205,15 @@ RGBTuple color_loader::from_rgb( const int r, const int g, const int b #include #include +ImVec4 cataimgui::imvec4_from_color( nc_color &color ) +{ + SDL_Color c = curses_color_to_SDL( color ); + return { static_cast( c.r / 255. ), + static_cast( c.g / 255. ), + static_cast( c.b / 255. ), + static_cast( c.a / 255. ) }; +} + cataimgui::client::client( const SDL_Renderer_Ptr &sdl_renderer, const SDL_Window_Ptr &sdl_window, const GeometryRenderer_Ptr &sdl_geometry ) : sdl_renderer( sdl_renderer ), @@ -417,24 +442,7 @@ void cataimgui::window::draw_colored_text( std::string const &text, nc_color &co if( i++ != 0 ) { ImGui::SameLine( 0, 0 ); } -#if !(defined(TILES) || defined(WIN32)) - int pair_id = color.get_index(); - pairs &pair = colorpairs[pair_id]; - - int palette_index = pair.FG != 0 ? pair.FG : pair.BG; - if( color.is_bold() ) { - palette_index += color_loader::COLOR_NAMES_COUNT / 2; - } - RGBTuple &rgbCol = rgbPalette[palette_index]; - ImGui::TextColored( { static_cast( rgbCol.Red / 255. ), static_cast( rgbCol.Green / 255. ), - static_cast( rgbCol.Blue / 255. ), static_cast( 255. ) }, - "%s", seg.c_str() ); -#else - SDL_Color c = curses_color_to_SDL( color ); - ImGui::TextColored( { static_cast( c.r / 255. ), static_cast( c.g / 255. ), - static_cast( c.b / 255. ), static_cast( c.a / 255. ) }, - "%s", seg.c_str() ); -#endif + ImGui::TextColored( color, "%s", seg.c_str() ); GImGui->LastItemData.ID = itemId; if( is_focused && !*is_focused ) { *is_focused = ImGui::IsItemFocused(); diff --git a/src/cata_imgui.h b/src/cata_imgui.h index b9c9b8cb21f23..551eb9d4f856a 100644 --- a/src/cata_imgui.h +++ b/src/cata_imgui.h @@ -15,6 +15,7 @@ struct input_event; struct point; struct ImVec2; +struct ImVec4; class Font; class input_context; @@ -78,6 +79,8 @@ class client void point_to_imvec2( point *src, ImVec2 *dest ); void imvec2_to_point( ImVec2 *src, point *dest ); +ImVec4 imvec4_from_color( nc_color &color ); + class window { std::unique_ptr p_impl; diff --git a/src/color.cpp b/src/color.cpp index eb325c8d031f7..81ef15d426c75 100644 --- a/src/color.cpp +++ b/src/color.cpp @@ -16,6 +16,7 @@ #include "filesystem.h" #include "flexbuffer_json-inl.h" #include "flexbuffer_json.h" +#include "imgui/imgui.h" #include "input_context.h" #include "json.h" #include "output.h" @@ -26,9 +27,12 @@ #include "translations.h" #include "ui.h" #include "ui_manager.h" -#if !(defined(TILES) || defined(WIN32)) #include "cata_imgui.h" -#endif + +nc_color::operator ImVec4() +{ + return cataimgui::imvec4_from_color( *this ); +} void nc_color::serialize( JsonOut &jsout ) const { diff --git a/src/color.h b/src/color.h index bddf649a54040..88290bd3b3ba6 100644 --- a/src/color.h +++ b/src/color.h @@ -344,6 +344,8 @@ enum hl_enum { NUM_HL }; +struct ImVec4; + class nc_color { private: @@ -357,6 +359,8 @@ class nc_color public: nc_color() : attribute_value( 0 ), index( 0 ) { } + operator ImVec4(); + // Most of the functions here are implemented in ncurses_def.cpp // (for ncurses builds) *and* in cursesport.cpp (for other builds). From 1e9c173fdc85f331962044ee8bc645b22cbf6da2 Mon Sep 17 00:00:00 2001 From: Daniel Brooks Date: Sun, 5 May 2024 00:35:36 -0700 Subject: [PATCH 2/5] make the color argument to draw_colored_text optional If you don't specify a color, it will use the current Text theme color as the base. If you do specify it, it will push the color you specify first and pop it after. Also, remove the internal color stack, since ImGui already has a stack of colors. --- src/cata_imgui.cpp | 41 ++++++++++++++++++++++++++++++----------- src/cata_imgui.h | 3 +++ src/popup.cpp | 3 +-- 3 files changed, 34 insertions(+), 13 deletions(-) diff --git a/src/cata_imgui.cpp b/src/cata_imgui.cpp index d5c7c940aee26..26b2750e78c83 100644 --- a/src/cata_imgui.cpp +++ b/src/cata_imgui.cpp @@ -393,20 +393,44 @@ void cataimgui::imvec2_to_point( ImVec2 *src, point *dest ) } } +static void PushOrPopColor( const std::string_view seg ) +{ + color_tag_parse_result tag = get_color_from_tag( seg, report_color_error::yes ); + switch( tag.type ) { + case color_tag_parse_result::open_color_tag: + ImGui::PushStyleColor( ImGuiCol_Text, tag.color ); + break; + case color_tag_parse_result::close_color_tag: + ImGui::PopStyleColor(); + break; + case color_tag_parse_result::non_color_tag: + // Do nothing + break; + } +} + void cataimgui::window::draw_colored_text( std::string const &text, const nc_color &color, float wrap_width, bool *is_selected, bool *is_focused, bool *is_hovered ) { nc_color color_cpy = color; - draw_colored_text( text, color_cpy, wrap_width, is_selected, is_focused, is_hovered ); + ImGui::PushStyleColor( ImGuiCol_Text, color_cpy ); + draw_colored_text( text, wrap_width, is_selected, is_focused, is_hovered ); + ImGui::PopStyleColor(); } void cataimgui::window::draw_colored_text( std::string const &text, nc_color &color, float wrap_width, bool *is_selected, bool *is_focused, bool *is_hovered ) +{ + ImGui::PushStyleColor( ImGuiCol_Text, color ); + draw_colored_text( text, wrap_width, is_selected, is_focused, is_hovered ); + ImGui::PopStyleColor(); +} + +void cataimgui::window::draw_colored_text( std::string const &text, + float wrap_width, bool *is_selected, bool *is_focused, bool *is_hovered ) { ImGui::PushID( text.c_str() ); ImGuiID itemId = GImGui->CurrentWindow->IDStack.back(); - std::stack color_stack; - color_stack.push( color ); size_t chars_per_line = size_t( wrap_width ); if( chars_per_line == 0 ) { chars_per_line = SIZE_MAX; @@ -418,7 +442,6 @@ void cataimgui::window::draw_colored_text( std::string const &text, nc_color &co std::vector folded_msg = foldstring( text, chars_per_line ); for( const std::string &line : folded_msg ) { - const auto color_segments = split_by_color( line ); if( is_selected != nullptr ) { ImGui::Selectable( "", is_selected ); @@ -431,18 +454,14 @@ void cataimgui::window::draw_colored_text( std::string const &text, nc_color &co } if( seg[0] == '<' ) { - const color_tag_parse_result::tag_type type = - update_color_stack( color_stack, seg, report_color_error::yes ); - if( type != color_tag_parse_result::non_color_tag ) { - seg = rm_prefix( seg ); - } + PushOrPopColor( seg ); + seg = rm_prefix( seg ); } - color = color_stack.empty() ? color : color_stack.top(); if( i++ != 0 ) { ImGui::SameLine( 0, 0 ); } - ImGui::TextColored( color, "%s", seg.c_str() ); + ImGui::TextUnformatted( seg.c_str() ); GImGui->LastItemData.ID = itemId; if( is_focused && !*is_focused ) { *is_focused = ImGui::IsItemFocused(); diff --git a/src/cata_imgui.h b/src/cata_imgui.h index 551eb9d4f856a..ca4e3ac35ca9f 100644 --- a/src/cata_imgui.h +++ b/src/cata_imgui.h @@ -97,6 +97,9 @@ class window void draw_colored_text( std::string const &text, nc_color &color, float wrap_width = 0.0F, bool *is_selected = nullptr, bool *is_focused = nullptr, bool *is_hovered = nullptr ); + void draw_colored_text( std::string const &text, + float wrap_width = 0.0F, bool *is_selected = nullptr, + bool *is_focused = nullptr, bool *is_hovered = nullptr ); bool action_button( const std::string &action, const std::string &text ); bool has_button_action(); std::string get_button_action(); diff --git a/src/popup.cpp b/src/popup.cpp index 151fb7326d526..818ec2c6d8bc3 100644 --- a/src/popup.cpp +++ b/src/popup.cpp @@ -58,8 +58,7 @@ void query_popup_impl::draw_controls() mouse_selected_option = -1; for( const std::string &line : parent->folded_msg ) { - nc_color col = parent->default_text_color; - draw_colored_text( line, col ); + draw_colored_text( line, parent->default_text_color ); } if( !parent->buttons.empty() ) { From 4fbc960c94e96d9acdf77eb0132c4fe6bfef5153 Mon Sep 17 00:00:00 2001 From: Daniel Brooks Date: Sun, 5 May 2024 22:44:31 -0700 Subject: [PATCH 3/5] suppress lint warning about implict conversions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The conversion is not expensive enough to worry about, and we don't have any functions that can take both an nc_color and an ImVec4 so it isn’t ambiguous either. --- src/color.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/color.h b/src/color.h index 88290bd3b3ba6..ae7194d800c84 100644 --- a/src/color.h +++ b/src/color.h @@ -359,7 +359,7 @@ class nc_color public: nc_color() : attribute_value( 0 ), index( 0 ) { } - operator ImVec4(); + operator ImVec4(); // NOLINT(google-explicit-constructor): the conversion is not expensive // Most of the functions here are implemented in ncurses_def.cpp // (for ncurses builds) *and* in cursesport.cpp (for other builds). From e902206220c6a2ba10819222dc42589786e5cb4f Mon Sep 17 00:00:00 2001 From: Katie McKean Date: Fri, 17 May 2024 13:49:00 -0400 Subject: [PATCH 4/5] Added code to guarantee that any colors pushed to the ImGui color style stack are popped back off --- src/cata_imgui.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/cata_imgui.cpp b/src/cata_imgui.cpp index 26b2750e78c83..a3c729bd2d7db 100644 --- a/src/cata_imgui.cpp +++ b/src/cata_imgui.cpp @@ -430,6 +430,7 @@ void cataimgui::window::draw_colored_text( std::string const &text, float wrap_width, bool *is_selected, bool *is_focused, bool *is_hovered ) { ImGui::PushID( text.c_str() ); + int startColorStackCount = GImGui->ColorStack.Size; ImGuiID itemId = GImGui->CurrentWindow->IDStack.back(); size_t chars_per_line = size_t( wrap_width ); if( chars_per_line == 0 ) { @@ -472,7 +473,10 @@ void cataimgui::window::draw_colored_text( std::string const &text, } } - + for( int curColorStackCount = GImGui->ColorStack.Size; curColorStackCount > startColorStackCount; + curColorStackCount-- ) { + ImGui::PopStyleColor(); + } ImGui::PopID(); } From dec5cf3e5c5279d6d4d216632fdb7ea155add458 Mon Sep 17 00:00:00 2001 From: Daniel Brooks Date: Sun, 19 May 2024 08:46:40 -0700 Subject: [PATCH 5/5] ensure that unbalanced tags do not pop too many colors off the stack --- src/cata_imgui.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/cata_imgui.cpp b/src/cata_imgui.cpp index a3c729bd2d7db..c76cd74b0d14d 100644 --- a/src/cata_imgui.cpp +++ b/src/cata_imgui.cpp @@ -393,7 +393,7 @@ void cataimgui::imvec2_to_point( ImVec2 *src, point *dest ) } } -static void PushOrPopColor( const std::string_view seg ) +static void PushOrPopColor( const std::string_view seg, int minimumColorStackSize ) { color_tag_parse_result tag = get_color_from_tag( seg, report_color_error::yes ); switch( tag.type ) { @@ -401,7 +401,9 @@ static void PushOrPopColor( const std::string_view seg ) ImGui::PushStyleColor( ImGuiCol_Text, tag.color ); break; case color_tag_parse_result::close_color_tag: - ImGui::PopStyleColor(); + if( GImGui->ColorStack.Size > minimumColorStackSize ) { + ImGui::PopStyleColor(); + } break; case color_tag_parse_result::non_color_tag: // Do nothing @@ -455,7 +457,7 @@ void cataimgui::window::draw_colored_text( std::string const &text, } if( seg[0] == '<' ) { - PushOrPopColor( seg ); + PushOrPopColor( seg, startColorStackCount ); seg = rm_prefix( seg ); }