Skip to content

Commit

Permalink
Merge pull request #2582 from rouault/fix_proj_clone
Browse files Browse the repository at this point in the history 
Fix proj_clone() to work on 'meta' coordinate operation PJ* objects that can be returned by proj_create_crs_to_crs()
  • Loading branch information
@rouault
rouault authored Mar 17, 2021
2 parents 1b07ef0 + cbbf3d6 commit ee19a2d
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 18 deletions.
9 changes: 5 additions & 4 deletions src/4D_api.cpp
View file
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
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
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
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

0 comments on commit ee19a2d

Please sign in to comment.