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

Cannot draw over horizontal padding area when child's border size is non zero #3312

Closed
Vennor opened this issue Jun 21, 2020 · 14 comments
Closed

Comments

@Vennor
Copy link

Vennor commented Jun 21, 2020

Hi!

I noticed something weird related to padding and border size. I tried to find the reason in the sources but gave up after some time without success. Consider the code below.

//#define DRAW_BORDER

if (ImGui::Begin("Window"))
{
    ImGui::PushStyleColor(ImGuiCol_ChildBg, ImVec4(0.0f, 0.0f, 0.0f, 1.0f)); // Black background used for clarity.
    ImGui::PushStyleVar(ImGuiStyleVar_WindowPadding, ImVec2(30.0f, 30.0f)); // Big padding used for clarity.

#if defined(DRAW_BORDER)
    ImGui::PushStyleVar(ImGuiStyleVar_ChildBorderSize, 1.0f);
#else
    ImGui::PushStyleVar(ImGuiStyleVar_ChildBorderSize, 0.0f);
#endif

    if (ImGui::BeginChild("Child", ImVec2(128.0f, 64.0f), true))
    {
        ImGui::SetCursorPos(ImVec2(-5.0f, -5.0f)); // Used for clarity, problem visible even when (0.0f, 0.0f) is used.
        ImGui::Text("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"); // Long text used for clarity.
    }
    ImGui::EndChild();

    ImGui::PopStyleVar(2);
    ImGui::PopStyleColor();
}
ImGui::End();

Depending on whether border size is 0 or non 0, these are the effects I'm seeing.

NoBlackBars BlackBars

Somehow when the border is non 0 whatever I draw in the child is not drawn on the left and right side - equal in size to the padding - while top and bottom are unaffected. This doesn't happen if there's no border around the child. It also doesn't seem like an overdraw with - in this case - solid color since when the background is left semi transparent there's no text visible on the sides too, so it's rather a clipping issue.

Is this a bug or am I missing something?

I'm using 1.76 from vcpkg on Windows with custom OpenGL 3 binding and SFML.

@ocornut
Copy link
Owner

ocornut commented Jun 23, 2020

Hello,

Background:

Whenever the child has a border by default we enable WindowPadding on the window. WindowPadding will make your initial position ~window->Pos + style.WindowPadding. It does make sense if you think about it, whenever child windows are used without border it makes more sense that items are horizontally aligned at the same X position you where we were before submitting the child window, whereas when you have a border it does make more sense to add the padding.

Initially this was designed to be tied to the bool borders parameter of BeginChild() but the fact that style.ChildBorderSize interfer makes things more confusing in that example since a style change does introduce an unexpected discontinuity.

The ImGuiWindowFlags_AlwaysUseWindowPadding flag was designed to enforce that padding in both cases:

// Ensure child windows without border uses style.WindowPadding
// (ignored by default for non-bordered child windows, because more convenient)
ImGuiWindowFlags_AlwaysUseWindowPadding = 1 << 16,

Clipping:

The default window clipping rectangle adds that spacing of WindowPadding.x*0.5f horizontally, and no clipping on the vertical axis. I made this choice in 1.0 because we tend to have a bias toward scrolling vertically, having no clipping-padding on the vertical axis tends to make it more noticeable that there's something to scroll to.

If we remove it on the horizontal axis that's the effect we'll get everywhere:

image
vs currently
image

I think the current look is a little more pleasant and less crampy

I agree it's a little arbitrary and I'm tempted to completely remove it, by turning

window->InnerClipRect.Min.x = ImFloor(0.5f + window->InnerRect.Min.x + ImMax(ImFloor(window->WindowPadding.x * 0.5f), window->WindowBorderSize));
window->InnerClipRect.Min.y = ImFloor(0.5f + window->InnerRect.Min.y + top_border_size);
window->InnerClipRect.Max.x = ImFloor(0.5f + window->InnerRect.Max.x - ImMax(ImFloor(window->WindowPadding.x * 0.5f), window->WindowBorderSize);
window->InnerClipRect.Max.y = ImFloor(0.5f + window->InnerRect.Max.y - window->WindowBorderSize);

into

window->InnerClipRect.Min.x = ImFloor(0.5f + window->InnerRect.Min.x + window->WindowBorderSize);
window->InnerClipRect.Min.y = ImFloor(0.5f + window->InnerRect.Min.y + top_border_size);
window->InnerClipRect.Max.x = ImFloor(0.5f + window->InnerRect.Max.x - window->WindowBorderSize);
window->InnerClipRect.Max.y = ImFloor(0.5f + window->InnerRect.Max.y - window->WindowBorderSize);

@ocornut ocornut added the layout label Jun 23, 2020
@Vennor
Copy link
Author

Vennor commented Jun 23, 2020

I see. Thank you for the explanation. It makes sense to clip the edges in the aspect of presentation - it certainly looks better! - but the solution seems kinda hacky and is a likely pitfall, especially since it depends on the presence of border.

A good solution might be to entirely leave it up to the user whether or not they want to use the flag. Clipping could even be the default behavior removed by setting a flag. What do you think about that?

@ocornut
Copy link
Owner

ocornut commented Jun 23, 2020

especially since it depends on the presence of border.

Only indirectly. Clipping only depends on WindowPadding, and for child windows WindowPadding depends on BorderSize.

leave it up to the user whether or not they want to use the flag

Which flag?
You can always set WindowPadding to 0.0f to disabling clipping in that region.

@Vennor
Copy link
Author

Vennor commented Jun 23, 2020

Which flag?

ImGuiWindowFlags_AlwaysUseWindowPadding. Maybe make it so that if one wants clipping they have to specify the flag regardless of border or padding. Or the other way around - clip by default and suppress it when eg. ImGuiWindowFlags_NeverUseWindowPadding is specified.

You can always set WindowPadding to 0.0f to disabling clipping in that region.

That's what I do at the moment but then I follow it with SetCursorPosition to manually add the padding to the elements that in my case should have it - since some others shouldn't. But now that I think about it maybe I just have a wrong approach.

@ocornut
Copy link
Owner

ocornut commented Jun 23, 2020

I don't think you need to use SetCursorPos(), you can use Indent() to offset the X position for new line, and future WorkRect API will allow you to manipulate that on all four directions.

@Vennor
Copy link
Author

Vennor commented Jun 27, 2020

Either way I managed to work around it, so I think we can close the issue if this is fine and expected. Unless you want to change it. :)

@ocornut
Copy link
Owner

ocornut commented May 3, 2024

Unrelated to main topic, but as a general tip, don't do that:

#if defined(DRAW_BORDER)
    ImGui::PushStyleVar(ImGuiStyleVar_ChildBorderSize, 1.0f);
#else
    ImGui::PushStyleVar(ImGuiStyleVar_ChildBorderSize, 0.0f);
#endif

When you can do:

if (io.KeyCtrl)
    ImGui::PushStyleVar(ImGuiStyleVar_ChildBorderSize, 1.0f);
else
    ImGui::PushStyleVar(ImGuiStyleVar_ChildBorderSize, 0.0f);

or

static bool enable_border = true;
ImGui::CheckBox("enableborder", &enable_border);
if (enable_border)
    ImGui::PushStyleVar(ImGuiStyleVar_ChildBorderSize, 1.0f);
else
    ImGui::PushStyleVar(ImGuiStyleVar_ChildBorderSize, 0.0f);

Both are more flexible than a compile-time flag, and make it easier to understand/debug a situation.

@ocornut
Copy link
Owner

ocornut commented May 3, 2024

As per #7540 see how we can change the inner cliprect boundaries:

window_padding_clip_rect

Code (note the neat use of ImGuiOnceUponAFrame)

> // .. in Begin()
static ImGuiOnceUponAFrame oaf;
static int padding_mode = 0;
if (oaf)
{
    ImGui::Begin("padding mode");
    ImGui::RadioButton("0.0", &padding_mode, 0);
    ImGui::RadioButton("0.5", &padding_mode, 1);
    ImGui::RadioButton("1.0", &padding_mode, 2);
    ImGui::End();
}
float window_padding_factor = padding_mode * 0.5f;
window->InnerClipRect.Min.x = ImTrunc(0.5f + window->InnerRect.Min.x + ImMax(ImTrunc(window->WindowPadding.x * window_padding_factor), window->WindowBorderSize));
window->InnerClipRect.Min.y = ImTrunc(0.5f + window->InnerRect.Min.y + top_border_size);
window->InnerClipRect.Max.x = ImTrunc(0.5f + window->InnerRect.Max.x - ImMax(ImTrunc(window->WindowPadding.x * window_padding_factor), window->WindowBorderSize));
window->InnerClipRect.Max.y = ImTrunc(0.5f + window->InnerRect.Max.y - window->WindowBorderSize);
window->InnerClipRect.ClipWithFull(host_rect);

it's a little bit frustrating imho because none is perfect, but going for 0.0 seems like the wise and obvious simplification so I am likely going to go for it.

@ocornut
Copy link
Owner

ocornut commented May 3, 2024

If some people are reading this could you try with 0.0 and see if any of the difference it introduces feel problematic or detrimental to your app?

@ocornut
Copy link
Owner

ocornut commented May 3, 2024

I went and pushed this change, I think it's for the best 0b30947
Also generally makes it easier to draw stuff in edge of windows without requiring a clip rect change.

@ocornut ocornut closed this as completed May 3, 2024
@cfillion
Copy link
Contributor

cfillion commented May 3, 2024

A small nitpick: menu bar text now overflows over the border in the docking branch when the coordinates are negative (#6861). Replacing IM_ROUND by ImFloor in BeginMenuBar solves it.

ImRect clip_rect(IM_ROUND(bar_rect.Min.x + window->WindowBorderSize), IM_ROUND(bar_rect.Min.y + window->WindowBorderSize), IM_ROUND(ImMax(bar_rect.Min.x, bar_rect.Max.x - ImMax(window->WindowRounding, window->WindowBorderSize))), IM_ROUND(bar_rect.Max.y));

Screen Shot 2024-05-03 at 10 23 15

@ocornut
Copy link
Owner

ocornut commented May 3, 2024

This looks like an issue indeed but I don't quite comprehend how it could be linked to 0b30947, since afaik nothing in that path rely on InnerClipRect. Can you confirm that a few commits before the merge the issue wasn't there?

@cfillion
Copy link
Contributor

cfillion commented May 3, 2024

Ah you're right, it is indeed unrelated. I didn't check/notice that menubars weren't using the window's inner clip rect to begin with...

@ocornut
Copy link
Owner

ocornut commented May 3, 2024

Pushed your fix f5d1852
Thank you!

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

No branches or pull requests

3 participants