list view completely mangled in WINE
Hello, in Crystal Impact's Diamond the list view is completely mangled and only barely recognizable as list view. Result in WINE: http://www.physik.fu-berlin.de/~tburnus/wine/diamond_list_wine.png Result in Windows: http://www.physik.fu-berlin.de/~tburnus/wine/diamond_list.png I'd be really pleased if someone could look at the problem. The demo version of the program is free and available from http://www.crystalimpact.com/download/diamond/demo/dm2demo.exe (In addition to this list-view problem the application does off-screen rendering which is not supported by WINE; one needs to disable rendering in order use the program. See WINE bug http://bugs.winehq.com/show_bug.cgi?id=2320 ) Tobias
"Tobias" == Tobias Burnus <burnus(a)gmx.de> writes:
Tobias> Hello, in Crystal Impact's Diamond the list view is completely Tobias> mangled and only barely recognizable as list view. Tobias> Result in WINE: Tobias> http://www.physik.fu-berlin.de/~tburnus/wine/diamond_list_wine.png Tobias> Result in Windows: Tobias> http://www.physik.fu-berlin.de/~tburnus/wine/diamond_list.png Consider running with native comctl32 as a workaround. Wine comctl32 still has a long way to go... ... Bye -- Uwe Bonnes bon(a)elektron.ikp.physik.tu-darmstadt.de Institut fuer Kernphysik Schlossgartenstrasse 9 64289 Darmstadt --------- Tel. 06151 162516 -------- Fax. 06151 164321 ----------
On Fri, Jul 30, 2004 at 05:26:57PM +0200, Tobias Burnus wrote:
Hello,
in Crystal Impact's Diamond the list view is completely mangled and only barely recognizable as list view.
Tobias, I'll try to look into it. I have been however immensely busy lately, so please remind me if I don't get back to you soon. -- Dimi.
Tobias Burnus wrote: [snip] Can you try the attached patch please? Regards, Filip Changelog: - Don't update infoPtr->dwStyle in LISTVIEW_WindoProc. It's already handled in LISTVIEW_StyleChanged and LISTVIEW_Create processing. - Fix TOOLBAR_DrawMasked to correctly use image list mask. Index: comctl32/toolbar.c =================================================================== --- comctl32/toolbar.c (revision 1) +++ comctl32/toolbar.c (working copy) @@ -658,8 +658,7 @@ /* Create src image */ hdcImage = CreateCompatibleDC(hdc); - hbmImage = CreateBitmap(cx, cy, GetDeviceCaps(hdc,PLANES), - GetDeviceCaps(hdc,BITSPIXEL), NULL); + hbmImage = CreateCompatibleBitmap(hdc, cx, cy); SelectObject(hdcImage, hbmImage); ImageList_DrawEx(himl, index, hdcImage, 0, 0, cx, cy, RGB(0xff, 0xff, 0xff), RGB(0,0,0), ILD_NORMAL); @@ -670,8 +669,8 @@ SelectObject(hdcMask, hbmMask); /* Remove the background and all white pixels */ - SetBkColor(hdcImage, ImageList_GetBkColor(himl)); - BitBlt(hdcMask, 0, 0, cx, cy, hdcImage, 0, 0, SRCCOPY); + ImageList_DrawEx(himl, index, hdcMask, 0, 0, cx, cy, + RGB(0xff, 0xff, 0xff), RGB(0,0,0), ILD_MASK); SetBkColor(hdcImage, RGB(0xff, 0xff, 0xff)); BitBlt(hdcMask, 0, 0, cx, cy, hdcImage, 0, 0, NOTSRCERASE); Index: comctl32/listview.c =================================================================== --- comctl32/listview.c (revision 1) +++ comctl32/listview.c (working copy) @@ -117,7 +117,6 @@ * -- LVM_GETISEARCHSTRINGW, LVM_GETISEARCHSTRINGA * -- LVM_GETTILEINFO, LVM_SETTILEINFO * -- LVM_GETTILEVIEWINFO, LVM_SETTILEVIEWINFO - * -- LVM_GETTOOLTIPS, LVM_SETTOOLTIPS * -- LVM_GETUNICODEFORMAT, LVM_SETUNICODEFORMAT * -- LVM_GETVIEW, LVM_SETVIEW * -- LVM_GETWORKAREAS, LVM_SETWORKAREAS @@ -8782,11 +8781,6 @@ if (!infoPtr && (uMsg != WM_CREATE)) return DefWindowProcW(hwnd, uMsg, wParam, lParam); - if (infoPtr) - { - infoPtr->dwStyle = GetWindowLongW(hwnd, GWL_STYLE); - } - switch (uMsg) { case LVM_APPROXIMATEVIEWRECT:
Hello, Filip Navara wrote:
Can you try the attached patch please? The patch improves the situation a lot! Even Save List works now! Thanks!
Before: http://www.physik.fu-berlin.de/~tburnus/wine/diamond_list_wine.png Afterwards: http://www.physik.fu-berlin.de/~tburnus/wine/diamond_list_wine2.png The only problem I still have is the horizontal scrolling in listviews. If I reduce the window size, horizontal scrollbars appear and everything looks fine (http://www.physik.fu-berlin.de/~tburnus/wine/diamond_list_wine3a.png). Now, I click on the scrollbar to scroll to the rightmost columns; while the header is correct, the list itself contains the FIRST columns not the columns which belong to the column headers http://www.physik.fu-berlin.de/~tburnus/wine/diamond_list_wine3b.png Regards, Tobias
Tobias Burnus wrote:
The patch improves the situation a lot! Even Save List works now! Thanks!
Great! [snip]
Now, I click on the scrollbar to scroll to the rightmost columns; while the header is correct, the list itself contains the FIRST columns not the columns which belong to the column headers http://www.physik.fu-berlin.de/~tburnus/wine/diamond_list_wine3b.png
Should be fixed in the attached patch, but there is still one problem with refreshing of the list view after shrinking the column headers. Unfortunetly I don't have time to fix that. Regards, Filip
Hello, Tobias Burnus wrote:
The patch improves the situation a lot! Even Save List works now! Thanks! I have another strangeness with multi-select lists (e.g. the lists in Diamond but this might affect other lists as well):
* I go to a line, press shift and then the arrow keys. If I use arrow down, I can select as many lines as I want. If I use arrow up, I can only select two lines: The line I'm in and the one below, the others further below are unselected as I move upwards. * Multi-select with ctrl doesn't work using the keyboard. Select a line, press ctrl, move up/down to another line and press space. Expected: This line is selected as well. Actual result: Nothing happens (Not a bug, but a missing handy feature: In Windows, double clicking between two column headers (cursor: <-|->) adjusts the column-width so that everything fits. In WINE nothing happens. (Work-around: go to the same place and adjust manually.)) Regards, Tobias, who hopes that the submitted patches get applied to CVS.
On Sat, Jul 31, 2004 at 05:52:21AM +0200, Filip Navara wrote:
Tobias Burnus wrote: [snip]
Can you try the attached patch please?
Changelog: - Don't update infoPtr->dwStyle in LISTVIEW_WindoProc. It's already handled in LISTVIEW_StyleChanged and LISTVIEW_Create processing.
- if (infoPtr) - { - infoPtr->dwStyle = GetWindowLongW(hwnd, GWL_STYLE); - } - switch (uMsg) { case LVM_APPROXIMATEVIEWRECT:
Can you please explain why updating it is a problem? Yes, we may be doing more work than needed, but at most it should be a no-op. In fact, we used to call 'GetWindowLongW(hwnd, GWL_STYLE)' every time we needed to get to the dwStyle, so updating it in there should be OK. BTW, I've tried to install the demo, but the installer dies on my version of wine. Does it work for you, or did you use Windows for the installation? -- Dimi.
Dimitrie O. Paun wrote:
Can you please explain why updating it is a problem? Yes, we may be doing more work than needed, but at most it should be a no-op. In fact, we used to call 'GetWindowLongW(hwnd, GWL_STYLE)' every time we needed to get to the dwStyle, so updating it in there should be OK.
Unfortunetly I can't explain it, because I don't understand it myself. I have inserted debug messages on all the places where the listview display mode can be changed and none of them appeared. After going once more through the whole code I found this. Obviously it seemed redundant and the only reason for doing that was workarounding problems with changed WS_VSCROLL and WS_HSCROLL styles (which correctly don't recieve WM_STYLECHANGED notifications from the scrolling code). So I tried removing it and ... whoa ... to my surprise it started to work.
BTW, I've tried to install the demo, but the installer dies on my version of wine. Does it work for you, or did you use Windows for the installation?
Yes, I installed it on Windows and even most of done most of the testing on Windows with Wine COMCTL32. Before sending the patch to wine-patches I checked if it works as desired on Wine... - Filip
On Tue, Aug 03, 2004 at 01:42:28AM +0200, Filip Navara wrote:
Unfortunetly I can't explain it, because I don't understand it myself. I
In which case I must object to the patch :( It seems we're just hiding a problem in a very convoluted way, don't you think?
have inserted debug messages on all the places where the listview display mode can be changed and none of them appeared. After going once more through the whole code I found this. Obviously it seemed redundant and the only reason for doing that was workarounding problems with changed WS_VSCROLL and WS_HSCROLL styles (which correctly don't recieve WM_STYLECHANGED notifications from the scrolling code). So I tried removing it and ... whoa ... to my surprise it started to work.
The problem is that WM_STYLECHANGED is sometimes sent, sometimes not, and it seems so much easier to just update it every time we come in so we can be sure we're using the latest value, as we should. It's redundant, I agree, but it's neglijable in terms of performance so I've put it in due to it's simplicity. This way the dwStyle has a very simple to understand semantics. In fact, I've meant to remove it eventually, just like you did, but only as an optimization. However, doing so to fix a bug in a way that we don't understand worries me greatly. I'd rather keep it in, and fix the problem. -- Dimi.
Dimitrie O. Paun wrote:
On Tue, Aug 03, 2004 at 01:42:28AM +0200, Filip Navara wrote:
Unfortunetly I can't explain it, because I don't understand it myself. I
In which case I must object to the patch :( It seems we're just hiding a problem in a very convoluted way, don't you think?
Well, I agree, the patch shouldn't go in, but I don't have currently time to investigate the issue behind this. Maybe in a week or so. If you want to give it a try, feel free to do so... - Filip
Dimitrie O. Paun wrote: [snip]
The problem is that WM_STYLECHANGED is sometimes sent, sometimes not, and it seems so much easier to just update it every time we come in so we can be sure we're using the latest value, as we should. It's redundant, I agree, but it's neglijable in terms of performance so I've put it in due to it's simplicity. This way the dwStyle has a very simple to understand semantics. In fact, I've meant to remove it eventually, just like you did, but only as an optimization. However, doing so to fix a bug in a way that we don't understand worries me greatly. I'd rather keep it in, and fix the problem.
I did another test. I intserted a code into the ListView window procedure to print a message every time the internal dwStyle member doesn't match the one returned by GetWindowLong with GWL_STYLE. To my little surprise they were different right from the beginning (even for WM_CREATE message). The one returned by GetWindowLong didn't contain the listview specific flags, WS_VSCROLL/WS_HSCROLL sometimes didn't match (as I explained why) and for the WS_CREATE message the WS_VISIBLE flag was different. I searched my local MSDN copy in order to find why this happens and the only relevant article I found is this: http://support.microsoft.com/default.aspx?scid=kb;EN-US;Q83366. The "SUMMARY" part of the article sort of explains why this happens (at least according to my understanding) and so I think the patch is OK. Do you agree? - Filip
On Tue, Aug 03, 2004 at 03:17:27AM +0200, Filip Navara wrote:
I did another test. I intserted a code into the ListView window procedure to print a message every time the internal dwStyle member doesn't match the one returned by GetWindowLong with GWL_STYLE. To my little surprise they were different right from the beginning (even for WM_CREATE message). The one returned by GetWindowLong didn't contain the listview specific flags,
If so, how did the listview work at all until now?!?
WS_VSCROLL/WS_HSCROLL sometimes didn't match (as I explained why) and for the WS_CREATE message the WS_VISIBLE flag was different. I searched my local MSDN copy in order to find why this happens and the only relevant article I found is this: http://support.microsoft.com/default.aspx?scid=kb;EN-US;Q83366. The
Good stuff, maybe we should link to it from WineHQ
"SUMMARY" part of the article sort of explains why this happens (at least according to my understanding) and so I think the patch is OK. Do you agree?
Almost. If it does go in though, we need to: -- fix a few more places where we look at WS_[VH]SCROLL -- understand why did the listview work at all if the LVS_ stuff was missing -- do an audit (or add a Janitorial task) to audit and fix the other controls as well. (of course, only the 1st point is really needed for the patch to go in, but the other two would be nice to have). -- Dimi.
Dimitrie O. Paun wrote: [snip]
Almost. If it does go in though, we need to: -- fix a few more places where we look at WS_[VH]SCROLL
Ok, I have that done in my local tree and will send it tonight propably.
-- understand why did the listview work at all if the LVS_ stuff was missing
I tried to run RegEdit with the same version of COMCTL32 and it preserved the listview specific flags. Maybe somebody with access to MFC source code (that is used by the Diamond application) can find out more. The application calls "CCtrlView::CCtrlView(WC_LISTVIEW, WS_VISIBLE | WS_CHILD | WS_BORDER)", so it must propably inject the listview flags later on, but I don't know enough about MFC internals to tell you how it does that (the window procedure of the list view is subclassed BTW). - Filip
Dimitrie O. Paun wrote:
On Tue, Aug 03, 2004 at 01:42:28AM +0200, Filip Navara wrote:
have inserted debug messages on all the places where the listview display mode can be changed and none of them appeared. After going once more through the whole code I found this. Obviously it seemed redundant and the only reason for doing that was workarounding problems with changed WS_VSCROLL and WS_HSCROLL styles (which correctly don't recieve WM_STYLECHANGED notifications from the scrolling code). So I tried removing it and ... whoa ... to my surprise it started to work.
The problem is that WM_STYLECHANGED is sometimes sent, sometimes not, and it seems so much easier to just update it every time we come in so we can be sure we're using the latest value, as we should.
The toolbar control works fine just using WM_STYLECHANGED to notify when it needs to update its internal state and it seems to send the message for all style changes that it needs (it doesn't depend on WS_ styles).
It's redundant, I agree, but it's neglijable in terms of performance so I've put it in due to it's simplicity. This way the dwStyle has a very simple to understand semantics. In fact, I've meant to remove it eventually, just like you did, but only as an optimization. However, doing so to fix a bug in a way that we don't understand worries me greatly. I'd rather keep it in, and fix the problem.
Rob
Hello, Dimitrie O. Paun wrote:
BTW, I've tried to install the demo, but the installer dies on my version of wine. Does it work for you, or did you use Windows for the installation?
I used WINE (more precisely CrossOver Office 3.0.1 and CXO nightly) to install the program, which worked flawlessly. Using the CXO config file, it also installed with the WINE CVS version. Tobias
participants (5)
-
Dimitrie O. Paun -
Filip Navara -
Robert Shearman -
Tobias Burnus -
Uwe Bonnes