From 4d1f12d1e27954c05d50535262f277c03d86f116 Mon Sep 17 00:00:00 2001 From: Andreas Fabri Date: Tue, 28 Sep 2021 10:41:50 +0100 Subject: [PATCH 1/4] Unset fail bit and add assertions about the state of a stream --- .../include/CGAL/IO/read_off_points.h | 3 ++- .../include/CGAL/IO/read_xyz_points.h | 1 + .../test_deprecated_io_point_set.cpp | 18 ++++++++++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/Point_set_processing_3/include/CGAL/IO/read_off_points.h b/Point_set_processing_3/include/CGAL/IO/read_off_points.h index 8fee80ba6bf..4241636c6ea 100644 --- a/Point_set_processing_3/include/CGAL/IO/read_off_points.h +++ b/Point_set_processing_3/include/CGAL/IO/read_off_points.h @@ -190,12 +190,13 @@ bool read_OFF(std::istream& is, put(normal_map, pwn, normal); // normal_map[&pwn] = normal *output++ = pwn; pointsRead++; + } // ...or skip comment line } // Skip remaining lines } - + is.clear(is.rdstate() & ~std::ios_base::failbit); // set by getline return true; } diff --git a/Point_set_processing_3/include/CGAL/IO/read_xyz_points.h b/Point_set_processing_3/include/CGAL/IO/read_xyz_points.h index a414ddb839a..88caefb1ea9 100644 --- a/Point_set_processing_3/include/CGAL/IO/read_xyz_points.h +++ b/Point_set_processing_3/include/CGAL/IO/read_xyz_points.h @@ -179,6 +179,7 @@ bool read_XYZ(std::istream& is, return false; } } + is.clear(is.rdstate() & ~std::ios_base::failbit); // set by getline return true; } diff --git a/Point_set_processing_3/test/Point_set_processing_3/test_deprecated_io_point_set.cpp b/Point_set_processing_3/test/Point_set_processing_3/test_deprecated_io_point_set.cpp index 701692d16f3..adad74c7dd0 100644 --- a/Point_set_processing_3/test/Point_set_processing_3/test_deprecated_io_point_set.cpp +++ b/Point_set_processing_3/test/Point_set_processing_3/test_deprecated_io_point_set.cpp @@ -122,8 +122,11 @@ is.close(); assert(ok); assert(ps.size() == 3); #endif + + //PLY os.open("tmp.ply"); +assert(os.good()); ok = CGAL::write_ply_points_with_properties(os, points, CGAL::make_ply_point_writer (CGAL::First_of_pair_property_map()), std::make_pair(GetRedMap(),CGAL::PLY_property("red")), @@ -132,9 +135,11 @@ ok = CGAL::write_ply_points_with_properties(os, points, std::make_pair(GetAlphaMap(), CGAL::PLY_property("alpha")) ); os.close(); +assert(! os.fail()); assert(ok); is.open("tmp.ply"); +assert(is.good()); points.clear(); ok = CGAL::read_ply_points_with_properties(is, std::back_inserter (points), CGAL::make_ply_point_reader(CGAL::First_of_pair_property_map()), @@ -145,48 +150,61 @@ ok = CGAL::read_ply_points_with_properties(is, std::back_inserter (points), CGAL::PLY_property("blue"), CGAL::PLY_property("alpha"))); is.close(); +assert(! is.fail()); assert(ok); assert(points.size() == 3); assert(points[1].second[1] == 255); os.open("tmp.ply"); +assert(os.good()); ok = CGAL::write_ply_points(os, ps, CGAL::parameters::all_default()); os.close(); +assert(! os.fail()); assert(ok); is.open("tmp.ply"); +assert(is.good()); ps.clear(); ok = CGAL::read_ply_points(is, std::back_inserter (ps), CGAL::parameters::all_default()); is.close(); +assert(! is.fail()); assert(ok); assert(ps.size() == 3); //OFF os.open("tmp.off"); +assert(os.good()); ok = CGAL::write_off_points(os, ps, CGAL::parameters::all_default()); os.close(); +assert(! os.fail()); assert(ok); is.open("tmp.off"); +assert(is.good()); ps.clear(); ok = CGAL::read_off_points(is, std::back_inserter (ps), CGAL::parameters::all_default()); is.close(); +assert(! is.fail()); assert(ok); assert(ps.size() == 3); //XYZ os.open("tmp.xyz"); +assert(os.good()); ok = CGAL::write_xyz_points(os, ps, CGAL::parameters::all_default()); os.close(); +assert(! os.fail()); assert(ok); is.open("tmp.xyz"); +assert(is.good()); ps.clear(); ok = CGAL::read_xyz_points(is, std::back_inserter (ps), CGAL::parameters::all_default()); is.close(); +assert(! is.fail()); assert(ok); assert(ps.size() == 3); } From 716dcaf6c9ec8fe423e1f654c39607efa6191f4f Mon Sep 17 00:00:00 2001 From: Andreas Fabri Date: Tue, 28 Sep 2021 13:57:40 +0200 Subject: [PATCH 2/4] Apply suggestions from code review Co-authored-by: Laurent Rineau --- Point_set_processing_3/include/CGAL/IO/read_off_points.h | 4 +++- Point_set_processing_3/include/CGAL/IO/read_xyz_points.h | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Point_set_processing_3/include/CGAL/IO/read_off_points.h b/Point_set_processing_3/include/CGAL/IO/read_off_points.h index 4241636c6ea..1a459a48a56 100644 --- a/Point_set_processing_3/include/CGAL/IO/read_off_points.h +++ b/Point_set_processing_3/include/CGAL/IO/read_off_points.h @@ -196,7 +196,9 @@ bool read_OFF(std::istream& is, } // Skip remaining lines } - is.clear(is.rdstate() & ~std::ios_base::failbit); // set by getline + if(is.eof()) { + is.clear(is.rdstate() & ~std::ios_base::failbit); // set by getline + } return true; } diff --git a/Point_set_processing_3/include/CGAL/IO/read_xyz_points.h b/Point_set_processing_3/include/CGAL/IO/read_xyz_points.h index 88caefb1ea9..0de735fadf8 100644 --- a/Point_set_processing_3/include/CGAL/IO/read_xyz_points.h +++ b/Point_set_processing_3/include/CGAL/IO/read_xyz_points.h @@ -179,7 +179,9 @@ bool read_XYZ(std::istream& is, return false; } } - is.clear(is.rdstate() & ~std::ios_base::failbit); // set by getline + if(is.eof()) { + is.clear(is.rdstate() & ~std::ios_base::failbit); // set by getline + } return true; } From 551ff77fb7315f012398003aef7213355f614f7a Mon Sep 17 00:00:00 2001 From: Andreas Fabri Date: Wed, 29 Sep 2021 13:31:45 +0100 Subject: [PATCH 3/4] Do not open() and close() just one stream --- .../test_deprecated_io_point_set.cpp | 234 ++++++++++-------- 1 file changed, 125 insertions(+), 109 deletions(-) diff --git a/Point_set_processing_3/test/Point_set_processing_3/test_deprecated_io_point_set.cpp b/Point_set_processing_3/test/Point_set_processing_3/test_deprecated_io_point_set.cpp index adad74c7dd0..5383be04e5a 100644 --- a/Point_set_processing_3/test/Point_set_processing_3/test_deprecated_io_point_set.cpp +++ b/Point_set_processing_3/test/Point_set_processing_3/test_deprecated_io_point_set.cpp @@ -77,134 +77,150 @@ points[1] = std::make_pair(Point_3(0,1,0), Color{0,255,0,255}); points[2] = std::make_pair(Point_3(0,0,1), Color{0,0,255,255}); -std::ofstream os; -std::ifstream is; bool ok; std::vector ps; ps.push_back(Point_3(1,0,0)); ps.push_back(Point_3(0,1,0)); ps.push_back(Point_3(0,0,1)); + + //LAS #ifdef CGAL_LINKED_WITH_LASLIB -os.open("tmp.las", std::ios::binary); -ok = CGAL::write_las_points_with_properties(os, points, - CGAL::make_las_point_writer(CGAL::First_of_pair_property_map()), - std::make_pair(GetRedMap(),CGAL::LAS_property::R()), - std::make_pair(GetGreenMap(), CGAL::LAS_property::G()), - std::make_pair(GetBlueMap(), CGAL::LAS_property::B()), - std::make_pair(GetAlphaMap(), CGAL::LAS_property::I()) - ); -os.close(); -assert(ok); -points.clear(); -is.open("tmp.las", std::ios::binary); -ok = CGAL::read_las_points_with_properties(is, std::back_inserter (points), - CGAL::make_las_point_reader(CGAL::First_of_pair_property_map()), - std::make_tuple(CGAL::Second_of_pair_property_map(), - CGAL::Construct_array(), - CGAL::LAS_property::R(), - CGAL::LAS_property::G(), - CGAL::LAS_property::B(), - CGAL::LAS_property::I())); -is.close(); -assert(ok); -assert(points.size() == 3); -assert(points[1].second[1] == 255); -os.open("tmp.las", std::ios_base::binary); -CGAL::write_las_points(os, ps, CGAL::parameters::all_default()); -os.close(); -assert(ok); -ps.clear(); -is.open("tmp.las", std::ios::binary); -ok = CGAL::read_las_points(is, std::back_inserter (ps),CGAL::parameters::all_default()); -is.close(); -assert(ok); -assert(ps.size() == 3); + { + std::ofstream os("tmp1.las", std::ios::binary); + ok = CGAL::write_las_points_with_properties(os, points, + CGAL::make_las_point_writer(CGAL::First_of_pair_property_map()), + std::make_pair(GetRedMap(),CGAL::LAS_property::R()), + std::make_pair(GetGreenMap(), CGAL::LAS_property::G()), + std::make_pair(GetBlueMap(), CGAL::LAS_property::B()), + std::make_pair(GetAlphaMap(), CGAL::LAS_property::I()) + ); + assert(ok); + + } + + { + points.clear(); + std::ifstream is("tmp1.las", std::ios::binary); + ok = CGAL::read_las_points_with_properties(is, std::back_inserter (points), + CGAL::make_las_point_reader(CGAL::First_of_pair_property_map()), + std::make_tuple(CGAL::Second_of_pair_property_map(), + CGAL::Construct_array(), + CGAL::LAS_property::R(), + CGAL::LAS_property::G(), + CGAL::LAS_property::B(), + CGAL::LAS_property::I())); + assert(ok); + assert(points.size() == 3); + assert(points[1].second[1] == 255); + } + + { + std::ofstream os("tmp2.las", std::ios_base::binary); + CGAL::write_las_points(os, ps, CGAL::parameters::all_default()); + assert(ok); + } + + { + ps.clear(); + std::ifstream is("tmp2.las", std::ios::binary); + ok = CGAL::read_las_points(is, std::back_inserter (ps),CGAL::parameters::all_default()); + assert(ok); + assert(ps.size() == 3); + } #endif //PLY -os.open("tmp.ply"); -assert(os.good()); -ok = CGAL::write_ply_points_with_properties(os, points, - CGAL::make_ply_point_writer (CGAL::First_of_pair_property_map()), - std::make_pair(GetRedMap(),CGAL::PLY_property("red")), - std::make_pair(GetGreenMap(), CGAL::PLY_property("green")), - std::make_pair(GetBlueMap(), CGAL::PLY_property("blue")), - std::make_pair(GetAlphaMap(), CGAL::PLY_property("alpha")) - ); -os.close(); -assert(! os.fail()); -assert(ok); + { + std::ofstream os("tmp1.ply"); + assert(os.good()); + ok = CGAL::write_ply_points_with_properties(os, points, + CGAL::make_ply_point_writer (CGAL::First_of_pair_property_map()), + std::make_pair(GetRedMap(),CGAL::PLY_property("red")), + std::make_pair(GetGreenMap(), CGAL::PLY_property("green")), + std::make_pair(GetBlueMap(), CGAL::PLY_property("blue")), + std::make_pair(GetAlphaMap(), CGAL::PLY_property("alpha")) + ); + assert(! os.fail()); + assert(ok); + } -is.open("tmp.ply"); -assert(is.good()); -points.clear(); -ok = CGAL::read_ply_points_with_properties(is, std::back_inserter (points), - CGAL::make_ply_point_reader(CGAL::First_of_pair_property_map()), - std::make_tuple(CGAL::Second_of_pair_property_map(), - CGAL::Construct_array(), - CGAL::PLY_property("red"), - CGAL::PLY_property("green"), - CGAL::PLY_property("blue"), - CGAL::PLY_property("alpha"))); -is.close(); -assert(! is.fail()); -assert(ok); -assert(points.size() == 3); -assert(points[1].second[1] == 255); + { + std::ifstream is("tmp1.ply"); + assert(is.good()); + points.clear(); + ok = CGAL::read_ply_points_with_properties(is, std::back_inserter (points), + CGAL::make_ply_point_reader(CGAL::First_of_pair_property_map()), + std::make_tuple(CGAL::Second_of_pair_property_map(), + CGAL::Construct_array(), + CGAL::PLY_property("red"), + CGAL::PLY_property("green"), + CGAL::PLY_property("blue"), + CGAL::PLY_property("alpha"))); + assert(! is.fail()); + assert(ok); + assert(points.size() == 3); + assert(points[1].second[1] == 255); + } -os.open("tmp.ply"); -assert(os.good()); -ok = CGAL::write_ply_points(os, ps, CGAL::parameters::all_default()); -os.close(); -assert(! os.fail()); -assert(ok); + { + std::ofstream os("tmp2.ply"); + assert(os.good()); + ok = CGAL::write_ply_points(os, ps, CGAL::parameters::all_default()); + assert(! os.fail()); + assert(ok); + } -is.open("tmp.ply"); -assert(is.good()); -ps.clear(); -ok = CGAL::read_ply_points(is, std::back_inserter (ps), - CGAL::parameters::all_default()); -is.close(); -assert(! is.fail()); -assert(ok); -assert(ps.size() == 3); + { + std::ifstream is("tmp2.ply"); + assert(is.good()); + ps.clear(); + ok = CGAL::read_ply_points(is, std::back_inserter (ps), + CGAL::parameters::all_default()); + assert(! is.fail()); + assert(ok); + assert(ps.size() == 3); + } //OFF -os.open("tmp.off"); -assert(os.good()); -ok = CGAL::write_off_points(os, ps, CGAL::parameters::all_default()); -os.close(); -assert(! os.fail()); -assert(ok); + { + std::ofstream os("tmp.off"); + assert(os.good()); + ok = CGAL::write_off_points(os, ps, CGAL::parameters::all_default()); + assert(! os.fail()); + assert(ok); + } -is.open("tmp.off"); -assert(is.good()); -ps.clear(); -ok = CGAL::read_off_points(is, std::back_inserter (ps), - CGAL::parameters::all_default()); -is.close(); -assert(! is.fail()); -assert(ok); -assert(ps.size() == 3); + { + std::ifstream is("tmp.off"); + assert(is.good()); + ps.clear(); + ok = CGAL::read_off_points(is, std::back_inserter (ps), + CGAL::parameters::all_default()); + assert(! is.fail()); + assert(ok); + assert(ps.size() == 3); + } //XYZ -os.open("tmp.xyz"); -assert(os.good()); -ok = CGAL::write_xyz_points(os, ps, CGAL::parameters::all_default()); -os.close(); -assert(! os.fail()); -assert(ok); + { + std::ofstream os("tmp.xyz"); + assert(os.good()); + ok = CGAL::write_xyz_points(os, ps, CGAL::parameters::all_default()); + assert(! os.fail()); + assert(ok); + } -is.open("tmp.xyz"); -assert(is.good()); -ps.clear(); -ok = CGAL::read_xyz_points(is, std::back_inserter (ps), - CGAL::parameters::all_default()); -is.close(); -assert(! is.fail()); -assert(ok); -assert(ps.size() == 3); + { + std::ifstream is("tmp.xyz"); + assert(is.good()); + ps.clear(); + ok = CGAL::read_xyz_points(is, std::back_inserter (ps), + CGAL::parameters::all_default()); + assert(! is.fail()); + assert(ok); + assert(ps.size() == 3); + } } From b3a7a7dc69c07ec15bd144b0dbd3a52f653e961f Mon Sep 17 00:00:00 2001 From: Andreas Fabri Date: Wed, 29 Sep 2021 13:44:42 +0100 Subject: [PATCH 4/4] untabify --- .../test_deprecated_io_point_set.cpp | 58 +++++++++---------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/Point_set_processing_3/test/Point_set_processing_3/test_deprecated_io_point_set.cpp b/Point_set_processing_3/test/Point_set_processing_3/test_deprecated_io_point_set.cpp index 5383be04e5a..218f4f42b63 100644 --- a/Point_set_processing_3/test/Point_set_processing_3/test_deprecated_io_point_set.cpp +++ b/Point_set_processing_3/test/Point_set_processing_3/test_deprecated_io_point_set.cpp @@ -90,12 +90,12 @@ ps.push_back(Point_3(0,0,1)); { std::ofstream os("tmp1.las", std::ios::binary); ok = CGAL::write_las_points_with_properties(os, points, - CGAL::make_las_point_writer(CGAL::First_of_pair_property_map()), - std::make_pair(GetRedMap(),CGAL::LAS_property::R()), - std::make_pair(GetGreenMap(), CGAL::LAS_property::G()), - std::make_pair(GetBlueMap(), CGAL::LAS_property::B()), - std::make_pair(GetAlphaMap(), CGAL::LAS_property::I()) - ); + CGAL::make_las_point_writer(CGAL::First_of_pair_property_map()), + std::make_pair(GetRedMap(),CGAL::LAS_property::R()), + std::make_pair(GetGreenMap(), CGAL::LAS_property::G()), + std::make_pair(GetBlueMap(), CGAL::LAS_property::B()), + std::make_pair(GetAlphaMap(), CGAL::LAS_property::I()) + ); assert(ok); } @@ -104,13 +104,13 @@ ps.push_back(Point_3(0,0,1)); points.clear(); std::ifstream is("tmp1.las", std::ios::binary); ok = CGAL::read_las_points_with_properties(is, std::back_inserter (points), - CGAL::make_las_point_reader(CGAL::First_of_pair_property_map()), - std::make_tuple(CGAL::Second_of_pair_property_map(), - CGAL::Construct_array(), - CGAL::LAS_property::R(), - CGAL::LAS_property::G(), - CGAL::LAS_property::B(), - CGAL::LAS_property::I())); + CGAL::make_las_point_reader(CGAL::First_of_pair_property_map()), + std::make_tuple(CGAL::Second_of_pair_property_map(), + CGAL::Construct_array(), + CGAL::LAS_property::R(), + CGAL::LAS_property::G(), + CGAL::LAS_property::B(), + CGAL::LAS_property::I())); assert(ok); assert(points.size() == 3); assert(points[1].second[1] == 255); @@ -137,12 +137,12 @@ ps.push_back(Point_3(0,0,1)); std::ofstream os("tmp1.ply"); assert(os.good()); ok = CGAL::write_ply_points_with_properties(os, points, - CGAL::make_ply_point_writer (CGAL::First_of_pair_property_map()), - std::make_pair(GetRedMap(),CGAL::PLY_property("red")), - std::make_pair(GetGreenMap(), CGAL::PLY_property("green")), - std::make_pair(GetBlueMap(), CGAL::PLY_property("blue")), - std::make_pair(GetAlphaMap(), CGAL::PLY_property("alpha")) - ); + CGAL::make_ply_point_writer (CGAL::First_of_pair_property_map()), + std::make_pair(GetRedMap(),CGAL::PLY_property("red")), + std::make_pair(GetGreenMap(), CGAL::PLY_property("green")), + std::make_pair(GetBlueMap(), CGAL::PLY_property("blue")), + std::make_pair(GetAlphaMap(), CGAL::PLY_property("alpha")) + ); assert(! os.fail()); assert(ok); } @@ -152,13 +152,13 @@ ps.push_back(Point_3(0,0,1)); assert(is.good()); points.clear(); ok = CGAL::read_ply_points_with_properties(is, std::back_inserter (points), - CGAL::make_ply_point_reader(CGAL::First_of_pair_property_map()), - std::make_tuple(CGAL::Second_of_pair_property_map(), - CGAL::Construct_array(), - CGAL::PLY_property("red"), - CGAL::PLY_property("green"), - CGAL::PLY_property("blue"), - CGAL::PLY_property("alpha"))); + CGAL::make_ply_point_reader(CGAL::First_of_pair_property_map()), + std::make_tuple(CGAL::Second_of_pair_property_map(), + CGAL::Construct_array(), + CGAL::PLY_property("red"), + CGAL::PLY_property("green"), + CGAL::PLY_property("blue"), + CGAL::PLY_property("alpha"))); assert(! is.fail()); assert(ok); assert(points.size() == 3); @@ -178,7 +178,7 @@ ps.push_back(Point_3(0,0,1)); assert(is.good()); ps.clear(); ok = CGAL::read_ply_points(is, std::back_inserter (ps), - CGAL::parameters::all_default()); + CGAL::parameters::all_default()); assert(! is.fail()); assert(ok); assert(ps.size() == 3); @@ -198,7 +198,7 @@ ps.push_back(Point_3(0,0,1)); assert(is.good()); ps.clear(); ok = CGAL::read_off_points(is, std::back_inserter (ps), - CGAL::parameters::all_default()); + CGAL::parameters::all_default()); assert(! is.fail()); assert(ok); assert(ps.size() == 3); @@ -218,7 +218,7 @@ ps.push_back(Point_3(0,0,1)); assert(is.good()); ps.clear(); ok = CGAL::read_xyz_points(is, std::back_inserter (ps), - CGAL::parameters::all_default()); + CGAL::parameters::all_default()); assert(! is.fail()); assert(ok); assert(ps.size() == 3);