boostorg / gil

Boost.GIL - Generic Image Library | Requires C++14 since Boost 1.80
https://boostorg.github.io/gil
Boost Software License 1.0
179 stars 163 forks source link

Ensure all examples build without error #628

Closed yogsothoth closed 2 years ago

yogsothoth commented 3 years ago

Description

Add all examples to example/Jamfile Add necessary 'using' for libpng Bring small fixes to examples' source files (remove unused variables)

Environment:

References

See #436

Tasklist

yogsothoth commented 3 years ago

I have a few questions:

mloskot commented 3 years ago

Hi, first thanks for your contribution, it's helpful.

Should we update the documentation to reference the missing examples?

I don't think we should care about describing the examples in the documentation. In fact, I'd clear the examples from the documentation. Instead, examples should be described in comment block at the top of .cpp files (i.e. what an example is about, what feature it presents). There also should be README.md files which describe any particularities about the examples (i.e. build requirements, specific data required, etc.). Then, the documentation should only link to the documentation folder (i.e. on GitHub).

Should we leave only the compilation targets (not the exe) in the final Jamfile?

I'm not sure I understand. We certainly want to let users to build (compile+link) examples using the b2 and the Jamfile-s we provide.

I've had issues with linking libpng that I'm not sure how to fix.

You'd have to be more specific what you are doing, what is the output, errors, etc. Feel free to open new issue for that problem.

Here is (new home of) B2 with its documentation: https://www.bfgroup.xyz/b2/manual/main/index.html

yogsothoth commented 3 years ago

I don't think we should care about describing the examples in the documentation. In fact, I'd clear the examples from the documentation. Instead, examples should be described in comment block at the top of .cpp files (i.e. what an example is about, what feature it presents). There also should be README.md files which describe any particularities about the examples (i.e. build requirements, specific data required, etc.). Then, the documentation should only link to the documentation folder (i.e. on GitHub).

Alright, I will include that in the tasklist then.

I'm not sure I understand. We certainly want to let users to build (compile+link) examples using the b2 and the Jamfile-s we provide.

Apologies, I wasn't being super clear: should the default target in the top Jamfile also include producing the executable files for the examples now, or do we leave it as it is today (i.e. build-project test)?

I've had issues with linking libpng that I'm not sure how to fix.

You'd have to be more specific what you are doing, what is the output, errors, etc. Feel free to open new issue for that problem.

Indeed, I just didn't want to turn this thread into a support ticket. I will open a separate ticket.

Here is (new home of) B2 with its documentation: https://www.bfgroup.xyz/b2/manual/main/index.html

I read it a few times and couldn't find the section that would help me with my problem so far... I'm clearly missing something.

yogsothoth commented 3 years ago

Regarding the issue with linking: it was a problem with the project-cache.jam not being updated. Deleting this file and rebuilding fixed it.

mloskot commented 3 years ago

Yes, the GIL's top-level Jamfile can also build the examples.

AFAICT, all Boost's CI services do b2 libs/<name>/test so they won't spend any (unnecessary) time on building examples.

yogsothoth commented 3 years ago

I just updated the PR with some explanations for the examples. I also remarked that some of them don't include any mention of any license. Shall I take the opportunity to ensure they all do?

mloskot commented 3 years ago

Good catch, yes, all source files should have the licence/copyright header comment, like this:

https://github.com/boostorg/gil/blob/537f8ae82313167bf5696b5daec265eb3bf6e8c2/example/dynamic_image.cpp#L1-L7

yogsothoth commented 2 years ago

Thank you for your remarks!

I've updated the attributions and the Jamfile indent. I have prepared Readme files with a synopsis, and some indications of what the examples require, in a different commit; I will update the PR.

yogsothoth commented 2 years ago

I noticed example/convolve2d.cpp was a bit off compared to the other examples: one output file had a png extension but a jpeg_tag{}, it ended with a cin(), contained redundant, commented out code, etc.

I cleaned it up.

This is the last of the changes I can see we can bring here: if you find all is well, then I think we can add the target example to the root Jamfile.

yogsothoth commented 2 years ago

Thank you for your time!

I just added the target example to the root Jamfile; marking this as Ready for review.

mloskot commented 2 years ago

Thank you for your contribution