Re: [8/21] comctl32: Add basic structure for IImageList interface
Owen Rudge wrote:
--- dlls/comctl32/imagelist.c | 318 ++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 317 insertions(+), 1 deletions(-) Hi.
+/************************************************************************* + * IImageList implementation + */ + +typedef struct { + const IImageListVtbl *lpVtbl; + LONG ref; + HIMAGELIST hImageList; +} ImageListImpl; + You can't do that. HIMAGELIST should be the same thing as IImageList. See here: 1) from commctrl.h --- #ifdef __cplusplus FORCEINLINE HIMAGELIST IImageListToHIMAGELIST(struct IImageList *himl) { return reinterpret_cast<HIMAGELIST>(himl); } #else #define IImageListToHIMAGELIST(himl) ((HIMAGELIST)(himl)) #endif --- 2) SHGetImageList docs from http://msdn.microsoft.com/en-us/library/bb762185%28VS.85%29.aspx ---* Remarks* The *IImageList* pointer type, such as that returned in the /ppv/ parameter, can be cast as an HIMAGELIST as needed; for example, for use in a list view. Conversely, an HIMAGELIST can be cast as a pointer to an *IImageList.* ---
You can't do that. HIMAGELIST should be the same thing as IImageList.
Hmm. Looking at the HIMAGELIST/IImageList internals in a debugger on Windows, the layout appears to be entirely different to the HIMAGELIST layout defined in dlls/comctl32/imagelist.h (as you'd expect -the vtable, etc, needs accommodating). Judging by the comments in imagelist.h, the layout of the structure was designed to imitate the Windows layout (or an earlier version thereof). This layout is not officially published anywhere, however (nor is the new one), so I presume it will be acceptable to modify it to fit the vtable and reference count, etc, in? It might conceivably cause problems for any pre-comctl32-v6 app that tries to poke around the internals (not that they should be anyway), but I can't see any way around that other than having full WinSXS support implemented and having two separate versions of comctl32, as in Windows itself. Cheers, -- Owen Rudge http://www.owenrudge.net/
This layout is not officially published anywhere, however (nor is the new one), so I presume it will be acceptable to modify it to fit the vtable and reference count, etc, in? It might conceivably cause problems for any pre-comctl32-v6 app that tries to poke around the internals (not that they should be anyway), but I can't see any way around that other than having full WinSXS support implemented and having two separate versions of comctl32, as in Windows itself.
Hmm, to clarify this, I think I've managed to answer my own question, in that I gather I should just make the changes to HIMAGELIST to incorporate the vtable, etc, into it, abolishing ImageListImpl. I also understand now that I shouldn't have written that little test program, as I did somewhat naïvely, to verify whether the internal structure on Windows matched that in the Wine header, as it could be construed as disassembly. This is a mistake I won't be making again. I'll make the appropriate changes tomorrow and re-submit if there are no other issues that need dealing with. Cheers, -- Owen Rudge http://www.owenrudge.net/
Owen Rudge wrote:
This layout is not officially published anywhere, however (nor is the new one), so I presume it will be acceptable to modify it to fit the vtable and reference count, etc, in? Personally I don't see another way to do so. It might conceivably cause problems for any pre-comctl32-v6 app that tries to poke around the internals (not that they should be anyway), Exactly. but I can't see any way around that other than having full WinSXS support implemented and having two separate versions of comctl32, as in Windows itself. Agreed. This is actually what stopped me when I tried to implement this interface in my local tree. Our current HIMAGELIST layout already is a result of debugging - offsets are about to match. The ideal way is to match SxS system here and add aV6 target (btw, listview has some minor visual difference in v6 comparing to <=5 caused by different padding). Hmm, to clarify this, I think I've managed to answer my own question, in that I gather I should just make the changes to HIMAGELIST to incorporate the vtable, etc, into it, abolishing ImageListImpl. I also understand now that I shouldn't have written that little test program, as I did somewhat naïvely, to verify whether the internal structure on Windows matched that in the Wine header, as it could be construed as disassembly. This is a mistake I won't be making again. Don't know if getting such dumps is allowed or not. But we got current layout some way... I'll make the appropriate changes tomorrow and re-submit if there are no other issues that need dealing with.
Cheers,
On Mi, 2009-08-12 at 23:31 +0100, Owen Rudge wrote:
You can't do that. HIMAGELIST should be the same thing as IImageList.
Hmm. Looking at the HIMAGELIST/IImageList internals in a debugger on Windows, the layout appears to be ...
:-( Disassembling Windows Code is not allowed for Wine. You should have know that and you should know the result. -- By by ... Detlef
Hi Detler,
Disassembling Windows Code is not allowed for Wine. You should have know that and you should know the result.
I'd just like to explain what it was exactly that I did, to possibly clear up any confusion. After submitting a set of patches, and receiving the comment that HIMAGELIST/IImageList should be the same, I was wondering about maintaining internal compatibility with the "old" structure, since it looked to me as if the existing HIMAGELIST structure had been specifically ordered to be compatible with Windows (see the commit at http://tinyurl.com/mfwl54 for instance - I presumed there was a reason behind this involving application compatibility). I then wrote a very simple little test program which took the existing Wine structural definition of HIMAGELIST, and then cast that onto the Windows structure, and performed a couple of tests to compare various values (eg, the "magic" value), to see if they were compatible. This was the extent of my "debugging" of the code, and I did not then make use of any of the seemingly-nonsensical values the program returned. The code in the header file in the patch (http://tinyurl.com/l4ffln) is the only part that was "affected" by this, and I simply moved the two structure members I had previous defined in another structure to the HIMAGELIST structure. I made no effort to further investigate or make compatible the structure with the native Windows structure. Additionally, at no time did I actively "disassemble" any Windows code, or do anything more than compare the first few values in the structure with this test program. I have since been made aware that even this is possibly unacceptable, and I understand that I may have made a mistake in doing so. Possibly I screwed up a bit, I accept that. I would just like to reiterate however that it was a very crude form of "debugging", as detailed above, and that no changes to the code were ultimately made as a result of it. No other code I have ever written has involved similar practices, and I would personally argue that this piece of code itself is for the largest part unaffected. I appreciate your comments, and hopefully this message will help explain things. It was obviously never my intention to put the project into jeopardy by replicating MS code directly (and, of course, this is something I have not done anyway). Cheers, -- Owen Rudge http://www.owenrudge.net/
participants (3)
-
Detlef Riekenberg -
Nikolay Sivov -
Owen Rudge