ownerdraw paint recursion in listbox

[Alexander Yaworsky]

Hello,

recently i tried to install some application and it hung when i
tried to select options.
It uses listbox with ownerdraw items with checkboxes. When listbox
is initially painted everything is ok. But when i try to select other
item, an extra WM_PAINT is sent to listbox when application draws item.
This causes infinite recursion.
I've been implemented a simple workaround adding boolean to listbox
structure but i have doubts. I briefly looked at some other controls
and had not noticed similar countermeasures.
Have somebody ever seen such behaviour?
It would be interesting to test other controls sending WM_PAINT from
ownerdraw handler but i'm short of time to do it soon.
Should this workaround implemented in this way or something
is wrong deeper with WM_PAINT handling?

Thanks.

[Dmitry Timoshkov]

"Alexander Yaworsky"  wrote:
> recently i tried to install some application and it hung when i
> tried to select options.
> It uses listbox with ownerdraw items with checkboxes. When listbox
> is initially painted everything is ok. But when i try to select other
> item, an extra WM_PAINT is sent to listbox when application draws item.
> This causes infinite recursion.
> I've been implemented a simple workaround adding boolean to listbox
> structure but i have doubts. I briefly looked at some other controls
> and had not noticed similar countermeasures.
> Have somebody ever seen such behaviour?
> It would be interesting to test other controls sending WM_PAINT from
> ownerdraw handler but i'm short of time to do it soon.
> Should this workaround implemented in this way or something
> is wrong deeper with WM_PAINT handling?

If you have a small test case or a snippet of the +relay log showing
the problem I'll have a look.

[Alexander Yaworsky]

Dmitry Timoshkov wrote:
> If you have a small test case or a snippet of the +relay log showing
> the problem I'll have a look.

http://migusoft.ru/misc/trace.bz2

with added ERR()s, see attachment

I cut the tail of log just before the first err:listbox. Note that
there are two listboxes on the form. Both are initialized ok,
wrong things begin after WM_KEYDOWN.

Index: listbox.c
===================================================================
RCS file: /home/wine/wine/dlls/user/listbox.c,v
retrieving revision 1.5
diff -u -r1.5 listbox.c
--- listbox.c	26 Oct 2004 22:03:00 -0000	1.5
+++ listbox.c	23 Nov 2004 04:00:07 -0000
@@ -103,6 +103,7 @@
     HFONT       font;           /* Current font */
     LCID          locale;       /* Current locale for string comparisons */
     LPHEADCOMBO   lphc;		/* ComboLBox */
+//    BOOL        painting;
 } LB_DESCR;

@@ -204,6 +205,7 @@
 static INT LISTBOX_GetCurrentPageSize( LB_DESCR *descr )
 {
     INT i, height;
+ERR("\n");
     if (!(descr->style & LBS_OWNERDRAWVARIABLE)) return descr->page_size;
     for (i = descr->top_item, height = 0; i < descr->nb_items; i++)
     {
@@ -223,6 +225,7 @@
 {
     INT max, page;

+ERR("\n");
     if (descr->style & LBS_OWNERDRAWVARIABLE)
     {
         page = descr->height;
@@ -255,6 +258,7 @@
 {
     SCROLLINFO info;

+ERR("\n");
     /* Check the listbox scroll bar flags individually before we call
        SetScrollInfo otherwise when the listbox style is WS_HSCROLL and
        no WS_VSCROLL, we end up with an uninitialized, visible horizontal
@@ -325,7 +329,9 @@
  */
 static LRESULT LISTBOX_SetTopItem( LB_DESCR *descr, INT index, BOOL scroll )
 {
-    INT max = LISTBOX_GetMaxTopIndex( descr );
+    INT max;
+ERR("\n");
+max = LISTBOX_GetMaxTopIndex( descr );
     if (index > max) index = max;
     if (index < 0) index = 0;
     if (descr->style & LBS_MULTICOLUMN) index -= index % descr->page_size;
@@ -383,6 +389,7 @@
 static void LISTBOX_UpdatePage( LB_DESCR *descr )
 {
     INT page_size;
+ERR("\n");

if ((descr->item_height == 0) || (page_size = descr->height / descr->item_height) < 1)
                        page_size = 1;
@@ -403,6 +410,7 @@
 static void LISTBOX_UpdateSize( LB_DESCR *descr )
 {
     RECT rect;
+ERR("\n");

GetClientRect( descr->self, &rect );
     descr->width  = rect.right - rect.left;
@@ -455,6 +463,7 @@
  */
 static LRESULT LISTBOX_GetItemRect( LB_DESCR *descr, INT index, RECT *rect )
 {
+ERR("\n");
     /* Index <= 0 is legal even on empty listboxes */
     if (index && (index >= descr->nb_items)) return -1;
     SetRect( rect, 0, 0, descr->width, descr->height );
@@ -508,6 +517,7 @@
 {
     INT index = descr->top_item;

+ERR("\n");
     if (!descr->nb_items) return -1;  /* No items */
     if (descr->style & LBS_OWNERDRAWVARIABLE)
     {
@@ -555,6 +565,7 @@
    		       INT index, UINT action, BOOL ignoreFocus )
 {
     LB_ITEMDATA *item = NULL;
+ERR("index=%d\n",index);
     if (index < descr->nb_items) item = &descr->items[index];

if (IS_OWNERDRAW(descr))
@@ -654,6 +665,7 @@
  */
 static void LISTBOX_SetRedraw( LB_DESCR *descr, BOOL on )
 {
+ERR("\n");
     if (on)
     {
         if (!(descr->style & LBS_NOREDRAW)) return;
@@ -686,6 +698,7 @@
     HFONT oldFont = 0;
     HBRUSH hbrush, oldBrush = 0;

+ERR("\n");
     /* Do not repaint the item if the item is not visible */
     if (!IsWindowVisible(descr->self)) return;
     if (descr->style & LBS_NOREDRAW)
@@ -716,6 +729,7 @@
 {
     LB_ITEMDATA *item;

+ERR("\n");
     nb_items += LB_ARRAY_GRANULARITY - 1;
     nb_items -= (nb_items % LB_ARRAY_GRANULARITY);
     if (descr->items) {
@@ -743,6 +757,7 @@
  */
 static BOOL LISTBOX_SetTabStops( LB_DESCR *descr, INT count, LPINT tabs, BOOL short_ints )
 {
+ERR("\n");
     if (!(descr->style & LBS_USETABSTOPS)) return TRUE;
     if (descr->tabs) HeapFree( GetProcessHeap(), 0, descr->tabs );
     if (!(descr->nb_tabs = count))
@@ -777,6 +792,7 @@
  */
 static LRESULT LISTBOX_GetText( LB_DESCR *descr, INT index, LPWSTR buffer, BOOL unicode )
 {
+ERR("\n");
     if ((index < 0) || (index >= descr->nb_items)) return LB_ERR;
     if (HAS_STRINGS(descr))
     {
@@ -818,6 +834,7 @@
 {
     INT index, min, max, res = -1;

+ERR("\n");
     if (!(descr->style & LBS_SORT)) return -1;  /* Add it at the end */
     min = 0;
     max = descr->nb_items;
@@ -861,6 +878,7 @@
 {
     INT min, max, res = -1;

+ERR("\n");
     if (!HAS_STRINGS(descr))
         return LISTBOX_FindStringPos( descr, str, FALSE );
     min = 0;
@@ -906,6 +924,7 @@
     INT i;
     LB_ITEMDATA *item;

+ERR("\n");
     if (start >= descr->nb_items) start = -1;
     item = descr->items + start + 1;
     if (HAS_STRINGS(descr))
@@ -967,6 +986,7 @@
     INT i, count;
     LB_ITEMDATA *item = descr->items;

+ERR("\n");
     if (!(descr->style & LBS_MULTIPLESEL) ||
         (descr->style & LBS_NOSEL))
       return LB_ERR;
@@ -984,6 +1004,7 @@
     INT i, count;
     LB_ITEMDATA *item = descr->items;

+ERR("\n");
     if (!(descr->style & LBS_MULTIPLESEL)) return LB_ERR;
     for (i = count = 0; (i < descr->nb_items) && (count < max); i++, item++)
         if (item->selected) array[count++] = (INT16)i;
@@ -999,6 +1020,7 @@
     INT i, count;
     LB_ITEMDATA *item = descr->items;

+ERR("\n");
     if (!(descr->style & LBS_MULTIPLESEL)) return LB_ERR;
     for (i = count = 0; (i < descr->nb_items) && (count < max); i++, item++)
         if (item->selected) array[count++] = i;
@@ -1017,8 +1039,12 @@
     HFONT oldFont = 0;
     HBRUSH hbrush, oldBrush = 0;

+ERR("\n");
     if (descr->style & LBS_NOREDRAW) return 0;

+//    if( descr->painting ) return 0;
+//    descr->painting = TRUE;
+
     SetRect( &rect, 0, 0, descr->width, descr->height );
     if (descr->style & LBS_MULTICOLUMN)
         rect.right = rect.left + descr->column_width;
@@ -1118,6 +1144,7 @@
     }
     if (oldFont) SelectObject( hdc, oldFont );
     if (oldBrush) SelectObject( hdc, oldBrush );
+//    descr->painting = FALSE;
     return 0;
 }

@@ -1132,6 +1159,7 @@
 {
     RECT rect;

+ERR("\n");
     if (LISTBOX_GetItemRect( descr, index, &rect ) == 1)
     {
         if (descr->style & LBS_NOREDRAW)
@@ -1165,6 +1193,7 @@
  */
 static LRESULT LISTBOX_GetItemHeight( LB_DESCR *descr, INT index )
 {
+ERR("\n");
     if (descr->style & LBS_OWNERDRAWVARIABLE)
     {
         if ((index < 0) || (index >= descr->nb_items)) return LB_ERR;
@@ -1179,6 +1208,7 @@
  */
 static LRESULT LISTBOX_SetItemHeight( LB_DESCR *descr, INT index, INT height, BOOL repaint )
 {
+ERR("\n");
     if (!height) height = 1;

if (descr->style & LBS_OWNERDRAWVARIABLE)
@@ -1210,6 +1240,7 @@
 {
     INT diff;

+ERR("\n");
     if (pos > descr->horz_extent - descr->width)
         pos = descr->horz_extent - descr->width;
     if (pos < 0) pos = 0;
@@ -1236,6 +1267,7 @@
  */
 static LRESULT LISTBOX_SetHorizontalExtent( LB_DESCR *descr, INT extent )
 {
+ERR("\n");
     if (!descr->horz_extent || (descr->style & LBS_MULTICOLUMN))
         return LB_OKAY;
     if (extent <= 0) extent = 1;
@@ -1255,6 +1287,7 @@
  */
 static LRESULT LISTBOX_SetColumnWidth( LB_DESCR *descr, INT width)
 {
+ERR("\n");
     if (width == descr->column_width) return LB_OKAY;
     TRACE("[%p]: new column width = %d\n", descr->self, width );
     descr->column_width = width;
@@ -1274,6 +1307,7 @@
     HFONT oldFont = 0;
     TEXTMETRICW tm;

+ERR("\n");
     descr->font = font;

if (!(hdc = GetDCEx( descr->self, 0, DCX_CACHE )))
@@ -1300,6 +1334,7 @@
 {
     INT top;

+ERR("\n");
     if (index <= descr->top_item) top = index;
     else if (descr->style & LBS_MULTICOLUMN)
     {
@@ -1337,6 +1372,7 @@
 {
     INT oldfocus = descr->focus_item;

+ERR("\n");
     if (descr->style & LBS_NOSEL) return LB_ERR;
     if ((index < 0) || (index >= descr->nb_items)) return LB_ERR;
     if (index == oldfocus) return LB_OKAY;
@@ -1362,6 +1398,7 @@
 {
     INT i;

+ERR("\n");
     /* A few sanity checks */

if (descr->style & LBS_NOSEL) return LB_ERR;
@@ -1401,6 +1438,7 @@
 static LRESULT LISTBOX_SetSelection( LB_DESCR *descr, INT index,
                                      BOOL on, BOOL send_notify )
 {
+ERR("\n");
     TRACE( "index=%d notify=%s\n", index, send_notify ? "YES" : "NO" );

if (descr->style & LBS_NOSEL)
@@ -1444,6 +1482,7 @@
 {
     INT oldfocus = descr->focus_item;

+ERR("\n");
     if ((index <  0) || (index >= descr->nb_items))
         return;

@@ -1497,6 +1536,7 @@
     INT max_items;
     INT oldfocus = descr->focus_item;

+ERR("\n");
     if (index == -1) index = descr->nb_items;
     else if ((index < 0) || (index > descr->nb_items)) return LB_ERR;
     if (!descr->items) max_items = 0;
@@ -1585,6 +1625,7 @@
     DWORD data = 0;
     LRESULT ret;

+ERR("\n");
     if (HAS_STRINGS(descr))
     {
         static const WCHAR empty_stringW[] = { 0 };
@@ -1623,6 +1664,7 @@
      *       It's probably better to send it too often than not
      *       often enough, so this is what we do here.
      */
+ERR("\n");
     if (IS_OWNERDRAW(descr) || descr->items[index].data)
     {
         DELETEITEMSTRUCT dis;
@@ -1650,6 +1692,7 @@
     LB_ITEMDATA *item;
     INT max_items;

+ERR("\n");
     if ((index == -1) && (descr->nb_items > 0)) index = descr->nb_items - 1;
     else if ((index < 0) || (index >= descr->nb_items)) return LB_ERR;

@@ -1714,6 +1757,7 @@
 {
     INT i;

+ERR("\n");
     for (i = 0; i < descr->nb_items; i++) LISTBOX_DeleteItem( descr, i );
     if (descr->items) HeapFree( GetProcessHeap(), 0, descr->items );
     descr->nb_items      = 0;
@@ -1732,6 +1776,7 @@
 {
     LRESULT ret;

+ERR("\n");
     if (HAS_STRINGS(descr)) return LB_ERR;
     /* FIXME: this is far from optimal... */
     if (count > descr->nb_items)
@@ -1761,6 +1806,7 @@
     WIN32_FIND_DATAW entry;
     int pos;

+ERR("\n");
     /* don't scan directory if we just want drives exclusively */
     if (attrib != (DDL_DRIVES | DDL_EXCLUSIVE)) {
         /* scan directory */
@@ -1834,6 +1880,7 @@
 {
     SCROLLINFO info;

+ERR("\n");
     if (descr->style & LBS_MULTICOLUMN) return 0;
     switch(scrollReq)
     {
@@ -1879,6 +1926,7 @@
     SCROLLINFO info;
     INT page;

+ERR("\n");
     if (descr->style & LBS_MULTICOLUMN)
     {
         switch(scrollReq)
@@ -1965,6 +2013,7 @@
     short gcWheelDelta = 0;
     UINT pulScrollLines = 3;

+ERR("\n");
     SystemParametersInfoW(SPI_GETWHEELSCROLLLINES,0, &pulScrollLines, 0);

gcWheelDelta -= delta;
@@ -1983,7 +2032,9 @@
  */
 static LRESULT LISTBOX_HandleLButtonDown( LB_DESCR *descr, DWORD keys, INT x, INT y )
 {
-    INT index = LISTBOX_GetItemFromPoint( descr, x, y );
+    INT index;
+ERR("\n");
+index = LISTBOX_GetItemFromPoint( descr, x, y );
     TRACE("[%p]: lbuttondown %d,%d item %d\n", descr->self, x, y, index );
     if (!descr->caret_on && (descr->in_focus)) return 0;

@@ -2079,6 +2130,7 @@
     RECT clientRect, screenRect;
     POINT mousePos;

+ERR("\n");
     mousePos.x = x;
     mousePos.y = y;

@@ -2154,6 +2206,7 @@
  */
 static LRESULT LISTBOX_HandleLButtonUp( LB_DESCR *descr )
 {
+ERR("\n");
     if (LISTBOX_Timer != LB_TIMER_NONE)
         KillSystemTimer( descr->self, LB_TIMER_ID );
     LISTBOX_Timer = LB_TIMER_NONE;
@@ -2176,6 +2229,7 @@
  */
 static LRESULT LISTBOX_HandleTimer( LB_DESCR *descr, INT index, TIMER_DIRECTION dir )
 {
+ERR("\n");
     switch(dir)
     {
     case LB_TIMER_UP:
@@ -2210,6 +2264,7 @@
  */
 static LRESULT LISTBOX_HandleSystemTimer( LB_DESCR *descr )
 {
+ERR("\n");
     if (!LISTBOX_HandleTimer( descr, descr->focus_item, LISTBOX_Timer ))
     {
         KillSystemTimer( descr->self, LB_TIMER_ID );
@@ -2230,6 +2285,7 @@
     INT index;
     TIMER_DIRECTION dir = LB_TIMER_NONE;

+ERR("\n");
     if (!descr->captured) return;

if (descr->style & LBS_MULTICOLUMN)
@@ -2276,6 +2332,7 @@
 {
     INT caret = -1;
     BOOL bForceSelection = TRUE; /* select item pointed to by focus_item */
+ERR("\n");
     if ((IS_MULTISELECT(descr)) || (descr->selected_item == descr->focus_item))
         bForceSelection = FALSE; /* only for single select list */

@@ -2389,6 +2446,7 @@
     INT caret = -1;
     WCHAR str[2];

+ERR("\n");
     str[0] = charW;
     str[1] = '\0';

@@ -2422,6 +2480,7 @@
     MEASUREITEMSTRUCT mis;
     RECT rect;

+ERR("\n");
     if (!(descr = HeapAlloc( GetProcessHeap(), 0, sizeof(*descr) )))
         return FALSE;

@@ -2451,6 +2510,7 @@
     descr->font          = 0;
     descr->locale        = 0;  /* FIXME */
     descr->lphc		 = lphc;
+//    descr->painting      = FALSE;

if (is_old_app(descr) && ( descr->style & ( WS_VSCROLL | WS_HSCROLL ) ) )
     {
@@ -2508,6 +2568,7 @@
  */
 static BOOL LISTBOX_Destroy( LB_DESCR *descr )
 {
+ERR("\n");
     LISTBOX_ResetContent( descr );
     SetWindowLongW( descr->self, 0, 0 );
     HeapFree( GetProcessHeap(), 0, descr );
@@ -2525,6 +2586,7 @@
     LPHEADCOMBO lphc = 0;
     LRESULT ret;

+ERR("msg=%u\n",msg);
     if (!descr)
     {
         if (!IsWindow(hwnd)) return 0;
@@ -3148,6 +3210,7 @@
  */
 static LRESULT WINAPI ListBoxWndProcA( HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam )
 {
+ERR("\n");
     return ListBoxWndProc_common( hwnd, msg, wParam, lParam, FALSE );
 }

@@ -3156,5 +3219,6 @@
  */
 static LRESULT WINAPI ListBoxWndProcW( HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam )
 {
+ERR("\n");
     return ListBoxWndProc_common( hwnd, msg, wParam, lParam, TRUE );
 }

[Dmitry Timoshkov]

"Alexander Yaworsky"  wrote:
> http://migusoft.ru/misc/trace.bz2
>
> with added ERR()s, see attachment
>
> I cut the tail of log just before the first err:listbox. Note that
> there are two listboxes on the form. Both are initialized ok,
> wrong things begin after WM_KEYDOWN.

Thanks, now I see where the problem is. In responce to WM_DRAWITEM the app does:

SendMessage(hwndListbox, LB_GETITEMRECT, 0, &rc);
InvalidateRect(hwndListbox, &rc, FALSE);
UpdateWindow(hwndListbox);

which leads to an infinite loop.

A test program is needed to investigate what happens under Windows in that
case. If you have time please don't wait for me, go ahead and write one,
I'll have time to look at the problem only on weekend.

P.S.
In order to eliminate a lot of garbage in the relay logs I'm using the following
filter in ~/.wine/config:

[Debug]
"RelayExclude" =
"RtlEnterCriticalSection;RtlLeaveCriticalSection;_EnterSysLevel;_LeaveSysLevel;_ConfirmSysLevel;LOCAL_Alloc;LOCAL_Lock;LOCAL_Unlock;
LOCAL_Free;GDI_GetObjPtr;GDI_ReleaseObj;WIN_GetPtr;WIN_FindWndPtr;WIN_ReleaseWndPtr;USER_Unlock"

It makes relay logs a lot smaller and much more readable, give it a try.

--
Dmitry.

[Dmitry Timoshkov]

"Alexander Yaworsky"  wrote:
> recently i tried to install some application and it hung when i
> tried to select options.
> It uses listbox with ownerdraw items with checkboxes. When listbox
> is initially painted everything is ok. But when i try to select other
> item, an extra WM_PAINT is sent to listbox when application draws item.
> This causes infinite recursion.

Could you create a message log of the problem with spy++ under windows?
I'm interested to see what happens after you press the down key.

[Dmitry Timoshkov]

"Dmitry Timoshkov"  wrote:
> "Alexander Yaworsky"  wrote:
> > http://migusoft.ru/misc/trace.bz2
> >
> > with added ERR()s, see attachment
> >
> > I cut the tail of log just before the first err:listbox. Note that
> > there are two listboxes on the form. Both are initialized ok,
> > wrong things begin after WM_KEYDOWN.
>
> Thanks, now I see where the problem is. In responce to WM_DRAWITEM the app does:
>
> SendMessage(hwndListbox, LB_GETITEMRECT, 0, &rc);
> InvalidateRect(hwndListbox, &rc, FALSE);
> UpdateWindow(hwndListbox);
>
> which leads to an infinite loop.

While browsing my old not answered e-mails I've found this one. Does
the attached patch help? It restricts the control to draw outside of
the item rect but not the whole control rect as it currently does.

[Alexander Yaworsky]

Dmitry Timoshkov wrote:
> While browsing my old not answered e-mails I've found this one. Does
> the attached patch help? It restricts the control to draw outside of
> the item rect but not the whole control rect as it currently does.

No, it does not help.

[Dmitry Timoshkov]

"Alexander Yaworsky"  wrote:
> > While browsing my old not answered e-mails I've found this one. Does
> > the attached patch help? It restricts the control to draw outside of
> > the item rect but not the whole control rect as it currently does.
>
> No, it does not help.

Yes, I know, the test I sent to wine-patches and which Alexandre already
committed confirmed that my listbox patch is wrong. Alexander, can I get
a copy of your app to see what happens? Contact me privately if you wish.