diff --git a/EXPLORER_STATE.md b/EXPLORER_STATE.md index 874d66a..c1cb930 100644 --- a/EXPLORER_STATE.md +++ b/EXPLORER_STATE.md @@ -175,6 +175,7 @@ viewer [creates Cesium viewer; reads readHash() once] | `facetFilters` | `:979` | `phase1`, `db` | `#materialFilterBody`, `#contextFilterBody`, `#objectTypeFilterBody`; facet count text | — | — | | `tableView` | `:1071` | `facetFilters` | `#tableContainer`, `#samplesTable`, `tr.selected` class | prev/next; max input; **change** on all four facet bodies; table-row clicks | `replaceState` via `buildHash` from table-row click (sets `#pid` directly, mirrors sample-mode globe click) | | `zoomWatcher` | `:1246` | `phase1`, `facetFilters`, `db` | facet count text; stats; phase msg; sample card; samples list | source filter `change`; material/context/object_type `change`; `camera.changed`; `camera.moveEnd` (sub-threshold pan settle, #205); `window` `hashchange`; share button; search button; in-map search input keydown; sidebar search input `input` (mirror) and keydown (world-scope submit) | `pushState` and `replaceState` via `buildHash` (camera changed/moveEnd, mode flip, sample fly, **share button**); `replaceState` via `writeQueryState` (filter changes, search submit) | +| `tableView` (post table-v2 viewport coupling) | — | `facetFilters`, `viewer` | also listens to `viewer.camera.moveEnd` to re-scope table to viewport bbox | — | — | | `perfPanel` | `:1910` | `phase1` | `#perfPanel` floating div | close button | — | Note that **two cells register `change` listeners on the four facet container @@ -515,6 +516,57 @@ where a failed page left old DOM visible but pageRowsByPid empty. to the current page only — sufficient since only the visible page has clickable rows. +**Viewport coupling** (PR #219, added shortly after table v2): both +the page and count queries now include +`viewerBboxSQL('latitude', 'longitude', 0.3)` which expands to +`AND latitude BETWEEN AND AND ` (with +dateline-crossing handling, including the post-padding wrap case +where a non-wrapping rect spills past ±180 after the 30% expansion). + +**Known pre-existing wrap bug NOT fixed in this PR.** +`loadViewportSamples()` in `zoomWatcher` (the point-mode sample +loader) has its own padding logic that does *not* split the +longitude predicate when the padded rectangle wraps the +antimeridian — `bounds.east - bounds.west` is meaningless for a +wrapping viewport, and the resulting single `BETWEEN` clause can +return zero matches. This PR's table queries now route through +`viewerBboxSQL()` and split the wrapped predicate correctly, so the +**table is fine** at the dateline; the bug is now only in the +point-mode count (phase-msg "Samples in View"). At wrapping +viewports the two surfaces will therefore diverge: table shows the +correct row set, phase-msg can read zero or undercount. Scoped out +as a follow-up because the user's primary complaint (table=6M vs +phase-msg=153 at Crete) is unrelated to the dateline. Right fix is +to share the `viewerBboxSQL`-style normalization with point-mode. +The 0.3 padding factor matches the 30% padding the point-mode +sample loader applies, so the table row count agrees with the +"Samples in View" stat box / phase-msg count on **fresh** point-mode +loads. Caveat: when the user pans within the point-mode cached +bounds, `loadViewportSamples()` short-circuits and reuses +`cachedTotalCount`, while the table re-queries the current padded +bbox on every `moveEnd` — so cache-hit pans can show a small +divergence (e.g. table=147, phase-msg=153 from the prior fetch). +This is a known minor follow-up; the primary bug (table=6M while +phase-msg=153) is fully resolved. `doSearch('area')` calls the same +helper without a padFactor (= undefined, no padding), since search +has always been documented as "samples within the current map view." + +The table re-fires on `viewer.camera.moveEnd`, so panning/zooming +the globe re-scopes the table. **Null-bbox handling**: if the camera +can't produce a view rectangle (off-globe; rare), `viewerBboxSQL()` +returns `null`. The table treats this as a status state ("No globe +area in view; pan or zoom the globe to see samples.") rather than +falling back to a no-bbox query — that's the bug shape PR #219 was +filed against. Area search keeps its existing world fallback. + +**Boot ordering**: the viewer cell's `once()` postRender handler now +sets `viewer._initialCameraApplied = true` after `setView`. The +table defers its first refresh until either (a) the flag is true at +cell-run time, or (b) the `camera.moveEnd` listener fires (which it +will, once `setView` lands). This avoids the brief flash of +world-view rows at boot when the URL hash specifies a deep-zoom +camera. + --- ## 7. Facet-count contract diff --git a/explorer.qmd b/explorer.qmd index db1be7d..05e4bdf 100644 --- a/explorer.qmd +++ b/explorer.qmd @@ -742,6 +742,70 @@ function facetFilterSQL() { return ` AND pid IN (SELECT DISTINCT pid FROM read_parquet('${facets_url}') WHERE ${conds.join(' AND ')})`; } +// Build a viewport-bbox SQL predicate for the given lat/lng column names, +// using the viewer's current view rectangle. Returns null when the camera +// can't produce a rectangle (off-globe; rare). Callers must decide what +// to do with null — fall back to world (search) vs. show empty state +// (table). NEVER fall back to no-bbox-filter for surfaces that are +// labeled "in view" — that recreates the bug shape PR #219 was filed for. +// +// padFactor expands the rectangle in each direction by N × span. This +// matches the 30% padding the point-mode sample loader applies (so the +// table's "samples in view" count agrees with the phase-msg count). +// Pass 0 for exact-viewport semantics (area search uses this). +// +// Dateline crossing: when west > east the rectangle wraps the antimeridian; +// emit two longitude ranges OR'd together. +// +// Used by: +// - tableView (PR #219): scopes the samples table to the current viewport +// so "samples in view" in cluster/point mode == table row count. +// - doSearch('area') (#178 light path): exact-viewport text search. +function viewerBboxSQL(latCol, lngCol, padFactor) { + if (typeof viewer === 'undefined') return null; + const rect = viewer.camera.computeViewRectangle(viewer.scene.globe.ellipsoid); + if (!rect) return null; + let south = Cesium.Math.toDegrees(rect.south); + let north = Cesium.Math.toDegrees(rect.north); + let west = Cesium.Math.toDegrees(rect.west); + let east = Cesium.Math.toDegrees(rect.east); + if (padFactor && padFactor > 0) { + const latPad = (north - south) * padFactor; + // Original longitude span. Wraps the antimeridian iff west > east. + const originalSpan = (west > east) ? (180 - west) + (east + 180) : east - west; + const lngPad = originalSpan * padFactor; + south -= latPad; + north += latPad; + west -= lngPad; + east += lngPad; + // Clamp latitude to valid degrees so the SQL doesn't BETWEEN past + // the poles (no data there but it reads odd). + if (south < -90) south = -90; + if (north > 90) north = 90; + // Normalize longitude after padding. Three cases: + // - Total span >= 360: padding consumed the globe; emit a full + // longitude predicate (BETWEEN -180 AND 180, no wrap). + // - west pushed below -180 OR east above 180: rotate the + // endpoint by 360 so it lands in [-180, 180]. The wrap-detect + // below picks up the new wrap state. This is the case Codex + // PR #219 round 2 caught: a non-wrapping rect like + // west=170, east=179 padded by 30% becomes (167.3, 181.7); + // east must wrap to -178.3 and the predicate must split into + // two ranges. + const totalSpan = originalSpan + 2 * lngPad; + if (totalSpan >= 360) { + west = -180; east = 180; + } else { + if (west < -180) west += 360; + if (east > 180) east -= 360; + } + } + const lngClause = (west > east) + ? `(${lngCol} BETWEEN ${west} AND 180 OR ${lngCol} BETWEEN -180 AND ${east})` + : `${lngCol} BETWEEN ${west} AND ${east}`; + return ` AND ${latCol} BETWEEN ${south} AND ${north} AND ${lngClause}`; +} + // === Cross-filter facet count UI helpers === function applyFacetCounts(facetKey, countsMap) { const baseline = (viewer && viewer._baselineCounts) ? viewer._baselineCounts[facetKey] : null; @@ -1017,6 +1081,11 @@ viewer = { } else { v.camera.setView({ destination: globalRect }); } + // Signal to dependent cells (e.g. tableView) that the URL-hydrated + // camera position has been applied. Anything that snapshots the + // viewer bbox at boot should wait for this rather than running + // against the default-constructed camera. + v._initialCameraApplied = true; v.scene.postRender.removeEventListener(once); }; v.scene.postRender.addEventListener(once); @@ -1504,7 +1573,7 @@ tableView = { } } - async function loadCount(genAtStart) { + async function loadCount(genAtStart, bboxSQL) { try { // pid IS NOT NULL mirrors the page query so the total counts only // rows that are actually pageable. Without this, totalRows could @@ -1516,6 +1585,7 @@ tableView = { WHERE pid IS NOT NULL ${sourceFilterSQL('source')} ${facetFilterSQL()} + ${bboxSQL} `); if (genAtStart !== pageGen) return true; // DuckDB-WASM returns BigInt for COUNT(*); coerce safely. @@ -1532,7 +1602,7 @@ tableView = { } } - async function loadPage(p, genAtStart) { + async function loadPage(p, genAtStart, bboxSQL) { try { const offset = p * TABLE_PAGE_SIZE; // AND pid IS NOT NULL: belt-and-suspenders. pid is the primary @@ -1546,6 +1616,7 @@ tableView = { WHERE pid IS NOT NULL ${sourceFilterSQL('source')} ${facetFilterSQL()} + ${bboxSQL} ORDER BY pid LIMIT ${TABLE_PAGE_SIZE} OFFSET ${offset} `); @@ -1575,15 +1646,38 @@ tableView = { return `${total} sample${totalRows === 1 ? '' : 's'} match the current filters.`; } + // 0.3 = match the 30% padding the point-mode sample loader uses, so + // the table's row count agrees with the "Samples in View" stat box + // and phase-msg count. + const BBOX_PAD_FACTOR = 0.3; + async function refreshAll() { const gen = ++pageGen; currentPage = 0; totalRows = null; + // Snapshot the viewport bbox at refresh-start time so the count and + // page queries see the same predicate. The user could pan during + // the async window; pageGen invalidates the in-flight result, and + // moveEnd will fire a fresh refreshAll() against the new bbox. + const bboxSQL = viewerBboxSQL('latitude', 'longitude', BBOX_PAD_FACTOR); + if (bboxSQL === null) { + // No view rectangle (rare; off-globe). Don't fall back to a + // no-bbox query — that's the bug shape that motivated PR #219. + // Clear the table and surface a status message. + pageRows = []; + pageRowsByPid = new Map(); + totalRows = 0; + lastPageFailed = false; + renderTable(); + setLoading(false); + setMeta('No globe area in view; pan or zoom the globe to see samples.', true); + return; + } setLoading(true); - setMetaLoading('Loading samples matching filters...'); + setMetaLoading('Loading samples in view...'); // Stale-while-loading: leave existing pageRows rendered (dimmed by // .is-loading) so the table doesn't blink to empty. - const [countOk, pageOk] = await Promise.all([loadCount(gen), loadPage(0, gen)]); + const [countOk, pageOk] = await Promise.all([loadCount(gen, bboxSQL), loadPage(0, gen, bboxSQL)]); if (gen !== pageGen) return; setLoading(false); if (pageOk && countOk) { @@ -1600,9 +1694,18 @@ tableView = { async function refreshPage(newPage) { const gen = ++pageGen; + // Page navigation uses the CURRENT viewport bbox. Counting was + // already done against this bbox (refreshAll caches totalRows for + // the filter+bbox combo until the next refreshAll). + const bboxSQL = viewerBboxSQL('latitude', 'longitude', BBOX_PAD_FACTOR); + if (bboxSQL === null) { + // User somehow paged while the camera went off-globe; refresh + // re-scopes properly. + return refreshAll(); + } setLoading(true); setMetaLoading(`Loading page ${newPage + 1}...`); - const ok = await loadPage(newPage, gen); + const ok = await loadPage(newPage, gen, bboxSQL); if (gen !== pageGen) return; setLoading(false); if (ok) { @@ -1627,8 +1730,35 @@ tableView = { document.getElementById('contextFilterBody')?.addEventListener('change', refreshAll); document.getElementById('objectTypeFilterBody')?.addEventListener('change', refreshAll); + // Re-scope the table to the viewport on every camera settle. moveEnd + // fires once per user gesture (after the pan/zoom ends), so this is + // already debounced by user input. pageGen invalidates any racing + // in-flight query. This listener also covers the boot case: when the + // viewer's once() handler does the URL-hydrated setView, moveEnd + // fires once and the initial table query runs against the correct + // bbox. + if (typeof viewer !== 'undefined') { + viewer.camera.moveEnd.addEventListener(() => { + refreshAll(); + }); + } + window.refreshSamplesTable = refreshAll; - refreshAll(); + // Boot: defer the initial refresh until the URL-hydrated camera has + // been applied (otherwise the first query runs against the + // default-constructed camera, briefly showing the wrong table). + // viewer.camera.moveEnd will fire when once() does setView and trigger + // the refresh via the listener above. If the camera is already + // settled (e.g. fast-path re-runs of this cell), refresh immediately. + if (typeof viewer !== 'undefined' && viewer._initialCameraApplied) { + refreshAll(); + } else { + // Show the loading state so the user sees the spinner immediately + // rather than a stale "Loading samples matching the current + // filters..." default-meta line. + setLoading(true); + setMetaLoading('Loading samples in view...'); + } return "active"; } @@ -2721,8 +2851,8 @@ zoomWatcher = { // the viewport predicate; THEN top-50. let results; if (effectiveScope === 'area') { - const rect = viewer.camera.computeViewRectangle(viewer.scene.globe.ellipsoid); - if (!rect) { + const bboxSQL = viewerBboxSQL('l.latitude', 'l.longitude'); + if (!bboxSQL) { // Camera couldn't produce a view rectangle (shouldn't // happen in practice; defensive). Fall through to the // world query so the user gets results, with a console @@ -2730,21 +2860,13 @@ zoomWatcher = { console.warn('Area scope requested but no view rectangle; falling back to world.'); results = await runWorldQuery(); } else { - const south = Cesium.Math.toDegrees(rect.south); - const north = Cesium.Math.toDegrees(rect.north); - const west = Cesium.Math.toDegrees(rect.west); - const east = Cesium.Math.toDegrees(rect.east); - const lngClause = (west > east) - ? `(l.longitude BETWEEN ${west} AND 180 OR l.longitude BETWEEN -180 AND ${east})` - : `l.longitude BETWEEN ${west} AND ${east}`; results = await db.query(` SELECT f.pid, f.label, f.source, l.latitude, l.longitude, f.place_name, (${score}) AS relevance_score FROM read_parquet('${facets_url}') f INNER JOIN read_parquet('${lite_url}') l USING (pid) WHERE ${searchWhere} - AND l.latitude BETWEEN ${south} AND ${north} - AND ${lngClause} + ${bboxSQL} ${sourceFilterSQL('f.source')} ${facetFilterSQL()} ORDER BY relevance_score DESC, f.label diff --git a/tests/playwright/explorer-map-overlay.spec.js b/tests/playwright/explorer-map-overlay.spec.js index e67feca..0b7dc13 100644 --- a/tests/playwright/explorer-map-overlay.spec.js +++ b/tests/playwright/explorer-map-overlay.spec.js @@ -150,6 +150,57 @@ test.describe('Map search overlay — Cesium toolbar coexistence (#200 / M-1A)', } }); + test('table is bbox-scoped: deep-zoom URL yields far fewer rows than world zoom', async ({ page }) => { + await page.setViewportSize({ width: 1280, height: 900 }); + + // Load at world zoom and read total. + await page.goto(`${BASE_URL}${EXPLORER_PATH}#v=1&lat=20&lng=0&alt=10000000`, { + waitUntil: 'domcontentloaded', + timeout: 60000, + }); + await page.waitForSelector('#cesiumContainer', { timeout: 30000 }); + await waitForBootReady(page); + await expect(page.locator('#tablePageInfo')).toContainText(/Page 1 of \d+/, { timeout: 60000 }); + const worldText = await page.locator('#tablePageInfo').textContent(); + const worldTotal = parseInt((worldText.match(/of ([\d,]+)\)$/) || [, '0'])[1].replace(/,/g, ''), 10); + expect(worldTotal).toBeGreaterThan(100000); // world view → millions of samples + + // Fly to Crete-area deep zoom (matches the live-site URL the user reported). + // Use Promise.race-style: kick off the flight, then wait for the table's + // pager text to *change* from the world view total. Just polling + // aria-busy is racy because aria-busy is already false from the boot + // load, and may briefly stay false between flight start and moveEnd. + await page.evaluate(async () => { + const v = await window._ojs.ojsConnector.mainModule.value('viewer'); + v.scene.requestRenderMode = false; + v.camera.flyTo({ + destination: Cesium.Cartesian3.fromDegrees(25.5610, 35.0104, 21299), + duration: 1.0, + }); + }); + + // Wait for the pager total to change (different total ⇒ refresh against + // new bbox completed). Use the captured worldText as the not-this guard. + await expect.poll( + async () => await page.locator('#tablePageInfo').textContent(), + { timeout: 60000, intervals: [250, 500, 1000] } + ).not.toBe(worldText); + // And aria-busy should land at false after the page+count complete. + await expect(page.locator('#tableContainer')).toHaveAttribute('aria-busy', 'false', { timeout: 60000 }); + // Belt-and-suspenders: confirm pager actually contains "Page 1 of N (..)" + // before parsing — without this, a blank/error pager would parseInt to 0 + // and pass the < worldTotal assertion vacuously. + await expect(page.locator('#tablePageInfo')).toContainText(/Page 1 of \d+ \([\d,]+-[\d,]+ of [\d,]+\)/, { timeout: 30000 }); + const zoomedText = await page.locator('#tablePageInfo').textContent(); + const zoomedTotal = parseInt((zoomedText.match(/of ([\d,]+)\)$/) || [, '0'])[1].replace(/,/g, ''), 10); + expect(zoomedTotal).toBeGreaterThan(0); + + // The deep-zoom bbox should match far fewer rows than the world view — + // sanity threshold rather than an exact value (data can change). + expect(zoomedTotal).toBeLessThan(worldTotal); + expect(zoomedTotal).toBeLessThan(10000); + }); + test('table v2: pagination is server-side, pager shows Page X of Y, Next loads new rows', async ({ page }) => { await page.setViewportSize({ width: 1280, height: 900 }); await page.goto(`${BASE_URL}${EXPLORER_PATH}#v=1&lat=20&lng=0&alt=10000000`, {