Skip to content

sync release font build parameters with spx - #330

Merged
joeykchen merged 1 commit into
spx4.4.1from
agent/sync-spx-font-build-params
Aug 7, 2026
Merged

sync release font build parameters with spx#330
joeykchen merged 1 commit into
spx4.4.1from
agent/sync-spx-font-build-params

Conversation

@joeykchen

Copy link
Copy Markdown

Summary

  • align Godot release CI text-server settings with SPX local builds
  • enable the Advanced text server and built-in HarfBuzz
  • disable the Fallback text server to keep font shaping and rasterization consistent across PC and Web targets

Root cause

SPX buildctl uses module_text_server_adv_enabled=true, module_text_server_fb_enabled=false, and builtin_harfbuzz=true, while the Godot release workflow used the opposite TextServer selection. That produced different font rendering results.

Validation

  • YAML parsing passed
  • git diff --check passed
  • go test ./internal/cmd/buildctl/engine passed in the SPX workspace

@joeykchen
joeykchen marked this pull request as ready for review August 7, 2026 03:26
@joeykchen
joeykchen merged commit 8afdba9 into spx4.4.1 Aug 7, 2026
36 checks passed

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review summary

This PR flips the release engine's text-server build flags to match spx: it enables the Advanced text server, disables the Fallback server, and adds builtin_harfbuzz=true, with an explanatory comment. The change is coherent and correctly paired (one text server shipped, HarfBuzz is a genuine dependency of the Advanced server). Security is fine — builtin_harfbuzz=true pins the vendored HarfBuzz from the checked-in engine tree, which is the upstream release default and improves build reproducibility.

A few non-blocking notes are inline. No blocking issues.

Additional context-only note (no inline location): enabling the Advanced text server + built-in HarfBuzz while disabling only the lightweight Fallback server will grow template/binary size, which is mildly at odds with the optimize=size flag in COMMON_ARGS — most relevant for the size-sensitive web minigame/miniprogram targets. It also invalidates the SCons cache once, so expect one round of longer uncached builds across all platform jobs. Both are expected given the stated font-shaping requirement; flagging for awareness only.

module_msdfgen_enabled=false
module_text_server_adv_enabled=false
module_text_server_fb_enabled=true
module_text_server_adv_enabled=true

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Minor / maintainability: these three edited lines (module_text_server_adv_enabled=true, module_text_server_fb_enabled=false, builtin_harfbuzz=true) drop the trailing space that every surrounding token line in this folded scalar (>-) carries.

This is functionally harmless — YAML folding inserts exactly one space per newline regardless of trailing whitespace, so the emitted common_args stays correctly space-separated. But the mixed trailing-whitespace inside one block makes future diffs noisy and the folded output harder to reason about. Consider normalizing the whole COMMON_ARGS block to consistent trailing whitespace (ideally none, since the fold adds the separator).

@@ -40,8 +43,9 @@ env:
disable_3d_physics=true
disable_navigation=true
module_msdfgen_enabled=false

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Since this PR is specifically syncing font-related build flags with spx's engineCommonArgs, it's worth confirming module_msdfgen_enabled=false is intentionally left unchanged — i.e. spx's local config also disables msdfgen. If spx enables it, the sync is incomplete; otherwise no change needed.

disable_3d=true
EDITOR_ARGS: >-
debug_symbols=true
# Keep the release engine aligned with spx's local buildctl engineCommonArgs.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The rationale comment is accurate and valuable. One low-severity note: the "aligned with spx's local buildctl engineCommonArgs" claim can't be verified from this repo, so it could silently drift out of sync if spx changes. Consider referencing the specific spx file/path so a future reviewer can check alignment.

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