Skip to content

fix(print-layout): improve atlas feature framing #1783

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
giswqs merged 5 commits into from
Aug 8, 2026
Merged

fix(print-layout): improve atlas feature framing #1783

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
71aa3d9
fix(print-layout): improve atlas feature framing
giswqs Aug 8, 2026
dea711d
fix(print-layout): clear stale atlas masks
giswqs Aug 8, 2026
1a7b448
fix(print-layout): harden atlas export state
giswqs Aug 8, 2026
60cdc8a
fix(print-layout): finalize atlas review feedback
giswqs Aug 8, 2026
91e72bd
fix(print-layout): preserve CSS pixel atlas bounds
giswqs Aug 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
141 changes: 122 additions & 19 deletions apps/geolibre-desktop/src/components/layout/PrintLayoutDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import {
import {
computeScaleRatio,
drawLayout,
mapBodyAspectRatio,
PAPER_SIZES,
resolvePageSize,
type BodyCorner,
Expand All @@ -58,6 +59,7 @@ import {
DEFAULT_TABLE_ROWS,
layerRows,
MAX_TABLE_ROWS,
rowForAtlasFeature,
rowsWithinBounds,
type ChartBlockType,
} from "../../lib/print-data-blocks";
Expand Down Expand Up @@ -91,9 +93,11 @@ import {
} from "../../lib/print-layout-export";
import {
atlasEntryName,
atlasViewportFrame,
buildAtlasPages,
buildLineAtlasPages,
collectAtlasFeatures,
geometryBounds,
hasLineGeometry,
MAX_LINE_ATLAS_PAGES,
expandBounds,
Expand All @@ -106,6 +110,7 @@ import {
type AtlasPage,
type AtlasTokenContext,
} from "../../lib/print-atlas";
import { clearAtlasFeatureMask, showAtlasFeatureMask } from "../../lib/print-atlas-mask";

interface PrintLayoutDialogProps {
open: boolean;
Expand Down Expand Up @@ -287,6 +292,7 @@ export function PrintLayoutDialog({
const [tableMaxRows, setTableMaxRows] = useState(DEFAULT_TABLE_ROWS);
const [tablePosition, setTablePosition] = useState<BodyCorner>("bottom-left");
const [tableFilterToPage, setTableFilterToPage] = useState(true);
const [tableFilterToAtlasFeature, setTableFilterToAtlasFeature] = useState(false);
const [showDataChart, setShowDataChart] = useState(false);
const [chartLayerId, setChartLayerId] = useState("");
const [chartTitle, setChartTitle] = useState("");
Expand Down Expand Up @@ -318,6 +324,7 @@ export function PrintLayoutDialog({
const [atlasNameField, setAtlasNameField] = useState("");
const [atlasExtentMode, setAtlasExtentMode] = useState<"margin" | "scale">("margin");
const [atlasMarginPct, setAtlasMarginPct] = useState(10);
const [atlasMaskEnabled, setAtlasMaskEnabled] = useState(false);
const [atlasScale, setAtlasScale] = useState("50000");
const [atlasSortField, setAtlasSortField] = useState("");
const [atlasSortDescending, setAtlasSortDescending] = useState(false);
Expand Down Expand Up @@ -629,7 +636,10 @@ export function PrintLayoutDialog({
if (!atlasActiveRef.current) recapture();
} else if (!open && wasOpenRef.current && !drawingRef.current) {
// Closing for good (not to draw): take the extent box off the map.
if (map) clearPrintExtent(map);
if (map) {
clearPrintExtent(map);
clearAtlasFeatureMask(map);
}
}
wasOpenRef.current = open;
}, [open, projectName, recapture, mapControllerRef, extentBbox]);
Expand Down Expand Up @@ -657,6 +667,7 @@ export function PrintLayoutDialog({
idleRecaptureRef.current = null;
}
clearPrintExtent(map);
clearAtlasFeatureMask(map);
}
},
[mapControllerRef],
Expand Down Expand Up @@ -812,6 +823,17 @@ export function PrintLayoutDialog({
() => atlasLayers.find((l) => l.id === atlasLayerId) ?? null,
[atlasLayers, atlasLayerId],
);
const atlasMaskAvailable = useMemo(
() =>
atlasCoverage === "features" &&
Boolean(
atlasLayer?.geojson?.features.some(
(feature) =>
feature.geometry?.type === "Polygon" || feature.geometry?.type === "MultiPolygon",
),
),
[atlasCoverage, atlasLayer],
);
// The per-vertex geometry walk runs once per coverage layer; sort/filter
// edits below only re-iterate these lightweight per-feature records.
const atlasFeatureInfos = useMemo(
Expand Down Expand Up @@ -881,6 +903,14 @@ export function PrintLayoutDialog({
const currentAtlasPage = atlasEnabled ? (atlasPages[clampedAtlasIndex] ?? null) : null;
const atlasActive = atlasEnabled && atlasPageCount > 0;
atlasActiveRef.current = atlasActive;
// The mask is a temporary live-map layer. Remove it immediately when the
// option, atlas, or dialog is turned off instead of waiting for another
// camera drive that may never happen.
useEffect(() => {
if (open && atlasActive && atlasMaskEnabled && atlasMaskAvailable) return;
const map = mapControllerRef.current?.getMap();
if (map) clearAtlasFeatureMask(map);
}, [open, atlasActive, atlasMaskEnabled, atlasMaskAvailable, mapControllerRef]);
const atlasFilterValid = atlasFilterPredicate !== null;
const atlasScaleValid = atlasExtentMode !== "scale" || Number(atlasScale) > 0;
// A floor (not just > 0) keeps a mistyped tiny length from cutting a long
Expand Down Expand Up @@ -921,6 +951,9 @@ export function PrintLayoutDialog({
() => atlasLayers.find((l) => l.id === chartLayerId) ?? null,
[atlasLayers, chartLayerId],
);
const tableUsesAtlasLayer = Boolean(
atlasEnabled && atlasLayer && tableLayer?.id === atlasLayer.id,
);
const tableFields = useMemo(
() => (tableLayer?.geojson ? listAtlasFields(tableLayer.geojson.features) : []),
[tableLayer],
Expand Down Expand Up @@ -1093,14 +1126,19 @@ export function PrintLayoutDialog({
const displayTableRows = useMemo(
() =>
showDataTable
? rowsForBlock(tableFeatureInfos, tableAllRows, tableFilterToPage, displayFilterBounds)
? tableFilterToAtlasFeature && tableUsesAtlasLayer && currentAtlasPage
? rowForAtlasFeature(tableAllRows, currentAtlasPage.sourceIndex)
: rowsForBlock(tableFeatureInfos, tableAllRows, tableFilterToPage, displayFilterBounds)

@github-actions github-actions Bot Aug 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Edge case: when tableFilterToAtlasFeature is checked but currentAtlasPage is momentarily null (e.g. the atlas filter currently yields 0 pages, or a newly selected page hasn't finished driving), this silently falls back to rowsForBlock(...) (page-extent/full-layer rows) instead of showing an empty table. That contradicts the hint text ("This takes priority over the page extent filter") and could surprise a user who expects "only the current feature" to mean "none" rather than "whatever the page-extent filter would show."

Same pattern applies to the export path at line ~1709 (tableFilterToAtlasFeature && tableUsesAtlasLayer ? rowForAtlasFeature(...) : rowsForBlock(...)), though there pages[i] is always defined so it's less reachable in practice.

Confidence: medium — this is a real behavioral gap, though it only surfaces in the narrow window where the atlas is enabled with a matching table layer but has no current page.

: [],
[
showDataTable,
rowsForBlock,
tableFeatureInfos,
tableAllRows,
tableFilterToPage,
tableFilterToAtlasFeature,
tableUsesAtlasLayer,
currentAtlasPage,
displayFilterBounds,
],
);
Expand Down Expand Up @@ -1163,25 +1201,49 @@ export function PrintLayoutDialog({
// Drive the live map to one atlas page's extent and capture it. Margin mode
// grows the feature's box before fitting; fixed-scale mode fits first, then
// corrects the zoom by the log2 ratio difference (like applyScale) and
// recaptures. Returns the capture plus the map's final visible bounds, so
// the data blocks can filter to what the page actually shows.
// recaptures. Returns the capture plus the print frame's final visible
// bounds, so data blocks exclude the part of the live map that cover-crop
// removes from the page.
const captureAtlasPage = useCallback(
async (page: AtlasPage): Promise<{ cap: CapturedMap; viewBounds: AtlasBounds }> => {
const map = mapControllerRef.current?.getMap();
if (!map) throw new Error("Map is not ready");
const ctx: AtlasTokenContext = {
name: page.name,
pageNumber: page.index + 1,
total: atlasPageCount,
properties: page.properties,
};
const pageOptions: LayoutOptions = {
...options,
title: substituteAtlasTokens(options.title, ctx),
subtitle: substituteAtlasTokens(options.subtitle, ctx),
footerText: substituteAtlasTokens(options.footerText, ctx),
};
const containMap = Boolean(map.getLayer(GRATICULE_LABEL_LAYER_ID));
const canvas = map.getCanvas();
const viewportWidth = canvas.clientWidth || canvas.width;
const viewportHeight = canvas.clientHeight || canvas.height;

@github-actions github-actions Bot Aug 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

canvas.clientWidth/clientHeight are CSS pixels, but the || canvas.width / || canvas.height fallback (used when the client size reads 0, e.g. a not-yet-laid-out or hidden canvas) returns device pixels (canvas.width = dispW * dpr per line ~1608). atlasViewportFrame's result feeds both fitBounds's padding and the map.unproject([x, y]) calls a few lines below (line ~1307), both of which expect CSS-pixel screen coordinates. If the fallback path is ever hit on a HiDPI display, the computed padding/crop rectangle will be off by the device pixel ratio, skewing the atlas fit and the frameBounds used to filter data blocks.

Confidence: low-to-medium — this only triggers when clientWidth/clientHeight are 0 while an atlas capture runs, which should be rare since the map must be visible to drive the camera, but the fallback silently produces wrong units rather than failing loudly.

const targetAspect = containMap
? viewportWidth / Math.max(1, viewportHeight)
: mapBodyAspectRatio(pageOptions);
const viewportFrame = atlasViewportFrame(viewportWidth, viewportHeight, targetAspect);
const coverageFeature = atlasLayer?.geojson?.features[page.sourceIndex];
if (atlasMaskEnabled) showAtlasFeatureMask(map, coverageFeature);
else clearAtlasFeatureMask(map);
const [w, s, e, n] = expandBounds(page.bounds, atlasFitMarginPct);
map.fitBounds(
[
[w, s],
[e, n],
],
{ animate: false, padding: 0 },
{ animate: false, padding: viewportFrame.padding },
);
await waitForAtlasSettle(map);
// Mirror recapture: an active graticule draws coordinate labels at the
// map edges, so fit with "contain" to keep them un-cropped on every
// atlas page (mapFit is persistent state, so it must be set here too).
setMapFit(map.getLayer(GRATICULE_LABEL_LAYER_ID) ? "contain" : "cover");
setMapFit(containMap ? "contain" : "cover");
// Hide the drawn print-extent box while reading the buffer, as recapture
// does, so its outline is never baked into a page.
const capture = () => {
Expand All @@ -1199,17 +1261,8 @@ export function PrintLayoutDialog({
// a title/footer made purely of tokens can resolve to empty for a
// given feature, which collapses that row and changes the body height
// the scale is computed from.
const ctx: AtlasTokenContext = {
name: page.name,
pageNumber: page.index + 1,
total: atlasPageCount,
properties: page.properties,
};
const ratio = computeScaleRatio({
...options,
title: substituteAtlasTokens(options.title, ctx),
subtitle: substituteAtlasTokens(options.subtitle, ctx),
footerText: substituteAtlasTokens(options.footerText, ctx),
...pageOptions,
metersPerPixel: cap.metersPerPixel,
mapPixelRatio: cap.pixelRatio,
bearingDeg: cap.bearingDeg,
Expand All @@ -1235,10 +1288,24 @@ export function PrintLayoutDialog({
} else {
setAtlasScaleNotice(null);
}
const frameBounds = containMap
? null
: geometryBounds({
type: "MultiPoint",
coordinates: [
[viewportFrame.crop.left, viewportFrame.crop.top],
[viewportFrame.crop.right, viewportFrame.crop.top],
[viewportFrame.crop.right, viewportFrame.crop.bottom],
[viewportFrame.crop.left, viewportFrame.crop.bottom],
].map(([x, y]) => {
const point = map.unproject([x, y]);
return [point.lng, point.lat];
}),
});
const b = map.getBounds();
return {
cap,
viewBounds: [b.getWest(), b.getSouth(), b.getEast(), b.getNorth()],
viewBounds: frameBounds ?? [b.getWest(), b.getSouth(), b.getEast(), b.getNorth()],
};
},
[
Expand All @@ -1247,6 +1314,8 @@ export function PrintLayoutDialog({
atlasFitMarginPct,
atlasScale,
atlasPageCount,
atlasLayer,
atlasMaskEnabled,
waitForAtlasSettle,
options,
t,
Expand Down Expand Up @@ -1340,6 +1409,7 @@ export function PrintLayoutDialog({
atlasExtentMode,
atlasMarginPct,
atlasScale,
atlasMaskEnabled,
// Along-a-line coverage: a new segment length can keep the same page
// count (sourceIndex signature unchanged) while every extent moved.
atlasCoverage,
Expand Down Expand Up @@ -1635,7 +1705,9 @@ export function PrintLayoutDialog({
// Each page's table/chart re-filters to the extent the page's
// capture actually shows (not just the nominal feature bounds).
...buildBlocksFromRows(
rowsForBlock(tableFeatureInfos, tableAllRows, tableFilterToPage, viewBounds),
tableFilterToAtlasFeature && tableUsesAtlasLayer
? rowForAtlasFeature(tableAllRows, pages[i].sourceIndex)
: rowsForBlock(tableFeatureInfos, tableAllRows, tableFilterToPage, viewBounds),
rowsForBlock(chartFeatureInfos, chartAllRows, chartFilterToPage, viewBounds),
),
title: substituteAtlasTokens(options.title, ctx),
Expand Down Expand Up @@ -2191,6 +2263,20 @@ export function PrintLayoutDialog({
)}
</div>
)}
{atlasMaskAvailable && (
<div className="space-y-1.5">
<ToggleField
id="atlas-mask-outside"
label={t("printLayout.atlas.maskOutside")}
checked={atlasMaskEnabled}
disabled={atlasBusy}
onChange={setAtlasMaskEnabled}
/>
<p className="text-xs text-muted-foreground">
{t("printLayout.atlas.maskOutsideHint")}
</p>
</div>
)}
{/* Along-a-line pages follow the line's own chainage,
so ordering controls only apply per-feature mode. */}
{atlasCoverage === "features" && (
Expand Down Expand Up @@ -2705,11 +2791,26 @@ export function PrintLayoutDialog({
id="dt-filter-page"
label={t("printLayout.dataBlocks.filterToPage")}
checked={tableFilterToPage}
disabled={tableFilterToAtlasFeature && tableUsesAtlasLayer}
onChange={setTableFilterToPage}
/>
<p className="text-xs text-muted-foreground">
{t("printLayout.dataBlocks.filterToPageHint")}
</p>
{atlasEnabled && (
<>
<ToggleField
id="dt-filter-atlas-feature"
label={t("printLayout.dataBlocks.filterToAtlasFeature")}
checked={tableFilterToAtlasFeature}
disabled={!tableUsesAtlasLayer}
onChange={setTableFilterToAtlasFeature}
/>
<p className="text-xs text-muted-foreground">
{t("printLayout.dataBlocks.filterToAtlasFeatureHint")}
</p>
</>
)}
{!displayDataBlocks.dataTable && (
<p className="text-xs text-muted-foreground">
{t("printLayout.dataTable.noRows")}
Expand Down Expand Up @@ -3028,7 +3129,9 @@ export function PrintLayoutDialog({
// encodeURIComponent, which leaves ( ) unescaped, and an
// unquoted ) (common in SVG: translate(), rgba(), url(#id))
// would prematurely close the CSS url() token.
style={{ backgroundImage: `url("${svgSrc}")` }}
style={{
backgroundImage: `url("${svgSrc}")`,
}}
/>
);
}
Expand Down
6 changes: 5 additions & 1 deletion apps/geolibre-desktop/src/i18n/locales/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1323,7 +1323,9 @@
"titleLabel": "Heading",
"position": "Position",
"filterToPage": "Only features in the page extent",
"filterToPageHint": "Applies to atlas pages and a drawn print extent; otherwise the whole layer is used."
"filterToPageHint": "Applies to atlas pages and a drawn print extent; otherwise the whole layer is used.",
"filterToAtlasFeature": "Only the current atlas feature",
"filterToAtlasFeatureHint": "Available when the table uses the atlas coverage layer. This takes priority over the page extent filter."
},
"dataTable": {
"columns": "Columns",
Expand Down Expand Up @@ -1453,6 +1455,8 @@
"extentMargin": "Margin around feature",
"extentScale": "Fixed scale",
"marginLabel": "Margin (%)",
"maskOutside": "Mask area outside current feature",
"maskOutsideHint": "Applies a translucent inverted fill to emphasize the current atlas feature.",
"scaleLabel": "Scale",
"scaleRequired": "Enter a scale greater than zero.",
"sortField": "Sort by",
Expand Down
6 changes: 5 additions & 1 deletion apps/geolibre-desktop/src/i18n/locales/fr.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1315,7 +1315,9 @@
"titleLabel": "En-tête",
"position": "Position",
"filterToPage": "Seulement les entités dans l'emprise de la page",
"filterToPageHint": "S'applique aux pages d'atlas et à une emprise d'impression dessinée ; sinon, la couche entière est utilisée."
"filterToPageHint": "S'applique aux pages d'atlas et à une emprise d'impression dessinée ; sinon, la couche entière est utilisée.",
"filterToAtlasFeature": "Seulement l’entité courante de l’atlas",
"filterToAtlasFeatureHint": "Disponible lorsque la table utilise la couche de couverture de l’atlas. Ce filtre est prioritaire sur l’emprise de la page."
Comment on lines +1319 to +1320

@github-actions github-actions Bot Aug 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor style nit: these two new strings use a curly apostrophe (’) in "l’entité"/"l’atlas", while the rest of this file consistently uses a straight apostrophe ('), e.g. filterToPageHint right above uses "S'applique", "dataChart.noNumericFields" uses "n'a aucun", etc. Same for atlas.maskOutside/maskOutsideHint below. Worth normalizing for consistency with the rest of the catalog.

Confidence: low — purely cosmetic, doesn't affect functionality.

},
"dataTable": {
"columns": "Colonnes",
Expand Down Expand Up @@ -1445,6 +1447,8 @@
"extentMargin": "Marge autour de l'entité",
"extentScale": "Échelle fixe",
"marginLabel": "Marge (%)",
"maskOutside": "Masquer la zone hors de l’entité courante",
"maskOutsideHint": "Applique un remplissage inversé translucide pour mettre en évidence l’entité courante de l’atlas.",
"scaleLabel": "Échelle",
"scaleRequired": "Entrez une échelle supérieure à zéro.",
"sortField": "Trier par",
Expand Down
Loading
Loading