[PATCH v4 0/1] MR10809: comctl32: Fix VARHEIGHT rebar scaling when cyIntegral == 0
This is a test and fix for an edge case in the rebar component where `cyChild` is set to a larger value than `cyMaxChild` while `cyIntegral` is exactly 0 on a VARIABLEHEIGHT rebar. On Windows, at least the two Win11 machines I have acces to, the WinAPI seems to clamp the value to `cyMaxChild` so the test passes. On Wine, it does not clamp the value and the test fails, but passes with the fix. Specifically, Wine's behaviour results in the program EffectsEd3 from the modding tools of Call of Duty: World at War sizing its rebar height incorrectly, taking over the whole window and making the program unusable. With the fix, this is no longer the case and the program becomes usable. {width=255 height=189} - {width=351 height=185} -- v4: comctl32: Clamp rebar height to cyMaxChild when cyIntegral == 0 too https://gitlab.winehq.org/wine/wine/-/merge_requests/10809
From: Feli Rippmann <25101-Feli@users.noreply.gitlab.winehq.org> --- dlls/comctl32/rebar.c | 5 ++++- dlls/comctl32/tests/rebar.c | 22 +++++++++++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/dlls/comctl32/rebar.c b/dlls/comctl32/rebar.c index 27270a9b63c..cb4763cff9b 100644 --- a/dlls/comctl32/rebar.c +++ b/dlls/comctl32/rebar.c @@ -465,7 +465,10 @@ static int round_child_height(const REBAR_BAND *lpBand, int cyHeight) { int cy = 0; if (lpBand->cyIntegral == 0) - return cyHeight; + { + cy = min(cyHeight, lpBand->cyMaxChild); + return cy; + } cy = max(cyHeight - (int)lpBand->cyMinChild, 0); cy = lpBand->cyMinChild + (cy/lpBand->cyIntegral) * lpBand->cyIntegral; cy = min(cy, lpBand->cyMaxChild); diff --git a/dlls/comctl32/tests/rebar.c b/dlls/comctl32/tests/rebar.c index d2c1e68d354..7f5158d5561 100644 --- a/dlls/comctl32/tests/rebar.c +++ b/dlls/comctl32/tests/rebar.c @@ -246,7 +246,7 @@ static void rbsize_add_band(rbsize_result_t *rbsr, int left, int top, int right, static rbsize_result_t *rbsize_results; -#define rbsize_results_num 27 +#define rbsize_results_num 28 static void rbsize_results_init(void) { @@ -430,6 +430,10 @@ static void rbsize_results_init(void) rbsize_add_band(&rbsize_results[26], 0, 0, 90, 65, 0x40, 90); rbsize_add_band(&rbsize_results[26], 90, 0, 163, 65, 0x40, 90); rbsize_add_band(&rbsize_results[26], 163, 0, 226, 65, 0x40, 90); + + rbsize_results[27] = rbsize_init(0, 0, 672, 65, 56, 1, 3); + rbsize_add_row(&rbsize_results[27], 65); + rbsize_add_band(&rbsize_results[27], 0, 0, 672, 65, 0x40, 90); } static void rbsize_results_free(void) @@ -684,6 +688,22 @@ static void test_layout(void) DestroyWindow(hRebar); + /* VARHEIGHT resizing test with cyIntegral == 0 on a horizontal rebar */ + hRebar = create_rebar_control(0); + SetWindowLongA(hRebar, GWL_STYLE, GetWindowLongA(hRebar, GWL_STYLE) | RBS_AUTOSIZE); + rbi.fMask = RBBIM_CHILD | RBBIM_CHILDSIZE | RBBIM_SIZE | RBBIM_STYLE; + rbi.fStyle = RBBS_VARIABLEHEIGHT; + rbi.cx = 90; + rbi.cyMinChild = 40; + rbi.cyMaxChild = 65; + rbi.cyIntegral = 0; + rbi.cyChild = 111; + rbi.hwndChild = build_toolbar(0, hRebar); + SendMessageA(hRebar, RB_INSERTBANDA, -1, (LPARAM)&rbi); + check_sizes(); + + DestroyWindow(hRebar); + rbsize_results_free(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10809
On Mon May 25 08:35:46 2026 +0000, Zhiyi Zhang wrote:
At this commit, the test fails, so it will introduce CI failures. You can put the fix in the same commit since the fix is pretty small. ``` mrebar.c:720: Test failed: invalid rect (client) (0,0)-(672,111) - expected (0,0)-(672,65) mrebar.c:720: Test failed: Height mismatch for row 0 - 65 vs 111 mrebar.c:720: Test failed: invalid rect (band) (0,0)-(90,111) - expected (0,0)-(90,65) mrebar.c:720: Test failed: invalid rect (band) (90,0)-(180,111) - expected (90,0)-(180,65) mrebar.c:720: Test failed: invalid rect (band) (180,0)-(672,111) - expected (180,0)-(672,65) ``` Thank you, I have now put the test and fix in one commit. I had originally separated them into two commits because of the [Patch guidelines](https://gitlab.winehq.org/wine/wine/-/wikis/Submitting-Patches#patch-guideli...) in the wiki. Is there any guideline on when to separate them and when not to, that I could use in the future?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10809#note_141355
On Tue May 26 17:01:46 2026 +0000, Feli Rippmann wrote:
changed this line in [version 4 of the diff](/wine/wine/-/merge_requests/10809/diffs?diff_id=270436&start_sha=95ef6e27f99724f63211b7771cad1baa281d8950#382007f113cc98a69ffe5bb3875a73e4727dad0c_701_696) I have reduced the test to use just one band and reduced the number of variables that are being set in the band initialization. If it can be reduced further, I am open to suggestions. Thank you.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10809#note_141358
On Tue May 26 17:06:54 2026 +0000, Feli Rippmann wrote:
Thank you, I have now put the test and fix in one commit. I had originally separated them into two commits because of the [Patch guidelines](https://gitlab.winehq.org/wine/wine/-/wikis/Submitting-Patches#patch-guideli...) in the wiki. Is there any guideline on when to separate them and when not to, that I could use in the future? Yes, usually we separate the tests and the fix. Then, submit the tests first with a todo_wine. However, in this case, you can either introduce a todo_wine support in `check_sizes()` or merge the patches into one patch. Since the fix is small, adding todo_wine to `check_sizes()` seems unnecessary, and one patch is fine.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10809#note_141388
On Wed May 27 00:59:35 2026 +0000, Zhiyi Zhang wrote:
Yes, usually we separate the tests and the fix. Then, submit the tests first with a todo_wine. However, in this case, you can either introduce a todo_wine support in `check_sizes()` or merge the patches into one patch. Since the fix is small, adding todo_wine to `check_sizes()` seems unnecessary, and one patch is fine. Thank you for the explanation, it is very much appreciated. I think I have addressed all points brought up so far, including in the other review comment. Could you review the new diff at your convenience? Thanks!
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10809#note_143293
participants (3)
-
Feli Rippmann -
Feli Rippmann (＠Feli)
-
Zhiyi Zhang (＠zhiyi)