----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129807/#review101945 -----------------------------------------------------------
Could you test it with an SVG file that contains text? From what I remember, painting text requires a QGuiApplication to access the font database. Additionally, I do not like the hand-made command line parsing. We have QCommandLineParser for this. src/CMakeLists.txt (line 5) <https://git.reviewboard.kde.org/r/129807/#comment68193> Is it actually useful on non-Windows systems? If not, please add a conditional similar to the Apple tool above. src/tools/ksvg2ico/ksvg2ico.cpp (line 24) <https://git.reviewboard.kde.org/r/129807/#comment68194> Trailing whitespace src/tools/ksvg2ico/ksvg2ico.cpp (line 116) <https://git.reviewboard.kde.org/r/129807/#comment68196> Trailing whitespace src/tools/ksvg2ico/ksvg2ico.cpp (line 117) <https://git.reviewboard.kde.org/r/129807/#comment68197> Missing space after comma - Christoph Feck On Jan. 11, 2017, 9:15 p.m., René J.V. Bertin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129807/ > ----------------------------------------------------------- > > (Updated Jan. 11, 2017, 9:15 p.m.) > > > Review request for KDE Frameworks, kdewin and Christoph Feck. > > > Repository: kiconthemes > > > Description > ------- > > Recently I committed a few changes to `ecm_add_app_icon` that make use of the > fact that KIconThemes includes a `ksvg2icns` utility, making it possible to > generate an application icon from an SVG file instead of from a series of PNG > files. > > With the present patch I propose to add an updated and partly rewritten > version of the ageing `svg2ico` tool from the KDEWin project to KIconThemes. > The goal is to have a proper and up-to-date tool to generate application > icons from SVG on MS Windows too. > > I considered proposing it to KDEWin instead, but in the end I liked the idea > better to provide icon conversion utilities for Mac and Windows with a > framework, where they might also serve in a less KDE-centric context (and > cross-platform development). > I also played with the idea of integrating the png2ico step into the tool, > but noticed during testing that Matthias Benkmann's png2ico supports input > sizes up to 248x248 while KDEWin's png2ico is currently limited at 48x48 or > maybe even 32x32. Lacking a MSWin development system I'm not volunteering to > update that code. > > > Diffs > ----- > > src/CMakeLists.txt efba9e6 > src/tools/ksvg2ico/CMakeLists.txt PRE-CREATION > src/tools/ksvg2ico/ksvg2ico.cpp PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/129807/diff/ > > > Testing > ------- > > On Mac OS X 10.9 with Qt 5.7.1 and KF5 5.29.0 . > > > File Attachments > ---------------- > > .ico file generated from sc-apps-calligrakarbon.svgz using ksvg2ico with > Benkmann's png2ico > > https://git.reviewboard.kde.org/media/uploaded/files/2017/01/11/aa0973dc-1be8-4d36-82dc-8f7baa053fee__apps-calligrakarbon.ico > > > Thanks, > > René J.V. Bertin > >
