Re: [Boost-users] [Gil new_io] Some questions and remarks

Subject: Re: [Boost-users] [Gil new_io] Some questions and remarks
From: Olivier Tournaire (olitour_at_[hidden])
Date: 2010-03-02 04:06:28

Hi Christian,

2010/3/1 Christian Henning <chhenning_at_[hidden]>

> Hi Olivier, long email. Let's tackle it:
>

Sorry. Wanted to be clear enough ...

>
> On Sat, Feb 27, 2010 at 9:29 AM, Olivier Tournaire <olitour_at_[hidden]>
> wrote:
> > Hi all,
> >
> > I have just checkout gil-contributions svn repo
> > (http://code.google.com/p/gil-contributions/) in order to test new_io in
> > which I am really interested. Thank you for this very usefull job! I have
> > however some remarks and questions about some compilation issues.
>
> Thanks, glad it's useful to you. I'll try to resolve all the issues.
>
> >
> > 0) The code seems to have been written and tested on Windows. Am I right
> > (see my remarks thereafter)? For ex., why do you use "\\" in paths
> > (paths.hpp) and not "/"?
>
> Yes, I do my programming on Windows. I have MinGW in place to make
> sure my code compiles with gcc. Not ideal, but it has worked so far.
>
> > 1) Which Boost version is required? I first tried with 1.40, but it did
> not
> > work because of missing accessors in boost::gil::variant (index() and
> > bits()). Did they appear in 1.41 or 1.42? With 1.42, this issue is
> solved.
>
> I added this patch in November of last year. I think you need 1.42.
> I'll make a note in the documentation.
>
> > 2) While compiling tests, I ran into some issues. I am running the code
> on
> > an Ubuntu 9.10, with g++-4.3.4. I was able to fix some of them, but
> others
> > are still problematic:
> >    2.1) For each test, I have to add at the beginning the 2 following
> lines:
> >
> > #define BOOST_TEST_DYN_LINK
> > #define BOOST_TEST_MODULE my application tests
> >
> >          It probably comes from my little knowledge of boost
> > unit_test_framework. Could you explain?
>
> You do? I'll have to investigate that. I have no idea what these
> preprocessor symbols do. But thanks for pointing it out.
>

My mistake. I usually use dynamic linking. Which requires to use the define
directives pointed above. Note that I have created my own CMakeLists.txt.
So, forgive this error which came from me ...

>
> >
> >     2.2) For each test, the compiler gave the following error:
> >
> >
> /usr/local/include/boost/gil/extension/io_new/detail/dynamic_io_new.hpp:75:
> > error: there are no arguments to ‘io_error’ that depend on a template
> > parameter, so a declaration of ‘io_error’ must be available
> >
> >             I simply add in [...]/detail/dynamic_io_new.hpp an unclude
> > directive at the beginning of the file : #include
> > <boost/gil/extension/io/io_error.hpp>. Still do not understand how it can
> > work wihtout this ...
>
> This is fixed and will be applied to my repository asap. Seems it's
> been a while when I last double checked with gcc.
>
> >
> >      2.3) I had a similar error with [...]/formats/bmp/read.hpp:
> >
> > /usr/local/include/boost/gil/extension/io_new/formats/bmp/read.hpp:833:
> > error: there are no arguments to ‘get_info’ that depend on a template
> > parameter, so a declaration of ‘get_info’ must be available
> >
> >             I simply changed line 833 from get_info() to
> > parent_t::get_info(). Now works.
>
> Yep. Thanks. It's fixed.
>
> >
> >      2.4) Some typename are missing (still do not know how visual can
> infer
> > types ...):
> >             - [...]/formats/jpeg/read.hpp, line 336: typename
> Image::view_t
> >             - [...]/formats/bmp/is_allowed.hpp, line 73: typename
> > View::value_type
> >             - [...]/formats/pnm/read.hpp, line 419: typename
> Image::view_t
> >             Everything works with these changes.
>
> Well, MSVC gets it. I have fixed it too.
>
> >
> >      2.5) png_read_test failed. Obviously, this is a portability issue.
> PNG
> > filenames for testing are uppercase, while they are lowercase in the test
> > code. I think that is also missing in the code the path to png files. For
> > example, in basic_format_test, instead of
> >
> >                    test_file< gray1_image_t >( "basn0g01.png" );
> >             it soulhd be:
> >                    test_file< gray1_image_t >( png_in + "BASN0G01.PNG" );
> >            I did not rename all files, so I am unable to try the test.
> >
> Mhmm, it works on Windows. I'm not a Linux buff but if this creates
> problems on Linux, I'll fix that. Of course.
>
> >      2.6) Some tests fail to compile or link. Here are the errors:
> >
> [snip]
>
> Seems like a libpng problem. Please give me some time reviewing it.
> What version of png are you using?
>

* Yep, you are right. It is a problem with libpng (I have version 1.2.37).
Removing two lines fixed the problem (seems it is a known bug:
https://bugs.launchpad.net/ubuntu/+source/libpng/+bug/218409)
* pnm_read_test.cpp compiles fine if you add some instanciations in
pnm_tags.hpp (patch attached).
* jpeg_old_test.cpp is also compilable with slight changes. The compilation
problem appears on line 237 in formats/jpeg/write.hpp:

    return apply_operation_base< Types1
                               , image_write_info< jpeg_tag >
                               >( *arg1.bits() *
                                , arg1.index()
                                , info
                                , op
                                );

arg1.bits() is then passed to return_apply_operation_base which takes a
reference on its first argument. Since arg1.bits() is local, it is const and
thus cannot be used as a reference (patch attached). Not sure my solution is
the best one. Maybe better to change apply_operation_base to have as first
argument a const reference ...

I also attach a patch to fix paths in tiff_file_format_test.cpp.

Hope it helps.

> >
> >
> > Apart png_read_test and its case sensitive problem, all compiled and
> linked
> > tests run successfully. This is a really good new! However, could you
> please
> > help regarding the issues above?
> > Please note that if you are intersted in a Linux developper/tester for
> gil,
> > I am wiling to do some jobs ...
>
> That would be great. My system of using MinGW is pretty limited. In
> the past I had to do a lot of tweaking for gcc. It would be great to
> have someone helping out. Not just for the gil::io but also for gil
> overall.
>

OK. Really interested to help. Let me know. You may contact me in private.
May I have a write access to the svn repo?

>
> Thank you Olivier for your insight. I'll apply all changes to the
> gil::io_new trunk asap. By tomorrow, all fixes should be done.
>

Great. Checkouted and tested. Works perfectly.

Regards,
Olivier

>
> Regards,
> Christian
> _______________________________________________
> Boost-users mailing list
> Boost-users_at_[hidden]
> http://lists.boost.org/mailman/listinfo.cgi/boost-users
>