Georef output contract: alpha band for grayscale, srcnodata as mask fallback (#56)

CLAUDE.md

@@ -43,6 +43,10 @@ the cap 0.05, and the two readings of the sweep that were wrong first.
 `data-raw/mask_calibrate-border_threshold.R` reproduces everything from a directory of
 thumbnails, and `inst/extdata/mask_border_sweep.csv` ships the result so the test suite
 recomputes both constants rather than trusting them
+- `data-raw/mask_measure-interior_zeros.R` — the two measurements fly#56 took before changing
+the output contract: true black the old grayscale `-dstnodata 0` deleted, and how often
+`fly_mask_one()` declines. PSOCK over a thumbnail directory; refuses to report if any frame
+errored, because a closure missing on the workers once made every frame "error" and print zeros
 - `data-raw/height_calibrate-flying_height_slip.R` — pulls every catalogue centroid by year
 (1.67 million, cached under the gitignored `data-raw/.cache/`), samples MRDEM under the 7,156
 frames that decide the `flying_height` constants, and prints a producer line for every figure
@@ -235,22 +239,31 @@ of 264 frames** had under a tenth of theirs masked. `fly_mask()` replaces it wit
 seeded from the image border, so interior dark water survives — on bcb90128_213 a plain
 threshold takes 7.7% and this takes 2.5%, and the missing 5% is a lake.
 
-  **Output band counts do not change**, because `-srcalpha` excludes the alpha band from the
-warped band list. That is what let masking default to on without moving `stac_airphoto_bc`.
-Grayscale keeps `-dstnodata 0`, the weaker contract — fly#56. Do not re-derive the collar's
-shape from geometry or re-propose a circle; read `inst/notes/border-masking.md`
-
-  **That band-count claim is platform-conditional and was written as if it were not**
-(fly#68, 2026-09-21). It was measured on macOS; the three-platform CI added in fly#52
-found on its first run that **Windows yields 2 bands for a masked grayscale frame**
-against 1 with masking off. ubuntu and macOS give 1; RGB is 4 everywhere. So the stated
-reason masking could default to on — that `stac_airphoto_bc` need not move — does not
-hold for grayscale produced on Windows. A second symptom from the same root, on the same
-runner: GDAL reports *"Value 0 in the source dataset has been changed to 1 ... to avoid
-being treated as NoData"*, so genuine zeros are silently shifted in the warped output.
-`test-fly_georef_mask.R` **pins** the observed Windows value rather than skipping it, so
-it reddens if that platform moves in either direction — including the direction where
-fly#68 is fixed. Do not re-state the invariant unconditionally while fly#56 is open (it absorbed fly#68)
+  Masking does not change the band count, because `-srcalpha` excludes the alpha band from
+the warped band list. Do not re-derive the collar's shape from geometry or re-propose a
+circle; read `inst/notes/border-masking.md`
+
+- **Every georef output carries one alpha band, and `srcnodata` backs a declined mask**
+(v0.19.0, #56, absorbing #68 and #69) — grayscale warps with `-dstalpha` like RGB, so it
+is **2 bands (Gray + Alpha, no NoData), RGB 4** — measured on macOS / sf's GDAL 3.8.5,
+and `test-fly_georef_mask.R` asserts it unconditionally so the three-platform CI decides
+the rest; do not restate it as cross-platform fact from one machine, which is #68's error. v0.11.0 kept grayscale on
+`-dstnodata 0` so `stac_airphoto_bc` need not move, and that contract failed twice: GDAL
+**rewrote** (silently on sf's 3.8.5) genuine black as 1 to dodge the nodata value — measured on the output,
+**161 of 182** calibration grayscale frames warped axis-aligned (up to 3.6%) and 41 at a
+30-degree bearing (up to 0.26%); an early draft measured 30 degrees alone and quoted it as
+the loss, which is wrong for every bearingless thumbnail (isotropic pixels; an anisotropic
+scan resamples even axis-aligned) — and it was not one number, since the
+Windows runner gave 2 bands masked (#68). A UInt16 output's alpha is 0/65535, not 0/255.
+
+  `fly_georef()` **accepts** `mask = "border"` with `srcnodata`: `fly_georef_warp_opts()`
+is an `else if`, so `-srcnodata` reaches only a frame whose mask declined and never sits
+beside `-srcalpha`, which would delete the interior black the mask kept. The v0.11.0
+refusal rested on the premise that both reached GDAL; they never did. Keep the `else if`
+— it is now the whole of the safety. A decline with no `srcnodata` warns, because that
+collar is written as data. The mask declined **0 of 10,105** public thumbnails, so the
+fallback's reach is non-Byte scans nothing public can fixture. Read
+`inst/notes/border-masking.md`
 
 - **`flying_height` is held against `scale x focal_length` before the DEM route believes it,
 and the one identifiable error is repaired** (v0.12.0, #54) — the catalogue's `FLYING_HEIGHT`

NEWS.md

@@ -1,5 +1,8 @@
 # fly (development version)
 
+- **Breaking: grayscale georeferenced output is now 2 bands, Gray + Alpha, where it was 1 band with `NoData = 0`** ([#56](https://github.com/NewGraphEnvironment/fly/issues/56)). `fly_georef()` warps every frame with `-dstalpha`, so RGB stays 4 bands and, from a grayscale or RGB source, no output carries a NoData value. A consumer that reads band 1 alone, or treats 0 as fill, must now read the last band: 0 is fill, and on these outputs the opaque value is the datatype's maximum (255 on Byte, 65535 on UInt16). Under `-dstnodata 0`, GDAL rewrote genuine black inside the frame as 1 so it would not read as fill (or deleted it where `srcnodata = "0"` was also given). Measured on the output of the 182 grayscale frames the mask was calibrated on, it hit **161 frames** warped axis-aligned (every frame with no flight bearing): typically a few dozen pixels, up to 3.6% of a frame on roll bcb94081 (1250 x 1250 thumbnails on square footprints; an anisotropic scan resamples even when axis-aligned). On a footprint rotated 30 degrees it hit 41 frames, at most 0.26%, because resampling mixes a lone 0 with its neighbours. The Windows CI runner also gave a masked grayscale frame 2 bands where ubuntu and macOS gave 1 ([#68](https://github.com/NewGraphEnvironment/fly/issues/68)). Band count is now one number, measured on macOS with sf's GDAL 3.8.5 and asserted by the test suite on all three CI platforms. This supersedes 0.11.0's "nothing downstream changes band count". `stac_airphoto_bc` needs a COG-writer change and a republish of grayscale frames ([stac_airphoto_bc#36](https://github.com/NewGraphEnvironment/stac_airphoto_bc/issues/36)).
+- **`fly_georef(mask = "border", srcnodata = ...)` is now accepted, and `srcnodata` is the fallback for frames whose mask declined** ([#69](https://github.com/NewGraphEnvironment/fly/issues/69), absorbed into #56). The warp options were always either/or, so the 0.11.0 refusal guarded a combination GDAL never received and only made the fallback unreachable. `srcnodata` reaches a frame only when `fly_mask()` declined it, and never alongside a mask that ran. A frame whose mask declines with no `srcnodata` is now warned about, since its collar reaches the output as image data. The mask declined none of 10,105 public thumbnails, so on thumbnails this changes nothing; it matters for non-8-bit scans. `data-raw/mask_measure-interior_zeros.R` reproduces both figures.
+
 ## 0.18.0 (2026-09-28)
 
 - **Half the r ≈ 2 mass was a wrong scale, not a mislabelled lens, and it is now drawn at its true width** ([#72](https://github.com/NewGraphEnvironment/fly/issues/72)). v0.12.0 said the film frames around twice their nominal scale were a 305 mm lens catalogued as 153, so falling back to nominal scale was right. The flight logbooks and the spacing between adjacent frames, read per roll-height over all 252 sampled frames beyond the band, split it: **24 roll-heights (120 sampled frames; their keys reach at most 3,227 catalogue frames, fewer once terrain brings some inside the band, where they were already sized from their height) carry the height the crew flew beside a `scale` recorded too small** — mostly 1972–76 rolls at half the true denominator — and ship in `flying_height_rolls.csv` at factor 1, `scale_wrong`, under a new `tail` value, `"near_upper"`. The fallback drew them at 1/r of their width, half at r = 2; they are now `"corrected_roll_table"` and sized from their height. On 21 roll-heights (82 frames) the logbook writes a 12" lens, which confirms the lens reading there, and those stay on nominal scale, as do the rest; each is listed in `flying_height_rolls_excluded.csv` with its reason.

R/fly_georef.R

@@ -15,17 +15,17 @@
 #'   exist.
 #' @param overwrite If `FALSE` (default), skip files that already exist.
 #' @param mask How to handle the black collar around the exposed frame. `"border"`
-#'   (default) masks it with [fly_mask()]; `"none"` reproduces the pre-0.11.0 warp
-#'   exactly, including its `srcnodata` handling.
+#'   (default) masks it with [fly_mask()]; `"none"` warps unmasked, applying `srcnodata`
+#'   to every frame as before v0.11.0. Output carries an alpha band either way.
 #' @param mask_threshold How close to black a pixel must be to count as collar. Passed
 #'   to [fly_mask()]; the default is measured, see there.
-#' @param srcnodata Source nodata value passed to GDAL warp, matched **exactly**. Now
-#'   defaults to `NULL`, and is an error alongside `mask = "border"` — the two are
-#'   different answers to one question and combining them silently undoes the mask (see
-#'   **Nodata handling**). It reached `"0"` before v0.11.0, which was close to useless:
-#'   scanned black runs 3-12, so it masked a median of 0.16% of a frame against the 3.11%
-#'   actually there, and 128 of 264 measured frames had under a tenth of their collar
-#'   removed.
+#' @param srcnodata Source nodata value passed to GDAL warp, matched **exactly**, or
+#'   `NULL` (default). With `mask = "none"` it applies to every frame. With
+#'   `mask = "border"` it is the **fallback**: it applies only to a frame whose mask
+#'   [fly_mask()] declined, and never alongside a mask that ran (see **Nodata handling**).
+#'   On its own it is close to useless as a collar mask: scanned black runs 3-12, so
+#'   `"0"` masked a median of 0.16% of a frame against the 3.11% actually there, and 128
+#'   of 264 measured frames had under a tenth of their collar removed.
 #' @param dem Optional elevation raster passed to [fly_footprint()], sizing each
 #'   frame from its height above ground instead of the reported scale. See the
 #'   **Terrain** section of [fly_footprint()].
@@ -127,8 +127,13 @@
 #'
 #' **Nodata handling:** two sources of unwanted black pixels, handled separately.
 #'
-#' 1. **Warp fill** — GDAL creates black pixels outside the rotated source frame. RGB
-#'    images get an alpha band (`-dstalpha`); grayscale use `dstnodata=0`. Unchanged.
+#' 1. **Warp fill** — GDAL creates pixels outside the rotated source frame. Every output
+#'    carries an alpha band (`-dstalpha`) marking them, so **grayscale is written as 2
+#'    bands and RGB as 4** for a Byte grayscale or RGB source. Grayscale used
+#'    `-dstnodata 0` before v0.19.0, and GDAL kept a genuine 0 inside the frame from
+#'    reading as fill by rewriting it as 1 — silently on sf's GDAL 3.8.5 — or, where
+#'    `srcnodata = "0"` was also given, deleted it. A value cannot be both content and
+#'    the fill marker.
 #' 2. **The frame collar** — film holder edges, fiducial marks and chamfered corners.
 #'    Since v0.11.0 this is [fly_mask()]: a flood fill seeded from the image border, so
 #'    only darkness *reachable from the edge* is masked and interior dark water survives.
@@ -140,12 +145,24 @@
 #' present, so it did not mask the borders — and the real black it was said to cost is a
 #' median 0.16% of the frame, which *is* the collar rather than shadow.
 #'
-#' **`srcnodata` and `mask = "border"` are mutually exclusive, and GDAL will not say so.**
-#' All combinations of `-srcalpha` and `-srcnodata` run clean and return the expected band
-#' count. What they do is delete the interior black the mask exists to keep: on a synthetic
-#' frame carrying an 11x11 block of true black, adding `-srcnodata "0 0 0"` to the masked
-#' warp removed exactly those 121 pixels. So `fly_georef()` raises the error GDAL does not.
-#' Pass `mask = "none"` to get the old behaviour, `srcnodata` and all.
+#' **`srcnodata` with `mask = "border"` is a per-frame fallback, never a second mask.**
+#' [fly_mask()] declines some frames — among them an unreadable or non-8-bit source, and a
+#' mask that floods into the interior; `fly_mask()`'s `reason` column lists every path —
+#' and a declined frame is warped unmasked. Given no
+#' `srcnodata`, its collar is written as opaque data. Given one, the declined frame is
+#' warped with `-srcnodata` and every masked frame with `-srcalpha` alone. The two are
+#' never handed to GDAL together, because GDAL would apply both and the mask would lose:
+#' on a synthetic frame carrying an 11x11 block of true black, adding `-srcnodata "0 0 0"`
+#' to the masked warp removed exactly those 121 pixels. Before v0.19.0 this pair was
+#' refused on the premise that both reached GDAL; they never did.
+#'
+#' "Declined" means [fly_mask()] returned `masked = FALSE`; an error inside it fails the
+#' frame instead. A declined frame given no `srcnodata` is warned about, since its collar
+#' reaches the output as data. The fallback is a weak last resort: it matches **exact**
+#' values only, so it catches a collar written at exactly 0 and misses one scanned at
+#' 3-12, and on the frame it reaches it also removes any true black inside the frame. On
+#' all 10,105 public thumbnails measured (the 264 `mask_threshold` was calibrated on among
+#' them), the mask declined none.
 #'
 #' **Accuracy:** footprints assume a nadir camera angle, and without `dem`
 #' they also assume flat terrain. Passing `dem` sizes each frame from its
@@ -179,15 +196,9 @@ fly_georef <- function(fetch_result, photos_sf,
   }
 
   mask <- match.arg(mask)
-  # Refused rather than silently dropped. GDAL accepts both together and then deletes the
-  # interior black the mask exists to preserve, reporting nothing - so a caller who set
-  # `srcnodata` deliberately must be told their instruction and the mask disagree, not
-  # have one of them quietly win.
-  if (identical(mask, "border") && !is.null(srcnodata)) {
-    stop("`srcnodata` and `mask = \"border\"` are two answers to one question and GDAL ",
-         "applies both: the mask keeps interior black and `srcnodata` then deletes it, ",
-         "silently. Drop `srcnodata`, or pass `mask = \"none\"` to use it.", call. = FALSE)
-  }
+  # `srcnodata` alongside `mask = "border"` is the fallback for frames whose mask declines,
+  # not a second mask: `fly_georef_warp_opts()` emits one or the other per frame, never
+  # both. Refused before v0.19.0 on the mistaken premise that both reached GDAL (fly#69).
   if (identical(mask, "border")) fly_check_threshold(mask_threshold, "mask_threshold")
 
   auto_rotation <- identical(rotation, "auto")
@@ -476,6 +487,14 @@ georef_one <- function(src, fp, out_file, srcnodata = NULL, rotation = 180,
     if (isTRUE(res$masked) && file.exists(mask_file)) {
       warp_src <- mask_file
       masked <- TRUE
+    } else if (is.null(srcnodata)) {
+      # The worst of the four mask x srcnodata outcomes (fly#69): no mask and no fallback,
+      # so the collar is warped in as opaque data. Not every decline path warns inside
+      # `fly_mask_one()` — a non-Byte source returns quietly — so it is named here.
+      warning(basename(src), ": the frame-border mask declined (", res$reason, ") and ",
+              "no `srcnodata` was given, so the collar is written as image data. Pass ",
+              "`srcnodata` as a fallback for such frames. ",
+              "See `inst/notes/border-masking.md`.", call. = FALSE)
     }
   }
 
@@ -507,15 +526,17 @@ georef_one <- function(src, fp, out_file, srcnodata = NULL, rotation = 180,
 #' [fly_georef_gcps()] is — it is a thing that can be wrong while everything around it
 #' looks healthy, and an option vector is checkable offline where a warped GeoTIFF is not.
 #'
-#' Three rules, and the first is the one that matters:
+#' Three rules:
 #'
 #' * a masked source is read through `-srcalpha`, which forces the LAST band to be the
-#'   alpha band and excludes it from the warped band list. That is what keeps the output
-#'   band count identical to a pre-0.11.0 run — grayscale 1 band, RGB 4.
-#' * `-srcnodata` is emitted only when there is no mask. The two together are refused at
-#'   the `fly_georef()` boundary; this function must not paper over a combination that
-#'   reaches it, so it simply never emits both.
-#' * warp fill is unchanged: `-dstalpha` for RGB, `-dstnodata 0` for grayscale.
+#'   alpha band and excludes it from the warped band list, so masking does not change the
+#'   output band count.
+#' * `-srcnodata` is emitted only when there is no mask. This `else if` is what makes
+#'   `srcnodata` a safe fallback under `mask = "border"`: GDAL given both applies both,
+#'   and `-srcnodata` then deletes the interior black the mask kept.
+#' * warp fill is always `-dstalpha`, so grayscale is 2 bands and RGB 4 (fly#56). Nothing
+#'   is marked by value on the output. A genuine 0 still becomes transparent on the
+#'   `-srcnodata` leg, which reads it as source nodata; that is the fallback's known cost.
 #'
 #' @param n_bands Band count of the source, before any alpha band is appended.
 #' @param srcnodata The `srcnodata` argument, or `NULL`.
@@ -533,7 +554,7 @@ fly_georef_warp_opts <- function(n_bands, srcnodata, masked) {
     opts <- c(opts, "-srcnodata", src_val)
   }
 
-  if (is_rgb) c(opts, "-dstalpha") else c(opts, "-dstnodata", "0")
+  c(opts, "-dstalpha")
 }
 
 #' Convert flight bearing to GCP rotation

data-raw/mask_calibrate-border_threshold.R

@@ -279,8 +279,9 @@ for (src in reps) {
   has_gcp <- any(grepl("GCP", readLines(v, warn = FALSE)))
 
   o <- file.path(work, "chk_warp.tif")
-  opts <- c("-t_srs", "EPSG:3005", "-r", "bilinear", "-srcalpha")
-  opts <- if (nb0 >= 3) c(opts, "-dstalpha") else c(opts, "-dstnodata", "0")
+  # The package's own option vector, not a copy of it: this line hard-coded the pre-fly#56
+  # grayscale fill (`-dstnodata 0`) and would have kept printing the old band count.
+  opts <- fly_georef_warp_opts(nb0, NULL, masked = TRUE)
   sf::gdal_utils("warp", source = v, destination = o, options = opts)
 
   cat(sprintf("src bands %d -> nearblack %d [%s] -> vrt gcps %s -> warp %d bands\n",