Skip to content

CRS change crashes with grid-based projections when layer resolution limits are configured #12627 - #12669

Merged
rowheat02 merged 3 commits into
geosolutions-it:masterfrom
rowheat02:muricaResolutionFix
Aug 3, 2026
Merged

CRS change crashes with grid-based projections when layer resolution limits are configured #12627#12669
rowheat02 merged 3 commits into
geosolutions-it:masterfrom
rowheat02:muricaResolutionFix

Conversation

@rowheat02

@rowheat02 rowheat02 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Description

This PR fixes the CRS change when the target projection uses a datum grid and some layers have minResolution or maxResolution configured.

The resolution conversion was transforming a point to the target CRS and then using it with the source projection. This could generate invalid coordinates and crash the CRS change when the target projection requires a datum-grid correction.

While fixing this, we also found an issue when switching the CRS back and forth. For example:

EPSG:25830 → EPSG:4326 → EPSG:25830

The converted resolution limits were snapped to the resolution list, so they did not return to their original values. Because of this, layers that were visible before the CRS change could become invisible after switching back.

The limits are now converted using the meters-per-unit ratio without snapping, making the conversion reversible.

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x", remove the others)

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

Issue

What is the current behavior?

#12627
The CRS change can fail with datum-grid projections when layer resolution limits are configured. Switching the CRS back and forth can also change the limits and make visible layers invisible.

What is the new behavior?

The CRS changes without crashing, and layer resolution limits remain stable after multiple CRS changes.

Breaking change

Does this PR introduce a breaking change? (check one with "x", remove the other)

  • Yes, and I documented them in migration notes
  • No

Other useful information

@rowheat02 rowheat02 added this to the 2026.02.01 milestone Jul 16, 2026
@rowheat02
rowheat02 requested a review from offtherailz July 16, 2026 10:57
@cla-bot cla-bot Bot added the CLA Ready label Jul 16, 2026
@tdipisa
tdipisa marked this pull request as draft July 21, 2026 14:35
@tdipisa

tdipisa commented Jul 21, 2026

Copy link
Copy Markdown
Member

Converted to draft waiting for 2026.02.00 to be out

@tdipisa
tdipisa marked this pull request as ready for review July 22, 2026 16:32

@offtherailz offtherailz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good,
Two things before merge in inline suggestions you can apply:

  1. guard + warning if conversion fails
  2. fix to the unit coneversion (if passing, for instance, from meeters to degrees etc). I attached a test. Please verify that makes sense to you too.

Could you please verify that the changes I suggested still fits our use case and they work well also in different latitude with inverse conversion @rowheat02 ? thank you.

Comment thread web/client/utils/MapUtils.js Outdated
Comment thread web/client/utils/MapUtils.js Outdated
Comment thread web/client/actions/map.js Outdated
Comment thread web/client/utils/__tests__/MapUtils-test.js Outdated
Comment thread web/client/utils/__tests__/MapUtils-test.js
@offtherailz offtherailz assigned rowheat02 and unassigned offtherailz Jul 29, 2026
…on CRS change

The converted limits were snapped to a resolution of the target CRS, but
getResolutions returns the resolutions configured in mapOptions.view for any SRS,
which on the way back is the previous CRS ladder: metres ended up snapped onto
degrees. Convert through the meters-per-unit ratio instead, the same one the map
view uses to keep the scale, and drop the snap so the conversion is reversible.
@offtherailz

offtherailz commented Jul 30, 2026

Copy link
Copy Markdown
Member

Pushed a commit on top of yours with the fix for the problem you found.

The conversion itself was fine after the suggestions; what broke the round trip was the snap.
snapToResolution used getResolutions(targetCRS), and that returns whatever is in
mapOptions.view.resolutions for any SRS because
updateMapResolutionsOnCrsChangeEpic writes that array with view.projection stripped. Since the
thunk runs before that epic, on the way back the limits in metres were snapped onto the previous
CRS ladder in degrees: maxResolution became ~0.703125 metres, i.e. the layer only showed up below
1:2657. Snapping between two ladders is also a ratchet, so the limits shrank on every switch.

So the limits are now converted through the meters-per-unit ratio of the two CRS - the same ratio
closestMatchedZoom uses to keep the map scale, which is what keeps the layer's relation with the
view resolution - and they are no longer snapped, so the conversion is reversible. No point
transform is involved anymore, which also removes the original #12627 crash at the root.

What to test:

  • a map in EPSG:25830 with a layer having min/maxResolution, switch to EPSG:4326 and back: the
    layer stays visible and the limits return to the original values (repeat a few times, they must
    not drift);
  • the Visibility limits panel in the layer settings must keep showing the same scale denominators
    in every CRS (the limit is a scale, only its unit changes);
  • after the round trip, zooming out past the max scale must still hide the layer, so the threshold
    is still where it was.

Two pre-existing problems came up while testing this, unrelated to the PR; I will open them
separately: the map scale drifts one zoom level per CRS switch, and mapOptions.view.resolutions
is saved without `view.projectis lying for any other caller.

@offtherailz
offtherailz self-requested a review July 31, 2026 15:20
@rowheat02
rowheat02 merged commit a7edb1d into geosolutions-it:master Aug 3, 2026
15 checks passed
@offtherailz

Copy link
Copy Markdown
Member

Backport failed for 2026.02.xx: couldn't find remote ref 2026.02.xx.
Please ensure that this GitHub repo has a branch named 2026.02.xx.

@allyoucanmap

Copy link
Copy Markdown
Contributor

@rowheat02 the automatic backport failed, please provide a new backport

@rowheat02

Copy link
Copy Markdown
Contributor Author

Backport for 2026.02.xx has been created manually: #12762

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CRS change crashes with grid-based projections when layer resolution limits are configured

4 participants