search

ocornut / imgui_test_engine

Dear ImGui Automation Engine & Test Suite
386 stars 40 forks source link

TestSuite: added "docking_dockspace_copy_no_remap" #20

Open PathogenDavid opened 1 year ago

PathogenDavid commented 1 year ago

Regression test for https://github.com/ocornut/imgui/issues/6035

Verified by intentionally regressing https://github.com/ocornut/imgui/commit/693967637266ff48fd247821acd0c6cdf66eda6d

PathogenDavid commented 1 year ago

This test was originally failing only for Linux docking on CI.

I did not have time to fully investigate why it was failing, but based on a hunch from a similar issue with https://github.com/ocornut/imgui_test_engine/pull/19 I guessed it was due to Window0 being too small, and indeed adding a SetNextWindowSize call for it fixed the issue.

I can investigate further if you find that fix unsatisfactory. It'd definitely be nice to know why Linux docking CI in particular seems to find these issues, but it might be a deep rabbit hole too. (I tested locally with WSLg and it passed there, which suggests it might be CI-specific rather than Linux docking-specific.)

I know for https://github.com/ocornut/imgui_test_engine/pull/19 I ended up figuring out that ImGuiTestContext::GetWindowTitlebarPoint was picking an invalid point on super small windows (had a note to report / investigate a fix later -- IIRC it looked like it was due to not avoiding the close/window menu buttons.), might just be the same issue manifesting its self slightly differently here.

ocornut commented 1 year ago

I know for https://github.com/ocornut/imgui_test_engine/pull/19 I ended up figuring out that ImGuiTestContext::GetWindowTitlebarPoint was picking an invalid point on super small windows (had a note to report / investigate a fix later -- IIRC it looked like it was due to not avoiding the close/window menu buttons.), might just be the same issue manifesting its self slightly differently here.

Thanks for the PR. That’s worth investigating separately, perhaps with its own test. And I guess maybe that function should be GetWindowTitlebarPointForMoving. Otherwise if we can make that function return a failure state, perhaps the higher-level users eg WindowMove() can decide to fallback to moving position directly using SetWindowPos(), although in principle ideally we’d try to steer the test engine toward behaving more like a real user.

ocornut commented 1 year ago

While we are here I suggest adding some basic sanity tests to check for the behavior of DockBuilderCopyDockSpace(). Which I guess would involve e.g. docking stuff in one dockspace, running copy and checking other dockspace. Depending on amount of content we can rename this to docking_dockspace_copy to add a new test docking_dockspace_copy and keep this one separate.