Review Request 112009: Update thumbnail support for Microsoft Windows executables and images, and use WINAPI when on Windows.

[Patrick Spendrin]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112009/#review37623
-----------------------------------------------------------



kioslave/thumbnail/CMakeLists.txt
<http://git.reviewboard.kde.org/r/112009/#comment27795>

    this could be moved out of the parentheses right?



kioslave/thumbnail/icoutils_win.cpp
<http://git.reviewboard.kde.org/r/112009/#comment27796>

    I don't think we should use dynamic loading of those functions here, only for those functions that might not be available in one of the Windows versions we support (XPsp3, Vista, 7, 8). For most of these functions you can simply use the linked version (e.g. directly use LoadLibraryEx instead of loading kernel32, finding the function, using that function etc.) That might save you quite some code. In case one of the functions is not available, you might need a workaround or a proper error handling.



kioslave/thumbnail/windowsexecreator.h
<http://git.reviewboard.kde.org/r/112009/#comment27797>

    style issue: check that you keep the eol at the end of the file (your patch seems to have "no eol at the end of file" in it)



kioslave/thumbnail/windowsexecreator.cpp
<http://git.reviewboard.kde.org/r/112009/#comment27798>

    You might simply return the output of !IcoUtils::loadIcoImageFromExe here ;-)



kioslave/thumbnail/windowsimagecreator.cpp
<http://git.reviewboard.kde.org/r/112009/#comment27799>

    See above comment.


- Patrick Spendrin


On Aug. 11, 2013, 1:59 p.m., Andrius da Costa Ribas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112009/
> -----------------------------------------------------------
> 
> (Updated Aug. 11, 2013, 1:59 p.m.)
> 
> 
> Review request for KDE Runtime, kdewin and Pali Rohár.
> 
> 
> Description
> -------
> 
> This patch intends to enable Windows exe/dll thumbnailing by using winapi. It derives from the unsubmitted patch from Pali Rohár from https://svn.reviewboard.kde.org/r/5156/ as a starting point. I've made a few adjustments on the original patch, split that patch into a common part and a icoutils-specific part, and then created the winapi-based part to replace the icoutils one on Windows (porting icoutils to windows wasn't going to be easy).
> 
> 
> Diffs
> -----
> 
>   kioslave/thumbnail/CMakeLists.txt b81339b 
>   kioslave/thumbnail/icoutils.h 6468bc1 
>   kioslave/thumbnail/icoutils.cpp 31db85d 
>   kioslave/thumbnail/icoutils_common.cpp PRE-CREATION 
>   kioslave/thumbnail/icoutils_icotools.cpp PRE-CREATION 
>   kioslave/thumbnail/icoutils_win.cpp PRE-CREATION 
>   kioslave/thumbnail/windowsexecreator.h a407982 
>   kioslave/thumbnail/windowsexecreator.cpp 9e24aee 
>   kioslave/thumbnail/windowsexethumbnail.desktop f10efef 
>   kioslave/thumbnail/windowsimagecreator.h 0b68cc6 
>   kioslave/thumbnail/windowsimagecreator.cpp 08b063d 
> 
> Diff: http://git.reviewboard.kde.org/r/112009/diff/
> 
> 
> Testing
> -------
> 
> Tested on a Windows 7 64-bit machine, with intel compiler (32-bit).
> Tested using ico files and both 32-bit and 64-bit executables and dlls, including jumbo-size icons.
> I've used QLibrary for all winapi functions in order to avoid issues with MinGW compiler, but I don't have a MinGW setup to check.
> 
> Not tested on *nix, but the original patch was not changed except for iterating order in the common part and namespacing.
> 
> 
> File Attachments
> ----------------
> 
> screenshot
>   http://git.reviewboard.kde.org/media/uploaded/files/2013/08/11/Icons.png
> 
> 
> Thanks,
> 
> Andrius da Costa Ribas
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20130812/cd43e122/attachment.htm>
-------------- next part --------------
_______________________________________________
Kde-windows mailing list
Kde-windows-RoXCvvDuEio at public.gmane.org
https://mail.kde.org/mailman/listinfo/kde-windows