Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Text customization to support disable, underline, strikethrough, hili… #5288

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ForrestFeng
Copy link

Change color within text / colored text

Enhanced the ImGui::TextUnformated() method to make it support arbitrary text color, underline, strikethrough, highlight, mask. This makes the imgui text more expressive.

Please check the demo/result

@ocornut
Copy link
Owner

ocornut commented May 5, 2022

Hello!
Thanks for the PR. Could you post the demo/result and images here too simply?

@ForrestFeng
Copy link
Author

ForrestFeng commented May 5, 2022

Change color within text / colored text

I have enhanced the ImGui::TextUnformated() method to make it support arbitrary text color, underline, strikethrough, highlight, mask

I aslo add one demo for this feature.

image

The code for the demo is shown below.

The new TextUnformatted signature was changed to support this feature. The wrapped, disabled, and customization are newly added args with default values. So it should be compatible with existing code.

IMGUI_API void TextUnformatted(const char* text, const char* text_end = NULL, bool wrapped = false, bool disabled = false, const ImTextCustomization *customization = NULL);

       IMGUI_DEMO_MARKER("Widgets/Text/Text Customization");
        if (ImGui::TreeNode("Text Customization Simple"))
        {
            ImColor red(255, 0, 0, 255);
            ImColor green(0, 255, 0, 255);
            ImColor blue(0, 0, 255, 255);
            ImColor yellow(255, 255, 0, 255); 
            ImColor brown(187, 126, 0, 255);
            ImColor cyan(0, 255, 255, 255);
            ImColor magenta(255, 0, 255, 125);

            const char* text = "The quick red fox jumps over the green box.";
            ImTextCustomization tc;

            ImGui::NewLine();
            ImGui::Text("Color the whole text");            
            ImGui::TextUnformatted(text, NULL, true, false, &tc.Range(text).TextColor(green));

            ImGui::NewLine();
            ImGui::Text("Color the substring of the text");
            ImGui::TextUnformatted(text, NULL, true, false, &tc.Clear().Range(text + 14, text + 17).TextColor(red).Range(text + 39, text + 42).TextColor(green));

            ImGui::NewLine();
            ImGui::Text("Underline");
            ImGui::TextUnformatted(text, NULL, true, false, &tc.Clear().Range(text).Unerline());

            ImGui::NewLine();
            ImGui::Text("Underline with color");
            ImGui::TextUnformatted(text, NULL, true, false, &tc.Clear().Range(text).Unerline(blue));

            ImGui::NewLine();
            ImGui::Text("Strikethrough");
            ImGui::TextUnformatted(text, NULL, true, false, &tc.Clear().Range(text).Strkethrough());

            ImGui::NewLine();
            ImGui::Text("Strikethrough with color");
            ImGui::TextUnformatted(text, NULL, true, false, &tc.Clear().Range(text).Strkethrough(red));

            ImGui::NewLine();
            ImGui::Text("Hilight the text with brown");
            ImGui::TextUnformatted(text, NULL, true, false, &tc.Clear().Range(text).Highlight(brown));

            ImGui::NewLine();
            ImGui::Text("Mask the text so it is not readable");
            ImGui::TextUnformatted(text, NULL, true, false, &tc.Clear().Range(text+10, text+17).Mask());

            ImGui::NewLine();
            ImGui::Text("Mask the text with color");
            ImGui::TextUnformatted(text, NULL, true, false, &tc.Clear().Range(text+10, text+17).Mask(cyan));

            ImGui::NewLine();
            ImGui::Text("Mask the text with semitransparent color");
            ImGui::TextUnformatted(text, NULL, true, false, &tc.Clear().Range(text+10, text+28).Mask(magenta));
            ImGui::NewLine();

            ImGui::TreePop();
        }

Those text styles are within a text and can be combined as needed.

Here is another demo. It allows the user to adjust the ranges to apply a style and see the result.

image

The code is on Text Customization Branch

Any comments please let me know.

If it is possible I will create a pull request when ready. Thanks.

@ForrestFeng
Copy link
Author

Add one more video for this feature.

ImGui.Text.Customization.Video_2022-05-04_132237.mp4

@ForrestFeng
Copy link
Author

ForrestFeng commented May 5, 2022

Hello! Thanks for the PR. Could you post the demo/result and images here too simply?

The demo/result and images are copied here. Hope it be useful for others who need this feature.

I may mistakenly close this PR. I do not know how to open it again.

@ForrestFeng ForrestFeng closed this May 5, 2022
@ocornut ocornut reopened this May 5, 2022
@ForrestFeng
Copy link
Author

Seems there are some warnings are treated as errors. Should I fix them and update the PR again?

imgui.h Outdated
};

// Find the style for given position
Style GetStyleByPositin(const char* char_pos) const
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a typo in "Positin"?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

imgui_draw.cpp Outdated
@@ -3535,11 +3535,12 @@ void ImFont::RenderChar(ImDrawList* draw_list, float size, const ImVec2& pos, Im
float x = IM_FLOOR(pos.x);
float y = IM_FLOOR(pos.y);
draw_list->PrimReserve(6, 4);
draw_list->PrimRectUV(ImVec2(x + glyph->X0 * scale, y + glyph->Y0 * scale), ImVec2(x + glyph->X1 * scale, y + glyph->Y1 * scale), ImVec2(glyph->U0, glyph->V0), ImVec2(glyph->U1, glyph->V1), col);
draw_list->PrimRectUV(ImVec2(x + glyph->X0 * scale, y + glyph->Y0 * scale), ImVec2(x + glyph->X1 * scale, y + glyph->Y1 * scale),
ImVec2(glyph->U0, glyph->V0), ImVec2(glyph->U1, glyph->V1), col);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary reformat?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

imgui_draw.cpp Outdated
ImDrawList _draw_list_text(ImGui::GetDrawListSharedData());

// Create a dedicated draw list for text. The reson is that the highlight texts must draw before the text.
// The highlight texts is genrated during the glyph drawing process.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo "genrated"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

imgui_draw.cpp Outdated
ImDrawList* draw_list_text = nullptr;


ImDrawList _draw_list_text(ImGui::GetDrawListSharedData());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

result of ImGui::GetDrawListSharedData() could be put into the if (customization) scope

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ImGui::GetDrawListSharedData() is moved to if (customization) scope. The cost is to new and delete a ImDrawList when drawing the text with customization.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That mentioned cost + the fact it means draw list buffers will realloc+copy every frame and not be amortized is very largely in the range of unacceptably slow in Dear ImGui world. It makes me worry about the veracity of your other statement about performances.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the alloc/free is expensive we can revert to the old logic. The old logic creates a draw_list_text on the stack this may save some time for us.

For the copy logic adding text draw list to the main draw_lists is shown as below. The text draw list has to be created anyway, so the creation cost is the same. The additional cost is the copy logic.

  1. The VtxBuffer is copied with memcpy should be fast.
  2. The IdxBuffer is copied with a for loop, may cost a few CPU clocks, but it cannot be avoided. If you have good suggestion, please advise.
    if (customization)
    {    
       ...
       // Append draw_list_text to the main draw_lists
        {
            unsigned int old_inx_buffer_sz = draw_list->IdxBuffer.Size;
            unsigned int old_vtx_buffer_sz = draw_list->VtxBuffer.Size;
            draw_list->PrimReserve(draw_list_text->IdxBuffer.Size, draw_list_text->VtxBuffer.Size);
            // draw_list_text->VtxBuffer buffer can be memcopied, it contains no relative data
            memcpy(draw_list->_VtxWritePtr, draw_list_text->VtxBuffer.Data, draw_list_text->VtxBuffer.Size * sizeof(ImDrawVert));
            // draw_list_text->IdxBuffer buffer cann't because the index value in the IdxBuffer points to draw_list_text->VtxBuffer. It need aligned to draw_list->VtxBuffer.
            for (int i = 0; i < draw_list_text->IdxBuffer.Size; i++)
            {
                draw_list->IdxBuffer[old_inx_buffer_sz + i] = (ImDrawIdx)(old_vtx_buffer_sz + draw_list_text->IdxBuffer[i]);
            }
            draw_list->_VtxWritePtr += draw_list_text->VtxBuffer.Size;
            draw_list->_IdxWritePtr += draw_list_text->IdxBuffer.Size;
            draw_list->_VtxCurrentIdx += vtx_current_idx;
        }

     ...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That mentioned cost + the fact it means draw list buffers will realloc+copy every frame and not be amortized is very largely in the range of unacceptably slow in Dear ImGui world. It makes me worry about the veracity of your other statement about performances.

Is there a performance benchmark for text, it would be a good idea to test it and compare the performance results.

TextEx(text, text_end, ImGuiTextFlags_NoWidthForLargeClippedText);
ImGuiContext& g = *GImGui;
bool need_backup = (g.CurrentWindow->DC.TextWrapPos < 0.0f); // Keep existing wrap position if one is already set
if (need_backup && warpped)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo "warpped"?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@Namek
Copy link

Namek commented Sep 17, 2022

Awesome work! (btw I only skimmed through it, it's not a full review)

Just a question, do you have any idea about the performance impact when the PR's features are not used? Does it change anything?

@CodingMadness
Copy link

May I ask (cause I am in the process of making use quite bit of the TextColoring mechanisms) if this entire topic is still WIP or safely useable? thanks.

@ForrestFeng
Copy link
Author

ForrestFeng commented Nov 23, 2022

May I ask (cause I am in the process of making use quite bit of the TextColoring mechanisms) if this entire topic is still WIP or safely useable? thanks.

It is still WIP. Not merged to master branch yet.

@ForrestFeng
Copy link
Author

ForrestFeng commented Nov 23, 2022

Awesome work! (btw I only skimmed through it, it's not a full review)

Just a question, do you have any idea about the performance impact when the PR's features are not used? Does it change anything?

Thanks for reviewing. For the performance impacts:

  1. There is no performace impact if no customization is applied to the text.
  2. If the text customization is applied, the cost is just some line segments (underlines, strikethroughs) and some filled rectangles(highlight, masks) depends on how many customization applied. The performance should be not a issue in my opinion.

@ForrestFeng ForrestFeng force-pushed the Text_Customization branch 5 times, most recently from 381d66b to 38e2b0f Compare November 23, 2022 11:15
@Fuuzetsu
Copy link

@ForrestFeng I have a question about this feature: how does it interact with TreeNode? In #728 it shows that it's possible to colour the text by pushing a style: great, but it doesn't work if one wants to have more fancy colouring in the label.

@Fuuzetsu
Copy link

@ForrestFeng I have a question about this feature: how does it interact with TreeNode? In #728 it shows that it's possible to colour the text by pushing a style: great, but it doesn't work if one wants to have more fancy colouring in the label.

Just to be super clear on what I mean, here's what we can do today just be setting styles and calling Text without spacing in between:

vvv

The question is whether this PR will let us do anything about those uncoloured nodes that are currently TreeNode labels.

@ForrestFeng
Copy link
Author

@ForrestFeng I have a question about this feature: how does it interact with TreeNode? In #728 it shows that it's possible to colour the text by pushing a style: great, but it doesn't work if one wants to have more fancy colouring in the label.

Just to be super clear on what I mean, here's what we can do today just be setting styles and calling Text without spacing in between:

vvv

The question is whether this PR will let us do anything about those uncoloured nodes that are currently TreeNode labels.


@Fuuzetsu to avoid misunderstanding can you provide a sample code for what you did on the TreeNode labels? So that I can try to recreate it in my way.

Thanks for your comments.

@Fuuzetsu
Copy link

@Fuuzetsu to avoid misunderstanding can you provide a sample code for what you did on the TreeNode labels? So that I can try to recreate it in my way.

Actually, I had a look at some of the existing applications linked in imgui README and saw that they are doing exactly what I want.

I'm using rust bindings so I apologise but it seems I can do something like this:

                    let node = TreeNode::new(&ident)
                        .label::<&str, &str>(&lbl)
                        // Don't render arrow unless we're aggregating orders.
                        .leaf(!is_aggregate);

                    let open = node.push(ui);
                    draw_ctx.ui.same_line_with_spacing(0.0, 0.0);
                    draw_ctx.ui.text_colored(
                        imgui_utils::srgb_f32!(#7e77c8).as_imcolor32f(),
                        "XXXXXXXXXXXX",
                    );

to produce "fancy" partially style labels. So feel free to ignore me, I answered my own question and I think the work here doesn't need to worry about TreeNode at all: one would just use your TextUnformatted as normal and it'll just work. Sorry for the noise.

col

@ocornut
Copy link
Owner

ocornut commented Dec 13, 2022 via email

Quintus added a commit to Quintus/ilmendur that referenced this pull request Dec 28, 2022
@ocornut
Copy link
Owner

ocornut commented Jan 10, 2023

I will be spending a little time evaluating some of the ideas from #5288 (here) and #3130 (and #902), they are similar in essence and both PR are giving a good amount of ideas and starting points.

I think we need to standardize a set of modifiers while being able to:

  • specify them inline (e.g. within a literal string)
  • specify them as external modifiers (driven by code and/or data). this has been the approach of both PR but whereas it is a definitive requirement for formatting external data (think InputText() syntax highlight), many typical use cases would likely be best embedded inline in strings.
  • and as with what I mentioned in Add text alignment support for wrapped paragraphs #4251 (comment) it is essential we tick all the boxes of flexibility, maintainability, ease of use, performances.

Thanks for this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants