Fix WFS PostGIS path SRSNAME handling and snap datum-shift error by meyerlor · Pull Request #6684 · 3liz/lizmap-web-client

meyerlor · 2026-04-02T11:01:48Z

While working on the snapping migration to OL10, I just found a mismatch happening when snapping to layers which looks like a datum-shift error:

This PR fixes it, it is aimed for 3.9 and 3.10 - the Ol10 will handle snapping differently and i'll keep in mind to watch for this behaviour once i continue working on that for 3.11 (or 4.X ! :) )

mind84 · 2026-04-02T15:25:52Z

Hi @meyerlor

I did some quick tests, I tried to enable snapping on the following project:

repository=testsrepository&project=form_edition_value_relation_field

The snap is only on the list layer :

but the server returns an incorrect bbox/geometries for the list layer.

Project is in 3857, list layer is in 4326

Am I doing something wrong? Can you replicate the problem?

Thanks!

meyerlor · 2026-04-02T19:11:50Z

@mind84 you are right — none of the test projects use a datum-shifted CRS, so the e2e test checks only the protocol change (WFS 1.1.0 + SRSNAME) rather than the coordinate values directly. Adding a project with e.g. EPSG:28992 or EPSG:25832 (my usual one) would also require NTv2 grid files on the test QGIS Server. The test only proves the new code path is taken, which eliminates the client-side proj4js transform by construction — the coordinate accuracy guarantee comes from QGIS Server/PROJ once SRSNAME is present.

mind84 · 2026-04-03T11:01:51Z

Hi @meyerlor,

I took a quick look, so please correct me if I'm wrong!

The main problem is that the WFS call is made to PostGIS for DB layers, see getFeaturePostgres method in WFSRequest.php class. The query that is made has a fixed CRS of 4326 for bboxes and geometries.

The following example shows the query generated with the new method:

WITH source AS (
         SELECT "id", "code", "label", "code_parent", "geom" AS "geosource" FROM "tests_projects"."form_edition_vr_list" WHERE True LIMIT 1000
        )
        --SELECT row_to_json(fc, True) AS geojson
        --FROM (
        --    SELECT
        --        'FeatureCollection' As type,
        --        array_to_json(array_agg(f)) As features
        SELECT row_to_json(f, True) AS geojson
            FROM (
                SELECT
                    'Feature' AS type,
         Concat(
            'list',
            '.',
            "id") AS id,
                        array_to_json(ARRAY[
                            ST_XMin(ST_Transform(ST_Envelope(lg.geosource::geometry), 4326)),
                            ST_YMin(ST_Transform(ST_Envelope(lg.geosource::geometry), 4326)),
                            ST_XMax(ST_Transform(ST_Envelope(lg.geosource::geometry), 4326)),
                            ST_YMax(ST_Transform(ST_Envelope(lg.geosource::geometry), 4326))
                        ]) As bbox,
            
                        ST_AsGeoJSON(ST_Transform(lg.geosource::geometry, 4326))::json As geometry,
            
                    row_to_json(
                        ( SELECT l FROM
                            (
                                SELECT "id", "code", "label", "code_parent"
                            ) As l
                        )
                    ) As properties
                FROM source As lg
            ) As f
        --) As fc

Geometries are always returned in 4326.

Also bbox is not considered, probably due to the BBOX sintax passed as param:

-899310.3675109865,5331047.836042481,1412145.3675109865,6412173.163957519,EPSG:3857

The CRS added as the fifth element seems to be a format not supported by QGIS Server (maybe is related to Geoserver?) -> EDIT: QGIS Server supports this format

Ignoring the BBOX causes all features to be requested, which can cause performance issues.

It would probably be necessary to modify this method to get a Postgis side conversion, or alternatively force the QGIS Server request with the WFS force_qgis parameter.

@rldhont @mdouchin what do you think?

Thanks!

meyerlor · 2026-04-03T13:14:02Z

Hi @mind84, good catch — you're right on both. Both issues are in the PostGres direct path (getfeaturePostgres), which is what gets hit for PG-backed layers:

BBOX ignored: getBboxSql rejects anything that isn't exactly 4 comma-separated values (line 751). WFS 1.1.0 appends the CRS as a 5th element (x,y,x,y,EPSG:3857), so the check fails silently and no spatial filter is applied → all features get returned. That explain the perf issue you saw.
Geometry hardcoded to 4326: Lines 1184–1185 always do ST_Transform(..., 4326) regardless of SRSNAME. Meanwhile the JS now assumes coordinates come back in map projection and skips reprojection — so on a 3857 map you get 4326 coords plotted as-is. Thats the mismatch in your screenshot.

Both are pre-existing limitation of the PG path.. Simplest fix imo: add FORCE_QGIS: '1' to the snap WFS call. That bypasses the direct SQL path and goes trough QGIS Server, which handle WFS 1.1.0 + SRSNAME + 5-element BBOX correctly and is also the right place for PROJ/NTv2 datum shifts anyway. Snap is already capped at 1000 features so the extra overhead doesn't really matter here.

Should I make that change and update the test? Fixing the PG path itself (BBOX parsing + SRSNAME awareness) would be worth doing but i'd keep that as a separate issue? I'm keen on migrating the whole snapping function to OL10 soonish.

I'll wait for @rldhont / @mdouchin ..! :)

rldhont · 2026-04-03T14:21:56Z

@meyerlor you are just facing the fact that when QGIS Server returns GeoJSON in EPSG:4326, it truncates coordinates at 6 number after comma. Like I explain here : #6456 (comment)

The solution is to request the GetFeature as GML or to add an SRSNAME and in PHP to force to QGIS Server when SRSNAME is provided.

mind84 · 2026-04-03T15:56:27Z

@meyerlor @rldhont

I'm a little worried about bypassing PG in this case, it's a very useful method for increasing performance.

mdouchin · 2026-04-03T16:37:32Z

I'm a little worried about bypassing PG in this case, it's a very useful method for increasing performance.

I agree !

meyerlor · 2026-04-06T16:06:49Z

Hi @rldhont @mind84 @mdouchin,
Thanks for the feedback. I worked on both issues raised — the BBOX parsing and the hardcoded EPSG:4326 — directly in the PostGIS path, so the performance benefit of the direct PG query is preserved.

BBOX with CRS (5th element): getBboxSql now recognises the WFS 1.1.0 format x,y,x,y,EPSG:XXXX. The CRS is extracted and the envelope is reprojected to the layer's native SRID via ST_Transform(ST_MakeEnvelope(..., inputSrid), layerSrid). An explicit SRSNAME parameter takes priority over the 5th BBOX element.
Geometry output in requested SRSNAME: setGeojsonSql now transforms geometry and bbox to the requested output SRID instead of always using 4326.

Both changes have unit tests covering EPSG:4326, 3857, and 2154.

Is this ok to go forward like this?

My other assumptions about precision were wrong - i closed that PR. (PostGIS ST_AsGeoJSON defaults to 9 decimal digits regardless of CRS.

mind84 · 2026-04-14T09:52:11Z

Hi @meyerlor!

In my opinion, there are no significant issues with modifying the PG function to handle SRSNAME. I think it's worth considering that:

Precision issues can't be completely avoided; the best solution is to reduce the geometry precision to a fixed grid.
There are several methods in the code that perform WFS requests; some of these assume the returned geometry is in 4326 format, so it's probably worth doing a general consistency check on these methods.

Cheers

meyerlor · 2026-04-16T12:57:27Z

Hi @mind84, thanks for bringin in that perspective!

Both of your points are fair, and I'd rather not widen the scope of this PR to adress them — I think an own issue / PR would be more apropriate?

Like:

Precision / fixed grid — agreed. ST_AsGeoJSON already caps at 9 digits which is sub-mm for projected CRSes, but an explicit ST_SnapToGrid per
layer SRID would make output deterministic across reprojections. I can open a dedicated issue to it sooner or later.
Consistency across WFS callers — also agreed. Several paths still implicitly assume 4326 output (e.g. qgisVectorLayer::editableFeatures(), search,
osm controllers, plus a few JS consumers). I will start a new issue listing the suspects so we can work through them one at a time - both issues are not top-priority i think?

If that is ok with you, I guess this PR is kind of ready for a final pass. @rldhont @mdouchin — any remaining concerns?

rldhont

To verify that the changes you've made are correctly transforming the geometries, you can add tests in https://github.com/3liz/lizmap-web-client/blob/master/tests/end2end/playwright/requests-wfs.spec.js, thanks.

rldhont

I have made a review of snap.spec.js.

Can you add a test in requests-wfs.spec.js, to check the ability to support SRSName for WFS 1.1.0 with Lizmap eb Client. You can reuse the request made by snap.

rldhont · 2026-04-20T09:15:31Z

+        const snapWfsRequestPromise = page.waitForRequest(
+            request => request.method() === 'POST'
+                && request.postData()?.includes('GetFeature') === true
+                && request.postData()?.includes('form_edition_snap_point') === true
+        );


I invite you to enhance project's waitForGetFeatureRequest method to be able to pass a FEATURETYPE parameter like waitForGetMapRequest has a LAYERS parameter.

rldhont · 2026-04-20T09:20:08Z

+
+        // Allow any in-flight requests to settle before checking the flag.
+        await page.waitForTimeout(300);


Add a responseExpect test to check that await snapWfsRequest.response() is a GeoJSON. And you cwill be able to reduce the waitForTimeout with the certitude that the DescribeFeatureType request will not be send.

rldhont · 2026-04-20T09:21:23Z

+        const snapWfsRequestPromise = page.waitForRequest(
+            request => request.method() === 'POST'
+                && request.postData()?.includes('GetFeature') === true
+                && request.postData()?.includes('snap_datum_shift_target') === true
+        );


Use your new waitForGetFeatureRequest method with FEATURETYPE parameter.

rldhont · 2026-04-20T09:23:38Z

+        const bbox = postData.get('BBOX') ?? '';
+        expect(bbox, `BBOX must be 5-element WFS 1.1.0 format (x,y,x,y,EPSG:3857), got: "${bbox}"`
+        ).toMatch(/^-?[\d.]+,-?[\d.]+,-?[\d.]+,-?[\d.]+,EPSG:3857$/);
+    });


Add a responseExpect test to check that await snapWfsRequest.response() is a GeoJSON.

rldhont · 2026-04-20T09:25:17Z

+        );
+
+        const formRequest = await project.openEditingFormWithLayer('snap_datum_shift_edit');
+        await formRequest.response();


Add a responseExpect to check that await formRequest.response() is an HTML.

rldhont · 2026-04-20T09:26:35Z

+        const snapWfsResponsePromise = page.waitForResponse(
+            response => response.request().method() === 'POST'
+                && response.request().postData()?.includes('GetFeature') === true
+                && response.request().postData()?.includes('snap_datum_shift_target') === true
+        );


Use your new waitForGetFeatureRequest method with FEATURETYPE parameter.

rldhont · 2026-04-20T09:26:55Z

+        );
+
+        const formRequest = await project.openEditingFormWithLayer('snap_datum_shift_edit');
+        await formRequest.response();


Check the response.

rldhont · 2026-04-20T09:27:42Z

+            contentType: 'application/json',
+        });
+
+        const geojson = JSON.parse(responseBody);


Suggested change

const geojson = JSON.parse(responseBody);

const geojson = await snapWfsResponse.json();

rldhont · 2026-04-20T09:28:31Z

+
+        await page.getByRole('tab', { name: 'Digitization' }).click();
+
+        const snapWfsResponse = await snapWfsResponsePromise;


Check that the response is GeoJSON

rldhont · 2026-04-20T09:30:01Z


-        await Promise.all([getSnappingPolygonFeatureRequestPromise, getSnappingPolygonDescribeFeatureRequestPromise])
+        await getSnappingPolygonFeatureRequestPromise;



Wait for the response and check it to be able to remove waitForTimeout

… error When loading snap source features, the previous code made a WFS 1.0.0 request without SRSNAME. QGIS Server returned GeoJSON in EPSG:4326, and OL2 transformed coordinates to the map projection client-side. For CRS that require datum grid shifts (e.g. RD New / EPSG:28992, Swiss LV95, Belgian Lambert), OL2 uses a simplified Helmert approximation rather than the full NTv2 grid that QGIS Server uses internally. This caused a systematic ~cm offset between WMS-rendered feature positions and WFS snap source positions. Backport of fix from main branch, adapted for OL2 APIs used in 3.9.

Add a test that confirms the snapping GetFeature request is issued with VERSION=1.1.0 and an explicit SRSNAME matching the map projection, and that no DescribeFeatureType request is sent alongside it. Also remove DescribeFeatureType waits from the existing snap panel test — the new WFS 1.1.0 path bypasses getFeatureData() so no DescribeFeatureType is triggered when loading snap source features.

…rmat Make the PostGIS direct WFS path honour the SRSNAME parameter: geometries and bounding boxes are now transformed to the requested output CRS instead of being hardcoded to EPSG:4326. Also parse the WFS 1.1.0 five-element BBOX (x,y,x,y,EPSG:XXXX) so the spatial filter works for cross-CRS layers. Includes unit tests, e2e tests, and a dedicated QGIS test project with an EPSG:2154 snap layer to exercise datum-shift reprojection.

…ft tests Add snap_on_start: True to form_edition_snap_datum_shift.qgs.cfg so snapping auto-activates when the editing form is displayed (matching the behaviour of form_edition_multilayer_snap). Without this flag the snap WFS request never fires automatically. Also fix both datum-shift tests to await formRequest.response() before clicking the Digitization tab, ensuring the form HTML has been rendered and edition-tabs is visible before Playwright tries to interact with it.

… errors Review follow-up on the PostGIS WFS SRSNAME handling added earlier in this branch. - Extract parseSrsnameSrid() helper in WFSRequest; both getBboxSql() and getfeaturePostgres() now share the same parsing logic and accept the common OGC forms (EPSG:xxxx, urn:ogc:def:crs:EPSG::xxxx, HTTP URI). - Unify the error policy on malformed SRSNAME: both methods now log a warning via appContext->logMessage and fall back to a safe default (layer SRID for the input BBOX, EPSG:4326 for the output geometry). Previously getBboxSql silently dropped the BBOX filter (returning all rows) while getfeaturePostgres silently defaulted to 4326 — the asymmetry produced the worst-of-both-worlds on garbage input. - Introduce DEFAULT_OUTPUT_SRID class constant to replace the scattered 4326 literal. - Snapping.js: surface a user-visible error message when a WFS snap request fails or returns a non-FeatureCollection payload (e.g. an OGC ExceptionReport as JSON). Notification fires once per refresh batch so repeated failures don't spam the UI. - Tests: add parseSrsnameSrid data-driven test; update getBboxSql tests to reflect the new fallback-with-log behaviour; inject ContextForTests so invalid-SRSNAME branches don't null-deref.

parseSrsnameSrid only splits on ':' so "http://.../EPSG/0/4326" never parsed as claimed. Correct the docblock to list only the colon-separated forms and remove the HTTP-URI case from the data provider. No production-code impact.

…t CRS Add testSetGeojsonSqlOutputCrs / getSetGeojsonSqlOutputCrsData to assert that both the feature geometry and the bounding-box geometry are wrapped in ST_Transform(…, outputSrid) for EPSG:4326 (default), EPSG:3857 (Web Mercator / snap use-case) and EPSG:2056 (Swiss national grid). This closes the gap identified in review: the existing tests covered SRSNAME→ST_Transform for the BBOX *input* filter (getBboxSql), but did not exercise the *output* reprojection path in setGeojsonSql that is the core fix for the datum-shift regression.

…pec.js - project.js: enhance waitForGetFeatureRequest() to accept an optional featureType parameter (mirrors the LAYERS filter on waitForGetMapRequest), avoiding inline predicates in callers. - snap.spec.js: - Import expect-response fixtures and use responseExpect().toBeGeoJson() / .toBeHtml() in place of bare await / waitForTimeout(300) calls, giving explicit assertions instead of implicit success. - Replace all inline page.waitForRequest / page.waitForResponse lambdas with project.waitForGetFeatureRequest(featureType) and .response(). - Use snapWfsResponse.json() directly instead of text() + JSON.parse(). - requests-wfs.spec.js: add 'WFS 1.1.0 GetFeature with 5-element BBOX' test that exercises the CRS-as-BBOX-suffix format introduced by WFS 1.1.0 (the same request shape snap uses) against the datum-shift project and verifies that returned geometry coordinates are in the requested EPSG:3857 range, not in EPSG:4326 or the layer's native EPSG:2154.

…ect edition form content-type assertion - Proxy.php normalizeParams(): skip float conversion for non-numeric elements (e.g. the 5th WFS 1.1.0 BBOX element "EPSG:3857") to avoid producing SRID 0 in downstream ST_MakeEnvelope calls - snap.spec.js: jResponseHtmlFragment sends text/plain, not text/html; use toBeTextPlain() to match the actual content-type of edition form responses

…bsent When a WFS 1.1.0 client embeds the CRS as the 5th BBOX element without sending a separate SRSNAME parameter, getfeaturePostgres was falling back to EPSG:4326 for output geometries. Apply the same SRSNAME extraction logic used by getBboxSql so the output coordinates match the requested CRS.

Update Docker Hub username in e2e_tests workflow

e314854

github-actions Bot added this to the 3.11.0 milestone Apr 2, 2026

meyerlor added backport release_3_8 backport release_3_9 backport release_3_10 labels Apr 2, 2026

mdouchin requested review from mind84 and rldhont April 2, 2026 12:52

Merge branch '3liz:master' into master

2fe6236

meyerlor force-pushed the fix/snap-wfs-map-projection branch 2 times, most recently from cf366d9 to 831f2a2 Compare April 6, 2026 15:54

meyerlor force-pushed the fix/snap-wfs-map-projection branch 5 times, most recently from 752f977 to 957fe3d Compare April 6, 2026 19:07

rldhont added the editing form label Apr 7, 2026

meyerlor mentioned this pull request Apr 7, 2026

fix: increase ST_AsGeoJSON precision to 9 decimal digits #6699

Closed

meyerlor changed the title ~~Fix/snap wfs map projection~~ Fix WFS PostGIS path SRSNAME handling and snap datum-shift error Apr 16, 2026

rldhont requested changes Apr 19, 2026

View reviewed changes

meyerlor requested a review from rldhont April 20, 2026 08:53

rldhont requested changes Apr 20, 2026

View reviewed changes

meyerlor and others added 11 commits April 21, 2026 06:55

test: drop SRSNAME HTTP-URI case from parseSrsnameSrid

d354860

parseSrsnameSrid only splits on ':' so "http://.../EPSG/0/4326" never parsed as claimed. Correct the docblock to list only the colon-separated forms and remove the HTTP-URI case from the data provider. No production-code impact.

ci: retrigger

af47dd1

meyerlor force-pushed the fix/snap-wfs-map-projection branch from 27f6bfe to 6d8a28f Compare April 21, 2026 04:55

Merge branch 'master' into fix/snap-wfs-map-projection

545dd31


		// Allow any in-flight requests to settle before checking the flag.
		await page.waitForTimeout(300);

	const geojson = JSON.parse(responseBody);
	const geojson = await snapWfsResponse.json();


		await page.getByRole('tab', { name: 'Digitization' }).click();

		const snapWfsResponse = await snapWfsResponsePromise;


		await Promise.all([getSnappingPolygonFeatureRequestPromise, getSnappingPolygonDescribeFeatureRequestPromise])
		await getSnappingPolygonFeatureRequestPromise;

Uh oh!

Conversation

meyerlor commented Apr 2, 2026

Uh oh!

mind84 commented Apr 2, 2026

Uh oh!

meyerlor commented Apr 2, 2026

Uh oh!

mind84 commented Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

meyerlor commented Apr 3, 2026

Uh oh!

rldhont commented Apr 3, 2026

Uh oh!

mind84 commented Apr 3, 2026

Uh oh!

mdouchin commented Apr 3, 2026

Uh oh!

meyerlor commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mind84 commented Apr 14, 2026

Uh oh!

meyerlor commented Apr 16, 2026

Uh oh!

rldhont left a comment

Choose a reason for hiding this comment

Uh oh!

rldhont left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mind84 commented Apr 3, 2026 •

edited

Loading

meyerlor commented Apr 6, 2026 •

edited

Loading