Skip to content

svg: rasterization moves below the exporter; the water preset declares its absorption - #108

Merged
krazyjakee merged 3 commits into
masterfrom
svg-below-export
Aug 8, 2026
Merged

svg: rasterization moves below the exporter; the water preset declares its absorption#108
krazyjakee merged 3 commits into
masterfrom
svg-below-export

Conversation

@krazyjakee

Copy link
Copy Markdown
Owner

Two changes an engine consuming the lowered SceneGraph needs, both of which
also remove a reach-around Studio was already doing.

Requested by a downstream consumer (moggy, krazyjakee/moggy#457), but neither
change is engine-specific — both are about the built-in surfaces being
reachable and self-describing on their own terms.

1. mogen-svg: the rasterizer moves below the exporter

Rasterization shipped as a pre-export pass (#106). That is still the right
shape for rewriting a SceneGraph's texture paths and layering bytes over a
TextureSource, and it is unchanged here. What was in the wrong place was the
renderer underneath it.

Every consumer holding a Material and a path sits beside the exporter, not
after it. svg.rs's own module docs already named the problem — "the consumer
this pass cannot reach" — and Studio's viewport solved it by reaching back into
mogen-export for is_svg / render_svg / resolve_svg_size. An engine that
reads the lowered SceneGraph and decodes textures itself cannot do even that
without linking the whole exporter, oxipng / fbxcel / meshopt included,
for three pure functions over (bytes, size, wrap).

So the policy moves into a new mogen-svg crate — mogen-core plus
resvg/usvg, both pure Rust — and the pass stays in mogen-export, which
re-exports the four public items. mogen_export::is_svg and friends still
resolve, so Studio needs no edit. textures-svg becomes dep:mogen-svg.

There is still exactly one implementation of "what pixels does this SVG
produce", which was the point of the original decision; it is simply reachable
now. Verified to cross-compile to wasm32-unknown-unknown, as the old module
claimed for itself.

2. The built-in water preset declares absorption

shader="water" was a bare on/off switch: the preset was seeded with an empty
param list, so an author could not say how absorbing the water is, and a
consumer reading the lowered Material found a shader name and nothing else.

It now declares absorption (float, default 1.5), resolved through the
ordinary ShaderDecl::resolve_param path — shader_params (absorption=2.5)
overrides it exactly as for a user-declared shader. The built-in stays the
first client of the general system rather than acquiring a special case, which
is the property #107 is heading toward anyway.

The default is deliberately not zero: zero is the "behaves exactly like clear
glass" case, so defaulting to it would make shader="water" a no-op that
reports success. The test asserts the override wins, that a silent material
gets the default, and — the control — that the default is not the disabled
value.

docs/dsl.md documents the parameter in both places water is described.

Verification

  • cargo test --locked -p mogen-svg -p mogen-export -p mogen-dsl -p mogen-core — 18 suites, all green (664 in mogen-dsl, 27 in svg_textures)
  • cargo check --workspace --all-targets — clean
  • cargo check -p mogen-svg --target wasm32-unknown-unknown — clean

Not run: the Studio GL paths, which need a display seat this machine may not
have. Studio is untouched by construction — it compiles against the same
re-exported names — but that is an argument, not an observed run.

🤖 Generated with Claude Code

krazyjakee and others added 3 commits August 8, 2026 17:52
… can reach it

Rasterization shipped as a pre-export pass, which is the right shape for
rewriting a SceneGraph's texture paths and is unchanged here. What was in the
wrong place was the renderer underneath it.

Every consumer holding a Material and a path sits *beside* the exporter rather
than after it. Studio's live viewport already reached back into mogen-export
for is_svg/render_svg/resolve_svg_size, and an engine that consumes the lowered
SceneGraph and decodes textures itself could not reach them at all without
linking the whole exporter — oxipng, fbxcel and meshopt included — for three
pure functions over `(bytes, size, wrap)`.

So the policy moves to a new `mogen-svg`: mogen-core plus resvg/usvg, both pure
Rust, and it cross-compiles to wasm32-unknown-unknown (checked). The pass stays
in mogen-export, which re-exports the four public items so `mogen_export::is_svg`
and friends keep resolving and Studio needs no edit. `textures-svg` becomes
`dep:mogen-svg`.

There is still exactly one implementation of "what pixels does this SVG
produce", which was the point of the original split; it is simply reachable now.

Verified: -p mogen-svg (12), -p mogen-export (5 svg unit + 27 svg_textures),
-p mogen-core, -p mogen-dsl (663), cargo check --workspace --all-targets, and
cargo check -p mogen-svg --target wasm32-unknown-unknown.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`shader="water"` was a bare on/off switch. The preset was seeded with an empty
param list, so an author had no way to say *how* absorbing the water is, and a
consumer reading the lowered Material had nothing to read — the shader name and
nothing else.

Declaring `absorption` (float, default 1.5) fixes both ends through the
ordinary machinery: `shader_params (absorption=2.5)` overrides it and
`ShaderDecl::resolve_param` resolves it, exactly as for a user-declared shader.
The built-in stays the first client of the general system rather than acquiring
a special case.

The default is deliberately not zero. Zero is the "behaves exactly like clear
glass" case, so defaulting to it would make `shader="water"` a no-op that
reports success. The test asserts the override wins, that a silent material
gets the default, and — the control — that the default is not the disabled
value.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The lowering-level test added in this PR only checks that
ShaderDecl::resolve_param resolves the right value on the SceneGraph;
nothing exercised the actual deliverable — a downstream engine reading
node.extras.shader.params off the exported GLB. Mirrors the existing
`declared_shader_params_merge_defaults_with_overrides` coverage for
user-declared shaders, but for the built-in water preset's default and
an authored override.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lbcid9DKhPVDy6eGeJi5na

Copy link
Copy Markdown
Owner Author

Reviewed this PR against code quality, file structure, test coverage, and a visual check of the rendering path it touches. One real gap found and fixed; everything else checks out.

Findings (ranked by severity)

1. Low — missing integration test for the new absorption param actually reaching the export (fixed)

The PR's new lowering test (crates/mogen-dsl/src/lower/tests/shader.rs) only checks that ShaderDecl::resolve_param resolves the right value on the SceneGraph. Nothing exercised the actual deliverable this feature is for — per the PR description, "a consumer reading the lowered Material" (i.e. a downstream engine) needs the value in the exported glTF's node.extras.shader.params, and there was no test pinning that, unlike the existing declared_shader_params_merge_defaults_with_overrides coverage for user-declared shaders.

Fixed in dccebe9: added builtin_water_shader_projects_absorption_default_and_override to crates/mogen-export/tests/shader.rs, asserting both the default (1.5) and an authored override (2.5) land in the exported extras. Also manually verified end-to-end through the real mogen CLI (release build, not just cargo test):

// node.extras for shader_params (absorption=2.5)
{ "shader": { "name": "water", "params": { "absorption": 2.5 } } }

2. Informational, no action needed — Studio's live preview doesn't consume the new param yet

crates/mogen-studio/src/viewer/flatten.rs:586 and user_shaders.rs:101 still hard-code the built-in water branch to shader id 1 with an empty param list, so shader_params (absorption=...) has no visible effect in Studio's GL viewport today. This is a pre-existing, explicitly tracked limitation (user_shaders.rs cites #107 by name), not a regression introduced by this PR — the data-model-level work here is a reasonable prerequisite for that follow-up. Flagging only so it's not mistaken for a bug if someone tries shader_params (absorption=…) in Studio and sees no change.

Verification performed

  • cargo check --workspace --all-targets --locked — clean.
  • cargo test --locked -p mogen-svg -p mogen-export -p mogen-dsl -p mogen-core — all green, including the new test.
  • Confirmed the mogen-export::svgmogen-svg crate split is behavior-preserving: built examples/features/svg_texture.mog through the actual release mogen binary and extracted the embedded rasterized textures from the produced GLB. dots.svg (wrap/tile mode) and emblem.svg (fit mode) both rasterize cleanly with correct colors and seamless wrap-around, no artifacts.
  • Confirmed feature wiring (textures-svg = ["textures", "dep:mogen-svg"]) and re-exports (mogen_export::{is_svg, render_svg, resolve_svg_size, MAX_SVG_SIZE}) are intact — mogen-studio/src/viewer/gl_util.rs compiles unchanged against them, matching the PR's "Studio needs no edit" claim.
  • No file touched by this PR exceeds 1000 lines; no split needed.

Nice PR — the crate split is clean and the doc comments carry real signal.


Generated by Claude Code

@krazyjakee
krazyjakee merged commit fce0908 into master Aug 8, 2026
@krazyjakee
krazyjakee deleted the svg-below-export branch August 8, 2026 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants