diff --git a/client/src/components/Journey/JourneyMap.test.tsx b/client/src/components/Journey/JourneyMap.test.tsx index f7b1d3310..3e56ba393 100644 --- a/client/src/components/Journey/JourneyMap.test.tsx +++ b/client/src/components/Journey/JourneyMap.test.tsx @@ -1,4 +1,4 @@ -// FE-COMP-JOURNEYMAP-001 to FE-COMP-JOURNEYMAP-054 +// FE-COMP-JOURNEYMAP-001 to FE-COMP-JOURNEYMAP-055 vi.mock('../../api/websocket', () => ({ connect: vi.fn(), @@ -38,9 +38,11 @@ vi.mock('leaflet', () => { getCenter: vi.fn(() => ({ lat: 0, lng: 0 })), zoomIn: vi.fn(), zoomOut: vi.fn(), - // The photo layer subscribes to zoom/pan and clusters in screen space (#1614). + // The photo and mark layers subscribe to zoom/pan and cluster in world + // pixels at the current zoom (#1614, #2889). on: vi.fn(), off: vi.fn(), + project: vi.fn(() => ({ x: 0, y: 0 })), latLngToContainerPoint: vi.fn(() => ({ x: 0, y: 0 })), }; return { @@ -76,6 +78,8 @@ import type { Mock } from 'vitest'; import L from 'leaflet'; import JourneyMap from './JourneyMap'; import type { JourneyMapHandle } from './JourneyMap'; +import { mercatorWorldPx } from './journeyMapMarks'; +import type { JourneyMapStay } from '@trek/shared'; // maplibre-gl-leaflet hangs the vector basemap into Leaflet's tile pane. The mock // only has to be callable and hand back a layer with the three methods the callers @@ -782,9 +786,8 @@ describe('JourneyMap', () => { }); // #2254 — the map effect defers its opening fitBounds/setView to a rAF, so this - // effect can draw while the map still has no center or zoom. Clustering - // projects to container pixels, which throws in that window and took the whole - // public journey page down with it. + // effect can draw while the map still has no center or zoom. Projecting in that + // window throws and took the whole public journey page down with it. it('FE-COMP-JOURNEYMAP-050: waits out a map that has no view yet, then draws on moveend', () => { const noView = () => { throw new Error('Set map center and zoom first.'); }; let hasView = false; @@ -794,12 +797,12 @@ describe('JourneyMap', () => { // The mock hands out one shared map instance, so it can be reached — and // stripped of its view — before the component ever builds one. Typed down to // the three spies this needs: L.map's signature promises a real Leaflet map. - type MapProbe = { getCenter: Mock; latLngToContainerPoint: Mock; on: Mock }; + type MapProbe = { getCenter: Mock; project: Mock; on: Mock }; L.map(document.createElement('div')); const map = mockedMap() as MapProbe; vi.mocked(L.map).mockClear(); map.getCenter.mockImplementation(() => (hasView ? { lat: 0, lng: 0 } : noView())); - map.latLngToContainerPoint.mockImplementation(() => (hasView ? { x: 0, y: 0 } : noView())); + map.project.mockImplementation(() => (hasView ? { x: 0, y: 0 } : noView())); try { // Rendering must survive the viewless window, and draw nothing in it. @@ -820,7 +823,7 @@ describe('JourneyMap', () => { expect(photoHtml()[0]).toContain('/uploads/late.jpg'); } finally { map.getCenter.mockImplementation(() => ({ lat: 0, lng: 0 })); - map.latLngToContainerPoint.mockImplementation(() => ({ x: 0, y: 0 })); + map.project.mockImplementation(() => ({ x: 0, y: 0 })); } }); @@ -868,6 +871,65 @@ describe('JourneyMap', () => { expect(divIconHtml(0)).not.toContain('#16a34a'); expect(divIconHtml(1)).toContain('#16a34a'); }); + + // #2889: a pan used to shift the screen grid the layers cluster on, so photos + // near a cell edge changed groups, counts and thumbnails without any zoom. + it('FE-COMP-JOURNEYMAP-055: panning keeps the photo and mark groups, only a zoom regroups them', () => { + const photos = [ + { id: 'p1', lat: 45, lng: 5, thumbUrl: '/uploads/a.jpg' }, + { id: 'p2', lat: 45, lng: 5.05, thumbUrl: '/uploads/b.jpg' }, + ]; + const marks = [ + { kind: 'stay' as const, key: 's1', lat: 45, lng: 5, color: '#14b8a6', stay: {} as JourneyMapStay }, + { kind: 'stay' as const, key: 's2', lat: 45, lng: 5.05, color: '#14b8a6', stay: {} as JourneyMapStay }, + ]; + L.map(document.createElement('div')); + const map = mockedMap() as { project: Mock; latLngToContainerPoint: Mock; getZoom: Mock; on: Mock; off: Mock }; + vi.mocked(L.map).mockClear(); + let zoom = 3; + let pan = 0; + map.getZoom.mockImplementation(() => zoom); + map.project.mockImplementation(([lat, lng]: [number, number], z: number) => mercatorWorldPx(lat, lng, z, 256)); + map.latLngToContainerPoint.mockImplementation(([lat, lng]: [number, number]) => { + const world = mercatorWorldPx(lat, lng, zoom, 256); + return { x: world.x - pan, y: world.y }; + }); + const drawnSince = (from: number) => + vi.mocked(L.divIcon).mock.calls.slice(from).map(c => String(c[0]?.html)).filter(h => h.includes('/uploads/') || h.includes('#14b8a6')); + const moveEnd = () => act(() => { + const gone = new Set(map.off.mock.calls.map(c => c[1])); + for (const [event, handler] of map.on.mock.calls) if (event === 'moveend' && !gone.has(handler)) (handler as () => void)(); + }); + + try { + render(); + let from = vi.mocked(L.divIcon).mock.calls.length; + moveEnd(); + const first = drawnSince(from); + expect(first.filter(h => h.includes('/uploads/'))).toEqual([expect.stringContaining('>2<')]); + expect(first.filter(h => h.includes('#14b8a6'))).toHaveLength(1); + expect(map.project).toHaveBeenCalledWith([45, 5.05], 3); + expect(map.latLngToContainerPoint).not.toHaveBeenCalled(); + + // A pan that puts a 64 px cell edge between the two photos on screen. + const mid = (mercatorWorldPx(45, 5, 3, 256).x + mercatorWorldPx(45, 5.05, 3, 256).x) / 2; + pan = mid - (Math.floor(mid / 64) + 0.5) * 64; + from = vi.mocked(L.divIcon).mock.calls.length; + moveEnd(); + expect(drawnSince(from)).toEqual(first); + + zoom = 12; + from = vi.mocked(L.divIcon).mock.calls.length; + moveEnd(); + const zoomed = drawnSince(from); + expect(zoomed.filter(h => h.includes('/uploads/'))).toHaveLength(2); + expect(zoomed.filter(h => h.includes('#14b8a6'))).toHaveLength(2); + } finally { + map.getZoom.mockImplementation(() => 10); + map.project.mockImplementation(() => ({ x: 0, y: 0 })); + map.latLngToContainerPoint.mockImplementation(() => ({ x: 0, y: 0 })); + } + }); }); /** The shared marker spy with the `remove` a hidden layer is taken off by. */ diff --git a/client/src/components/Journey/JourneyMap.tsx b/client/src/components/Journey/JourneyMap.tsx index afedc547c..4d3071e5f 100644 --- a/client/src/components/Journey/JourneyMap.tsx +++ b/client/src/components/Journey/JourneyMap.tsx @@ -11,7 +11,7 @@ import { escapeHtml, type JourneyTrack } from '@trek/shared' import { ensureJourneyPopupStyle, formatMarkerDate, journeyPopupHtml } from './journeyMapPopup' import { PLANNED_BADGE_SVG, type JourneyMapMark } from './journeyMapMarks' import { isPlannedTrack, type JourneyMapLayerVisibility } from './journeyMapLayers' -import { NOTHING_DRAWN, drawLeafletMarks, switchLeafletLayers, type DrawnLayers, type LeafletLayerKind } from './journeyLeafletLayers' +import { NOTHING_DRAWN, drawLeafletMarks, leafletWorldPx, switchLeafletLayers, type DrawnLayers, type LeafletLayerKind } from './journeyLeafletLayers' export interface MapMarkerItem { id: string @@ -509,8 +509,8 @@ function JourneyMap( }, [tileUrl]) // Photo layer (#1614). Its own effect on purpose: photos arriving must not tear - // down and rebuild the map the way the entry effect does. Redrawn on zoom and - // pan because the clustering is done in screen space. + // down and rebuild the map the way the entry effect does. Redrawn after every + // move, but the clusters sit on a world grid, so only a zoom regroups them. useEffect(() => { const map = mapRef.current if (!map) return @@ -522,7 +522,7 @@ function JourneyMap( // The initial view (setView/fitBounds) is set on a deferred rAF in the // map-build effect above, so this can run before the map has a center/zoom - // — latLngToContainerPoint throws in that window. Skip; the moveend/zoomend + // and nothing can be projected in that window. Skip; the moveend/zoomend // listener below redraws once the deferred view lands. try { map.getCenter() @@ -531,7 +531,7 @@ function JourneyMap( } const group = L.layerGroup() - for (const cluster of clusterPhotos(photos, (lat, lng) => map.latLngToContainerPoint([lat, lng]))) { + for (const cluster of clusterPhotos(photos, leafletWorldPx(map))) { const marker = L.marker([cluster.lat, cluster.lng], { icon: L.divIcon({ className: '', @@ -563,7 +563,7 @@ function JourneyMap( // The plan's places and the stays of day reports (JRN-04), in their own layer // like the photos: below the entry pins and above the thumbnails, regrouped - // on every move because marks are only joined where they would overlap. + // on zoom because marks are only joined where they would overlap. useEffect(() => { const map = mapRef.current if (!map) return diff --git a/client/src/components/Journey/JourneyMapGL.test.tsx b/client/src/components/Journey/JourneyMapGL.test.tsx index c7956ed60..cfaf838d6 100644 --- a/client/src/components/Journey/JourneyMapGL.test.tsx +++ b/client/src/components/Journey/JourneyMapGL.test.tsx @@ -35,8 +35,9 @@ const gl = vi.hoisted(() => { off: vi.fn(), once: vi.fn(), loaded: vi.fn(() => true), - // Container pixels from lng/lat, ten per degree: enough to tell near from far. - project: vi.fn(([lng, lat]: [number, number]) => ({ x: lng * 10, y: lat * 10 })), + // Container pixels, ten per degree. The layers must not group by them: they + // shift with every pan (#2889). + project: vi.fn(), remove: vi.fn(), resize: vi.fn(), flyTo: vi.fn(), @@ -182,6 +183,7 @@ beforeEach(() => { return gl.map }) gl.map.getZoom.mockReturnValue(10) + gl.map.project.mockImplementation(([lng, lat]: [number, number]) => ({ x: lng * 10, y: lat * 10 })) gl.map.getSource.mockReturnValue(null) vi.mocked(isStandardFamily).mockReturnValue(false) vi.mocked(supportsCustom3d).mockReturnValue(false) @@ -553,9 +555,10 @@ describe('JourneyMapGL', () => { { id: 'b', lat: 45.1, lng: 5.1, thumbUrl: '/api/photos/2/thumbnail' }, { id: 'c', lat: 48, lng: 11, thumbUrl: '/api/photos/3/thumbnail' }, ] + gl.map.getZoom.mockReturnValue(4) render() const thumbs = gl.markers.filter(m => m.element.innerHTML.includes('/api/photos/')) - // a and b fall in one 64px bucket at ten pixels a degree, c in its own. + // At zoom 4 a and b lie three world pixels apart in one 64px cell, c far off. expect(thumbs).toHaveLength(2) expect(thumbs[0].element.innerHTML).toContain('>2<') thumbs[0].element.click() @@ -563,6 +566,45 @@ describe('JourneyMapGL', () => { expect(gl.map.on).toHaveBeenCalledWith('moveend', expect.any(Function)) }) + it('FE-COMP-JMAPGL-2889: panning keeps the photo and mark groups, only a zoom regroups them', () => { + withToken() + const photos = [ + { id: 'a', lat: 45, lng: 5, thumbUrl: '/api/photos/1/thumbnail' }, + { id: 'b', lat: 45, lng: 5.1, thumbUrl: '/api/photos/2/thumbnail' }, + ] + const marks = [ + { kind: 'stay', key: 's1', lat: 45, lng: 5, color: '#14b8a6', stay: {} }, + { kind: 'stay', key: 's2', lat: 45, lng: 5.1, color: '#14b8a6', stay: {} }, + ] + gl.map.getZoom.mockReturnValue(4) + render() + const drawn = () => { + const shown = gl.markers.filter(m => !m.remove.mock.calls.length) + return { + photos: shown.filter(m => m.element.innerHTML.includes('/api/photos/')).map(m => [m.lngLat, m.element.innerHTML]), + marks: shown.filter(m => m.element.innerHTML.includes('#14b8a6')).map(m => [m.lngLat, m.element.innerHTML]), + } + } + const moveEnd = () => act(() => { + const gone = new Set(gl.map.off.mock.calls.map(c => c[1])) + for (const [event, handler] of gl.map.on.mock.calls) if (event === 'moveend' && !gone.has(handler)) (handler as () => void)() + }) + const before = drawn() + expect(before.photos).toHaveLength(1) + expect(before.photos[0][1]).toContain('>2<') + expect(before.marks).toHaveLength(1) + + // A pan moves every container point; at this offset b crosses a cell edge on screen. + gl.map.project.mockImplementation(([lng, lat]: [number, number]) => ({ x: lng * 10 - 19, y: lat * 10 })) + moveEnd() + expect(drawn()).toEqual(before) + + gl.map.getZoom.mockReturnValue(12) + moveEnd() + expect(drawn().photos).toHaveLength(2) + expect(drawn().marks).toHaveLength(2) + }) + it('FE-COMP-JMAPGL-026: focusMarker on an unknown id is a no-op', () => { withToken() const ref = React.createRef() diff --git a/client/src/components/Journey/JourneyMapGL.tsx b/client/src/components/Journey/JourneyMapGL.tsx index a8de124ad..fc3839fd7 100644 --- a/client/src/components/Journey/JourneyMapGL.tsx +++ b/client/src/components/Journey/JourneyMapGL.tsx @@ -7,7 +7,7 @@ import { buildGlMap } from '../Map/glSupport' import type { JourneyTrack } from '@trek/shared' import { SHOT_INDEX_ATTR, ensureJourneyPopupStyle, formatMarkerDate, journeyPopupHtml } from './journeyMapPopup' import { clusterPhotos, photoMarkerHtml, type MapPhoto } from './journeyPhotoLayer' -import { MARK_CLUSTER_PX, PLANNED_BADGE_SVG, clusterPoints, markHitSvg, type JourneyMapMark } from './journeyMapMarks' +import { MARK_CLUSTER_PX, PLANNED_BADGE_SVG, clusterPoints, glWorldPx, markHitSvg, type JourneyMapMark } from './journeyMapMarks' import { isPlannedTrack, type JourneyMapLayerVisibility } from './journeyMapLayers' export interface JourneyMapGLHandle { @@ -526,9 +526,10 @@ function JourneyMapGL( }, [entries, stableTrail, stableTracks, glProvider, glStyle, mapboxToken, enableMapbox3d, mapboxQuality, fullScreen, paddingBottom]) // Photo layer (#2453), the GL twin of the Leaflet one: its own effect so photos - // arriving do not rebuild the map, redrawn on every move because the clustering - // is done in screen space. Same dependencies as the build effect above, so it - // attaches to the map that effect just made. + // arriving do not rebuild the map. Redrawn after every move, but the clusters + // sit on a world grid, so only a zoom regroups them; a pan, a turn or a tilt + // leaves them as they were. Same dependencies as the build effect above, so + // it attaches to the map that effect just made. useEffect(() => { const map = mapRef.current if (!map) return @@ -539,7 +540,7 @@ function JourneyMapGL( const draw = () => { clear() if (!photos?.length) return - for (const cluster of clusterPhotos(photos, (lat, lng) => map.project([lng, lat]))) { + for (const cluster of clusterPhotos(photos, glWorldPx(map))) { const el = document.createElement('div') el.style.cssText = 'position:relative;width:48px;height:48px;cursor:pointer' el.innerHTML = photoMarkerHtml(cluster.members[0].thumbUrl, cluster.members.length) @@ -589,7 +590,7 @@ function JourneyMapGL( }, [plannedEntryIds, setMarkerStyle]) // The plan's places and the stays (JRN-04), the GL twin of the Leaflet layer: - // regrouped on every move, between the photos and the entry pins. + // regrouped on zoom, between the photos and the entry pins. useEffect(() => { const map = mapRef.current if (!map) return @@ -608,7 +609,7 @@ function JourneyMapGL( } catch { /* empty bounds */ } } } - for (const hit of clusterPoints(marks, (lat, lng) => map.project([lng, lat]), MARK_CLUSTER_PX)) { + for (const hit of clusterPoints(marks, glWorldPx(map), MARK_CLUSTER_PX)) { const look = markHitSvg(hit.members) const el = document.createElement('div') el.style.cssText = `width:${look.width}px;height:${look.height}px;cursor:pointer` diff --git a/client/src/components/Journey/journeyLeafletLayers.ts b/client/src/components/Journey/journeyLeafletLayers.ts index 42827a263..511b720e2 100644 --- a/client/src/components/Journey/journeyLeafletLayers.ts +++ b/client/src/components/Journey/journeyLeafletLayers.ts @@ -1,6 +1,6 @@ import L from 'leaflet'; import type { JourneyMapLayerVisibility } from './journeyMapLayers'; -import { MARK_CLUSTER_PX, clusterPoints, markHitSvg, type JourneyMapMark } from './journeyMapMarks'; +import { MARK_CLUSTER_PX, clusterPoints, markHitSvg, type JourneyMapMark, type WorldPx } from './journeyMapMarks'; /** The layers of the Leaflet journey map a switch takes off and puts back (JRN-04). */ export type LeafletLayerKind = keyof JourneyMapLayerVisibility; @@ -29,6 +29,15 @@ export function switchLeafletLayers( return next; } +/** + * World pixels at the map's current zoom, through the map's own CRS so a + * GCJ-02 basemap groups where it draws. Only meaningful once the map has a view. + */ +export function leafletWorldPx(map: L.Map): WorldPx { + const zoom = map.getZoom(); + return (lat, lng) => map.project([lat, lng], zoom); +} + /** * The plan's places and the stays as one layer: below the entry pins, above * the photo thumbnails, joined only where they would overlap on screen. @@ -47,7 +56,7 @@ export function drawLeafletMarks( return null; } const group = L.layerGroup(); - for (const hit of clusterPoints(marks, (lat, lng) => map.latLngToContainerPoint([lat, lng]), MARK_CLUSTER_PX)) { + for (const hit of clusterPoints(marks, leafletWorldPx(map), MARK_CLUSTER_PX)) { const look = markHitSvg(hit.members); const marker = L.marker([hit.lat, hit.lng], { icon: L.divIcon({ diff --git a/client/src/components/Journey/journeyMapMarks.test.ts b/client/src/components/Journey/journeyMapMarks.test.ts index fd9b8c04e..b086be11b 100644 --- a/client/src/components/Journey/journeyMapMarks.test.ts +++ b/client/src/components/Journey/journeyMapMarks.test.ts @@ -1,4 +1,4 @@ -// FE-JOURNEY-MAPMARKS-001 to FE-JOURNEY-MAPMARKS-003 +// FE-JOURNEY-MAPMARKS-001 to FE-JOURNEY-MAPMARKS-005 // // JRN-04: the plan and stay marks both renderers draw, and how they join. import type { JourneyMapPlace, JourneyMapStay } from '@trek/shared'; @@ -6,7 +6,9 @@ import { describe, expect, it } from 'vitest'; import { MARK_CLUSTER_PX, clusterPoints, + glWorldPx, markHitSvg, + mercatorWorldPx, planPinSvg, safeColor, type JourneyMapMark, @@ -54,4 +56,39 @@ describe('journey map marks', () => { expect(safeColor(null)).toBe('#64748b'); expect(planPinSvg('red" onload="x')).not.toContain('onload'); }); + + it('FE-JOURNEY-MAPMARKS-004: world pixels are Web Mercator at the zoom, doubling per step and clamped at the poles', () => { + expect(mercatorWorldPx(0, 0, 0, 512)).toEqual({ x: 256, y: 256 }); + expect(mercatorWorldPx(0, -180, 0, 512).x).toBe(0); + const z5 = mercatorWorldPx(35.01, 135.68, 5, 512); + const z6 = mercatorWorldPx(35.01, 135.68, 6, 512); + expect(z6.x).toBeCloseTo(z5.x * 2, 6); + expect(z6.y).toBeCloseTo(z5.y * 2, 6); + expect(mercatorWorldPx(90, 0, 0, 512).y).toBeCloseTo(0, 3); + expect(mercatorWorldPx(-90, 0, 0, 512).y).toBeCloseTo(512, 3); + }); + + it('FE-JOURNEY-MAPMARKS-005: groups on the GL world grid hold however the view is panned, and split on zoom (#2889)', () => { + const marks = [planMark('a', 35.0), planMark('b', 35.0005), planMark('c', 35.001)]; + const keys = (hits: { members: JourneyMapMark[] }[]) => hits.map((h) => h.members.map((m) => m.key)); + + // What a container point does: the same world position, less the pan. + const onScreen = (pan: number) => + keys( + clusterPoints( + marks, + (lat, lng) => { + const world = mercatorWorldPx(lat, lng, 13, 512); + return { x: world.x, y: world.y - pan }; + }, + MARK_CLUSTER_PX + ) + ); + const screenGroupings = new Set(Array.from({ length: MARK_CLUSTER_PX }, (_, pan) => JSON.stringify(onScreen(pan)))); + expect(screenGroupings.size).toBeGreaterThan(1); + + const atZoom = (zoom: number) => keys(clusterPoints(marks, glWorldPx({ getZoom: () => zoom }), MARK_CLUSTER_PX)); + expect(atZoom(13)).toEqual([['a', 'b', 'c']]); + expect(atZoom(18)).toEqual([['a'], ['b'], ['c']]); + }); }); diff --git a/client/src/components/Journey/journeyMapMarks.ts b/client/src/components/Journey/journeyMapMarks.ts index 360bdb225..e36f5ee6e 100644 --- a/client/src/components/Journey/journeyMapMarks.ts +++ b/client/src/components/Journey/journeyMapMarks.ts @@ -15,7 +15,7 @@ export type JourneyMapMark = export const PLAN_FALLBACK_COLOR = '#64748b'; /** - * Marks closer than this on screen are one hit: the reader could not pick one + * Marks in one cell of this size are one hit: the reader could not pick one * of them anyway. Tighter than the photo grid, since a mark is a pin and not a * thumbnail, so marks a street apart stay apart. */ @@ -27,19 +27,49 @@ const STAY_DOT = 14; const MARK_CLUSTER_SIZE = 28; /** - * Grid clustering in screen space, shared by both renderers: points whose - * projected position falls in the same `px` cell become one, anchored on the - * first so the pin points where that member is. `project` is the engine's own - * lat/lng to container-pixel conversion. + * A position in world pixels: the whole map laid out at the current zoom, with + * the origin at its north-west corner. Unlike a container point it does not + * move when the map is panned, rotated or tilted. + */ +export type WorldPx = (lat: number, lng: number) => { x: number; y: number }; + +const MAX_MERCATOR_LAT = 85.0511287798; + +/** + * Web Mercator world pixels at `zoom` for a tile of `tileSize` pixels. Both GL + * engines lay out their world on 512 px tiles; Leaflet projects through its + * own CRS instead, so a GCJ-02 basemap keeps its offset. + */ +export function mercatorWorldPx(lat: number, lng: number, zoom: number, tileSize: number): { x: number; y: number } { + const size = tileSize * 2 ** zoom; + const phi = (Math.max(-MAX_MERCATOR_LAT, Math.min(MAX_MERCATOR_LAT, lat)) * Math.PI) / 180; + return { + x: ((lng + 180) / 360) * size, + y: ((1 - Math.log(Math.tan(phi) + 1 / Math.cos(phi)) / Math.PI) / 2) * size, + }; +} + +/** World pixels at a GL map's current zoom, free of its centre, bearing and pitch. */ +export function glWorldPx(map: { getZoom(): number }): WorldPx { + const zoom = map.getZoom(); + return (lat, lng) => mercatorWorldPx(lat, lng, zoom, 512); +} + +/** + * Grid clustering shared by both renderers: points whose world position falls + * in the same `px` cell become one, anchored on the first so the pin points + * where that member is. The grid is laid over the world and not over the + * viewport, so panning keeps every group as it is and only a zoom regroups + * (#2889). */ export function clusterPoints( points: readonly T[], - project: (lat: number, lng: number) => { x: number; y: number }, + toWorldPx: WorldPx, px: number ): { lat: number; lng: number; members: T[] }[] { const buckets = new Map(); for (const point of points) { - const pt = project(point.lat, point.lng); + const pt = toWorldPx(point.lat, point.lng); const key = `${Math.round(pt.x / px)}:${Math.round(pt.y / px)}`; const list = buckets.get(key); if (list) list.push(point); diff --git a/client/src/components/Journey/journeyPhotoLayer.test.ts b/client/src/components/Journey/journeyPhotoLayer.test.ts index 1980cacf9..f2aaa69e6 100644 --- a/client/src/components/Journey/journeyPhotoLayer.test.ts +++ b/client/src/components/Journey/journeyPhotoLayer.test.ts @@ -3,7 +3,7 @@ import { describe, expect, it } from 'vitest' import { clusterPhotos, photoMarkerHtml } from './journeyPhotoLayer' describe('journey photo layer (#2453)', () => { - it('FE-JOURNEY-PHOTOLAYER-001: buckets by screen position, anchors on the first photo, badges a count', () => { + it('FE-JOURNEY-PHOTOLAYER-001: buckets by world position, anchors on the first photo, badges a count', () => { const photos = [ { id: '1', lat: 1, lng: 1, thumbUrl: '/a.jpg' }, { id: '2', lat: 1.2, lng: 1.2, thumbUrl: '/b.jpg' }, diff --git a/client/src/components/Journey/journeyPhotoLayer.ts b/client/src/components/Journey/journeyPhotoLayer.ts index 4cab72e14..e6281e0e3 100644 --- a/client/src/components/Journey/journeyPhotoLayer.ts +++ b/client/src/components/Journey/journeyPhotoLayer.ts @@ -1,4 +1,4 @@ -import { clusterPoints } from './journeyMapMarks' +import { clusterPoints, type WorldPx } from './journeyMapMarks' /** A photo that knows where it was taken (#1614). */ export interface MapPhoto { @@ -9,24 +9,23 @@ export interface MapPhoto { } /** - * Grid clustering in screen space, shared by the Leaflet and the GL journey map. + * Grid clustering of the photos, shared by the Leaflet and the GL journey map. * * The Journey maps have never had clustering, and the library the planner uses * hangs off react-leaflet while these maps drive their engines directly. - * Bucketing by rounded pixel position is a few lines, is deterministic, and is - * enough for the job: photos of one place collapse into one thumbnail with a - * count, and pulling the map apart separates them again. `project` is the - * engine's own lat/lng to container-pixel conversion. + * Bucketing by rounded world-pixel position is a few lines, is deterministic, + * and is enough for the job: photos of one place collapse into one thumbnail + * with a count, zooming in separates them again, and panning leaves them be. */ export const PHOTO_CLUSTER_PX = 64 export function clusterPhotos( photos: MapPhoto[], - project: (lat: number, lng: number) => { x: number; y: number }, + toWorldPx: WorldPx, ): { lat: number; lng: number; members: MapPhoto[] }[] { // Anchored on the first member rather than the centroid: the thumbnail shown // is that photo's, so the pin should point where that picture was taken. - return clusterPoints(photos, project, PHOTO_CLUSTER_PX) + return clusterPoints(photos, toWorldPx, PHOTO_CLUSTER_PX) } export function photoMarkerHtml(thumbUrl: string, count: number): string {