Skip to content

XPM: two display paths still substitute a face for the frame foreground (follow-up from #555) #556

Open

Description

@eval-exec

Found in the review of #555 (the fix for #550). Two paths that can reach an XPM decode still hand the decoder a face where GNU reads the frame's foreground, plus two smaller notes from the same review.

1. The (space :width (image …)) size path

crates/neomacs-layout-engine/src/display_pixel_calc.rs:178-180 passes inputs.default_fg as the frame foreground, with a comment ("the face stands in for the frame foreground it cannot name"). That lookup is not inert: a catalog miss schedules a decode, which then carries the substituted value.

2. The margin and display-property replacement entry points

crates/neomacs-layout-engine/src/display_source_resolver.rs:574 and :619 pass self.params.default_fg. That value comes from resolve_buffer_default_face -> resolve_remapped_default_face (crates/neomacs-layout-engine/src/neovm_bridge.rs:4613-4624), i.e. it is remap-aware. GNU's FRAME_FOREGROUND_PIXEL is the frame's foreground-color parameter (src/frame.h:1375), which face-remapping-alist does not touch, so a buffer that remaps default still diverges from GNU on an unresolvable color key. Untested.

The end state #550 asked for, applied to every path that can schedule a decode: the un-folded frame foreground plumbed to the decoder.

Verification: extend the xpm_named_colors GUI fixture with a remapped default face (and a size-only (space :width …) operand) and require the readback to match GNU's frame foreground, as the #550 decoys already do.

3. Smaller notes from the same review

  • #[serde(default)] frame_foreground on ImageColorContext deserializes an absent field to black (ImageRgb::default() is 0x000000, which that type's own doc calls "a real color instead of an accidental unspecified sentinel") rather than mirroring foreground, as from_pixels documents. Latent: no production serde consumer today.
  • crates/neomacs-infra/src/pinned.rs duplicates neomacs-test-fonts' private downloader (same lock, same 120/15/60s timeouts, same size cap, same verify-before-write and .partial+rename). Migrating the fonts crate onto pinned_file would leave one implementation.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions