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

Scroll bar flicker for child windows #7252

Open
GamingMinds-DanielC opened this issue Jan 22, 2024 · 6 comments
Open

Scroll bar flicker for child windows #7252

GamingMinds-DanielC opened this issue Jan 22, 2024 · 6 comments

Comments

@GamingMinds-DanielC
Copy link
Contributor

Version/Branch of Dear ImGui:

Version 1.90.2 WIP (19014), Branch: master/docking

Back-ends:

imgui_impl_win32.cpp + imgui_impl_dx11/dx12.cpp

Compiler, OS:

Windows 10/11 + MSVC 2019/2022

Full config/build information:

Dear ImGui 1.90.2 WIP (19014)
--------------------------------
sizeof(size_t): 8, sizeof(ImDrawIdx): 2, sizeof(ImDrawVert): 20
define: __cplusplus=199711
define: _WIN32
define: _WIN64
define: _MSC_VER=1929
define: _MSVC_LANG=201703
--------------------------------
io.BackendPlatformName: imgui_impl_win32
io.BackendRendererName: imgui_impl_dx11
io.ConfigFlags: 0x00000001
 NavEnableKeyboard
io.ConfigInputTextCursorBlink
io.ConfigWindowsResizeFromEdges
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x0000000E
 HasMouseCursors
 HasSetMousePos
 RendererHasVtxOffset
--------------------------------
io.Fonts: 1 fonts, Flags: 0x00000000, TexSize: 512,64
io.DisplaySize: 1264.00,761.00
io.DisplayFramebufferScale: 1.00,1.00
--------------------------------
style.WindowPadding: 8.00,8.00
style.WindowBorderSize: 1.00
style.FramePadding: 4.00,3.00
style.FrameRounding: 0.00
style.FrameBorderSize: 0.00
style.ItemSpacing: 8.00,4.00
style.ItemInnerSpacing: 4.00,4.00

Details:

Some background information about the actual application (test case is much simpler):
For a somewhat complex dialog, I have a variable amount of filters at the top in a child of a resizeable child. Whenever I add a filter, if the current height of the outer resizeable child is at its full height, I make it bigger (by setting size constraints accordingly for a single frame) to remain at full height. For the inner child, I set the window height (to the height of the outer child), content height and scroll position in advance to avoid flickering, but the scroll bar still flickers for one frame. Not a high priority since it doesn't impact functionality, but looks a bit annoying.

The test case is reduced to just a toggle changing the size of a single child window. Size and content size get set in advance, still the child window scroll bar flickers when it is toggled bigger. To capture the GIF, I limited the frame rate to 10.

The problem seems to be in ImGui::Begin(). There, use_current_size_for_scrollbar_y (and ..._x as well) will be false for the child. This results in the last size being used to calculate if a scroll bar is needed, despite the current size being available and accurate.

Screenshots/Video:

child_size_scroll_flicker

Minimal, Complete and Verifiable Example code:

// call once per frame in the update loop
void testChildSize()
{
	if ( ImGui::Begin( "TestChildSize" ) )
	{
		static bool isBig = false;

		if ( ImGui::Button( "Toggle Size" ) )
			isBig = !isBig;

		const float  lineHeight = ImGui::GetTextLineHeightWithSpacing();
		const ImVec2 childSize  = ImVec2( 0.0f, ( isBig ? 5.0f : 2.0f ) * lineHeight );

		ImGui::SetNextWindowSize( childSize );
		ImGui::SetNextWindowContentSize( childSize );

		if ( ImGui::BeginChild( "Lines", childSize ) )
		{
			ImGui::TextUnformatted( "Line 1" );
			ImGui::TextUnformatted( "Line 2" );

			if ( isBig )
			{
				ImGui::TextUnformatted( "Line 3" );
				ImGui::TextUnformatted( "Line 4" );
				ImGui::TextUnformatted( "Line 5" );
			}
		}
		ImGui::EndChild();
	}
	ImGui::End();
}
@ocornut
Copy link
Owner

ocornut commented Feb 28, 2024

Hello,

Thanks for the detailed report. This is indeed tricky, I have sort of an answer for the test case but I'd be interested to see a shot or pseudo-code for the your fuller case.

First of all - as you maybe found out, calling SetNextWindowSize() on a child-window is a bit ill-defined because it conflicts with BeginChild()'s size parameter. It may have an undocumented side-effect inside Begin() by nature of setting window_size_y_set_by_api. For now I'll focus on the fact that we don't need this call. But we'll probably need to consider it or clarify that behavior at some point.

For the specific of your test case, setting ImGuiChildFlags_AutoResizeY on the BeginChild() would also effectively fixes it as use_current_size_for_scrollbar_y will end up being true and we have a content size, but I'm not sure it'll apply to your real case.


I started thinking we could safely set use_current_size_for_scrollbar_y when window_size_y_set_by_api && window->ContentSizeExplicit.y != 0 (conceptually the right part is "window_contents_size_y_set_by_api"), but I'm wondering why we simplify don't do: use_current_size_for_scrollbar_y |= window_contents_size_y_set_by_api;. I am going to investigate this a little bit more.

ocornut added a commit to ocornut/imgui_test_engine that referenced this issue Feb 28, 2024
@ocornut
Copy link
Owner

ocornut commented Feb 28, 2024

Added a WIP test for this,
ocornut/imgui_test_engine@2226ad5

The part that pertain to this specific case being:

        // Verify reaction to altered size and contents size (#7252)
        // FIXME-TESTS: We should/could cover more cases by tracking bugs corrected related to the setup of ScrollbarY flag.
        vars.InSize = ImVec2(100, 100);
        vars.InDeclaredContentSize = ImVec2(100.0f, 100.0f - style.WindowPadding.y * 2.0f);
        vars.InSubmittedContentSize = ImVec2(100.0f, 100.0f - style.WindowPadding.y * 2.0f);
        ctx->Yield();
        IM_CHECK_EQ(vars.OutWindow->ScrollbarY, false);
        ctx->Yield();
        IM_CHECK_EQ(vars.OutWindow->ScrollbarY, false);
        // ...increase contents
        vars.InSize.y += 50.0f;
        vars.InDeclaredContentSize.y += 50.0f;
        vars.InSubmittedContentSize.y += 50.0f;
        ctx->Yield();
#if IMGUI_BROKEN_TESTS
        IM_CHECK_EQ(vars.OutWindow->ScrollbarY, false); // Expected/ideal
#else
        IM_CHECK_EQ(vars.OutWindow->ScrollbarY, true); // Current as of 1.90.4
#endif
        ctx->Yield();
        IM_CHECK_EQ(vars.OutWindow->ScrollbarY, false);

@ocornut
Copy link
Owner

ocornut commented Feb 28, 2024

Writing this down because it's a typical case where test suite helps understanding side-effects.

I changed the code in Begin() to:

        bool use_current_size_for_scrollbar_x = window_just_created || window_size_x_set_by_api;
        bool use_current_size_for_scrollbar_y = window_just_created || window_size_y_set_by_api;

And ran all tests, and noticed "table_synced_2" is failing.
If I right-click "Run Gui Func" on "table_synced_2" I noticed that resizing the main window shows flickering scrollbars with this change.

So I amended the first test:

if (vars.InSubmittedContentAuto)
    ImGui::Dummy(ImGui::GetContentRegionAvail());
// submit a Dummy() filling all available space
vars.InSize = ImVec2(100, 100);
vars.InDeclaredContentSize = ImVec2(-1.f, -1.f);
vars.InSubmittedContentSize = ImVec2(-1.f, -1.f);
vars.InSubmittedContentAuto = true;
ctx->Yield(2);
IM_CHECK_EQ(vars.OutWindow->ScrollbarY, false);
vars.OutScrollbarYOred = false;
ctx->WindowResize("//Test Window", ImVec2(50, 50));
IM_CHECK_EQ(vars.OutScrollbarYOred, false); // Verify that scrollbar never shown

And notice this patterns starts breaking if we use bool use_current_size_for_scrollbar_y = window_just_created || window_size_y_set_by_api; which is the core reason why we don't use "current size" in most frames or frames with SetNextWindowSize() call.
EDIT Above said has a mistake as we resize window while still calling SetNextWindowSize() every frame, should clear vars.InSize before calling WindowSize().

@GamingMinds-DanielC
Copy link
Contributor Author

Thanks for the detailed report. This is indeed tricky, I have sort of an answer for the test case but I'd be interested to see a shot or pseudo-code for the your fuller case.

Not the dialog mentioned in the original report, but a newer tool window with the exact same problem and less clutter...
filters_in_child_window

Not really pseudo-code, but an actual code snippet of the relevant part (no complete example code though):

	size_t numFilters = m_Filters.size();
	if ( numFilters > 0 )
	{
		float filterLineHeight   = ImGui::GetFrameHeightWithSpacing();
		float filterContHeight   = (float)numFilters * filterLineHeight;
		float filterMinHeight    = Core::min<float>( 2.0f * filterLineHeight, filterContHeight );
		float filterMaxHeight    = filterContHeight;
		bool  scrollToLastFilter = false;

		const float minHeightBelow = 160.0f;

		filterMaxHeight = Core::clamp<float>( floor( ( ImGui::GetWindowSize().y - minHeightBelow ) / filterLineHeight ) * filterLineHeight, filterMinHeight, filterMaxHeight );

		ImGuiChildFlags filterChildFlags = ImGuiChildFlags_ResizeY;

		if ( m_FilterContHeight < filterContHeight )
		{
			if ( m_FilterFullHeight )
			{
				filterMinHeight = filterMaxHeight;
				filterChildFlags |= ImGuiChildFlags_AutoResizeY; // workaround for child scroll bar flickering on adding filters
			}

			scrollToLastFilter = true;
		}

		m_FilterContHeight = filterContHeight;

		// handle size and scrolling
		ImGui::SetNextWindowSizeConstraints(
		    ImVec2( 0.0f, filterMinHeight ),
		    ImVec2( FLT_MAX, filterMaxHeight ) );
		ImGui::SetNextWindowContentSize( ImVec2( 0.0f, filterContHeight ) );

		if ( scrollToLastFilter )
			ImGui::SetNextWindowScroll( ImVec2( -1.0f, filterContHeight ) );

		if ( ImGui::BeginChild( "Filters", ImVec2( 0.0f, 0.0f ), filterChildFlags, ImGuiWindowFlags_NavFlattened | ImGuiWindowFlags_NoNavInputs ) )
		{
			m_FilterFullHeight = ImGui::GetWindowSize().y >= filterMaxHeight;

			// edit filters
			// ... (omitted for code snippet)
		}
		ImGui::EndChild();

		ImGui::Separator();
	}
	else
	{
		m_FilterContHeight = 0.0f;
		m_FilterFullHeight = true;
	}

I added the ImGuiChildFlags_AutoResizeY workaround you mentioned (for a single frame when making the child bigger), and it does indeed work. No scroll bar flickering in my this case. It also works in the dialog in which I first noticed the flicker.

The child window with the filters is only shown when filters are present. Minimum height is 2 filters if they are that much, If only one filter is present, the child window is a bit bigger than needed (probably an internal minimum size), that's why I test with >= when assigning m_FilterFullHeight. That member of my window class memorizes if the window was at full height last frame. If there are more than 2 filters, the user can resize the child. I only make it bigger automatically if a new filter gets added and the window was at full size before, otherwise I just scroll to the newly added filter.

ocornut added a commit that referenced this issue Feb 28, 2024
@ocornut
Copy link
Owner

ocornut commented Feb 28, 2024

I went with 0573513 for now, which seem reasonable and fixed some of my own tests.
It should fixes yours I think?

ocornut added a commit to ocornut/imgui_test_engine that referenced this issue Feb 28, 2024
ocornut added a commit to ocornut/imgui_test_engine that referenced this issue Feb 28, 2024
@GamingMinds-DanielC
Copy link
Contributor Author

GamingMinds-DanielC commented Feb 29, 2024

It should fixes yours I think?

I will update our library versions as soon as I get around to it (likely today, otherwise tomorrow), remove the workaround and test it.

Update:
I can now confirm that it fixes the flickering for me as well with the workaround reverted. But using ImGuiChildFlags_AutoResizeY for a frame instead of upping the minimum size seems to be a cleaner solution, so I probably will adapt my code a bit.

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

2 participants