TestSuite: added "nav_ctrl_tab_customization"

[PathogenDavid]

Functional test for https://github.com/ocornut/imgui/issues/4828

This adds two tests:

• nav_ctrl_tab_customization - For ensuring ConfigNavWindowingKeyNext/ConfigNavWindowingKeyPrev can be configured.
• nav_ctrl_tab_disabled - For ensuring setting them to 0 disables the keys.

I also added a test for Ctrl+Shift+Tab to nav_ctrl_tab_focusing as this key combo does not appear to be tested elsewhere.

---

While implementing these tests I noticed that the assert you get with this configuration is a bit confusing:

g->ConfigNavWindowingKeyNext = ImGuiKey_N;
g->ConfigNavWindowingKeyPrev = 0;

My expectation was that I'd get a next key with no modifier along with no prev key at all. Instead you get an assert about the two not having modifiers in common.

Not sure if this is a scenario you want to support, but I thought I'd mention it.

[PathogenDavid]

This test is failing on CI but only for Linux Docking.

I poked around a bit and figured out I could repro locally by docking the windows together before the test: https://github.com/PathogenDavid/imgui_test_engine/commit/a1e06b66ebf714ca65b63d9d424232343e73c295

Weirdly enough, Windows and macOS CI still pass with this change, so it might not repro locally for you either.

I added a printout and it appears that the nav window is a dock node?? It's ##DockNode_02 on CI and ##DockNode_01 on my machine, which definitely doesn't seem right.

I put a sleep in the test to see what was going on and found that nav focus was given to the window's tab:

As opposed to the whole window:

It seems like it's maybe reacting to the Alt to focus the tab bar instead of the Alt + N to perform windowing. Very odd that it happens in some situations but not in others. (I can consistently repro on the test engine, but not in the Dear ImGui demo apps.)

I don't imagine it's an ini thing since it's happening on CI, but here's mine in case it helps:

Click to expand...

```ini [Window][Debug##Default] Pos=60,60 Size=400,400 Collapsed=0 [Window][Dear ImGui Test Engine] Pos=44,37 Size=650,520 Collapsed=0 [Window][Dear ImGui Demo] Pos=1110,25 Size=276,680 Collapsed=0 DockId=0x00000004,0 [Window][Hello, world!] Pos=836,25 Size=272,680 Collapsed=0 DockId=0x00000003,0 [Window][Popup] Pos=696,413 Size=32,35 Collapsed=0 [Window][Test] Pos=60,60 Size=94,54 Collapsed=0 [Table][0x52E825D7,3] RefScale=13 Column 0 Width=48 Column 1 Width=70 Column 2 Weight=1.0000 [Docking][Data] DockNode ID=0x00000001 Pos=60,60 Size=65,48 Selected=0xC56529CC DockNode ID=0x00000002 Pos=836,25 Size=550,680 Split=X DockNode ID=0x00000003 Parent=0x00000002 SizeRef=272,680 Selected=0xD00C9CD4 DockNode ID=0x00000004 Parent=0x00000002 SizeRef=276,680 Selected=0xE87781F4 [TestEngine][Data] FilterTests=ctrl_tab FilterPerfs= LogHeight=150 CaptureTool=0 PerfTool=0 StackTool=0 CaptureEnabled=1 CaptureOnError=0 VideoCapturePathToEncoder= VideoCaptureParamsToEncoder=-hide_banner -loglevel error -r $FPS -f rawvideo -pix_fmt rgba -s $WIDTHx$HEIGHT -i - -threads 0 -y -preset ultrafast -pix_fmt yuv420p -crf 20 $OUTPUT GifCaptureParamsToEncoder=-hide_banner -loglevel error -r $FPS -f rawvideo -pix_fmt rgba -s $WIDTHx$HEIGHT -i - -threads 0 -y -filter_complex "split=2 [a] [b]; [a] palettegen [pal]; [b] [pal] paletteuse" $OUTPUT VideoCaptureExtension=.mp4 ```

[ocornut]

Again, being over-zealous in my comments for the sake of sharing some thoughts:

Intuitively I would say it may make sense for "nav_ctrl_tab_customization" and "nav_ctrl_tab_disabled" to be the same test. It's a case-by-case decision to make, but the general idea is that we end up with less code it is a win.

Since those ConfigNavWindowingKeyXXX flags are currently awkwardly in the main context struct they would need to be backed up in ImGuiTestEngine_RunTest() so they don't get overwritten by your change, it's a reasonable change to as they are config flags and that will be unnecessary if we get moved to IO eventually.

[ocornut]

I pushed https://github.com/ocornut/imgui/commit/89d09070e3fd0b5ea73a12f3592c3bbffe15ac61 Which should fix the issue that when ConfigNavWindowingKeyNext or ConfigNavWindowingKeyPrev use Alt, the key release would trigger menu change.

Tip: if you change

IM_CHECK(g.NavWindow == ctx->GetWindowByRef("Window 1"));

to

IM_CHECK_EQ(g.NavWindow, ctx->GetWindowByRef("Window 1"));

We have a window specific overload that display window name in the log.

[PathogenDavid]

Could you push https://github.com/ocornut/imgui/commit/89d09070e3fd0b5ea73a12f3592c3bbffe15ac61 to docking? Right now IMGUI_VERSION_NUM is ahead on docking but the fix is only on master. (Although ironically CI is passing on Linux docking now.)

---

Pushed a new version with the following changes:

• Combined nav_ctrl_tab_disabled into nav_ctrl_tab_customization
• Added backup/restore of keys to ImGuiTestEngine_RunTest
• Added intentionally docking the windows together to test https://github.com/ocornut/imgui/commit/89d09070e3fd0b5ea73a12f3592c3bbffe15ac61
• Changed IM_CHECK to IM_CHECK_EQ.

---

As an aside, it dawned on my the IM_CHECK usage was a copy+paste from nav_ctrl_tab_focusing since I used it as a basis for this test. Would you like me to go through and replace IM_CHECK usage with IM_CHECK_EQ/etc macros as appropriate?

---

Also do you have a preference between ammend+force push for addressing review comments vs separate commits?

[ocornut]

Merged as 2010d63, thanks!

Could you push https://github.com/ocornut/imgui/commit/89d09070e3fd0b5ea73a12f3592c3bbffe15ac61 to docking? Right now IMGUI_VERSION_NUM is ahead on docking but the fix is only on master. (Although ironically CI is passing on Linux docking now.)

Trying to reduce test suite adding pressure to merge too often, if you locally test that's fine. (I tested your test with the local merge too).

As an aside, it dawned on my the IM_CHECK usage was a copy+paste from nav_ctrl_tab_focusing since I used it as a basis for this test. Would you like me to go through and replace IM_CHECK usage with IM_CHECK_EQ/etc macros as appropriate?

It's not so important, I would wait until a thing break to replace them. You can also enable [X] Debug Break to see value in debugger on a test failure. Typically for a IM_CHECK(bool == false) there's next to zero value to use IM_CHECK_EQ(), but it's quite useful for ImGuiWindow* pointer.

Also do you have a preference between amend+force push for addressing review comments vs separate commits?

I would say for large features/work an amend that I would manually squash after review is better. For smaller one it's less important either way is ok.