Skip to content

Commit

Permalink
PB-1221 : partially out of bound KML was ignored entirely
Browse files Browse the repository at this point in the history
The KML extent detection was not performing as expected with KML that had only one dimension out of LV95 bounds.

To counteract that, I've reverted the logic in the extent calculation, transforming LV95 bounds into WGS84 to do the matching instead of transforming a potentially big WGS84 extent to LV95 (lead to big math error...)
  • Loading branch information
pakb committed Nov 26, 2024
1 parent 4549e68 commit 42ea977
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 24 deletions.
38 changes: 26 additions & 12 deletions src/utils/__tests__/extentUtils.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ import { getExtentIntersectionWithCurrentProjection } from '@/utils/extentUtils'

describe('Test extent utils', () => {
describe('reproject and cut extent within projection bounds', () => {
function expectExtentIs(toBeTested, expected, acceptableDelta = 0.5) {
expect(toBeTested).to.be.an('Array').lengthOf(4)
expected.forEach((value, index) => {
expect(toBeTested[index]).to.be.approximately(value, acceptableDelta)
})
}

it('handles well wrong inputs and returns null', () => {
expect(getExtentIntersectionWithCurrentProjection()).to.be.null
expect(getExtentIntersectionWithCurrentProjection(null, null, null)).to.be.null
Expand All @@ -18,7 +25,7 @@ describe('Test extent utils', () => {
const singleCoordinate = [8.2, 47.5]
const singleCoordinateInLV95 = reprojectAndRound(WGS84, LV95, singleCoordinate)
const extent = [singleCoordinate, singleCoordinate].flat()
expect(getExtentIntersectionWithCurrentProjection(extent, WGS84, LV95)).to.deep.equal([
expectExtentIs(getExtentIntersectionWithCurrentProjection(extent, WGS84, LV95), [
...singleCoordinateInLV95,
...singleCoordinateInLV95,
])
Expand All @@ -33,15 +40,21 @@ describe('Test extent utils', () => {
expect(getExtentIntersectionWithCurrentProjection(extent, WGS84, LV95)).to.be.null
})
it('reproject and cut an extent that is greater than LV95 extent on all sides', () => {
const projectedExtent = getExtentIntersectionWithCurrentProjection(
[-2.4, 35, 21.3, 51.7],
WGS84,
LV95
expectExtentIs(
getExtentIntersectionWithCurrentProjection([-2.4, 35, 21.3, 51.7], WGS84, LV95),
[...LV95.bounds.bottomLeft, ...LV95.bounds.topRight]
)
})
it('reproject and cut an extent that is partially bigger than LV95 bounds', () => {
expectExtentIs(
getExtentIntersectionWithCurrentProjection(
// extent of file linked to PB-1221
[-122.08, -33.85, 151.21, 51.5],
WGS84,
LV95
),
[...LV95.bounds.bottomLeft, ...LV95.bounds.topRight]
)
expect(projectedExtent).to.deep.equal([
...LV95.bounds.bottomLeft,
...LV95.bounds.topRight,
])
})
it('only gives back the portion of an extent that is within LV95 bounds', () => {
const singleCoordinateInsideLV95 = [7.54, 48.12]
Expand All @@ -51,9 +64,10 @@ describe('Test extent utils', () => {
singleCoordinateInsideLV95
)
const overlappingExtent = [0, 0, ...singleCoordinateInsideLV95]
expect(
getExtentIntersectionWithCurrentProjection(overlappingExtent, WGS84, LV95)
).to.deep.equal([...LV95.bounds.bottomLeft, ...singleCoordinateInLV95])
expectExtentIs(
getExtentIntersectionWithCurrentProjection(overlappingExtent, WGS84, LV95),
[...LV95.bounds.bottomLeft, ...singleCoordinateInLV95]
)
})
})
})
29 changes: 19 additions & 10 deletions src/utils/extentUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,26 +84,35 @@ export function getExtentIntersectionWithCurrentProjection(
) {
return null
}
let extentInCurrentProjection = flattenExtent(extent)
let currentProjectionAsExtentProjection = currentProjection.bounds.flatten
if (extentProjection.epsg !== currentProjection.epsg) {
extentInCurrentProjection = projExtent(
extentProjection,
// we used to reproject the extent here, but there's problem arising if current projection is LV95 and
// the extent is going a little bit out of Switzerland.
// As LV95 is quite location-locked, the further we get, the bigger the mathematical errors start growing.
// So to counteract that, we transform the current projection bounds in the extent projection to do the comparison.
currentProjectionAsExtentProjection = projExtent(
currentProjection,
extentInCurrentProjection
extentProjection,
currentProjectionAsExtentProjection
)
}
extentInCurrentProjection = getExtentIntersection(
extentInCurrentProjection,
currentProjection.bounds.flatten
let finalExtent = getExtentIntersection(
flattenExtent(extent),
currentProjectionAsExtentProjection
)
if (
!extentInCurrentProjection ||
!finalExtent ||
// OL now populates the extent with Infinity when nothing is in common, instead returning a null value
extentInCurrentProjection.every((value) => Math.abs(value) === Infinity)
finalExtent.every((value) => Math.abs(value) === Infinity)
) {
return null
}
return flattenExtent(extentInCurrentProjection)
if (extentProjection.epsg !== currentProjection.epsg) {
// if we transformed the current projection extent above, we now need to output the correct proj
finalExtent = projExtent(extentProjection, currentProjection, finalExtent)
}

return flattenExtent(finalExtent)
}

/**
Expand Down
4 changes: 2 additions & 2 deletions tests/cypress/tests-e2e/importToolFile.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ describe('The Import File Tool', () => {
)
const expectedLayerId = 'external-kml-file.kml'
const expectedOnlineLayerId = 'https://example.com/second-valid-kml-file.kml'
const acceptedDelta = 0.1
const acceptedDelta = 0.2
const checkLocation = (expected, result) => {
expect(result).to.be.an('Array')
expect(result.length).to.eq(2)
Expand Down Expand Up @@ -821,7 +821,7 @@ describe('The Import File Tool', () => {
cy.closeMenuIfMobile()

cy.get('[data-cy="window-close"]').click()
cy.get('[data-cy="ol-map"]').click(150, 250)
cy.get('[data-cy="ol-map"]').click(170, 250)

cy.log('Check that the error is displayed in the profile popup')
cy.get('[data-cy="show-profile"]').click()
Expand Down

0 comments on commit 42ea977

Please sign in to comment.