Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix proj_clone() to work on 'meta' coordinate operation PJ* objects that can be returned by proj_create_crs_to_crs() #2582

Merged
merged 1 commit into from
Mar 17, 2021
Merged

Fix proj_clone() to work on 'meta' coordinate operation PJ* objects that can be returned by proj_create_crs_to_crs() #2582

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/4D_api.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ double proj_roundtrip (PJ *P, PJ_DIRECTION direction, int n, PJ_COORD *coord) {

/**************************************************************************************/
int pj_get_suggested_operation(PJ_CONTEXT*,
const std::vector<CoordOperation>& opList,
const std::vector<PJCoordOperation>& opList,
const int iExcluded[2],
PJ_DIRECTION direction,
PJ_COORD coord)
Expand Down Expand Up @@ -1034,7 +1034,7 @@ static PJ* add_coord_op_to_list(
PJ* pjGeogToSrc,
PJ* pjGeogToDst,
bool isOffshore,
std::vector<CoordOperation>& altCoordOps) {
std::vector<PJCoordOperation>& altCoordOps) {
/*****************************************************************************/

double minxSrc;
Expand Down Expand Up @@ -1193,7 +1193,7 @@ PJ *proj_create_crs_to_crs (PJ_CONTEXT *ctx, const char *source_crs, const char


/*****************************************************************************/
std::vector<CoordOperation> pj_create_prepared_operations(PJ_CONTEXT *ctx,
std::vector<PJCoordOperation> pj_create_prepared_operations(PJ_CONTEXT *ctx,
const PJ *source_crs,
const PJ *target_crs,
PJ_OBJ_LIST* op_list)
Expand All @@ -1218,7 +1218,7 @@ std::vector<CoordOperation> pj_create_prepared_operations(PJ_CONTEXT *ctx,

try
{
std::vector<CoordOperation> preparedOpList;
std::vector<PJCoordOperation> preparedOpList;

// Iterate over source->target candidate transformations and reproject
// their long-lat bounding box into the source CRS.
Expand Down Expand Up @@ -1407,6 +1407,7 @@ PJ *proj_create_crs_to_crs_from_pj (PJ_CONTEXT *ctx, const PJ *source_crs, cons

P->alternativeCoordinateOperations = std::move(preparedOpList);
// The returned P is rather dummy
P->descr = "Set of coordinate operations";
P->iso_obj = nullptr;
P->fwd = nullptr;
P->inv = nullptr;
Expand Down
40 changes: 35 additions & 5 deletions src/iso19111/c_api.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,8 +434,11 @@ static const char *getOptionValue(const char *option,

/** \brief "Clone" an object.
*
* Technically this just increases the reference counter on the object, since
* PJ objects are immutable.
* The object might be used independently of the original object, provided that
* the use of context is compatible. In particular if you intend to use a
* clone in a different thread than the original object, you should pass a
* context that is different from the one of the original object (or later
* assing a different context with proj_assign_context()).
*
* The returned object must be unreferenced with proj_destroy() after use.
* It should be used by at most one thread at a time.
Expand All @@ -453,6 +456,18 @@ PJ *proj_clone(PJ_CONTEXT *ctx, const PJ *obj) {
return nullptr;
}
if (!obj->iso_obj) {
if (!obj->alternativeCoordinateOperations.empty()) {
auto newPj = pj_new();
if (newPj) {
newPj->descr = "Set of coordinate operations";
newPj->ctx = ctx;
for (const auto &altOp : obj->alternativeCoordinateOperations) {
newPj->alternativeCoordinateOperations.emplace_back(
PJCoordOperation(ctx, altOp));
}
}
return newPj;
}
return nullptr;
}
try {
Expand Down Expand Up @@ -1268,6 +1283,21 @@ static int proj_is_equivalent_to_internal(PJ_CONTEXT *ctx, const PJ *obj,
}
return false;
}

if (obj->iso_obj == nullptr && other->iso_obj == nullptr &&
!obj->alternativeCoordinateOperations.empty() &&
obj->alternativeCoordinateOperations.size() ==
other->alternativeCoordinateOperations.size()) {
for (size_t i = 0; i < obj->alternativeCoordinateOperations.size();
++i) {
if (obj->alternativeCoordinateOperations[i] !=
other->alternativeCoordinateOperations[i]) {
return false;
}
}
return true;
}

if (!obj->iso_obj || !other->iso_obj) {
return false;
}
Expand Down Expand Up @@ -7864,7 +7894,7 @@ struct PJ_OPERATION_LIST : PJ_OBJ_LIST {
PJ *source_crs;
PJ *target_crs;
bool hasPreparedOperation = false;
std::vector<CoordOperation> preparedOperations{};
std::vector<PJCoordOperation> preparedOperations{};

explicit PJ_OPERATION_LIST(PJ_CONTEXT *ctx, const PJ *source_crsIn,
const PJ *target_crsIn,
Expand All @@ -7874,7 +7904,7 @@ struct PJ_OPERATION_LIST : PJ_OBJ_LIST {
PJ_OPERATION_LIST(const PJ_OPERATION_LIST &) = delete;
PJ_OPERATION_LIST &operator=(const PJ_OPERATION_LIST &) = delete;

const std::vector<CoordOperation> &getPreparedOperations(PJ_CONTEXT *ctx);
const std::vector<PJCoordOperation> &getPreparedOperations(PJ_CONTEXT *ctx);
};

// ---------------------------------------------------------------------------
Expand All @@ -7899,7 +7929,7 @@ PJ_OPERATION_LIST::~PJ_OPERATION_LIST() {

// ---------------------------------------------------------------------------

const std::vector<CoordOperation> &
const std::vector<PJCoordOperation> &
PJ_OPERATION_LIST::getPreparedOperations(PJ_CONTEXT *ctx) {
if (!hasPreparedOperation) {
hasPreparedOperation = true;
Expand Down
49 changes: 40 additions & 9 deletions src/proj_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ typedef PJ_COORD (* PJ_OPERATOR) (PJ_COORD, PJ *);
#define PJD_GRIDSHIFT 3
#define PJD_WGS84 4 /* WGS84 (or anything considered equivalent) */

struct CoordOperation
struct PJCoordOperation
{
int idxInOriginalList;
double minxSrc = 0.0;
Expand All @@ -298,7 +298,7 @@ struct CoordOperation
double accuracy = -1.0;
bool isOffshore = false;

CoordOperation(int idxInOriginalListIn,
PJCoordOperation(int idxInOriginalListIn,
double minxSrcIn, double minySrcIn, double maxxSrcIn, double maxySrcIn,
double minxDstIn, double minyDstIn, double maxxDstIn, double maxyDstIn,
PJ* pjIn, const std::string& nameIn, double accuracyIn, bool isOffshoreIn):
Expand All @@ -311,9 +311,20 @@ struct CoordOperation
{
}

CoordOperation(const CoordOperation&) = delete;
PJCoordOperation(const PJCoordOperation&) = delete;

CoordOperation(CoordOperation&& other):
PJCoordOperation(PJ_CONTEXT* ctx, const PJCoordOperation& other):
idxInOriginalList(other.idxInOriginalList),
minxSrc(other.minxSrc), minySrc(other.minySrc), maxxSrc(other.maxxSrc), maxySrc(other.maxySrc),
minxDst(other.minxDst), minyDst(other.minyDst), maxxDst(other.maxxDst), maxyDst(other.maxyDst),
pj(proj_clone(ctx, other.pj)),
name(std::move(other.name)),
accuracy(other.accuracy),
isOffshore(other.isOffshore)
{
}

PJCoordOperation(PJCoordOperation&& other):
idxInOriginalList(other.idxInOriginalList),
minxSrc(other.minxSrc), minySrc(other.minySrc), maxxSrc(other.maxxSrc), maxySrc(other.maxySrc),
minxDst(other.minxDst), minyDst(other.minyDst), maxxDst(other.maxxDst), maxyDst(other.maxyDst),
Expand All @@ -324,9 +335,29 @@ struct CoordOperation
other.pj = nullptr;
}

CoordOperation& operator=(const CoordOperation&) = delete;
PJCoordOperation& operator=(const PJCoordOperation&) = delete;

bool operator == (const PJCoordOperation& other) const {
return idxInOriginalList == other.idxInOriginalList &&
minxSrc == other.minxSrc &&
minySrc == other.minySrc &&
maxxSrc == other.maxxSrc &&
maxySrc == other.maxySrc &&
minxDst == other.minxDst &&
minyDst == other.minyDst &&
maxxDst == other.maxxDst &&
maxyDst == other.maxyDst &&
name == other.name &&
proj_is_equivalent_to(pj, other.pj, PJ_COMP_STRICT) &&
accuracy == other.accuracy &&
isOffshore == other.isOffshore;
}

bool operator != (const PJCoordOperation& other) const {
return !(operator==(other));
}

~CoordOperation()
~PJCoordOperation()
{
proj_destroy(pj);
}
Expand Down Expand Up @@ -545,7 +576,7 @@ struct PJconsts {
/*************************************************************************************
proj_create_crs_to_crs() alternative coordinate operations
**************************************************************************************/
std::vector<CoordOperation> alternativeCoordinateOperations{};
std::vector<PJCoordOperation> alternativeCoordinateOperations{};
int iCurCoordOp = -1;

/*************************************************************************************
Expand Down Expand Up @@ -820,13 +851,13 @@ std::string PROJ_DLL pj_context_get_grid_cache_filename(PJ_CONTEXT *ctx);
void PROJ_DLL pj_context_set_user_writable_directory(PJ_CONTEXT* ctx, const std::string& path);
std::string PROJ_DLL pj_get_relative_share_proj(PJ_CONTEXT *ctx);

std::vector<CoordOperation> pj_create_prepared_operations(PJ_CONTEXT *ctx,
std::vector<PJCoordOperation> pj_create_prepared_operations(PJ_CONTEXT *ctx,
const PJ *source_crs,
const PJ *target_crs,
PJ_OBJ_LIST* op_list);

int pj_get_suggested_operation(PJ_CONTEXT *ctx,
const std::vector<CoordOperation>& opList,
const std::vector<PJCoordOperation>& opList,
const int iExcluded[2],
PJ_DIRECTION direction,
PJ_COORD coord);
Expand Down
38 changes: 38 additions & 0 deletions test/unit/test_c_api.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,10 @@ class CApi : public ::testing::Test {
PJ *m_obj = nullptr;
explicit ObjectKeeper(PJ *obj) : m_obj(obj) {}
~ObjectKeeper() { proj_destroy(m_obj); }
void clear() {
proj_destroy(m_obj);
m_obj = nullptr;
}

ObjectKeeper(const ObjectKeeper &) = delete;
ObjectKeeper &operator=(const ObjectKeeper &) = delete;
Expand Down Expand Up @@ -2738,6 +2742,40 @@ TEST_F(CApi, proj_clone) {

// ---------------------------------------------------------------------------

TEST_F(CApi, proj_clone_of_obj_with_alternative_operations) {
// NAD27 to NAD83
auto obj =
proj_create_crs_to_crs(m_ctxt, "EPSG:4267", "EPSG:4269", nullptr);
ObjectKeeper keeper(obj);
ASSERT_NE(obj, nullptr);

PJ_COORD c;
c.xyzt.x = 40.5;
c.xyzt.y = -60;
c.xyzt.z = 0;
c.xyzt.t = 2021;
PJ_COORD c_trans_ref = proj_trans(obj, PJ_FWD, c);
EXPECT_NE(c_trans_ref.xyzt.x, c.xyzt.x);
EXPECT_NEAR(c_trans_ref.xyzt.x, c.xyzt.x, 1e-3);
EXPECT_NEAR(c_trans_ref.xyzt.y, c.xyzt.y, 1e-3);

auto clone = proj_clone(m_ctxt, obj);
ObjectKeeper keeperClone(clone);
ASSERT_NE(clone, nullptr);

EXPECT_TRUE(proj_is_equivalent_to(obj, clone, PJ_COMP_STRICT));

keeper.clear();
obj = nullptr;
(void)obj;

PJ_COORD c_trans = proj_trans(clone, PJ_FWD, c);
EXPECT_EQ(c_trans.xyzt.x, c_trans_ref.xyzt.x);
EXPECT_EQ(c_trans.xyzt.y, c_trans_ref.xyzt.y);
}

// ---------------------------------------------------------------------------

TEST_F(CApi, proj_crs_alter_geodetic_crs) {
auto projCRS = proj_create_from_wkt(
m_ctxt,
Expand Down