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

TestSuite: added "docking_dockspace_copy_no_remap" #20

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

PathogenDavid
Copy link
Contributor

Regression test for ocornut/imgui#6035

Verified by intentionally regressing ocornut/imgui@6939676

@PathogenDavid PathogenDavid force-pushed the docking_dockspace_copy_no_remap branch from 137958b to 193469f Compare March 31, 2023 07:36
@PathogenDavid
Copy link
Contributor Author

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 #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 #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
Copy link
Owner

ocornut commented Mar 31, 2023

I know for #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
Copy link
Owner

ocornut commented Mar 31, 2023

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.

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.

2 participants