From 63c34efbeb2785edab4230c742c077f625617788 Mon Sep 17 00:00:00 2001 From: Kai Pastor Date: Fri, 7 Feb 2020 08:04:35 +0100 Subject: [PATCH 1/3] SelectCRSDialog: Remove GeorefAlternatives This type was used for a parameter which had two usages, both with the same actual value. With the parameter removed, we can simplify the constructor. This change facilitates further modifications. --- src/gui/select_crs_dialog.cpp | 21 +++++++-------------- src/gui/select_crs_dialog.h | 20 +------------------- src/templates/template_image.cpp | 2 -- 3 files changed, 8 insertions(+), 35 deletions(-) diff --git a/src/gui/select_crs_dialog.cpp b/src/gui/select_crs_dialog.cpp index 99566df54..8dae83af8 100644 --- a/src/gui/select_crs_dialog.cpp +++ b/src/gui/select_crs_dialog.cpp @@ -1,6 +1,6 @@ /* * Copyright 2012, 2013 Thomas Schöps - * Copyright 2012-2015 Kai Pastor + * Copyright 2012-2020 Kai Pastor * * This file is part of OpenOrienteering. * @@ -54,7 +54,6 @@ enum SpecialCRS { SelectCRSDialog::SelectCRSDialog( const Georeferencing& georef, QWidget* parent, - GeorefAlternatives alternatives, const QString& description ) : QDialog(parent, Qt::WindowSystemMenuHint | Qt::WindowTitleHint) , georef(georef) @@ -64,22 +63,16 @@ SelectCRSDialog::SelectCRSDialog( crs_selector = new CRSSelector(georef, nullptr); if (georef.isLocal()) - crs_selector->clear(); - - if (alternatives.testFlag(TakeFromMap) && !georef.isLocal()) - { - crs_selector->addCustomItem(tr("Same as map"), SpecialCRS::SameAsMap); - crs_selector->setCurrentIndex(0); // TakeFromMap - } - - if (alternatives.testFlag(Local) || georef.isLocal()) { + crs_selector->clear(); crs_selector->addCustomItem(tr("Local"), SpecialCRS::Local); - crs_selector->setCurrentIndex(0); // TakeFromMap or Local, both is fine. } - - if (alternatives.testFlag(Geographic) && !georef.isLocal()) + else + { + crs_selector->addCustomItem(tr("Same as map"), SpecialCRS::SameAsMap); crs_selector->addCustomItem(tr("Geographic coordinates (WGS84)"), SpecialCRS::Geographic); + } + crs_selector->setCurrentIndex(0); status_label = new QLabel(); button_box = new QDialogButtonBox(QDialogButtonBox::Cancel | QDialogButtonBox::Ok); diff --git a/src/gui/select_crs_dialog.h b/src/gui/select_crs_dialog.h index 8a7a5736e..434fe7cf5 100644 --- a/src/gui/select_crs_dialog.h +++ b/src/gui/select_crs_dialog.h @@ -1,6 +1,6 @@ /* * Copyright 2012, 2013 Thomas Schöps - * Copyright 2012-2015 Kai Pastor + * Copyright 2012-2020 Kai Pastor * * This file is part of OpenOrienteering. * @@ -42,19 +42,6 @@ class SelectCRSDialog : public QDialog { Q_OBJECT public: - - /** - * Georeferencing alternatives - */ - enum GeorefAlternative - { - TakeFromMap = 1 << 0, - Local = 1 << 1, - Geographic = 1 << 2, - None = 0 - }; - Q_DECLARE_FLAGS(GeorefAlternatives, GeorefAlternative) - /** * Creates a SelectCRSDialog. * @@ -67,7 +54,6 @@ Q_OBJECT SelectCRSDialog( const Georeferencing& georef, QWidget* parent, - GeorefAlternatives alternatives, const QString& description = QString() ); @@ -92,8 +78,4 @@ Q_OBJECT } // namespace OpenOrienteering - -Q_DECLARE_OPERATORS_FOR_FLAGS(OpenOrienteering::SelectCRSDialog::GeorefAlternatives) - - #endif diff --git a/src/templates/template_image.cpp b/src/templates/template_image.cpp index c7ac6a7a7..acbabf21d 100644 --- a/src/templates/template_image.cpp +++ b/src/templates/template_image.cpp @@ -258,7 +258,6 @@ bool TemplateImage::postLoadConfiguration(QWidget* dialog_parent, bool& /*out_ce SelectCRSDialog dialog( map->getGeoreferencing(), dialog_parent, - SelectCRSDialog::TakeFromMap | SelectCRSDialog::Geographic, tr("Select the coordinate reference system of the coordinates in the world file") ); if (dialog.exec() == QDialog::Rejected) continue; @@ -375,7 +374,6 @@ bool TemplateImage::trySetTemplateGeoreferenced(bool value, QWidget* dialog_pare SelectCRSDialog dialog( map->getGeoreferencing(), dialog_parent, - SelectCRSDialog::TakeFromMap | SelectCRSDialog::Geographic, tr("Select the coordinate reference system of the coordinates in the world file") ); if (dialog.exec() == QDialog::Rejected) return is_georeferenced; From d170bda507e57aa2c3eadc84eeeb552f437718dc Mon Sep 17 00:00:00 2001 From: Kai Pastor Date: Wed, 12 Feb 2020 08:29:02 +0100 Subject: [PATCH 2/3] TemplateImage: Revise georeferencing setup Handle CRS and transformation options more independently. Pass available georeferencing options to the CRS selection dialog, and initialize the "current" selection accordingly. Allow overriding of CRS information when it differs from the map, or when (re-)enabling georeferencing for an existing template. Allow world files to be used even with local georeferencing (assuming same CRS). Mitigates GH-1515 (slightly misplaced templates). --- src/gdal/gdal_image_reader.cpp | 11 +- src/gdal/gdal_template.cpp | 4 +- src/gui/select_crs_dialog.cpp | 29 ++- src/gui/select_crs_dialog.h | 4 + src/gui/widgets/template_list_widget.cpp | 2 +- src/templates/template_image.cpp | 230 +++++++++++-------- src/templates/template_image.h | 62 +++-- src/templates/template_image_open_dialog.cpp | 7 +- 8 files changed, 216 insertions(+), 133 deletions(-) diff --git a/src/gdal/gdal_image_reader.cpp b/src/gdal/gdal_image_reader.cpp index fee7089f5..635e38f56 100644 --- a/src/gdal/gdal_image_reader.cpp +++ b/src/gdal/gdal_image_reader.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2019 Kai Pastor + * Copyright 2019, 2020 Kai Pastor * * This file is part of OpenOrienteering. * @@ -284,12 +284,11 @@ TemplateImage::GeoreferencingOption GdalImageReader::readGeoTransform() auto const result = GDALGetGeoTransform(dataset, geo_transform.data()); if (result == CE_None) { - georef.type = TemplateImage::Georeferencing_GDAL; - georef.source = GDALGetDriverShortName(GDALGetDatasetDriver(dataset)); georef.crs_spec = toProjSpec(GDALGetProjectionRef(dataset)); - georef.pixel_to_world = { geo_transform[1], geo_transform[2], - geo_transform[4], geo_transform[5], - geo_transform[0], geo_transform[3] }; + georef.transform.source = GDALGetDriverShortName(GDALGetDatasetDriver(dataset)); + georef.transform.pixel_to_world = { geo_transform[1], geo_transform[2], + geo_transform[4], geo_transform[5], + geo_transform[0], geo_transform[3] }; } } return georef; diff --git a/src/gdal/gdal_template.cpp b/src/gdal/gdal_template.cpp index 14abc102e..744573647 100644 --- a/src/gdal/gdal_template.cpp +++ b/src/gdal/gdal_template.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2019 Kai Pastor + * Copyright 2019, 2020 Kai Pastor * * This file is part of OpenOrienteering. * @@ -85,7 +85,7 @@ bool GdalTemplate::loadTemplateFileImpl(bool configuring) available_georef = findAvailableGeoreferencing(reader.readGeoTransform()); if (!configuring && is_georeferenced) { - if (available_georef.front().type == Georeferencing_None) + if (!isGeoreferencingUsable()) { // Image was georeferenced, but georeferencing info is gone -> deny to load template setErrorString(::OpenOrienteering::TemplateImage::tr("Georeferencing not found")); diff --git a/src/gui/select_crs_dialog.cpp b/src/gui/select_crs_dialog.cpp index 8dae83af8..1247bdc2a 100644 --- a/src/gui/select_crs_dialog.cpp +++ b/src/gui/select_crs_dialog.cpp @@ -41,9 +41,10 @@ namespace OpenOrienteering { namespace { enum SpecialCRS { - SameAsMap = 1, - Local = 2, - Geographic = 3 + SameAsMap = 1, + Local = 2, + Geographic = 3, + TemplateFile = 4, }; @@ -52,10 +53,12 @@ enum SpecialCRS { SelectCRSDialog::SelectCRSDialog( + const TemplateImage::GeoreferencingOptions& options, const Georeferencing& georef, QWidget* parent, const QString& description ) : QDialog(parent, Qt::WindowSystemMenuHint | Qt::WindowTitleHint) + , options(options) , georef(georef) { setWindowModality(Qt::WindowModal); @@ -69,10 +72,11 @@ SelectCRSDialog::SelectCRSDialog( } else { + if (!options.template_file.crs_spec.isEmpty()) + crs_selector->addCustomItem(tr("From template file"), SpecialCRS::TemplateFile ); crs_selector->addCustomItem(tr("Same as map"), SpecialCRS::SameAsMap); crs_selector->addCustomItem(tr("Geographic coordinates (WGS84)"), SpecialCRS::Geographic); } - crs_selector->setCurrentIndex(0); status_label = new QLabel(); button_box = new QDialogButtonBox(QDialogButtonBox::Cancel | QDialogButtonBox::Ok); @@ -88,6 +92,20 @@ SelectCRSDialog::SelectCRSDialog( form_layout->addItem(Util::SpacerItem::create(this)); crs_selector->setDialogLayout(form_layout); + auto const& crs_spec = options.effective.crs_spec; + if (georef.isLocal()) + crs_selector->setCurrentIndex(0); + else if (crs_spec.isEmpty()) + crs_selector->setCurrentIndex(crs_selector->findData(SpecialCRS::SameAsMap)); + else if (crs_spec == options.template_file.crs_spec) + crs_selector->setCurrentIndex(crs_selector->findData(SpecialCRS::TemplateFile)); + else if (crs_spec == georef.getProjectedCRSSpec()) + crs_selector->setCurrentIndex(crs_selector->findData(SpecialCRS::SameAsMap)); + else if (crs_spec == Georeferencing::geographic_crs_spec) + crs_selector->setCurrentIndex(crs_selector->findData(SpecialCRS::Geographic)); + else + crs_selector->setCurrentCRS(CRSTemplateRegistry().find(QString::fromLatin1("PROJ.4")), { crs_spec }); + auto layout = new QVBoxLayout(); layout->addLayout(form_layout, 1); layout->addWidget(button_box, 0); @@ -114,6 +132,9 @@ QString SelectCRSDialog::currentCRSSpec() const case SpecialCRS::Geographic: spec = Georeferencing::geographic_crs_spec; break; + case SpecialCRS::TemplateFile: + spec = options.template_file.crs_spec; + break; default: spec = crs_selector->currentCRSSpec(); } diff --git a/src/gui/select_crs_dialog.h b/src/gui/select_crs_dialog.h index 434fe7cf5..2664ebd8c 100644 --- a/src/gui/select_crs_dialog.h +++ b/src/gui/select_crs_dialog.h @@ -27,6 +27,8 @@ #include #include +#include "templates/template_image.h" + class QDialogButtonBox; class QLabel; class QWidget; @@ -52,6 +54,7 @@ Q_OBJECT * Should explain what the selected CRS will be used for. */ SelectCRSDialog( + const TemplateImage::GeoreferencingOptions& options, const Georeferencing& georef, QWidget* parent, const QString& description = QString() @@ -69,6 +72,7 @@ Q_OBJECT void updateWidgets(); private: + const TemplateImage::GeoreferencingOptions& options; const Georeferencing& georef; CRSSelector* crs_selector; QLabel* status_label; diff --git a/src/gui/widgets/template_list_widget.cpp b/src/gui/widgets/template_list_widget.cpp index d60ef44d6..e1497dab3 100644 --- a/src/gui/widgets/template_list_widget.cpp +++ b/src/gui/widgets/template_list_widget.cpp @@ -1105,7 +1105,7 @@ void TemplateListWidget::changeGeorefClicked() if (templ->trySetTemplateGeoreferenced(new_value, this) != new_value) { QMessageBox::warning(this, tr("Error"), tr("Cannot change the georeferencing state.")); - georef_action->setChecked(false); + georef_action->setChecked(templ->isTemplateGeoreferenced()); } updateButtons(); } diff --git a/src/templates/template_image.cpp b/src/templates/template_image.cpp index acbabf21d..41e8c197e 100644 --- a/src/templates/template_image.cpp +++ b/src/templates/template_image.cpp @@ -1,6 +1,6 @@ /* * Copyright 2012, 2013 Thomas Schöps - * Copyright 2012-2019 Kai Pastor + * Copyright 2012-2020 Kai Pastor * * This file is part of OpenOrienteering. * @@ -89,8 +89,6 @@ TemplateImage::TemplateImage(const QString& path, Map* map) : Template(path, map) , georef(std::make_unique()) { - available_georef.push_back({Georeferencing_None, "no georeferencing information", {}, {}}); - const Georeferencing& georef = map->getGeoreferencing(); connect(&georef, &Georeferencing::projectionChanged, this, &TemplateImage::updateGeoreferencing); connect(&georef, &Georeferencing::transformationChanged, this, &TemplateImage::updateGeoreferencing); @@ -103,7 +101,6 @@ TemplateImage::TemplateImage(const TemplateImage& proto) // not copied: undo_index , available_georef(proto.available_georef) , georef(new Georeferencing(*proto.georef)) -, temp_crs_spec(proto.temp_crs_spec) { const Georeferencing& georef = map->getGeoreferencing(); connect(&georef, &Georeferencing::projectionChanged, this, &TemplateImage::updateGeoreferencing); @@ -141,8 +138,8 @@ void TemplateImage::saveTypeSpecificTemplateConfiguration(QXmlStreamWriter& xml) { // Follow map georeferencing XML structure xml.writeStartElement(QString::fromLatin1("crs_spec")); -// TODO: xml.writeAttribute(QString::fromLatin1("language"), "PROJ.4"); - xml.writeCharacters(temp_crs_spec); + /// \todo xml.writeAttribute(QString::fromLatin1("language"), "PROJ.4"); + xml.writeCharacters(available_georef.effective.crs_spec); xml.writeEndElement(/*crs_spec*/); } } @@ -151,8 +148,8 @@ bool TemplateImage::loadTypeSpecificTemplateConfiguration(QXmlStreamReader& xml) { if (is_georeferenced && xml.name() == QLatin1String("crs_spec")) { - // TODO: check specification language - temp_crs_spec = xml.readElementText(); + /// \todo check specification language + available_georef.effective.crs_spec = xml.readElementText(); } else xml.skipCurrentElement(); // unsupported @@ -196,11 +193,12 @@ bool TemplateImage::loadTemplateFileImpl(bool configuring) #ifdef MAPPER_USE_GDAL available_georef = findAvailableGeoreferencing(readGdalGeoTransform(template_path)); #else - available_georef = findAvailableGeoreferencing(); + available_georef = findAvailableGeoreferencing({}); #endif + if (!configuring && is_georeferenced) { - if (available_georef.front().type == Georeferencing_None) + if (!isGeoreferencingUsable()) { // Image was georeferenced, but georeferencing info is gone -> deny to load template setErrorString(tr("Georeferencing not found")); @@ -213,7 +211,7 @@ bool TemplateImage::loadTemplateFileImpl(bool configuring) return true; } -bool TemplateImage::postLoadConfiguration(QWidget* dialog_parent, bool& /*out_center_in_view*/) +bool TemplateImage::postLoadConfiguration(QWidget* dialog_parent, bool& out_center_in_view) { TemplateImageOpenDialog open_dialog(this, dialog_parent); open_dialog.setWindowModality(Qt::WindowModal); @@ -225,15 +223,11 @@ bool TemplateImage::postLoadConfiguration(QWidget* dialog_parent, bool& /*out_ce if (!open_dialog.isGeorefRadioChecked()) break; - Q_ASSERT(!available_georef.empty()); // implied by open_dialog.isGeorefRadioChecked() - temp_crs_spec = available_georef.front().crs_spec; if (map->getGeoreferencing().isLocal()) { // Make sure that the map is georeferenced. Georeferencing initial_georef(map->getGeoreferencing()); - if (!initial_georef.setProjectedCRS({}, temp_crs_spec) || initial_georef.isLocal()) - temp_crs_spec.clear(); - + initial_georef.setProjectedCRS({}, available_georef.effective.crs_spec); if (initial_georef.getMapRefPoint() == MapCoord{} && initial_georef.getProjectedRefPoint() == QPointF{}) { @@ -252,33 +246,47 @@ bool TemplateImage::postLoadConfiguration(QWidget* dialog_parent, bool& /*out_ce continue; } - if (temp_crs_spec.isEmpty()) + if (map->getGeoreferencing().getProjectedCRSSpec() == available_georef.effective.crs_spec) { - // Let the user select the coordinate reference system. + // For convenience, skip CRS selection. + break; + } + + if (!map->getGeoreferencing().isLocal()) + { + // Let user select the coordinate reference system. + // \todo Change description text below (no longer just for world files.) + Q_UNUSED(QT_TR_NOOP("Select the coordinate reference system of the georeferenced image.")) SelectCRSDialog dialog( + available_georef, map->getGeoreferencing(), dialog_parent, tr("Select the coordinate reference system of the coordinates in the world file") ); - if (dialog.exec() == QDialog::Rejected) - continue; - temp_crs_spec = dialog.currentCRSSpec(); + if (dialog.exec() == QDialog::Accepted) + { + available_georef.effective.crs_spec = dialog.currentCRSSpec(); + break; + } } - - break; } - is_georeferenced = open_dialog.isGeorefRadioChecked(); - if (is_georeferenced) + if (open_dialog.isGeorefRadioChecked()) + { calculateGeoreferencing(); + // If not both the template and the map are fully georeferenced, + // we use the transform, but don't claim the georeferenced state. + is_georeferenced = georef->isValid() + && !georef->isLocal() + && map->getGeoreferencing().isValid() + && !map->getGeoreferencing().isLocal(); + out_center_in_view = false; + } else { + is_georeferenced = false; transform.template_scale_x = open_dialog.getMpp() * 1000.0 / map->getScaleDenominator(); transform.template_scale_y = transform.template_scale_x; transform.template_shear = 0.0; - - //transform.template_x = main_view->getPositionX(); - //transform.template_y = main_view->getPositionY(); - updateTransformationMatrices(); } @@ -353,7 +361,8 @@ QPointF TemplateImage::calcCenterOfGravity(QRgb background_color) bool TemplateImage::canChangeTemplateGeoreferenced() { - return available_georef.front().type != Georeferencing_None; + // No need to care for CRS here and now: This is handled by the dialog ATM. + return !available_georef.effective.transform.source.isEmpty(); } bool TemplateImage::trySetTemplateGeoreferenced(bool value, QWidget* dialog_parent) @@ -367,22 +376,26 @@ bool TemplateImage::trySetTemplateGeoreferenced(bool value, QWidget* dialog_pare if (value) { // Cf. postLoadConfiguration - if (available_georef.front().type == Georeferencing_WorldFile - && temp_crs_spec.isEmpty()) - { - // Let user select the coordinate reference system, as this is not specified in world files - SelectCRSDialog dialog( - map->getGeoreferencing(), - dialog_parent, - tr("Select the coordinate reference system of the coordinates in the world file") ); - if (dialog.exec() == QDialog::Rejected) - return is_georeferenced; - temp_crs_spec = dialog.currentCRSSpec(); - } + // Let user select the coordinate reference system. + // \todo Change description text below (no longer just for world files.) + Q_UNUSED(QT_TR_NOOP("Select the coordinate reference system of the georeferenced image.")) + SelectCRSDialog dialog( + available_georef, + map->getGeoreferencing(), + dialog_parent, + tr("Select the coordinate reference system of the coordinates in the world file") ); + if (dialog.exec() == QDialog::Rejected) + return isTemplateGeoreferenced(); setTemplateAreaDirty(); - is_georeferenced = true; + available_georef.effective.crs_spec = dialog.currentCRSSpec(); calculateGeoreferencing(); + // If not both the template and the map are fully georeferenced, + // we use the transform, but don't claim the georeferenced state. + is_georeferenced = georef->isValid() + && !georef->isLocal() + && map->getGeoreferencing().isValid() + && !map->getGeoreferencing().isLocal(); setTemplateAreaDirty(); } else @@ -391,7 +404,7 @@ bool TemplateImage::trySetTemplateGeoreferenced(bool value, QWidget* dialog_pare } map->setTemplatesDirty(); } - return is_georeferenced; + return isTemplateGeoreferenced(); } void TemplateImage::updateGeoreferencing() @@ -400,42 +413,83 @@ void TemplateImage::updateGeoreferencing() updatePosFromGeoreferencing(); } -TemplateImage::GeoreferencingOptions TemplateImage::findAvailableGeoreferencing(TemplateImage::GeoreferencingOption&& hint) const + +namespace { + +TemplateImage::GeoreferencingOption worldFileOption(const QString& template_path) { - GeoreferencingOptions result; - - auto crs_spec = temp_crs_spec; // loaded or empty - if (crs_spec.isEmpty()) - crs_spec = hint.crs_spec; // loaded or empty - + auto option = TemplateImage::GeoreferencingOption {}; WorldFile world_file; if (world_file.tryToLoadForImage(template_path)) { - auto pixel_to_world = QTransform(world_file); - if (!crs_spec.isEmpty()) - { - Georeferencing tmp_georef; - tmp_georef.setProjectedCRS(QString{}, crs_spec); - if (tmp_georef.isGeographic()) - { - constexpr auto factor = qDegreesToRadians(1.0); - pixel_to_world = { - pixel_to_world.m11() * factor, pixel_to_world.m12() * factor, 0, - pixel_to_world.m21() * factor, pixel_to_world.m22() * factor, 0, - pixel_to_world.m31() * factor, pixel_to_world.m32() * factor, 1 - }; - } - } - result.push_back({Georeferencing_WorldFile, "World file", crs_spec, pixel_to_world}); + option.transform.source = "World file"; + option.transform.pixel_to_world = QTransform(world_file); + } + return option; +} + +} // namespace + + +TemplateImage::GeoreferencingOptions TemplateImage::findAvailableGeoreferencing(GeoreferencingOption template_file_option) const +{ + auto options = GeoreferencingOptions { + {}, + worldFileOption(template_path), + std::move(template_file_option), + }; + + // Try to select the effective CRS from: + // (1) current effective CRS, (2) template file, (3) map + if (!available_georef.effective.crs_spec.isEmpty()) + { + options.effective.crs_spec = available_georef.effective.crs_spec; + } + else if (!options.template_file.crs_spec.isEmpty()) + { + options.effective.crs_spec = options.template_file.crs_spec; + } + else if (!map->getGeoreferencing().getProjectedCRSSpec().isEmpty()) + { + options.effective.crs_spec = map->getGeoreferencing().getProjectedCRSSpec(); } - if (hint.type != Georeferencing_None) - result.push_back(std::move(hint)); + // Try to select the effective transform from: + // (1) world file, (2) template file + if (!options.world_file.transform.source.isEmpty()) + { + options.effective.transform = options.world_file.transform; + } + else if (!options.template_file.transform.source.isEmpty()) + { + options.effective.transform = options.template_file.transform; + } - Q_ASSERT(available_georef.back().type == Georeferencing_None); - result.push_back(available_georef.back()); + // Modify transform from geographic CRS + if (!options.effective.crs_spec.isEmpty() && !options.effective.transform.source.isEmpty()) + { + Georeferencing tmp_georef; + tmp_georef.setProjectedCRS(QString{}, options.effective.crs_spec); + if (tmp_georef.isGeographic()) + { + constexpr auto factor = qDegreesToRadians(1.0); + auto& pixel_to_world = options.effective.transform.pixel_to_world; + pixel_to_world = { + pixel_to_world.m11() * factor, pixel_to_world.m12() * factor, 0, + pixel_to_world.m21() * factor, pixel_to_world.m22() * factor, 0, + pixel_to_world.m31() * factor, pixel_to_world.m32() * factor, 1 + }; + } + } - return result; + return options; +} + +bool TemplateImage::isGeoreferencingUsable() const +{ + return !available_georef.effective.transform.source.isEmpty() + && (!available_georef.effective.crs_spec.isEmpty() + || map->getGeoreferencing().getProjectedCRSSpec().isEmpty()); } @@ -580,36 +634,18 @@ void TemplateImage::addUndoStep(const TemplateImage::DrawOnImageUndoStep& new_st void TemplateImage::calculateGeoreferencing() { - // Calculate georeferencing of image coordinates where the coordinate (0, 0) - // is mapped to the world position of the top-left corner of the top-left pixel - applyGeoreferencingOption(available_georef.front()); -} - -void TemplateImage::applyGeoreferencingOption(const GeoreferencingOption& option) -{ - if (option.type == Georeferencing_None) + if (!isGeoreferencingUsable()) { - qWarning("%s shall not be called for Georeferencing_None", Q_FUNC_INFO); + qWarning("%s must not be called with incomplete georeferencing", Q_FUNC_INFO); georef->setTransformationDirectly({}); return; } - georef.reset(new Georeferencing()); - switch (option.type) - { - case Georeferencing_WorldFile: - if (!temp_crs_spec.isEmpty()) - georef->setProjectedCRS(QString::fromUtf8(option.source), temp_crs_spec); - break; - case Georeferencing_GDAL: -#ifdef MAPPER_USE_GDAL - georef->setProjectedCRS(QString::fromUtf8(option.source), option.crs_spec); -#endif - break; - case Georeferencing_None: - break; - } - georef->setTransformationDirectly(option.pixel_to_world); + // Calculate georeferencing of image coordinates where the coordinate (0, 0) + // is mapped to the world position of the top-left corner of the top-left pixel + georef = std::make_unique(); + georef->setProjectedCRS(QString{}, available_georef.effective.crs_spec); + georef->setTransformationDirectly(available_georef.effective.transform.pixel_to_world); if (map->getGeoreferencing().isValid()) updatePosFromGeoreferencing(); } diff --git a/src/templates/template_image.h b/src/templates/template_image.h index eee48e848..0e8cbdf9c 100644 --- a/src/templates/template_image.h +++ b/src/templates/template_image.h @@ -1,6 +1,6 @@ /* * Copyright 2012, 2013 Thomas Schöps - * Copyright 2012-2019 Kai Pastor + * Copyright 2012-2020 Kai Pastor * * This file is part of OpenOrienteering. * @@ -35,7 +35,6 @@ #include #include #include -#include #include "templates/template.h" @@ -61,23 +60,36 @@ class TemplateImage : public Template { Q_OBJECT public: - enum GeoreferencingType + /** + * Information fragment about template image georeferencing. + * + * A GeoreferencingOption may carry information about the CRS and/or the + * pixel-to-world transform for a template image. The crs_spec is empty + * if the CRS is unknown. The transform's source is empty if the + * pixel-to-world transformation is unknown. + */ + struct GeoreferencingOption { - Georeferencing_None = 0, - Georeferencing_WorldFile, - Georeferencing_GDAL + struct Transform + { + QTransform pixel_to_world = {}; ///< The transformation from pixel space to CRS space. + QByteArray source = {}; ///< The source of the pixel-to-world transform. + }; + + QString crs_spec = {}; ///< The specification of the CRS uses for georeferencing. + Transform transform = {}; ///< The transformation from pixel space to CRS space. }; - struct GeoreferencingOption + /** + * A collection of alternative and/or complementary georeferencing information fragments. + */ + struct GeoreferencingOptions { - GeoreferencingType type; - QByteArray source; - QString crs_spec; - QTransform pixel_to_world; + GeoreferencingOption effective; ///< The effective option, if CRS and transform are defined. + GeoreferencingOption world_file; ///< A pixel-to-world transform from a world file. + GeoreferencingOption template_file; ///< CRS and/or pixel-to-world transform from the template file. }; - using GeoreferencingOptions = QVarLengthArray; - /** * Returns the filename extensions supported by this template class. */ @@ -134,11 +146,26 @@ public slots: protected: /** - * Searches for available georeferencing methods, using hint as additional - * provider of CRS and transformation information. + * Collects available georeferencing information. + * + * This function is meant to be used from loadTemplateFileImpl() to set the + * available_georef member. The parameter template_file_option is used to + * efficiently provide georeferencing information from the template file + * itself. */ - GeoreferencingOptions findAvailableGeoreferencing(GeoreferencingOption&& hint = {}) const; + GeoreferencingOptions findAvailableGeoreferencing(GeoreferencingOption template_file_option) const; + /** + * Tests if the available georeferencing options can be used with the current map. + * + * To be usable, the effective option must provide a transformation, and + * if the map's CRS spec is not empty (local georeferencing), the effective + * option's CRS spec must not be empty. + * + * Note that changing the state to georeferenced may still fail for invalid + * CRS specs. + */ + bool isGeoreferencingUsable() const; /** Information about an undo step for the paint-on-template functionality. */ struct DrawOnImageUndoStep @@ -157,7 +184,6 @@ public slots: void drawOntoTemplateUndo(bool redo) override; void addUndoStep(const DrawOnImageUndoStep& new_step); void calculateGeoreferencing(); - void applyGeoreferencingOption(const GeoreferencingOption& option); void updatePosFromGeoreferencing(); QImage image; @@ -170,8 +196,6 @@ public slots: GeoreferencingOptions available_georef; std::unique_ptr georef; - // Temporary storage for crs spec. Use georef instead. - QString temp_crs_spec; }; diff --git a/src/templates/template_image_open_dialog.cpp b/src/templates/template_image_open_dialog.cpp index 2c920b8c1..208cdaf4f 100644 --- a/src/templates/template_image_open_dialog.cpp +++ b/src/templates/template_image_open_dialog.cpp @@ -1,6 +1,6 @@ /* * Copyright 2012, 2013 Thomas Schöps - * Copyright 2012-2019 Kai Pastor + * Copyright 2012-2020 Kai Pastor * * This file is part of OpenOrienteering. * @@ -60,9 +60,8 @@ TemplateImageOpenDialog::TemplateImageOpenDialog(TemplateImage* templ, QWidget* double scale; templ->getMap()->getImageTemplateDefaults(use_meters_per_pixel, meters_per_pixel, dpi, scale); - auto const & georeferencing_option = templ->availableGeoreferencing().front(); - auto georef_source = georeferencing_option.source; - auto const georef_radio_enabled = georeferencing_option.type != TemplateImage::Georeferencing_None; + auto georef_source = templ->availableGeoreferencing().effective.transform.source; + auto const georef_radio_enabled = !georef_source.isEmpty(); // Georeferencing source translations which already existed in this context // and need to be preserved here, until moved to a different file. From a7c0ca1da597a50d7dc8a3203c4f9112f053d979 Mon Sep 17 00:00:00 2001 From: Kai Pastor Date: Wed, 12 Feb 2020 08:49:08 +0100 Subject: [PATCH 3/3] Template: Revise trySetTemplateGeoreferenced Don't return the resulting value, but indicate success or error. This is necessary to suppress an error message if the user intentionally cancelled CRS configuration on enabling geoerefencing. --- src/gui/widgets/template_list_widget.cpp | 4 ++-- src/templates/template.cpp | 6 +++--- src/templates/template.h | 11 +++++------ src/templates/template_image.cpp | 4 ++-- 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/gui/widgets/template_list_widget.cpp b/src/gui/widgets/template_list_widget.cpp index e1497dab3..c8008fa63 100644 --- a/src/gui/widgets/template_list_widget.cpp +++ b/src/gui/widgets/template_list_widget.cpp @@ -1,6 +1,6 @@ /* * Copyright 2012, 2013 Thomas Schöps - * Copyright 2012-2019 Kai Pastor + * Copyright 2012-2020 Kai Pastor * * This file is part of OpenOrienteering. * @@ -1102,7 +1102,7 @@ void TemplateListWidget::changeGeorefClicked() if (position_action->isChecked()) position_action->trigger(); } - if (templ->trySetTemplateGeoreferenced(new_value, this) != new_value) + if (!templ->trySetTemplateGeoreferenced(new_value, this)) { QMessageBox::warning(this, tr("Error"), tr("Cannot change the georeferencing state.")); georef_action->setChecked(templ->isTemplateGeoreferenced()); diff --git a/src/templates/template.cpp b/src/templates/template.cpp index e46be5aa7..d0dcdd379 100644 --- a/src/templates/template.cpp +++ b/src/templates/template.cpp @@ -1,6 +1,6 @@ /* * Copyright 2012, 2013 Thomas Schöps - * Copyright 2013-2019 Kai Pastor + * Copyright 2013-2020 Kai Pastor * * This file is part of OpenOrienteering. * @@ -676,9 +676,9 @@ bool Template::canChangeTemplateGeoreferenced() } // virtual -bool Template::trySetTemplateGeoreferenced(bool /*value*/, QWidget* /*dialog_parent*/) +bool Template::trySetTemplateGeoreferenced(bool value, QWidget* /*dialog_parent*/) { - return is_georeferenced; + return isTemplateGeoreferenced() == value; } diff --git a/src/templates/template.h b/src/templates/template.h index c0e09c64e..d16c27cfb 100644 --- a/src/templates/template.h +++ b/src/templates/template.h @@ -1,6 +1,6 @@ /* * Copyright 2012, 2013 Thomas Schöps - * Copyright 2012-2019 Kai Pastor + * Copyright 2012-2020 Kai Pastor * * This file is part of OpenOrienteering. * @@ -518,12 +518,11 @@ Q_OBJECT * Tries to change the usage of georeferencing data. * * If supported by the actual template, this function tries to switch the - * state between non-georeferenced and georeferenced. It returns the final - * state which may be the same as before if the change is not implemented - * or fails for other reasons. + * state between non-georeferenced and georeferenced. It returns false when + * an error occurred, and true if the change was successful or if it was + * explicitly cancelled by the user. * - * The default implementation changes nothing, and it just returns the - * current state. + * The default implementation returns true iff the state matches the value. */ virtual bool trySetTemplateGeoreferenced(bool value, QWidget* dialog_parent); diff --git a/src/templates/template_image.cpp b/src/templates/template_image.cpp index 41e8c197e..95bea9b8e 100644 --- a/src/templates/template_image.cpp +++ b/src/templates/template_image.cpp @@ -385,7 +385,7 @@ bool TemplateImage::trySetTemplateGeoreferenced(bool value, QWidget* dialog_pare dialog_parent, tr("Select the coordinate reference system of the coordinates in the world file") ); if (dialog.exec() == QDialog::Rejected) - return isTemplateGeoreferenced(); + return true; // not failed! setTemplateAreaDirty(); available_georef.effective.crs_spec = dialog.currentCRSSpec(); @@ -404,7 +404,7 @@ bool TemplateImage::trySetTemplateGeoreferenced(bool value, QWidget* dialog_pare } map->setTemplatesDirty(); } - return isTemplateGeoreferenced(); + return isTemplateGeoreferenced() == value; } void TemplateImage::updateGeoreferencing()