Share the GridMsg-to-renderer sync from felis-client via felis-client-core #442

Closed

opened 2026-09-25 12:07:55 +09:00 by natsukium · 1 comment

natsukium commented

2026-09-25 12:07:55 +09:00

Owner

Motivation

felis-client keeps two things in sync after each ShadowScreen::apply: the shadow screen, and renderer-side state that the shadow does not carry. ShadowScreen treats ThemeColor, PaletteColor and PaletteResetAll as no-ops (crates/felis-client-core/src/shadow.rs:355-359). felis-client then maps messages to renderer calls itself (crates/felis-client/src/event_handler.rs:405-445):

ThemeColor → apply_theme_override
PaletteColor → apply_palette_override
PaletteResetAll → reset_palette_overrides
RehydrateBegin → reset_session_colors
ModeFlags.reverse_video → set_reverse_video

Every other renderer consumer has to copy this mapping. The planned felis-reel (headless .fcast → video, which depends on the offscreen renderer issue) is the first such consumer. A copy silently goes stale when felis adds a message that affects the renderer but not the shadow. When that happens, a recording that changes colors through OSC 4/10/11 renders in the base theme, and nothing reports an error.

Proposal

Move the mapping into felis-client-core as a shared function, for example fn sync_renderer(msg: &GridMsg, sink: &mut impl RendererSink), where RendererSink is a small trait covering the override setters above. felis-client and felis-reel would both call it, so a new message would need handling in only one place.

Open question

felis-client-core has no renderer dependency, so the trait would decouple it from felis-render-wgpu. An alternative is to implement the trait in felis-render-wgpu behind a feature.

## Motivation felis-client keeps two things in sync after each `ShadowScreen::apply`: the shadow screen, and renderer-side state that the shadow does not carry. `ShadowScreen` treats `ThemeColor`, `PaletteColor` and `PaletteResetAll` as no-ops (`crates/felis-client-core/src/shadow.rs:355-359`). felis-client then maps messages to renderer calls itself (`crates/felis-client/src/event_handler.rs:405-445`): - `ThemeColor` → `apply_theme_override` - `PaletteColor` → `apply_palette_override` - `PaletteResetAll` → `reset_palette_overrides` - `RehydrateBegin` → `reset_session_colors` - `ModeFlags.reverse_video` → `set_reverse_video` Every other renderer consumer has to copy this mapping. The planned felis-reel (headless `.fcast` → video, which depends on the offscreen renderer issue) is the first such consumer. A copy silently goes stale when felis adds a message that affects the renderer but not the shadow. When that happens, a recording that changes colors through OSC 4/10/11 renders in the base theme, and nothing reports an error. ## Proposal Move the mapping into felis-client-core as a shared function, for example `fn sync_renderer(msg: &GridMsg, sink: &mut impl RendererSink)`, where `RendererSink` is a small trait covering the override setters above. felis-client and felis-reel would both call it, so a new message would need handling in only one place. ## Open question felis-client-core has no renderer dependency, so the trait would decouple it from felis-render-wgpu. An alternative is to implement the trait in felis-render-wgpu behind a feature.

natsukium commented

2026-09-25 21:39:16 +09:00

Author

Owner

This does not block the felis-reel PoC: the PoC copies the five arms from felis-client/src/event_handler.rs and this lands afterwards.

The drift has already happened

felis-web-component (src/lib.rs, pinned to an older felis) carries its own copy that handles only ThemeColor, in the older {channel, rgb} shape. PaletteColor, PaletteResetAll, RehydrateBegin and reverse video are dropped there without an error. felis-reel would be the third copy.

Design: return data instead of a trait

A RendererSink trait in felis-client-core cannot be implemented for felis_render_wgpu::Renderer from felis-client (orphan rule). The workarounds are a newtype in every consumer, or a new felis-render-wgpu → felis-client-core dependency edge. A pure function needs neither and also works from wasm:

pub enum RendererEffect {
    ThemeOverride { channel: ThemeChannel, rgb: Option<(u8, u8, u8)> },
    PaletteOverride { index: u8, rgb: Option<(u8, u8, u8)> },
    ResetPalette,
    ResetSessionColors,
    ReverseVideo(bool),
}

pub fn renderer_effect(msg: &GridMsg) -> Option<RendererEffect>;

What it guarantees, and what it does not

If renderer_effect matches GridMsg without a _ arm and consumers match RendererEffect exhaustively, a new message variant or a new effect breaks the build instead of rendering silently wrong. It does not catch a new renderer-affecting field inside an arm matched with .. (for example ModeFlags). The PR therefore adds:

a test that pins the effect for each renderer-affecting message, and
a test that felis-client routes through renderer_effect rather than its own mapping.

This does not block the felis-reel PoC: the PoC copies the five arms from `felis-client/src/event_handler.rs` and this lands afterwards. ## The drift has already happened felis-web-component (`src/lib.rs`, pinned to an older felis) carries its own copy that handles only `ThemeColor`, in the older `{channel, rgb}` shape. `PaletteColor`, `PaletteResetAll`, `RehydrateBegin` and reverse video are dropped there without an error. felis-reel would be the third copy. ## Design: return data instead of a trait A `RendererSink` trait in felis-client-core cannot be implemented for `felis_render_wgpu::Renderer` from felis-client (orphan rule). The workarounds are a newtype in every consumer, or a new felis-render-wgpu → felis-client-core dependency edge. A pure function needs neither and also works from wasm: ```rust pub enum RendererEffect { ThemeOverride { channel: ThemeChannel, rgb: Option<(u8, u8, u8)> }, PaletteOverride { index: u8, rgb: Option<(u8, u8, u8)> }, ResetPalette, ResetSessionColors, ReverseVideo(bool), } pub fn renderer_effect(msg: &GridMsg) -> Option<RendererEffect>; ``` ## What it guarantees, and what it does not If `renderer_effect` matches `GridMsg` without a `_` arm and consumers match `RendererEffect` exhaustively, a new message variant or a new effect breaks the build instead of rendering silently wrong. It does **not** catch a new renderer-affecting field inside an arm matched with `..` (for example `ModeFlags`). The PR therefore adds: - a test that pins the effect for each renderer-affecting message, and - a test that felis-client routes through `renderer_effect` rather than its own mapping.

natsukium referenced this issue from natsukium/felis-reel

2026-09-26 00:13:32 +09:00

feat: render .fcast recordings through felis's offscreen renderer #2

natsukium referenced this issue from a pull request that will close it,

2026-09-26 01:13:44 +09:00

client-core: map each grid message to its renderer effect #468

natsukium closed this issue

2026-09-26 01:25:47 +09:00

Commenting is not possible because the repository is archived.