- User Since
- Mar 5 2017, 7:19 PM (75 w, 6 d)
Nov 5 2017
Small nitpick on a coding style issue, but the code looks good, thanks for your work!
Oct 24 2017
LGTM, but I agree you could get rid of fitsplugin.h in lib/CMakeLists.txt
Oct 15 2017
Some minor requests, but we are getting there!
Oct 7 2017
Sep 30 2017
Sep 27 2017
I think the basic problem here that the static Qt plugin is put inside a shared library
Sep 24 2017
Why is this restricted to Linux? Does it work out of the box on Windows and macOS?
Sep 10 2017
The fits decoding is moved to imageformats as you requested.
I had huge problems with the static plugin registration, the old way did not work at all. I finally figured out a workaround hack, but I had to spend couple of hours on this. Explanation is written in app/CMakeLists.txt.
Looks good, thanks for simplifying exception handling code in Exiv2ImageLoader.
Aug 20 2017
- You don't need to provide write support: some formats are read-only (see README.md)
- Adding fits format to kimageformats also means Dolphin and the file dialogs can show thumbnails for .fits files, maybe that could be useful?
- It is true that you would have to duplicate some code to add metadata support in Gwenview. The right way to avoid that would be to create a Qt library to handle the fits image format and use it in kimageformats and Gwenview (and KStars maybe?). That's more work to do though, so I can understand if you don't want to go that way, but if you are already duplicating code from KStars to Gwenview to add fits support (I haven't checked if it is the case), then creating a library would make sense.
Aug 15 2017
It works for me:
[perso ~/src/colorpick/build] $ which gcc /usr/lib/ccache/gcc [perso ~/src/colorpick/build] $ cmake -G Ninja .. -- The C compiler identification is GNU 6.2.0 -- The CXX compiler identification is GNU 6.2.0 -- Check for working C compiler using: Ninja -- Check for working C compiler using: Ninja -- works -- Detecting C compiler ABI info -- Detecting C compiler ABI info - done -- Detecting C compile features -- Detecting C compile features - done -- Check for working CXX compiler using: Ninja -- Check for working CXX compiler using: Ninja -- works -- Detecting CXX compiler ABI info -- Detecting CXX compiler ABI info - done -- Detecting CXX compile features -- Detecting CXX compile features - done -- Configuring done -- Generating done -- Build files have been written to: /home/perso/src/colorpick/build [perso ~/src/colorpick/build] $ ccache -s cache directory /home/perso/.ccache primary config /home/perso/.ccache/ccache.conf secondary config (readonly) /etc/ccache.conf cache hit (direct) 297 cache hit (preprocessed) 15 cache miss 1390 called for link 1126 called for preprocessing 167 multiple source files 1 compile failed 80 preprocessor error 7 no input file 64 files in cache 1379 cache size 39.5 MB max cache size 1.1 GB [perso ~/src/colorpick/build] $ ninja [1/14] Automatic moc for target colorpick Generating moc_coloreditor.cpp Generating moc_colorpicker.cpp Generating moc_componenteditor.cpp Generating moc_contrastpreview.cpp Generating moc_window.cpp [14/14] Linking CXX executable src/colorpick [perso ~/src/colorpick/build] $ ccache -s cache directory /home/perso/.ccache primary config /home/perso/.ccache/ccache.conf secondary config (readonly) /etc/ccache.conf cache hit (direct) 297 cache hit (preprocessed) 15 cache miss 1401 called for link 1127 called for preprocessing 167 multiple source files 1 compile failed 80 preprocessor error 7 no input file 64 files in cache 1412 cache size 40.0 MB max cache size 1.1 GB
Looks good, but I think it would be better if most of the work were in https://cgit.kde.org/kimageformats.git/ instead, unless it is not possible for some reason. Doing it that way would avoid adding a bunch of ifs in Gwenview, but more importantly, would bring support to fits format to all KDE applications. It might still be necessary to add some fits-specific code to show the meta information in Gwenview.
Is this really necessary? Building with ccache is usually just a matter of making sure /usr/lib/ccache comes first in $PATH. Or am I missing something?
Aug 8 2017
Jul 12 2017
Jul 2 2017
Jun 24 2017
Ping, do you think this can go in?
Jun 21 2017
Ah, I got stuck with that too! but I solved it in a different way: I merged the direction (left-to-right, right-to-left) and center origin concepts into 3 modes, which can be switched with F2, F3 and F4. You can build the work branch of my scratch repo (firstname.lastname@example.org:clones/kruler/gateau/kruler) to give it a try.
Jun 19 2017
Jun 14 2017
Jun 6 2017
I love the idea, but what about caching? Cached thumbnails are supposed to follow https://specifications.freedesktop.org/thumbnail-spec/thumbnail-spec-latest.html#THUMBSIZE, which still, sadly, says 256.
Jun 3 2017
May 28 2017
Ah yes, using keyPressEvent() sounds like a good idea.
May 14 2017
Thanks a lot for fixing the mess I created. I just added a note about this to the article on my blog.