Re: [PATCH] TortoiseIDiff optionally using FreeImage.dll for image loading

Will Wilson wrote:
> Okay,
> This time with a bit more polish (TM)... :)

Thanks! Even more polished than my usual code :)

>>> To try it out simply compile a patched TortoiseIDiff.exe, drop the
>>> FreeImage.dll (see http://freeimage.sourceforge.net/download.html)
>> next
>>> to it & run the app.
>> Did that, but crashed immediately :)
>
> Hey, what were you expecting? That I tested it? ;) It worked standalone,
> without command-line parameters - although purely by chance!

I just happen to have set two paths to *.ico files in my debugger when I
started it - that's why it crashed ;)

>> Here, you could just use TCHAR instead of the #ifdef and using
> wchar_t*
>> and char*.
>
> Agreed & updated. I've made use of the MSLU for the past four years & so
> don't touch many _T macros these days...

Those macros are very useful I think. And since TSVN doesn't support
Win9x anymore, we don't have to deal with MSLU either.
Come to think of it: we wouldn't have to use the T macros either: we
only compile with UNICODE defined anyway.

>> - HMODULE hLib = LoadLibrary(_T("gdiplus.dll"));
>> - if (hLib)
>> {
>> - // we have gdiplus, so try loading the picture with that
>> one
>> - if (GdiplusStartup( &gdiplusToken, &gdiplusStartupInput,
>> NULL )==Ok)
>> - {
>> + HMODULE hFreeImageLib =
> LoadLibrary(_T("FreeImage.dll"));
>> + FreeImage_GetVersion_t FreeImage_GetVersion = NULL;
>>
>> You must not remove those lines. The FreeImage lib also uses GDI+ and
>> therefore needs the GdiplusStartup() call to work. That's also why it
>> crashed on me when I first tried it.
>
> Yep, also now fixed. However it still seems odd that the GDI+ DLL is
> released before GdiplusShutdown is called.

We only load the gdiplus.dll to check if it's installed on the system
and can be loaded. After that check, we can release it immediately
again. GdiplusStartup() load the dll itself, and due to reference
counting of loaded dll's we don't have to worry to actually unload the
dll completely by releasing it.

>> You've also removed our custom handling of icons. Since GDI+ only
> loads
>> the first icon in an icon file, we have to handle icons ourselves to
>> get
>> all the sizes and formats.
>
> What, you want icon support as well? ;) Fixed.

Well, sure I want that! After all, it took me quite a while to get that
working :)

Committed your patch in revision 7629 (with some minor tweaks by me).

The next step should be to have a function in CPicture which returns all
the supported image formats (so we can adjust the file-open dialog
filemask accordingly).

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tortoisesvn.tigris.org
For additional commands, e-mail: dev-help@tortoisesvn.tigris.org