Preparation for PR reprojection 'a la volee' #2422

ftoromanoff · 2024-10-02T12:27:32Z

Following PR #2272 that became a big PR. I extracted commits that are needed for the reprojection but are also independants :

support of meter and foot units in crs
parsing crs from metadata (for entwnie and copc data)
few simplification or deletion of useless code

Desplandis

I have a few comments for this PR (mainly on proj4 patching). Could you rename the commit "supp onPointsCreated" to "delete onPointsCreated callack"?

Desplandis · 2024-10-04T08:52:16Z

src/Core/Geographic/Crs.js

@@ -61,7 +64,7 @@ function toUnit(crs) {
        case 'EPSG:4326' : return UNIT.DEGREE;
        case 'EPSG:4978' : return UNIT.METER;
        default: {
-            const p = proj4.defs(formatToEPSG(crs));
+            const p = proj4.defs(crs.startsWith('TMS') ? formatToEPSG(crs) : crs);


Are we sure that there is no part of the code which depends on the assumption that a crs starts either by EPSG: or TMS:? Maybe this conversion is not needed anymore with #2412.

src/Core/Geographic/Crs.js

src/Layer/PointCloudLayer.js

Desplandis · 2024-10-04T09:36:12Z

src/Source/CopcSource.js

+            if (proj4.defs('unknown').type === 'COMPD_CS') {
+                console.warn('CopcSource: compound coordinate system is not yet supported.');
+                metadata.wkt = metadata.wkt.slice(metadata.wkt.search('PROJCS'), metadata.wkt.search(',VERT_CS'));
+                proj4.defs('unknown', metadata.wkt);
+            }
+
+            const projCS = proj4.defs('unknown');
+            if (projCS.AUTHORITY) {
+                const authority = Object.keys(projCS.AUTHORITY)[0];
+                this.crs = `${authority}:${projCS.AUTHORITY[authority]}`;
+                if (!proj4.defs(this.crs)) {
+                    proj4.defs(this.crs, proj4.defs('unknown'));
+                }
+            } else {
+                this.crs = projCS.name || 'EPSG:4326';
+            }


Seems we are patching a missing feature of proj4.js, I'm for proposing a PR directly upstream instead of patching it in iTowns...

proj4js doesn't support 'COMPD_CS' wkt. Thus it can parse it without error, but proj4.defs() can't have a ProjectionDefinition as 2nd parameters.
This would be a major change on proj4js side.

I simplified it and if ever proj4 accept my PR or a version of it, we might have to change one line in the test copc.js

Desplandis · 2024-10-04T09:41:04Z

src/Source/EntwinePointTileSource.js

+            if (metadata.srs) {
+                if (metadata.srs.authority && metadata.srs.horizontal) {
+                    this.crs = `${metadata.srs.authority}:${metadata.srs.horizontal}`;
+                    if (!proj4.defs(this.crs)) {
+                        proj4.defs(this.crs, metadata.srs.wkt);
+                    }
+                } else if (metadata.srs.wkt) {
+                    proj4.defs('unknown', metadata.srs.wkt);
+                    this.crs = proj4.defs('unknown').name;
+                    proj4.defs(this.crs, proj4.defs('unknown'));


Same here, the wkt parser breaks the type invariant that what type returned by proj4.defs(crs) (i.e. the ProjectionDefinition type) shall return a defined title property. We could use this property to uniquely define a projection.

I suggest that we could contribute to proj4 such fix (using srs.authority) to define the title property.

I did open a PR on proj4 github, to address that issue.
https://github.com/proj4js/wkt-parser/pulls

Desplandis · 2024-11-04T14:17:35Z

test/unit/entwine.js

+const eptSsAuthority = JSON.parse(eptFile);
+eptSsAuthority.srs = {
+    wkt: 'PROJCS["RGF93 v1 / Lambert-93",GEOGCS["RGF93 v1",DATUM["Reseau_Geodesique_Francais_1993_v1",SPHEROID["GRS 1980",6378137,298.257222101],TOWGS84[0,0,0,0,0,0,0]],PRIMEM["Greenwich",0,AUTHORITY["EPSG","8901"]],UNIT["degree",0.0174532925199433,AUTHORITY["EPSG","9122"]],AUTHORITY["EPSG","4171"]],PROJECTION["Lambert_Conformal_Conic_2SP"],PARAMETER["latitude_of_origin",46.5],PARAMETER["central_meridian",3],PARAMETER["standard_parallel_1",49],PARAMETER["standard_parallel_2",44],PARAMETER["false_easting",700000],PARAMETER["false_northing",6600000],UNIT["metre",1,AUTHORITY["EPSG","9001"]],AXIS["Easting",EAST],AXIS["Northing",NORTH],AUTHORITY["EPSG","2154"]]',
+    // wkt: 'PROJCS["WGS 84 / Pseudo-Mercator",GEOGCS["WGS 84",DATUM["WGS_1984",SPHEROID["WGS 84",6378137,298.257223563,AUTHORITY["EPSG","7030"]],AUTHORITY["EPSG","6326"]],PRIMEM["Greenwich",0,AUTHORITY["EPSG","8901"]],UNIT["degree",0.0174532925199433,AUTHORITY["EPSG","9122"]],AUTHORITY["EPSG","4326"]],PROJECTION["Mercator_1SP"],PARAMETER["central_meridian",0],PARAMETER["scale_factor",1],PARAMETER["false_easting",0],PARAMETER["false_northing",0],UNIT["metre",1,AUTHORITY["EPSG","9001"]],AXIS["X",EAST],AXIS["Y",NORTH],EXTENSION["PROJ4","+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs"],AUTHORITY["EPSG","3857"]]',


Could you please remove this comment?

…Back, errCallBack)

ftoromanoff force-pushed the prepAlaVolee branch 9 times, most recently from eefe617 to add66c8 Compare October 4, 2024 07:34

ftoromanoff marked this pull request as ready for review October 4, 2024 07:40

ftoromanoff force-pushed the prepAlaVolee branch from add66c8 to 3616127 Compare October 4, 2024 08:33

ftoromanoff requested a review from Desplandis October 4, 2024 08:41

Desplandis requested changes Oct 4, 2024

View reviewed changes

ftoromanoff force-pushed the prepAlaVolee branch 5 times, most recently from 28eca27 to b196453 Compare October 10, 2024 15:53

ftoromanoff force-pushed the prepAlaVolee branch from b196453 to 1f59481 Compare October 15, 2024 08:52

Desplandis reviewed Nov 4, 2024

View reviewed changes

ftoromanoff added 9 commits November 4, 2024 16:23

fix(geometryLayer): fix opacity magic number

2a32daa

fix(crs): fix proj4 unit 'meter' and add 'foot'

cb4ec49

fix(crs): add support for non EPSG projection

21a4584

refactor(PointCloudLayer): promise.catch/finally instead of then(Call…

f03b76a

…Back, errCallBack)

refactor(PointCloudLayer): delete onPointsCreated callback

3f101c2

refactor(entwineSource): read crs from metadata.srs

9aefa2c

refactor(CopcSource): use metadata.wkt to set source.crs

560d034

test(lasparser): add test for parseChunk

67c9ffa

refactor(test): change timeout

1843d7f

ftoromanoff force-pushed the prepAlaVolee branch from 1f59481 to 1843d7f Compare November 4, 2024 15:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Preparation for PR reprojection 'a la volee' #2422

Preparation for PR reprojection 'a la volee' #2422

ftoromanoff commented Oct 2, 2024 •

edited

Loading

Desplandis left a comment

Desplandis Oct 4, 2024

Desplandis Oct 4, 2024

ftoromanoff Oct 7, 2024 •

edited

Loading

ftoromanoff Oct 10, 2024

Desplandis Oct 4, 2024

ftoromanoff Oct 7, 2024

Desplandis Nov 4, 2024

ftoromanoff Nov 4, 2024

Preparation for PR reprojection 'a la volee' #2422

Are you sure you want to change the base?

Preparation for PR reprojection 'a la volee' #2422

Conversation

ftoromanoff commented Oct 2, 2024 • edited Loading

Desplandis left a comment

Choose a reason for hiding this comment

Desplandis Oct 4, 2024

Choose a reason for hiding this comment

Desplandis Oct 4, 2024

Choose a reason for hiding this comment

ftoromanoff Oct 7, 2024 • edited Loading

Choose a reason for hiding this comment

ftoromanoff Oct 10, 2024

Choose a reason for hiding this comment

Desplandis Oct 4, 2024

Choose a reason for hiding this comment

ftoromanoff Oct 7, 2024

Choose a reason for hiding this comment

Desplandis Nov 4, 2024

Choose a reason for hiding this comment

ftoromanoff Nov 4, 2024

Choose a reason for hiding this comment

ftoromanoff commented Oct 2, 2024 •

edited

Loading

ftoromanoff Oct 7, 2024 •

edited

Loading