{"action":"created","issue":{"url":"https://api.github.com/repos/boostorg/gil/issues/49","repository_url":"https://api.github.com/repos/boostorg/gil","labels_url":"https://api.github.com/repos/boostorg/gil/issues/49/labels{/name}","comments_url":"https://api.github.com/repos/boostorg/gil/issues/49/comments","events_url":"https://api.github.com/repos/boostorg/gil/issues/49/events","html_url":"https://github.com/boostorg/gil/issues/49","id":306076020,"number":49,"title":"Image checksum tests failing for variant=release and x64 arch","user":{"login":"mloskot","id":80741,"avatar_url":"https://avatars2.githubusercontent.com/u/80741?v=4","gravatar_id":"","url":"https://api.github.com/users/mloskot","html_url":"https://github.com/mloskot","followers_url":"https://api.github.com/users/mloskot/followers","following_url":"https://api.github.com/users/mloskot/following{/other_user}","gists_url":"https://api.github.com/users/mloskot/gists{/gist_id}","starred_url":"https://api.github.com/users/mloskot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mloskot/subscriptions","organizations_url":"https://api.github.com/users/mloskot/orgs","repos_url":"https://api.github.com/users/mloskot/repos","events_url":"https://api.github.com/users/mloskot/events{/privacy}","received_events_url":"https://api.github.com/users/mloskot/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/boostorg/gil/milestones/1","html_url":"https://github.com/boostorg/gil/milestone/1","labels_url":"https://api.github.com/repos/boostorg/gil/milestones/1/labels","id":3195885,"number":1,"title":"The Grand Merge","description":"A lot of work has gone into the `develop` branch over the years (notably IO extensions), but unfortunately hasn't made it into `master` yet. This milestone thus focuses on whatever work remains to be done to stabilize the `develop` branch before we can merge it to `master`, hopefully in time for the Boost 1.68 release.","creator":{"login":"stefanseefeld","id":1243351,"avatar_url":"https://avatars3.githubusercontent.com/u/1243351?v=4","gravatar_id":"","url":"https://api.github.com/users/stefanseefeld","html_url":"https://github.com/stefanseefeld","followers_url":"https://api.github.com/users/stefanseefeld/followers","following_url":"https://api.github.com/users/stefanseefeld/following{/other_user}","gists_url":"https://api.github.com/users/stefanseefeld/gists{/gist_id}","starred_url":"https://api.github.com/users/stefanseefeld/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/stefanseefeld/subscriptions","organizations_url":"https://api.github.com/users/stefanseefeld/orgs","repos_url":"https://api.github.com/users/stefanseefeld/repos","events_url":"https://api.github.com/users/stefanseefeld/events{/privacy}","received_events_url":"https://api.github.com/users/stefanseefeld/received_events","type":"User","site_admin":false},"open_issues":4,"closed_issues":3,"state":"open","created_at":"2018-03-18T15:42:57Z","updated_at":"2018-03-20T15:14:53Z","due_on":null,"closed_at":null},"comments":5,"created_at":"2018-03-16T21:03:55Z","updated_at":"2018-03-22T17:06:58Z","closed_at":null,"author_association":"MEMBER","body":"This issue is spawned by additon of `b2 variant=release` build jobs to AppVeyor in #46 which revealed that `test/image.cpp` is failing on **optimised x64 only** builds:\r\n```\r\nChecksum error in bgr121_basic_red_x (crc=2009802095 != 2137924839)\r\n```\r\nHere is the build https://ci.appveyor.com/project/stefanseefeld/gil/build/1.0.182-develop\r\n\r\nThis issue does not occur on Travis (GCC 5.4.1 and clang 4.2.1).\r\nI also was not able to reproduce it locally, on Ubuntu with GCC 5.4.0.\r\n\r\n## Failing checksum test cases\r\n\r\nModifying `test/image.cpp` to skip cases with missing CRC-s:\r\n```\r\n // Create a checksum for the given view and compare it with the reference checksum. Throw exception if different\r\n void checksum_image_test::check_view_impl(const rgb8c_view_t& img_view, const string& name) {\r\n+    if (_crc_map[name] == 0)\r\n+    {\r\n+        cerr << \"no checksum, skipping \" << name << endl;\r\n+        return;\r\n+    }\r\n+\r\n     boost::crc_32_type checksum_acumulator;\r\n     checksum_acumulator.process_bytes(img_view.row_begin(0),img_view.size()*3);\r\n```\r\n\r\nallowed to iterate over the test cases in `test/gil_reference_checksums.txt` (remove failing entry, run the test, repeat until all remaining cases pass) and identify the failing ones:\r\n\r\n```\r\nbgr121_basic_red_x 7f6e24e7\r\nbgr121_basic_white_x e4aaa1d3\r\nbgr121_views_90cw 8565efd8\r\nbgr121_views_flipped_ud 8f7f7d00\r\n```\r\n\r\nQuestion: what is special about these four that optimised test program binary is calculating different checksum?"},"comment":{"url":"https://api.github.com/repos/boostorg/gil/issues/comments/375383391","html_url":"https://github.com/boostorg/gil/issues/49#issuecomment-375383391","issue_url":"https://api.github.com/repos/boostorg/gil/issues/49","id":375383391,"user":{"login":"mloskot","id":80741,"avatar_url":"https://avatars2.githubusercontent.com/u/80741?v=4","gravatar_id":"","url":"https://api.github.com/users/mloskot","html_url":"https://github.com/mloskot","followers_url":"https://api.github.com/users/mloskot/followers","following_url":"https://api.github.com/users/mloskot/following{/other_user}","gists_url":"https://api.github.com/users/mloskot/gists{/gist_id}","starred_url":"https://api.github.com/users/mloskot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mloskot/subscriptions","organizations_url":"https://api.github.com/users/mloskot/orgs","repos_url":"https://api.github.com/users/mloskot/repos","events_url":"https://api.github.com/users/mloskot/events{/privacy}","received_events_url":"https://api.github.com/users/mloskot/received_events","type":"User","site_admin":false},"created_at":"2018-03-22T17:06:58Z","updated_at":"2018-03-22T17:06:58Z","author_association":"MEMBER","body":"![image-crc-bug](https://user-images.githubusercontent.com/80741/37784392-025f0926-2df8-11e8-9e4b-972e214e301e.png)\r\n\r\n`image_crc_minimal` runs series of test cases with `NxN` images where `N = {2, 3, 4, ..., N+1}` each time performs 3 steps:\r\n  - create empty `NxN` image\r\n  - fill image with red\r\n     - calculate CRC\r\n  - draw blue line diagonal\r\n     - calculate CRC\r\n\r\nBug: For VS2017 64-bit optimised build, the *blue line* step seems to truncate or damage the image in memory (see screenshot below).\r\n\r\nSource of the program that runs this minimal 'fuzz' testing above:\r\n\r\n```\r\nD:\\dev\\boost-gil-workshop\\crc (image-crc-bug-test)\r\nλ cat image_crc_minimal.cpp\r\n#define BOOST_CONFIG_SUPPRESS_OUTDATED_MESSAGE\r\n#ifdef _MSC_VER\r\n#pragma warning(disable:4244)\r\n#pragma warning(disable:4503)\r\n#pragma warning(disable:4714)\r\n#pragma warning(disable:4996)\r\n#endif\r\n#include <filesystem>\r\n#include <fstream>\r\n#include <iostream>\r\n#include <string>\r\n#include <vector>\r\n#include <boost/algorithm/hex.hpp>\r\n#include <boost/crc.hpp>\r\n#include <boost/gil/gil_all.hpp>\r\n#include <boost/gil/extension/dynamic_image/dynamic_image_all.hpp>\r\n#include <boost/mpl/vector.hpp>\r\nusing namespace std;\r\nusing namespace boost::gil;\r\nnamespace fs = std::experimental::filesystem;\r\n\r\nstruct image_test\r\n{\r\n    std::ostream& log_;\r\n    image_test(std::ostream& os) : log_(os) {}\r\n\r\n    void print_checksum(const rgb8c_view_t& img_view, const string& name)\r\n    {\r\n        boost::crc_32_type checksum_acumulator;\r\n        checksum_acumulator.process_bytes(img_view.row_begin(0), img_view.size() * 3);\r\n        cerr << \"Checksum (\" << name << \"):\\t\\t\" << std::hex << checksum_acumulator.checksum() << endl;\r\n        log_ << \"Checksum (\" << name << \"):\\t\\t\" << std::hex << checksum_acumulator.checksum() << endl;\r\n    }\r\n\r\n    void print_view_hex(const rgb8c_view_t& img_view)\r\n    {\r\n        auto const raw_size = img_view.size() * 3;\r\n        unsigned char const* raw_ptr = &img_view[0][0];\r\n        std::vector<unsigned char> raw(raw_ptr, raw_ptr + raw_size);\r\n        std::string out(raw_size * 2, char{0});\r\n        boost::algorithm::hex(raw, out.begin());\r\n        log_ << out << endl;\r\n    }\r\n\r\n    template <typename View>\r\n    void check_view(const View& img_view, const string& name)\r\n    {\r\n        rgb8_image_t rgb_img(img_view.dimensions());\r\n        copy_and_convert_pixels(img_view, view(rgb_img));\r\n        print_checksum(const_view(rgb_img), name);\r\n        print_view_hex(const_view(rgb_img));\r\n    }\r\n\r\n    template <typename Img>\r\n    void basic_test(typename Img::point_t const& dims)\r\n    {\r\n        using view_value_t = typename Img::view_t::value_type;\r\n\r\n        // make empty image\r\n        Img img(dims);\r\n        auto const& img_view = view(img);\r\n        //check_view(img_view, \"make\"); // possibly random garbage\r\n\r\n        // colors\r\n        rgb8_pixel_t red8(255, 0, 0), green8(0, 255, 0), blue8(0, 0, 255), white8(255, 255, 255);\r\n        view_value_t red, green, blue, white;\r\n        color_convert(red8, red);\r\n        color_convert(green8, green);\r\n        color_convert(blue8, blue);\r\n        color_convert(white8, white);\r\n\r\n        // fill it with red\r\n        fill(img_view.begin(), img_view.end(), red);\r\n        check_view(img_view, \"red  fill\");\r\n\r\n        // draw a blue line along the diagonal\r\n        {\r\n            auto loc = img_view.xy_at(0, img_view.height() - 1);\r\n            for (int y = 0; y < img_view.height(); ++y)\r\n            {\r\n                *loc = blue;\r\n                ++loc.x();\r\n                loc.y()--;\r\n            }\r\n            check_view(img_view, \"blue line\");\r\n        }\r\n    }\r\n};\r\n\r\n#ifdef NDEBUG\r\nauto const config_suffix = \"opt\";\r\n#else\r\nauto const config_suffix = \"dbg\";\r\n#endif\r\n#if defined(_WIN64) || defined(__x86_64__) || defined(__LLP64__) || (UINTPTR_MAX == 0xffffffffffffffffULL)\r\nauto const arch_suffix = \"x64\";\r\n#else\r\nauto const arch_suffix = \"x32\";\r\n#endif\r\n\r\nint main()\r\n{\r\n    try\r\n    {\r\n        using bgr121_ref_t = const bit_aligned_pixel_reference<boost::uint8_t, boost::mpl::vector3_c<int, 1, 2, 1>, bgr_layout_t, true>;\r\n        using bgr121_image_t = image<bgr121_ref_t, false>;\r\n        using dims_t = bgr121_image_t::point_t;\r\n\r\n        auto const this_path = fs::canonical(fs::path(__FILE__).parent_path());\r\n\r\n        auto const max_dim = 8; // modify if you need\r\n        for (int i = 2; i < max_dim + 1; i += 1)\r\n        {\r\n            auto const log_path = this_path / fs::path(\"test_\");\r\n            std::ostringstream os;\r\n            os << log_path << i << \"x\" << i << \"_\" << arch_suffix << \"_\" << config_suffix << \".log\";\r\n            auto log = os.str();\r\n\r\n            // if (i == 5) may randomly crash in opt, in gil::image<..>::deallocate\r\n\r\n            // test\r\n            std::ofstream ofs(log );\r\n            std::cerr << log << std::endl;\r\n            image_test test(ofs);\r\n            dims_t dims(i, i);\r\n            test.basic_test<bgr121_image_t>(dims);\r\n        }\r\n    }\r\n    catch (std::runtime_error const& e)\r\n    {\r\n        std::cerr << e.what() << std::endl;\r\n    }\r\n    return 0;\r\n}\r\n```\r\n\r\nDobule-check comparing the calculated CRC and hex dump of the test image:\r\n\r\n* 32-bit build is fine\r\n\r\n```\r\nD:\\dev\\boost-gil-workshop\\crc (image-crc-bug-test)\r\nλ for /L %i in (2, 1, 4) do diff -Nua test_%ix%i_x32_dbg.log test_%ix%i_x32_opt.log\r\n\r\ndiff -Nua test_2x2_x32_dbg.log test_2x2_x32_opt.log\r\n\r\ndiff -Nua test_3x3_x32_dbg.log test_3x3_x32_opt.log\r\n\r\ndiff -Nua test_4x4_x32_dbg.log test_4x4_x32_opt.log\r\n```\r\n\r\n* 64-bit reproduces the bug\r\n\r\n```\r\nD:\\dev\\boost-gil-workshop\\crc (image-crc-bug-test)\r\nλ for /L %i in (2, 1, 4) do diff -Nua test_%ix%i_x64_dbg.log test_%ix%i_x64_opt.log\r\n\r\ndiff -Nua test_2x2_x64_dbg.log test_2x2_x64_opt.log\r\n\r\ndiff -Nua test_3x3_x64_dbg.log test_3x3_x64_opt.log\r\n--- test_3x3_x64_dbg.log        2018-03-22 17:31:24.330980200 +0100\r\n+++ test_3x3_x64_opt.log        2018-03-22 16:16:02.111684000 +0100\r\n@@ -1,4 +1,4 @@\r\n Checksum (red  fill):          23a6f403\r\n FF0000FF0000FF0000FF0000FF0000FF0000FF0000FF0000FF0000\r\n-Checksum (blue line):          2e4950b4\r\n-FF0000FF00000000FFFF00000000FFFF00000000FFFF0000FF0000\r\n+Checksum (blue line):          ff43598\r\n+FF0000FF00000000FFFF0000FF0000FF00000000FFFF0000FF0000\r\n\r\ndiff -Nua test_4x4_x64_dbg.log test_4x4_x64_opt.log\r\n--- test_4x4_x64_dbg.log        2018-03-22 17:31:24.357974200 +0100\r\n+++ test_4x4_x64_opt.log        2018-03-22 16:16:02.138165400 +0100\r\n@@ -1,4 +1,4 @@\r\n Checksum (red  fill):          b803c24c\r\n FF0000FF0000FF0000FF0000FF0000FF0000FF0000FF0000FF0000FF0000FF0000FF0000FF0000FF0000FF0000FF0000\r\n-Checksum (blue line):          7313468d\r\n-FF0000FF0000FF00000000FFFF0000FF00000000FFFF0000FF00000000FFFF0000FF00000000FFFF0000FF0000FF0000\r\n+Checksum (blue line):          8570c3bc\r\n+FF00000000FFFF0000FF00000000FFFF0000FF0000FF0000FF00000000FFFF0000FF00000000FFFF0000FF0000FF0000\r\n```\r\n\r\nAs you can see, the bug can be reproduced with **image as small as 3x3**:\r\n\r\n![image-crc-bug-hex](https://user-images.githubusercontent.com/80741/37785927-a70e148c-2dfb-11e8-9fe5-51f0e7688e8a.png)\r\n\r\nI do not see clear reason to blame the `boost::crc_32__type`; I suspect bug in `image_view<>::locator` or `image_view` itself, of their specialisations."}}
{"actor":{"display_login":"mloskot"}}