[Olena-patches] last-svn-commit-179-g15a0571 Fix and improve the Magick++ I/O API wrapper.

* mln/io/magick/load.hh (io::magick::do_it): Enclose these helpers... (io::magick::impl::do_it): ...in a sub-namespace. (io::magick::load): Ensure a Magick++'s Quantum is an 8-bit value. Use a pixel view (Magick::Pixels) to access to pixel values. No longer pass the input filename to the `do_it' helper. Simplify the code. Improve the documentation. Aesthetic changes. * mln/io/magick/save.hh (io::magick::get_color): Enclose these helpers... (io::magick::impl::get_color): ...in a sub-namespace. Properly use Magick::Color. Properly pass the width and the height of the image. Use a pixel view (Magick::Pixels) to access to pixel values. Simplify the code. Aesthetic changes. * tests/io/magick/load.cc, * tests/io/magick/save.cc: Properly initialize Magick++. Exercice more cases. * tests/io/magick/Makefile.am (MOSTLYCLEANFILES): Update the list of files created by tests. --- milena/ChangeLog | 28 ++++++ milena/mln/io/magick/load.hh | 184 ++++++++++++++++++++---------------- milena/mln/io/magick/save.hh | 140 +++++++++++++++++----------- milena/tests/io/magick/Makefile.am | 8 +- milena/tests/io/magick/load.cc | 72 ++++++++++++-- milena/tests/io/magick/save.cc | 86 ++++++++++++++--- 6 files changed, 354 insertions(+), 164 deletions(-) diff --git a/milena/ChangeLog b/milena/ChangeLog index 214b7e6..b4e98cd 100644 --- a/milena/ChangeLog +++ b/milena/ChangeLog @@ -1,3 +1,31 @@ +2010-07-19 Roland Levillain &lt;roland(a)lrde.epita.fr&gt; + + Fix and improve the Magick++ I/O API wrapper. + + * mln/io/magick/load.hh + (io::magick::do_it): Enclose these helpers... + (io::magick::impl::do_it): ...in a sub-namespace. + (io::magick::load): Ensure a Magick++'s Quantum is an 8-bit value. + Use a pixel view (Magick::Pixels) to access to pixel values. + No longer pass the input filename to the `do_it' helper. + Simplify the code. + Improve the documentation. + Aesthetic changes. + * mln/io/magick/save.hh + (io::magick::get_color): Enclose these helpers... + (io::magick::impl::get_color): ...in a sub-namespace. + Properly use Magick::Color. + Properly pass the width and the height of the image. + Use a pixel view (Magick::Pixels) to access to pixel values. + Simplify the code. + Aesthetic changes. + * tests/io/magick/load.cc, + * tests/io/magick/save.cc: + Properly initialize Magick++. + Exercice more cases. + * tests/io/magick/Makefile.am (MOSTLYCLEANFILES): + Update the list of files created by tests. + 2010-04-26 Roland Levillain &lt;roland(a)lrde.epita.fr&gt; Clean Milena's tests' outputs during `make mostlyclean'. diff --git a/milena/mln/io/magick/load.hh b/milena/mln/io/magick/load.hh index 11d17bd..662e67c 100644 --- a/milena/mln/io/magick/load.hh +++ b/milena/mln/io/magick/load.hh @@ -1,4 +1,4 @@ -// Copyright (C) 2009 EPITA Research and Development Laboratory (LRDE) +// Copyright (C) 2009, 2010 EPITA Research and Development Laboratory (LRDE) // // This file is part of Olena. // @@ -28,13 +28,21 @@ /// \file /// -/// Define a function which loads an image of kind magick with -/// given path. +/// \brief Image intput routines based on Magick++. +/// +/// Do not forget to call Magick::InitializeMagick(*argv) +/// <em>before</em> using any of these functions, as advised by the +/// GraphicsMagick documentation +/// (http://www.graphicsmagick.org/Magick++/Image.html). + +# include <cstdlib> + +# include <Magick++.h> # include <mln/core/image/image2d.hh> + # include <mln/value/int_u8.hh> # include <mln/value/rgb8.hh> -# include <Magick++.h> namespace mln @@ -46,66 +54,83 @@ namespace mln namespace magick { - /*! Load a magick image in a Milena image. - * - * \param[out] ima A reference to the image which will receive - * data. - * \param[in] filename The source. - */ + /** Load data from a file into a Milena image using Magick++. + + \param[out] ima The image data are loaded into. + \param[in] filename The name of the input file. */ template <typename I> void load(Image<I>& ima, const std::string& filename); - /*! Load a magick image in a tiled image. - * - * \param[out] ima A reference to the image which will receive - * data. - * \param[in] filename The source. - */ - /*template <typename T> - void load(Image<tiled2d<T> >& ima, const std::string& filename);*/ + + // FIXME: Unfinished? +#if 0 + /** Load data from a file into a Milena tiled image using + Magick++. + + \param[out] ima The image data are loaded into. + \param[in] filename The name of the input file. */ + template <typename T> + void load(Image<tiled2d<T> >& ima, const std::string& filename); +#endif # ifndef MLN_INCLUDE_ONLY - inline - bool do_it(const value::rgb8& in, bool& out, const std::string& filename) + namespace impl { - if (in.red() == 255u && in.green() == 255u && in.blue() == 255u) + + inline + bool + do_it(const value::rgb8& in, bool& out) { - out = true; + if (in.red() != in.green() || in.green() != in.blue()) + { + std::cerr << + "error: attempt to load what looks like a color\n" + "(mln::value::rgb8) image into a Boolean (bool) image" << + std::endl; + return false; + } + if (in.red() != 0 && + in.red() != mln_max(value::rgb8::red_t)) + { + std::cerr << + "error: attempt to load what looks like a grayscale\n" + "(mln::value::int_u8) image into a Boolean (bool) image" << + std::endl; + return false; + } + + out = (in.red() != 0); return true; } - if (in.red() == 0u && in.green() == 0u && in.blue() == 0u) + + inline + bool + do_it(const value::rgb8& in, value::int_u8& out) { - out = false; + if (in.red() != in.green() || in.green() != in.blue()) + { + std::cerr << + "error: attempt to load what looks like a color\n" + "(mln::value::rgb8) image into a grayscale\n" + "(mln::int_u8 values) image" << std::endl; + return false; + } + + out = in.red(); return true; } - if (in.red() == in.green() && in.green() == in.blue()) - std::cerr << "error: trying to load '" << filename << "' which is a grayscale image into a bool image" << std::endl; - else - std::cerr << "error: trying to load '" << filename << "' which is a truecolor image into a bool image" << std::endl; - return false; - } - inline - bool do_it(const value::rgb8& in, value::int_u8& out, const std::string& filename) - { - if (in.red() == in.green() && in.green() == in.blue()) + inline + bool + do_it(const value::rgb8& in, value::rgb8& out) { - out = in.red(); + out = in; return true; } - std::cerr << "error: trying to load '" << filename << "' which is a truecolor image into a grayscale image" << std::endl; - return false; - } - inline - bool do_it(const value::rgb8& in, value::rgb8& out, const std::string& filename) - { - (void) filename; - out = in; - return true; - } + } // end of namespace mln::io::magick::impl template <typename I> @@ -114,58 +139,51 @@ namespace mln { trace::entering("mln::io::magick::load"); + // Ensure a Magick++'s Quantum is an 8-bit value. + mln::metal::equal<Magick::Quantum, unsigned char>::check(); + I& ima = exact(ima_); - //std::ifstream file(filename.c_str()); - //if (! file) - //{ - // std::cerr << "error: cannot open file '" << filename << "'!"; - // abort(); - //} - - Magick::Image im_file(filename); - im_file.modifyImage(); - im_file.type(Magick::TrueColorType); - int columns = im_file.columns(); - int rows = im_file.rows(); - /*std::cout << "width: " <<columns << std::endl; - std::cout << "height: " <<rows << std::endl; - std::cout << "depth: " <<im_file.depth() << std::endl; - std::cout << "format: " <<im_file.format() << std::endl; - std::cout << "magick: " <<im_file.magick() << std::endl;*/ - - const Magick::PixelPacket *pixel_cache = im_file.getConstPixels(0, 0, columns, rows); - - algebra::vec<mln_site_(I)::dim, unsigned int> vmin; - algebra::vec<mln_site_(I)::dim, unsigned int> vmax; - vmin[0] = 0; - vmin[1] = 0; - vmax[0] = rows - 1; - vmax[1] = columns - 1; - mln_site(I) pmin(vmin); - mln_site(I) pmax(vmax); + // FIXME: Handle Magick++'s exceptions (see either + // ImageMagick++'s or GraphicsMagick++'s documentation). + Magick::Image magick_ima(filename); + magick_ima.read(filename); + magick_ima.type(Magick::TrueColorType); + int nrows = magick_ima.rows(); + int ncols = magick_ima.columns(); + mln_site(I) pmin(0, 0); + mln_site(I) pmax(nrows - 1, ncols - 1); mln_concrete(I) result(box<mln_site(I)>(pmin, pmax)); initialize(ima, result); + + Magick::Pixels view(magick_ima); + // Note that `ncols' is passed before `nrows'. + Magick::PixelPacket* pixels = view.get(0, 0, ima.ncols(), ima.nrows()); mln_piter(I) p(ima.domain()); for_all(p) { - const Magick::PixelPacket *pixel = pixel_cache + (int) p.to_site().to_vec()[0] * columns - + (int) p.to_site().to_vec()[1]; - // FIXME: Quantum = 16bits but rgb is 8bits - value::rgb8 pix(pixel->red % 256, pixel->green % 256, pixel->blue % 256); + value::rgb8 c(pixels->red, pixels->green, pixels->blue); mln_value(I) res; - if (!do_it(pix, res, filename)) - abort(); + if (!impl::do_it(c, res)) + { + std::cerr << "while trying to load `" << filename << "'" + << std::endl; + abort(); + } ima(p) = res; + ++pixels; } trace::exiting("mln::io::magick::load"); } - /*template<typename T> + // FIXME: Unfinished? +#if 0 + template<typename T> inline - void load(Image<tiled2d<T> >& ima_, const std::string& filename) + void + load(Image<tiled2d<T> >& ima_, const std::string& filename) { trace::entering("mln::io::magick::load"); @@ -175,8 +193,8 @@ namespace mln ima = result; trace::exiting("mln::io::magick::load"); - }*/ - + } +#endif # endif // ! MLN_INCLUDE_ONLY diff --git a/milena/mln/io/magick/save.hh b/milena/mln/io/magick/save.hh index 0af7d21..d524564 100644 --- a/milena/mln/io/magick/save.hh +++ b/milena/mln/io/magick/save.hh @@ -1,4 +1,4 @@ -// Copyright (C) 2009 EPITA Research and Development Laboratory (LRDE) +// Copyright (C) 2009, 2010 EPITA Research and Development Laboratory (LRDE) // // This file is part of Olena. // @@ -28,16 +28,23 @@ /// \file /// -/// Define a function which saves an image of kind magick with -/// given path. +/// \brief Image output routines based on Magick++. /// -/// \todo At the moment it works; is it a miracle? +/// Do not forget to call Magick::InitializeMagick(*argv) +/// <em>before</em> using any of these functions, as advised by the +/// GraphicsMagick documentation +/// (http://www.graphicsmagick.org/Magick++/Image.html). + +# include <cstdlib> + +# include <Magick++.h> -# include <mln/core/image/image2d.hh> # include <mln/metal/equal.hh> + +# include <mln/core/image/image2d.hh> + # include <mln/value/int_u8.hh> # include <mln/value/rgb8.hh> -# include <Magick++.h> namespace mln @@ -49,84 +56,106 @@ namespace mln namespace magick { - /*! Save a Milena image in a magick image. - * - * \param[out] ima A reference to the image to save. - * \param[in] filename The output. - */ + /** Save a Milena image into a file using Magick++. + + \param[out] ima The image to save. + \param[in] filename The name of the output file. */ template <typename I> - void save(const Image<I>& ima, - const std::string& filename); + void + save(const Image<I>& ima, const std::string& filename); - /*! Save a Milena tiled image in a magick image. - * - * \param[out] ima A reference to the image to save. - * \param[in] filename The output. - */ - /*template <typename T> - void save(Image< tiled2d<T> >& ima, - const std::string& filename);*/ + + // FIXME: Unfinished? +#if 0 + /** Save a Milena tiled image into a file using Magick++. + + \param[out] ima The image to save. + \param[in] filename The name of the output file. */ + template <typename T> + void + save(const Image< tiled2d<T> >& ima, const std::string& filename); +#endif # ifndef MLN_INCLUDE_ONLY - inline - Magick::Color get_color(bool value) + namespace impl { - return Magick::ColorMono(value); - } - inline - Magick::Color get_color(const value::int_u8& value) - { - return Magick::ColorGray(256 - value); - } + inline + Magick::Color get_color(bool value) + { + return Magick::ColorMono(value); + } + + inline + Magick::Color get_color(const value::int_u8& value) + { + // Ensure a Magick++'s Quantum is an 8-bit value. + mln::metal::equal<Magick::Quantum, unsigned char>::check(); + return Magick::Color(value, value, value); + } + + inline + Magick::Color get_color(const value::rgb8& value) + { + // Ensure a Magick++'s Quantum is an 8-bit value. + mln::metal::equal<Magick::Quantum, unsigned char>::check(); + return Magick::Color(value.red(), value.green(), value.blue()); + } + + } // end of namespace mln::io::magick::impl - inline - Magick::Color get_color(const value::rgb8& value) - { - return Magick::ColorRGB(256 - value.red(), - 256 - value.green(), - 256 - value.blue()); - } template <typename I> inline - void save(const Image<I>& ima_, const std::string& filename) + void + save(const Image<I>& ima_, const std::string& filename) { trace::entering("mln::io::magick::save"); mln_precondition(mln_site_(I)::dim == 2); - const I& ima = exact(ima_); + // Turn this into a static check? if (!(mln::metal::equal<mln_value(I), bool>::value || mln::metal::equal<mln_value(I), value::int_u8>::value || mln::metal::equal<mln_value(I), value::rgb8>::value)) { - std::cerr << "error: trying to save an unsupported format" << std::endl; - std::cerr << "supported formats: binary, 8bits grayscale (int_u8), 8bits truecolor (rgb8)" << std::endl; + std::cerr << + "error: trying to save an unsupported format\n" + "supported formats are:\n" + " binary (bool)\n" + " 8-bit grayscale (mln::value::int_u8)\n" + " 3x8-bit truecolor (rgb8)" << std::endl; abort(); } - Magick::Image im_file; - im_file.size(Magick::Geometry(ima.nrows(), ima.ncols())); + const I& ima = exact(ima_); - Magick::PixelPacket* pixel_cache = im_file.getPixels(0, 0, ima.nrows(), ima.ncols()); - Magick::PixelPacket* pixel; + Magick::Image magick_ima; + // In the construction of a Geometry object, the width (i.e. + // `ncols') comes first, then the height (i.e. `nrows') + // follows. + magick_ima.size(Magick::Geometry(ima.ncols(), ima.nrows())); + + Magick::Pixels view(magick_ima); + // As above, `ncols' is passed before `nrows'. + Magick::PixelPacket* pixels = view.get(0, 0, ima.ncols(), ima.nrows()); mln_piter(I) p(ima.domain()); for_all(p) - { - pixel = pixel_cache + (int) p.to_site().to_vec()[0] * ima.ncols() - + (int) p.to_site().to_vec()[1]; - *pixel = get_color(ima(p)); - } - im_file.syncPixels(); - im_file.write(filename); + *pixels++ = impl::get_color(ima(p)); + + view.sync(); + magick_ima.write(filename); trace::exiting("mln::io::magick::save"); } - /*template <typename T> - void save(Image< tiled2d<T> >& ima_, const std::string& filename) + + // FIXME: Unfinished? +#if 0 + template <typename T> + void + save(const Image< tiled2d<T> >& ima_, const std::string& filename) { trace::entering("mln::io::magick::save"); @@ -135,7 +164,8 @@ namespace mln ima.buffer().write(filename); trace::exiting("mln::io::magick::save"); - }*/ + } +#endif # endif // ! MLN_INCLUDE_ONLY diff --git a/milena/tests/io/magick/Makefile.am b/milena/tests/io/magick/Makefile.am index f2d5f48..c04147f 100644 --- a/milena/tests/io/magick/Makefile.am +++ b/milena/tests/io/magick/Makefile.am @@ -29,5 +29,9 @@ save_SOURCES = save.cc TESTS = $(check_PROGRAMS) MOSTLYCLEANFILES = \ - save-tiny.ppm \ - save-tiny.png + save-tiny-temp.pbm \ + save-tiny-temp.pgm \ + save-tiny-temp.png \ + save-tiny.pbm \ + save-tiny.pgm \ + save-tiny.ppm diff --git a/milena/tests/io/magick/load.cc b/milena/tests/io/magick/load.cc index ce6db10..1525545 100644 --- a/milena/tests/io/magick/load.cc +++ b/milena/tests/io/magick/load.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2009 EPITA Research and Development Laboratory (LRDE) +// Copyright (C) 2009, 2010 EPITA Research and Development Laboratory (LRDE) // // This file is part of Olena. // @@ -23,26 +23,78 @@ // exception does not however invalidate any other reasons why the // executable file might be covered by the GNU General Public License. -#include <mln/core/image/image2d.hh> +#include <mln/io/magick/load.hh> + +#include <mln/io/pbm/load.hh> +#include <mln/io/pgm/load.hh> +#include <mln/io/ppm/load.hh> #include <mln/data/compare.hh> -#include <mln/io/magick/load.hh> -#include <mln/io/ppm/load.hh> +#include <mln/core/image/image2d.hh> #include <mln/value/rgb8.hh> #include "tests/data.hh" -int main() + +int main(int /* argc */, char* argv[]) { using namespace mln; - image2d<value::rgb8> lena_ppm; - io::ppm::load(lena_ppm, MLN_IMG_DIR "/tiny.ppm"); + /* From http://www.graphicsmagick.org/Magick++/Image.html: + + The InitializeMagick() function MUST be invoked before + constructing any Magick++ objects. This used to be optional, + but now it is absolutely required. This function initalizes + semaphores and configuration information necessary for the + software to work correctly. Failing to invoke + InitializeMagick() is likely to lead to a program crash or + thrown assertion. If the program resides in the same directory + as the GraphicsMagick files, then argv[0] may be passed as an + argument so that GraphicsMagick knows where its files reside, + otherwise NULL may be passed and GraphicsMagick will try to use + other means (if necessary). */ + Magick::InitializeMagick(*argv); + + // Compare Milena's built-in PBM loaded and Magick++'s support for PBM. + { + typedef image2d<bool> I; + I mln_lena; + io::pbm::load(mln_lena, MLN_IMG_DIR "/tiny.pbm"); + I magick_lena; + io::magick::load(magick_lena, MLN_IMG_DIR "/tiny.pbm"); + mln_assertion(mln_lena == magick_lena); + } + + // Compare Milena's built-in PGM loaded and Magick++'s support for PGM. + { + typedef image2d<value::int_u8> I; + I mln_lena; + io::pgm::load(mln_lena, MLN_IMG_DIR "/tiny.pgm"); + I magick_lena; + io::magick::load(magick_lena, MLN_IMG_DIR "/tiny.pgm"); + mln_assertion(mln_lena == magick_lena); + } + + // Compare Milena's built-in PPM loaded and Magick++'s support for PPM. + { + typedef image2d<value::rgb8> I; + I mln_lena; + io::ppm::load(mln_lena, MLN_IMG_DIR "/tiny.ppm"); + I magick_lena; + io::magick::load(magick_lena, MLN_IMG_DIR "/tiny.ppm"); + mln_assertion(mln_lena == magick_lena); + } - image2d<value::rgb8> lena_png; - io::magick::load(lena_png, MLN_TESTS_IMG_DIR "/tiny.png"); - mln_assertion(lena_png == lena_ppm); + // Check Magick++'s support for PNG. + { + typedef image2d<value::rgb8> I; + I lena_ppm; + io::ppm::load(lena_ppm, MLN_IMG_DIR "/tiny.ppm"); + I lena_png; + io::magick::load(lena_png, MLN_TESTS_IMG_DIR "/tiny.png"); + mln_assertion(lena_ppm == lena_png); + } } diff --git a/milena/tests/io/magick/save.cc b/milena/tests/io/magick/save.cc index a4efd12..9f5eae7 100644 --- a/milena/tests/io/magick/save.cc +++ b/milena/tests/io/magick/save.cc @@ -24,34 +24,92 @@ // executable file might be covered by the GNU General Public License. #include <mln/core/image/image2d.hh> + +#include <mln/data/compare.hh> + #include <mln/io/magick/load.hh> #include <mln/io/magick/save.hh> + +#include <mln/io/pbm/load.hh> +#include <mln/io/pbm/save.hh> + +#include <mln/io/pgm/load.hh> +#include <mln/io/pgm/save.hh> + #include <mln/io/ppm/load.hh> #include <mln/io/ppm/save.hh> -#include <mln/data/compare.hh> + #include "tests/data.hh" -#include <mln/io/magick/load.hh> +using namespace mln; -int main() -{ - using namespace mln; +template <typename T> +image2d<T> +test(const image2d<T>& lena_mln, const std::string& temp_filename) +{ point2d p(0,0); - image2d<value::rgb8> lena_mln; - io::ppm::load(lena_mln, MLN_IMG_DIR "/tiny.ppm"); - value::rgb8 c = lena_mln(p); + T c = lena_mln(p); - io::magick::save(lena_mln, "save-tiny.png"); + io::magick::save(lena_mln, temp_filename); + image2d<T> lena_im; + io::magick::load(lena_im, temp_filename); - image2d<value::rgb8> lena_im; - io::magick::load(lena_im, "save-tiny.png"); mln_assertion(lena_im(p) == c); - - io::ppm::save(lena_im, "save-tiny.ppm"); - mln_assertion(lena_im.domain() == lena_mln.domain()); mln_assertion(lena_im == lena_mln); + + return lena_im; +} + + +int main(int /* argc */, char* argv[]) +{ + using namespace mln; + + /* From http://www.graphicsmagick.org/Magick++/Image.html: + + The InitializeMagick() function MUST be invoked before + constructing any Magick++ objects. This used to be optional, + but now it is absolutely required. This function initalizes + semaphores and configuration information necessary for the + software to work correctly. Failing to invoke + InitializeMagick() is likely to lead to a program crash or + thrown assertion. If the program resides in the same directory + as the GraphicsMagick files, then argv[0] may be passed as an + argument so that GraphicsMagick knows where its files reside, + otherwise NULL may be passed and GraphicsMagick will try to use + other means (if necessary). */ + Magick::InitializeMagick(*argv); + + point2d p(0,0); + + // Grayscale values (PBM -> PBM -> PBM). + { + typedef image2d<bool> I; + I lena_mln; + io::pbm::load(lena_mln, MLN_IMG_DIR "/tiny.pbm"); + I lena_im = ::test(lena_mln, "save-tiny-temp.pbm"); + io::pbm::save(lena_im, "save-tiny.pbm"); + } + + // Grayscale values (PGM -> PGM -> PGM). + { + typedef image2d<value::int_u8> I; + I lena_mln; + io::pgm::load(lena_mln, MLN_IMG_DIR "/tiny.pgm"); + I lena_im = ::test(lena_mln, "save-tiny-temp.pgm"); + io::pgm::save(lena_im, "save-tiny.pgm"); + } + + // Color values (PPM -> PNG -> PPM). + { + typedef image2d<value::rgb8> I; + I lena_mln; + io::ppm::load(lena_mln, MLN_IMG_DIR "/tiny.ppm"); + I lena_im = ::test(lena_mln, "save-tiny-temp.png"); + io::ppm::save(lena_im, "save-tiny.ppm"); + } } -- 1.5.6.5