From f7123f3ce9db470cf118db282eb6277be2481fca Mon Sep 17 00:00:00 2001 From: Christian Sailer Date: Wed, 17 May 2017 22:56:06 +0100 Subject: [PATCH 1/5] Factor out grid min/max calculations to helper class + tests. Remove pointless checks from GridDialog (the QSpinner makes sure the result is always within the set limits, no point checking again) --- depthmapX/GraphDoc.cpp | 5 +-- depthmapX/GridDialog.cpp | 69 ++++----------------------------- depthmapX/GridDialog.h | 21 ++++------ salaTest/salaTest.pro | 3 +- salaTest/testgridproperties.cpp | 26 +++++++++++++ salalib/gridproperties.cpp | 13 +++++++ salalib/gridproperties.h | 31 +++++++++++++++ salalib/salalib.pro | 6 ++- 8 files changed, 92 insertions(+), 82 deletions(-) create mode 100644 salaTest/testgridproperties.cpp create mode 100644 salalib/gridproperties.cpp create mode 100644 salalib/gridproperties.h diff --git a/depthmapX/GraphDoc.cpp b/depthmapX/GraphDoc.cpp index d4ed08b9..bc648511 100644 --- a/depthmapX/GraphDoc.cpp +++ b/depthmapX/GraphDoc.cpp @@ -669,15 +669,14 @@ void QGraphDoc::OnEditGrid() if ( QMessageBox::Yes != QMessageBox::question(this, tr("depthmapX"), tr("This will clear existing points. Do you want to continue?"), QMessageBox::Yes|QMessageBox::No, QMessageBox::No) ) return; } - CGridDialog dlg; QtRegion r = m_meta_graph->SuperSpacePixel::getRegion(); - dlg.m_maxdimension = __max(r.width(), r.height()); + CGridDialog dlg(__max(r.width(), r.height())); if (QDialog::Accepted == dlg.exec()) { if (newmap) { m_meta_graph->PointMaps::addNewMap(); } - m_meta_graph->setGrid( dlg.m_spacing, Point2f(0.0f, 0.0f) ); + m_meta_graph->setGrid( dlg.getSpacing(), Point2f(0.0f, 0.0f) ); m_meta_graph->m_showgrid = true; SetUpdateFlag(NEW_TABLE); SetRedrawFlag(VIEW_ALL,REDRAW_GRAPH, NEW_DATA); diff --git a/depthmapX/GridDialog.cpp b/depthmapX/GridDialog.cpp index e74a5583..80f8d337 100644 --- a/depthmapX/GridDialog.cpp +++ b/depthmapX/GridDialog.cpp @@ -1,4 +1,5 @@ // Copyright (C) 2011-2012, Tasos Varoudis +// Copyright (C) 2017 Christian Sailer // This program is free software: you can redistribute it and/or modify // it under the terms of the GNU General Public License as published by @@ -15,30 +16,21 @@ #include "GridDialog.h" #include +#include -CGridDialog::CGridDialog(QWidget *parent) -: QDialog(parent) +CGridDialog::CGridDialog(double maxDimension, QWidget *parent) +: QDialog(parent), m_maxdimension(maxDimension) { setupUi(this); m_spacing = 0.01; - m_maxdimension = 1.0; } void CGridDialog::showEvent(QShowEvent * event) { - m_maxexponent = (int) floor(log10(m_maxdimension)) - 1; - m_minexponent = m_maxexponent - 2; - m_basemantissa = (int) floor(m_maxdimension / pow(10.0,double(m_maxexponent+1))); + GridProperties gp(m_maxdimension); + m_spacing = gp.getDefault(); - // current: - m_mantissa = m_basemantissa; - m_exponent = m_maxexponent - 1; - - m_spacing = (double) m_mantissa * pow(10.0, double(m_exponent)); - - double truemax = (double) 2 * m_mantissa * pow(10.0, double(m_maxexponent)); - double truemin = (double) m_mantissa * pow(10.0, double(m_minexponent)); - c_spacing_ctrl->setRange(truemin, truemax); + c_spacing_ctrl->setRange(gp.getMin(), gp.getMax()); UpdateData(false); } @@ -58,48 +50,6 @@ void CGridDialog::OnDeltaposSpinSpacing(double iDelta) void CGridDialog::OnOK() { UpdateData(true); - - double truemax = (double) 2 * m_mantissa * pow(10.0, double(m_maxexponent)); - double truemin = (double) m_mantissa * pow(10.0, double(m_minexponent)); - if (m_spacing > truemax || m_spacing < truemin) { - QString formatabsmin, formatmin, formatmax; - if (m_minexponent < 0) { - formatmin.sprintf("%%.%df", abs(m_minexponent)); - } - else { - formatmin = tr("%.0f"); - } - if (m_minexponent-1 < 0) { - formatabsmin.sprintf("%%.%df", abs(m_minexponent-1)); - } - else { - formatabsmin = tr("%.0f"); - } - if (m_maxexponent < 0) { - formatmax.sprintf("%%.%df" ,abs(m_maxexponent)); - } - else { - formatmax = tr("%.0f"); - } - QString absminstr, minstr, maxstr; - absminstr.sprintf(formatabsmin.toLatin1(), truemin/10); - minstr.sprintf(formatmin.toLatin1(), truemin); - maxstr.sprintf(formatmax.toLatin1(), truemax); - if (m_spacing >= truemin / 10 && m_spacing < truemin) { - QString msg; - msg = tr("You are below the suggested minimum grid spacing of ") + minstr + tr(". If you use this grid spacing, it may cause processing problems.\nAre you sure you want to proceed with this grid spacing?"); - if (QMessageBox::No == QMessageBox::question(this, tr("Question"), msg, QMessageBox::Yes | QMessageBox::No)) { - return; - } - } - else { - QString msg; - msg = tr("Please enter a spacing between ") + absminstr + tr(" (at the absolute minimum) and ") + maxstr; - QMessageBox::warning(this, "Notice", msg, QMessageBox::Ok, QMessageBox::Ok); - return; - } - } - accept(); } @@ -119,8 +69,3 @@ void CGridDialog::UpdateData(bool value) c_spacing_ctrl->setValue(m_spacing); } } - -void CGridDialog::userValue(double value) -{ - m_spacing = c_spacing_ctrl->value(); -} diff --git a/depthmapX/GridDialog.h b/depthmapX/GridDialog.h index 45d4cb48..afdbf059 100644 --- a/depthmapX/GridDialog.h +++ b/depthmapX/GridDialog.h @@ -1,4 +1,5 @@ // Copyright (C) 2011-2012, Tasos Varoudis +// Copyright (C) 2017 Christian Sailer // This program is free software: you can redistribute it and/or modify // it under the terms of the GNU General Public License as published by @@ -14,30 +15,22 @@ // along with this program. If not, see . #include "ui_GridDialog.h" -#include -#include -#include -#include - class CGridDialog : public QDialog, public Ui::CGridDialog { Q_OBJECT public: - CGridDialog(QWidget *parent = 0); - double m_spacing; - double m_maxdimension; - int m_minexponent; - int m_maxexponent; - int m_basemantissa; - int m_mantissa; - int m_exponent; + CGridDialog(double maxDimension, QWidget *parent = 0); void UpdateData(bool value); void showEvent(QShowEvent * event); + double getSpacing() const { return m_spacing; } + +private: + double m_spacing; + double m_maxdimension; private slots: void OnDeltaposSpinSpacing(double); void OnOK(); void OnCancel(); - void userValue(double value); }; diff --git a/salaTest/salaTest.pro b/salaTest/salaTest.pro index eb676714..292e48ee 100644 --- a/salaTest/salaTest.pro +++ b/salaTest/salaTest.pro @@ -8,7 +8,8 @@ INCLUDEPATH += ../ThirdParty/Catch SOURCES += main.cpp \ testentityparsing.cpp \ testpointmap.cpp \ - testlinkutils.cpp + testlinkutils.cpp \ + testgridproperties.cpp win32:Release:LIBS += -L../genlib/release -L../salalib/release win32:Debug:LIBS += -L../genlib/debug -L../salalib/debug diff --git a/salaTest/testgridproperties.cpp b/salaTest/testgridproperties.cpp new file mode 100644 index 00000000..f82ca1b6 --- /dev/null +++ b/salaTest/testgridproperties.cpp @@ -0,0 +1,26 @@ +// Copyright (C) 2017 Christian Sailer + +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +#include "catch.hpp" +#include "salalib/gridproperties.h" + +TEST_CASE("TestGridProperties", "Test the calculations of grid properties") +{ + double maxDimension = 4.583; + GridProperties gp(maxDimension); + REQUIRE(gp.getDefault() == Approx(0.04)); + REQUIRE(gp.getMax() == Approx(0.8)); + REQUIRE(gp.getMin() == Approx(0.004)); +} diff --git a/salalib/gridproperties.cpp b/salalib/gridproperties.cpp new file mode 100644 index 00000000..26193d21 --- /dev/null +++ b/salalib/gridproperties.cpp @@ -0,0 +1,13 @@ +#include "gridproperties.h" +#include + +GridProperties::GridProperties(double maxDimension) +{ + int maxexponent = (int) floor(log10(maxDimension)) - 1; + int minexponent = maxexponent - 2; + int mantissa = (int) floor(maxDimension / pow(10.0,double(maxexponent+1))); + + m_default = (double) mantissa * pow(10.0, double(maxexponent - 1)); + m_max = (double) 2 * mantissa * pow(10.0, double(maxexponent)); + m_min = (double) mantissa * pow(10.0, double(minexponent)); +} diff --git a/salalib/gridproperties.h b/salalib/gridproperties.h new file mode 100644 index 00000000..ad3d5714 --- /dev/null +++ b/salalib/gridproperties.h @@ -0,0 +1,31 @@ +// Copyright (C) 2017 Christian Sailer + +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +#pragma once + +class GridProperties +{ +public: + GridProperties(double maxDimension); + double getMin() const{return m_min;} + double getMax() const{return m_max;} + double getDefault() const{return m_default;} +private: + double m_max; + double m_min; + double m_default; + +}; + diff --git a/salalib/salalib.pro b/salalib/salalib.pro index 69797681..c538e2d4 100644 --- a/salalib/salalib.pro +++ b/salalib/salalib.pro @@ -38,7 +38,8 @@ SOURCES += \ topomet.cpp \ vertex.cpp \ entityparsing.cpp \ - linkutils.cpp + linkutils.cpp \ + gridproperties.cpp HEADERS += \ attributes.h \ @@ -63,7 +64,8 @@ HEADERS += \ vertex.h \ idepthmap.h \ entityparsing.h \ - linkutils.h + linkutils.h \ + gridproperties.h DISTFILES += \ salascript-tests.txt From 05b5be896d6e72b3acb8935ceca4a7374f54b663 Mon Sep 17 00:00:00 2001 From: Christian Sailer Date: Fri, 19 May 2017 14:38:23 +0100 Subject: [PATCH 2/5] Command line mode to prepare for VGA (grid, fill and connectivity graph) --- cliTest/cliTest.pro | 5 +- cliTest/testvisprepparser.cpp | 186 +++++++++++++++++++++++++++++ depthmapXcli/commandlineparser.cpp | 21 +--- depthmapXcli/depthmapXcli.pro | 7 +- depthmapXcli/linkparser.cpp | 17 +-- depthmapXcli/parsingutils.h | 28 +++++ depthmapXcli/runmethods.cpp | 57 +++++++++ depthmapXcli/runmethods.h | 2 + depthmapXcli/vgaparser.cpp | 11 +- depthmapXcli/visprepparser.cpp | 129 ++++++++++++++++++++ depthmapXcli/visprepparser.h | 58 +++++++++ 11 files changed, 480 insertions(+), 41 deletions(-) create mode 100644 cliTest/testvisprepparser.cpp create mode 100644 depthmapXcli/parsingutils.h create mode 100644 depthmapXcli/visprepparser.cpp create mode 100644 depthmapXcli/visprepparser.h diff --git a/cliTest/cliTest.pro b/cliTest/cliTest.pro index e5c02e50..d53491d1 100644 --- a/cliTest/cliTest.pro +++ b/cliTest/cliTest.pro @@ -20,7 +20,10 @@ SOURCES += main.cpp \ testperformancewriter.cpp \ testselfcleaningfile.cpp \ ../depthmapXcli/runmethods.cpp \ - ../depthmapXcli/modeparserregistry.cpp + ../depthmapXcli/modeparserregistry.cpp \ + testvisprepparser.cpp \ + ../depthmapXcli/visprepparser.cpp + HEADERS += \ ../depthmapXcli/commandlineparser.h \ diff --git a/cliTest/testvisprepparser.cpp b/cliTest/testvisprepparser.cpp new file mode 100644 index 00000000..b8e55618 --- /dev/null +++ b/cliTest/testvisprepparser.cpp @@ -0,0 +1,186 @@ +// Copyright (C) 2017 Christian Sailer + +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +#include +#include "depthmapXcli/visprepparser.h" +#include "argumentholder.h" +#include "selfcleaningfile.h" + +TEST_CASE("VisPrepParserFail", "Error cases") +{ + SECTION("Missing argument to pg") + { + VisPrepParser parser; + ArgumentHolder ah{"prog", "-pg", "-pp", "1.2,1.3"}; + REQUIRE_THROWS_WITH(parser.parse(ah.argc(), ah.argv()), Catch::Contains("-pg requires an argument")); + } + SECTION("Missing argument to pp") + { + VisPrepParser parser; + ArgumentHolder ah{"prog", "-pp"}; + REQUIRE_THROWS_WITH(parser.parse(ah.argc(), ah.argv()), Catch::Contains("-pp requires an argument")); + } + SECTION("Missing argument to pf") + { + VisPrepParser parser; + ArgumentHolder ah{"prog", "-pf", "-pg", "1.2"}; + REQUIRE_THROWS_WITH(parser.parse(ah.argc(), ah.argv()), Catch::Contains("-pf requires an argument")); + } + + SECTION("Non-numeric input to -pg") + { + VisPrepParser parser; + ArgumentHolder ah{"prog", "-pg", "foo"}; + REQUIRE_THROWS_WITH(parser.parse(ah.argc(), ah.argv()), Catch::Contains("-pg must be a number >0, got foo")); + } + + SECTION("rubbish input to -pp") + { + VisPrepParser parser; + ArgumentHolder ah{"prog", "-pp", "foo"}; + REQUIRE_THROWS_WITH(parser.parse(ah.argc(), ah.argv()), Catch::Contains("Invalid fill point provided (foo). Should only contain digits dots and commas")); + } + + SECTION("Non-existing file provide") + { + VisPrepParser parser; + ArgumentHolder ah{"prog", "-pg", "0.1", "-pf", "foo.csv"}; + REQUIRE_THROWS_WITH(parser.parse(ah.argc(), ah.argv()), Catch::Contains("Failed to load file foo.csv, error")); + } + + SECTION("Neiter points nor point file provided") + { + VisPrepParser parser; + ArgumentHolder ah{"prog", "-pg", "0.1"}; + REQUIRE_THROWS_WITH(parser.parse(ah.argc(), ah.argv()), Catch::Contains("Either -pp or -pf must be given")); + } + + SECTION("No grid") + { + VisPrepParser parser; + ArgumentHolder ah{"prog", "-pp", "0.1,1"}; + REQUIRE_THROWS_WITH(parser.parse(ah.argc(), ah.argv()), Catch::Contains("Use -pg to define grid")); + } + + SECTION("Points and pointfile provided") + { + VisPrepParser parser; + SelfCleaningFile scf("testpoints.csv"); + { + std::ofstream f("testpoints.csv"); + f << "x\ty\n1\t2\n" << std::flush; + } + ArgumentHolder ah{"prog", "-pg", "0.1", "-pp", "0.1,5.2", "-pf", "testpoints.csv"}; + REQUIRE_THROWS_WITH(parser.parse(ah.argc(), ah.argv()), Catch::Contains("-pf cannot be used together with -pp")); + } + + SECTION("Pointfile and points provided") + { + VisPrepParser parser; + SelfCleaningFile scf("testpoints.csv"); + { + std::ofstream f("testpoints.csv"); + f << "x\ty\n1\t2\n" << std::flush; + } + ArgumentHolder ah{"prog", "-pg", "0.1", "-pf", "testpoints.csv", "-pp", "0.1,5.2"}; + REQUIRE_THROWS_WITH(parser.parse(ah.argc(), ah.argv()), Catch::Contains("-pp cannot be used together with -pf")); + } + + SECTION("Malformed pointfile") + { + VisPrepParser parser; + SelfCleaningFile scf("testpoints.csv"); + { + std::ofstream f("testpoints.csv"); + f << "x\ty\n1\n" << std::flush; + } + ArgumentHolder ah{"prog", "-pg", "0.1", "-pf", "testpoints.csv"}; + REQUIRE_THROWS_WITH(parser.parse(ah.argc(), ah.argv()), Catch::Contains("Error parsing line: 1")); + } + + SECTION("Malformed point arg") + { + VisPrepParser parser; + SelfCleaningFile scf("testpoints.csv"); + { + std::ofstream f("testpoints.csv"); + f << "x\ty\n1\n" << std::flush; + } + ArgumentHolder ah{"prog", "-pg", "0.1", "-pp", "0.1"}; + REQUIRE_THROWS_WITH(parser.parse(ah.argc(), ah.argv()), Catch::Contains("Error parsing line: 0.1")); + } + + SECTION("Nonsensical visiblity restriction") + { + VisPrepParser parser; + ArgumentHolder ah{"prog", "-pr", "foo"}; + REQUIRE_THROWS_WITH(parser.parse(ah.argc(), ah.argv()), Catch::Contains("Restricted visibilyt of 'foo' makes no sense, use a positive number or -1 for unrestricted")); + } + + SECTION("Nonsensical visiblity restriction") + { + VisPrepParser parser; + ArgumentHolder ah{"prog", "-pr", "0.0"}; + REQUIRE_THROWS_WITH(parser.parse(ah.argc(), ah.argv()), Catch::Contains("Restricted visibilyt of '0.0' makes no sense, use a positive number or -1 for unrestricted")); + } + +} + +TEST_CASE("VisprepParserSuccess", "Read successfully") +{ + VisPrepParser parser; + double x1 = 1.0; + double y1 = 2.0; + double x2 = 1.1; + double y2 = 1.2; + double grid = 0.5; + std::stringstream gstring; + gstring << grid << std::flush; + + SECTION("Read from commandline") + { + std::stringstream p1; + p1 << x1 << "," << y1 << std::flush; + std::stringstream p2; + p2 << x2 << "," << y2 << std::flush; + + ArgumentHolder ah{"prog", "-pg", gstring.str(), "-pp", p1.str(), "-pp", p2.str(), "-pb", "-pr", "2.1"}; + parser.parse(ah.argc(), ah.argv()); + REQUIRE(parser.getBoundaryGraph()); + REQUIRE(parser.getMaxVisibility() == Approx(2.1)); + } + + SECTION("Read from file") + { + SelfCleaningFile scf("testpoints.csv"); + { + std::ofstream f(scf.Filename().c_str()); + f << "x\ty\n" << x1 << "\t" << y1 << "\n" + << x2 << "\t" << y2 << "\n" << std::flush; + } + ArgumentHolder ah{"prog", "-pg", gstring.str(), "-pf", scf.Filename()}; + parser.parse(ah.argc(), ah.argv() ); + REQUIRE_FALSE(parser.getBoundaryGraph()); + REQUIRE(parser.getMaxVisibility() == Approx(-1.0)); + } + + REQUIRE(parser.getGrid() == Approx(grid)); + auto points = parser.getFillPoints(); + REQUIRE(points.size() == 2); + REQUIRE(points[0].x == Approx(x1)); + REQUIRE(points[0].y == Approx(y1)); + REQUIRE(points[1].x == Approx(x2)); + REQUIRE(points[1].y == Approx(y2)); +} diff --git a/depthmapXcli/commandlineparser.cpp b/depthmapXcli/commandlineparser.cpp index 22529059..18b55f9c 100644 --- a/depthmapXcli/commandlineparser.cpp +++ b/depthmapXcli/commandlineparser.cpp @@ -16,6 +16,7 @@ #include "commandlineparser.h" #include "exceptions.h" #include "imodeparserfactory.h" +#include "parsingutils.h" #include #include #include @@ -60,10 +61,7 @@ void CommandLineParser::parse(size_t argc, char *argv[]) { throw CommandLineException("-m can only be used once"); } - if ( ++i >= argc || argv[i][0] == '-' ) - { - throw CommandLineException("-m requires an argument"); - } + ENFORCE_ARGUMENT("-m", i) for (auto iter = _parserFactory.getModeParsers().begin(), end = _parserFactory.getModeParsers().end(); iter != end; ++iter ) @@ -82,26 +80,17 @@ void CommandLineParser::parse(size_t argc, char *argv[]) } else if ( strcmp ("-f", argv[i]) == 0) { - if ( ++i >= argc || argv[i][0] == '-' ) - { - throw CommandLineException("-f requires an argument"); - } + ENFORCE_ARGUMENT("-f", i) _fileName = argv[i]; } else if ( strcmp ("-o", argv[i]) == 0) { - if ( ++i >= argc || argv[i][0] == '-' ) - { - throw CommandLineException("-o requires an argument"); - } + ENFORCE_ARGUMENT("-o", i) _outputFile = argv[i]; } else if ( strcmp ("-t", argv[i]) == 0) { - if ( ++i >= argc || argv[i][0] == '-' ) - { - throw CommandLineException("-t requires an argument"); - } + ENFORCE_ARGUMENT("-t", i) _timingFile = argv[i]; } else if ( strcmp("-s", argv[i]) == 0) diff --git a/depthmapXcli/depthmapXcli.pro b/depthmapXcli/depthmapXcli.pro index 8c92987c..26c6bb65 100644 --- a/depthmapXcli/depthmapXcli.pro +++ b/depthmapXcli/depthmapXcli.pro @@ -11,7 +11,8 @@ SOURCES += main.cpp \ vgaparser.cpp \ linkparser.cpp \ performancewriter.cpp \ - modeparserregistry.cpp + modeparserregistry.cpp \ + visprepparser.cpp HEADERS += \ commandlineparser.h \ @@ -25,7 +26,9 @@ HEADERS += \ performancesink.h \ imodeparser.h \ modeparserregistry.h \ - imodeparserfactory.h + imodeparserfactory.h \ + visprepparser.h \ + parsingutils.h win32:Release:LIBS += -L../genlib/release -L../salalib/release win32:Debug:LIBS += -L../genlib/debug -L../salalib/debug diff --git a/depthmapXcli/linkparser.cpp b/depthmapXcli/linkparser.cpp index 21e9b582..ce8c5ca2 100644 --- a/depthmapXcli/linkparser.cpp +++ b/depthmapXcli/linkparser.cpp @@ -18,18 +18,13 @@ #include "salalib/entityparsing.h" #include "exceptions.h" #include "runmethods.h" +#include "parsingutils.h" #include #include #include using namespace depthmapX; -namespace { - bool has_only_digits_dots_commas(const std::string &s){ - return s.find_first_not_of( "0123456789,." ) == std::string::npos; - } -} - void LinkParser::parse(int argc, char *argv[]) { std::string linksFile; @@ -47,10 +42,7 @@ void LinkParser::parse(int argc, char *argv[]) { throw CommandLineException("-lf can not be used in conjunction with -lnk"); } - if ( ++i >= argc || argv[i][0] == '-' ) - { - throw CommandLineException("-lf requires an argument"); - } + ENFORCE_ARGUMENT("-lf", i) linksFile = argv[i]; } else if ( strcmp ("-lnk", argv[i]) == 0) @@ -59,10 +51,7 @@ void LinkParser::parse(int argc, char *argv[]) { throw CommandLineException("-lf can not be used in conjunction with -lnk"); } - if ( ++i >= argc || argv[i][0] == '-' ) - { - throw CommandLineException("-lnk requires an argument"); - } + ENFORCE_ARGUMENT("-lnk", i) if (!has_only_digits_dots_commas(argv[i])) { std::stringstream message; diff --git a/depthmapXcli/parsingutils.h b/depthmapXcli/parsingutils.h new file mode 100644 index 00000000..d43fe16a --- /dev/null +++ b/depthmapXcli/parsingutils.h @@ -0,0 +1,28 @@ +// Copyright (C) 2017 Christian Sailer + +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +#pragma once + +#define ENFORCE_ARGUMENT(flag, counter)\ + if ( ++ ##counter >= argc || argv[ ##counter][0] == '-' )\ + {\ + throw CommandLineException(flag " requires an argument");\ + }\ + +namespace depthmapX{ + inline bool has_only_digits_dots_commas(const std::string &s){ + return s.find_first_not_of( "0123456789,." ) == std::string::npos; + } +} diff --git a/depthmapXcli/runmethods.cpp b/depthmapXcli/runmethods.cpp index 69d59d98..66653b90 100644 --- a/depthmapXcli/runmethods.cpp +++ b/depthmapXcli/runmethods.cpp @@ -22,6 +22,7 @@ #include #include #include +#include namespace dm_runmethods { @@ -105,4 +106,60 @@ namespace dm_runmethods std::cout << " ok" << std::endl; } + void fillGraph(MetaGraph& graph, const Point2f& point) + { + auto r = graph.getRegion(); + if (!r.contains(point)) + { + throw depthmapX::RuntimeException("Point outside of target region"); + } + graph.makePoints(point, 0, 0); + } + + void runVisualPrep( + const CommandLineParser &clp, + double gridSize, const std::vector &fillPoints, + double maxVisibility, + bool boundaryGraph, + IPerformanceSink &perfWriter) + { + auto mGraph = loadGraph(clp.getFileName().c_str(),perfWriter); + + std::cout << "Initial checks... " << std::flush; + auto state = mGraph->getState(); + if (~state & MetaGraph::LINEDATA) + { + throw depthmapX::RuntimeException("Graph must have line data before preparing VGA"); + } + // set grid + QtRegion r = mGraph->getRegion(); + + GridProperties gp(__max(r.width(), r.height())); + if ( gridSize > gp.getMax() || gridSize < gp.getMin()) + { + std::stringstream message; + message << "Chosen grid spacing " << gridSize << " is outside of the expected intervall of " + << gp.getMin() << " <= spacing <= " << gp.getMax() << std::flush; + throw depthmapX::RuntimeException(message.str()); + } + + std::cout << "ok\nSetting up grid... " << std::flush; + if (!mGraph->PointMaps::size() || mGraph->getDisplayedPointMap().isProcessed()) { + // this can happen if there are no displayed maps -- so flag new map required: + mGraph->addNewMap(); + } + DO_TIMED("Setting grid", mGraph->setGrid(gridSize, Point2f(0.0, 0.0))) + + std::cout << "ok\nFilling grid... " << std::flush; + DO_TIMED("Filling grid", + for_each(fillPoints.begin(), fillPoints.end(), [&mGraph](const Point2f &point)->void{fillGraph(*mGraph, point);})) + + + std::cout << "ok\nCalculating connectivity... " << std::flush; + DO_TIMED("Calculate Connectivity", mGraph->makeGraph(0, boundaryGraph ? 1 : 0, maxVisibility)) + std::cout << " ok\nWriting out result..." << std::flush; + DO_TIMED("Writing graph", mGraph->write(clp.getOuputFile().c_str(),METAGRAPH_VERSION, false)) + std::cout << " ok" << std::endl; + } + } diff --git a/depthmapXcli/runmethods.h b/depthmapXcli/runmethods.h index 231ed128..adb00fc3 100644 --- a/depthmapXcli/runmethods.h +++ b/depthmapXcli/runmethods.h @@ -23,9 +23,11 @@ #include class Line; +class Point2f; namespace dm_runmethods{ void linkGraph(const CommandLineParser &cmdP, const std::vector &mergeLines, IPerformanceSink &perfWriter ); void runVga(const CommandLineParser &cmdP, const VgaParser &vgaP, const IRadiusConverter &converter, IPerformanceSink &perfWriter ); + void runVisualPrep(const CommandLineParser &clp, double gridSize, const std::vector &fillPoints, double maxVisibility, bool boundaryGraph, IPerformanceSink &perfWriter); } #endif // RUNMETHODS_H diff --git a/depthmapXcli/vgaparser.cpp b/depthmapXcli/vgaparser.cpp index 7ccc6dd0..87abe8d9 100644 --- a/depthmapXcli/vgaparser.cpp +++ b/depthmapXcli/vgaparser.cpp @@ -18,6 +18,7 @@ #include #include "radiusconverter.h" #include "runmethods.h" +#include "parsingutils.h" using namespace depthmapX; @@ -42,10 +43,7 @@ void VgaParser::parse(int argc, char *argv[]) { throw CommandLineException("-vm can only be used once, modes are mutually exclusive"); } - if ( ++i >= argc || argv[i][0] == '-' ) - { - throw CommandLineException("-vm requires an argument"); - } + ENFORCE_ARGUMENT("-vm", i) if ( strcmp(argv[i], "isovist") == 0 ) { _vgaMode = VgaMode::ISOVIST; @@ -81,10 +79,7 @@ void VgaParser::parse(int argc, char *argv[]) } else if (strcmp(argv[i], "-vr") == 0) { - if ( ++i >= argc || argv[i][0] == '-' ) - { - throw CommandLineException("-vr requires an argument"); - } + ENFORCE_ARGUMENT("-vr", i) _radius = argv[i]; } ++i; diff --git a/depthmapXcli/visprepparser.cpp b/depthmapXcli/visprepparser.cpp new file mode 100644 index 00000000..0d95a5d5 --- /dev/null +++ b/depthmapXcli/visprepparser.cpp @@ -0,0 +1,129 @@ +// Copyright (C) 2017 Christian Sailer + +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +#include "visprepparser.h" +#include "exceptions.h" +#include "parsingutils.h" +#include "salalib/entityparsing.h" +#include "runmethods.h" +#include + +using namespace depthmapX; + +void VisPrepParser::parse(int argc, char ** argv) +{ + + std::vector points; + std::string pointFile; + for ( int i = 1; i < argc; ++i ) + { + if ( strcmp ("-pg", argv[i]) == 0) + { + if (m_grid >= 0) + { + throw CommandLineException("-pg can only be used once"); + } + ENFORCE_ARGUMENT("-pg", i) + m_grid = std::atof(argv[i]); + if (m_grid <= 0) + { + throw CommandLineException(std::string("-pg must be a number >0, got ") + argv[i]); + } + } + else if ( strcmp ("-pp", argv[i]) == 0) + { + if (!pointFile.empty()) + { + throw CommandLineException("-pp cannot be used together with -pf"); + } + ENFORCE_ARGUMENT("-pp", i) + if (!has_only_digits_dots_commas(argv[i])) + { + std::stringstream message; + message << "Invalid fill point provided (" + << argv[i] + << "). Should only contain digits dots and commas" + << flush; + throw CommandLineException(message.str().c_str()); + } + points.push_back(argv[i]); + } + else if ( strcmp("-pf", argv[i]) == 0 ) + { + if (!points.empty()) + { + throw CommandLineException("-pf cannot be used together with -pp"); + } + ENFORCE_ARGUMENT("-pf", i) + pointFile = argv[i]; + } + else if ( strcmp("-pr", argv[i]) == 0) + { + ENFORCE_ARGUMENT("-pr", i); + m_maxVisibility = std::atof(argv[i]); + if ( m_maxVisibility == 0.0) + { + std::stringstream message; + message << "Restricted visibilyt of '" << argv[i] << "' makes no sense, use a positive number or -1 for unrestricted"; + throw CommandLineException(message.str()); + } + } + else if ( strcmp("-pb", argv[i]) == 0 ) + { + m_boundaryGraph = true; + } + } + if (m_grid < 0) + { + throw CommandLineException("Use -pg to define grid"); + } + + if (pointFile.empty() && points.empty()) + { + throw CommandLineException("Either -pp or -pf must be given"); + } + + if(!pointFile.empty()) + { + std::ifstream pointsStream(pointFile); + if (!pointsStream) + { + std::stringstream message; + message << "Failed to load file " << pointFile << ", error " << strerror(errno) << flush; + throw depthmapX::RuntimeException(message.str().c_str()); + } + vector parsed = EntityParsing::parsePoints(pointsStream, '\t'); + m_fillPoints.insert(std::end(m_fillPoints), std::begin(parsed), std::end(parsed)); + } + else if(!points.empty()) + { + std::stringstream pointsStream; + pointsStream << "x,y"; + std::vector::iterator iter = points.begin(), end = + points.end(); + for ( ; iter != end; ++iter ) + { + pointsStream << "\n" << *iter; + } + vector parsed = EntityParsing::parsePoints(pointsStream, ','); + m_fillPoints.insert(std::end(m_fillPoints), std::begin(parsed), std::end(parsed)); + + } +} + +void VisPrepParser::run(const CommandLineParser &clp, IPerformanceSink &perfWriter) const +{ + dm_runmethods::runVisualPrep(clp, m_grid, m_fillPoints, m_maxVisibility, m_boundaryGraph, perfWriter); +} diff --git a/depthmapXcli/visprepparser.h b/depthmapXcli/visprepparser.h new file mode 100644 index 00000000..bbfaac3d --- /dev/null +++ b/depthmapXcli/visprepparser.h @@ -0,0 +1,58 @@ +// Copyright (C) 2017 Christian Sailer + +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . +#pragma once + +#include "imodeparser.h" +#include "genlib/p2dpoly.h" +#include + +class VisPrepParser : public IModeParser +{ +public: + VisPrepParser() : m_grid(-1.0), m_maxVisibility(-1.0), m_boundaryGraph(false) + {} + + virtual std::string getModeName() const + { + return "VISPREP"; + } + + virtual std::string getHelp() const + { + return "Mode options for VISPREP (visual analysis preparation) are:\n" \ + " -pg floating point number defining the grid spacing\n" \ + " -pp point where to fill. Can be repeated\n" \ + " -pf a file with a point per line to fill\n" \ + " -pr restrict visibility (-1 is unrestricted, default)\n" \ + " -pb Make boundary graph\n"; + } + + virtual void parse(int argc, char** argv); + + virtual void run(const CommandLineParser &clp, IPerformanceSink &perfWriter) const; + + double getGrid() const { return m_grid; } + std::vector getFillPoints() const { return m_fillPoints; } + bool getBoundaryGraph() const { return m_boundaryGraph; } + double getMaxVisibility() const { return m_maxVisibility; } + +private: + double m_grid; + std::vector m_fillPoints; + double m_maxVisibility; + bool m_boundaryGraph; +}; + + From 5d92a0128a3efa6e1141baee3f39c6fe4dc2559e Mon Sep 17 00:00:00 2001 From: Christian Sailer Date: Fri, 19 May 2017 15:05:57 +0100 Subject: [PATCH 3/5] Add parser to registry --- depthmapXcli/modeparserregistry.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/depthmapXcli/modeparserregistry.cpp b/depthmapXcli/modeparserregistry.cpp index ab4a248e..907b3254 100644 --- a/depthmapXcli/modeparserregistry.cpp +++ b/depthmapXcli/modeparserregistry.cpp @@ -1,6 +1,7 @@ #include "modeparserregistry.h" #include "linkparser.h" #include "vgaparser.h" +#include "visprepparser.h" void ModeParserRegistry::populateParsers() @@ -8,5 +9,6 @@ void ModeParserRegistry::populateParsers() // Register any mode parsers here REGISTER_PARSER(VgaParser); REGISTER_PARSER(LinkParser); + REGISTER_PARSER(VisPrepParser); // ********* } From b5da9ae748ab76f88efe1d9e2add247914ba8aee Mon Sep 17 00:00:00 2001 From: Christian Sailer Date: Mon, 22 May 2017 13:15:46 +0100 Subject: [PATCH 4/5] Rework from pull request review: Fix macro compilation on gcc and clang, typos and some signed/unsigned warnings --- cliTest/testlinkparser.cpp | 2 +- cliTest/testvisprepparser.cpp | 4 ++-- depthmapXcli/linkparser.cpp | 2 +- depthmapXcli/parsingutils.h | 4 ++-- depthmapXcli/runmethods.cpp | 2 +- depthmapXcli/vgaparser.cpp | 2 +- depthmapXcli/visprepparser.cpp | 2 +- 7 files changed, 9 insertions(+), 9 deletions(-) diff --git a/cliTest/testlinkparser.cpp b/cliTest/testlinkparser.cpp index db363f87..853de497 100644 --- a/cliTest/testlinkparser.cpp +++ b/cliTest/testlinkparser.cpp @@ -60,7 +60,7 @@ TEST_CASE("LINK args valid", "valid") { // for this set of tests a difference less than 0.001 should // suffice to signify that two floats are the same - const float EPSILON = 0.001; + const float EPSILON = 0.001f; { ArgumentHolder ah{"prog", "-f", "infile", "-o", "outfile", "-m", "LINK", "-lnk", "1.2,3.4,5.6,7.8"}; diff --git a/cliTest/testvisprepparser.cpp b/cliTest/testvisprepparser.cpp index b8e55618..c815e1e6 100644 --- a/cliTest/testvisprepparser.cpp +++ b/cliTest/testvisprepparser.cpp @@ -122,14 +122,14 @@ TEST_CASE("VisPrepParserFail", "Error cases") REQUIRE_THROWS_WITH(parser.parse(ah.argc(), ah.argv()), Catch::Contains("Error parsing line: 0.1")); } - SECTION("Nonsensical visiblity restriction") + SECTION("Nonsensical visibility restriction") { VisPrepParser parser; ArgumentHolder ah{"prog", "-pr", "foo"}; REQUIRE_THROWS_WITH(parser.parse(ah.argc(), ah.argv()), Catch::Contains("Restricted visibilyt of 'foo' makes no sense, use a positive number or -1 for unrestricted")); } - SECTION("Nonsensical visiblity restriction") + SECTION("Nonsensical visibility restriction") { VisPrepParser parser; ArgumentHolder ah{"prog", "-pr", "0.0"}; diff --git a/depthmapXcli/linkparser.cpp b/depthmapXcli/linkparser.cpp index ce8c5ca2..4e7129d1 100644 --- a/depthmapXcli/linkparser.cpp +++ b/depthmapXcli/linkparser.cpp @@ -30,7 +30,7 @@ void LinkParser::parse(int argc, char *argv[]) std::string linksFile; std::vector manualLinks; - for ( size_t i = 1; i < argc; ) + for ( int i = 1; i < argc; ) { if ( strcmp ("-lf", argv[i]) == 0) { diff --git a/depthmapXcli/parsingutils.h b/depthmapXcli/parsingutils.h index d43fe16a..3a7f7817 100644 --- a/depthmapXcli/parsingutils.h +++ b/depthmapXcli/parsingutils.h @@ -14,9 +14,9 @@ // along with this program. If not, see . #pragma once - +#define PASTE(arg) arg #define ENFORCE_ARGUMENT(flag, counter)\ - if ( ++ ##counter >= argc || argv[ ##counter][0] == '-' )\ + if ( ++ PASTE(counter) >= argc || argv[ PASTE(counter)][0] == '-' )\ {\ throw CommandLineException(flag " requires an argument");\ }\ diff --git a/depthmapXcli/runmethods.cpp b/depthmapXcli/runmethods.cpp index 66653b90..0ab5aa0e 100644 --- a/depthmapXcli/runmethods.cpp +++ b/depthmapXcli/runmethods.cpp @@ -138,7 +138,7 @@ namespace dm_runmethods if ( gridSize > gp.getMax() || gridSize < gp.getMin()) { std::stringstream message; - message << "Chosen grid spacing " << gridSize << " is outside of the expected intervall of " + message << "Chosen grid spacing " << gridSize << " is outside of the expected interval of " << gp.getMin() << " <= spacing <= " << gp.getMax() << std::flush; throw depthmapX::RuntimeException(message.str()); } diff --git a/depthmapXcli/vgaparser.cpp b/depthmapXcli/vgaparser.cpp index 87abe8d9..4c5c5924 100644 --- a/depthmapXcli/vgaparser.cpp +++ b/depthmapXcli/vgaparser.cpp @@ -34,7 +34,7 @@ VgaParser::VgaParser() : _vgaMode(VgaMode::NONE), _globalMeasures(false), _local void VgaParser::parse(int argc, char *argv[]) { - for ( size_t i = 1; i < argc; ) + for ( int i = 1; i < argc; ) { if ( strcmp ("-vm", argv[i]) == 0) diff --git a/depthmapXcli/visprepparser.cpp b/depthmapXcli/visprepparser.cpp index 0d95a5d5..9416b541 100644 --- a/depthmapXcli/visprepparser.cpp +++ b/depthmapXcli/visprepparser.cpp @@ -76,7 +76,7 @@ void VisPrepParser::parse(int argc, char ** argv) if ( m_maxVisibility == 0.0) { std::stringstream message; - message << "Restricted visibilyt of '" << argv[i] << "' makes no sense, use a positive number or -1 for unrestricted"; + message << "Restricted visibility of '" << argv[i] << "' makes no sense, use a positive number or -1 for unrestricted"; throw CommandLineException(message.str()); } } From 730434fbbc5b93954e9089c5ff37d3666f0ac907 Mon Sep 17 00:00:00 2001 From: Christian Sailer Date: Mon, 22 May 2017 13:47:36 +0100 Subject: [PATCH 5/5] Fix typo in expected exception message in unit test --- cliTest/testvisprepparser.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cliTest/testvisprepparser.cpp b/cliTest/testvisprepparser.cpp index c815e1e6..e1865745 100644 --- a/cliTest/testvisprepparser.cpp +++ b/cliTest/testvisprepparser.cpp @@ -126,14 +126,14 @@ TEST_CASE("VisPrepParserFail", "Error cases") { VisPrepParser parser; ArgumentHolder ah{"prog", "-pr", "foo"}; - REQUIRE_THROWS_WITH(parser.parse(ah.argc(), ah.argv()), Catch::Contains("Restricted visibilyt of 'foo' makes no sense, use a positive number or -1 for unrestricted")); + REQUIRE_THROWS_WITH(parser.parse(ah.argc(), ah.argv()), Catch::Contains("Restricted visibility of 'foo' makes no sense, use a positive number or -1 for unrestricted")); } SECTION("Nonsensical visibility restriction") { VisPrepParser parser; ArgumentHolder ah{"prog", "-pr", "0.0"}; - REQUIRE_THROWS_WITH(parser.parse(ah.argc(), ah.argv()), Catch::Contains("Restricted visibilyt of '0.0' makes no sense, use a positive number or -1 for unrestricted")); + REQUIRE_THROWS_WITH(parser.parse(ah.argc(), ah.argv()), Catch::Contains("Restricted visibility of '0.0' makes no sense, use a positive number or -1 for unrestricted")); } }