Uh oh!
There was an error while loading. Please reload this page.
Guide figures render with a bundled font instead of the host's - #5668
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
shai-almogforce-pushed
the
devguide-figure-harness
branch
from
September 2, 2026 07:11
506b447 to
8a91850CompareContributor
Cloudflare Preview
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:8a91850b97
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:de62de4b98
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:2989351749
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:365153a366
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
Contributor
Developer Guide build artifacts are available for download from this workflow run:
Developer Guide quality checks: |
shai-almogforce-pushed
the
devguide-figure-harness
branch
from
September 2, 2026 09:56
691841e to
2e7dd10CompareThere was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:2e7dd10fe6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
shai-almogforce-pushed
the
devguide-figure-harness
branch
from
September 2, 2026 10:06
2e7dd10 to
f02771eCompareshai-almog
changed the base branch from
devguide-structure-and-guards
to
devguide-guardsSeptember 2, 2026 10:12
shai-almogforce-pushed
the
devguide-figure-harness
branch
2 times, most recently
from
September 2, 2026 11:29
6832347 to
1bc8e6aCompareshai-almogforce-pushed
the
devguide-guards
branch
from
September 2, 2026 11:39
212956c to
6d0560cCompareshai-almogforce-pushed
the
devguide-figure-harness
branch
from
September 2, 2026 11:39
1bc8e6a to
ae956d3Compareshai-almogforce-pushed
the
devguide-guards
branch
from
September 2, 2026 11:58
b96d132 to
d82c0eaCompareshai-almogforce-pushed
the
devguide-figure-harness
branch
from
September 2, 2026 11:58
ae956d3 to
eecf05dCompareshai-almogforce-pushed
the
devguide-guards
branch
from
September 2, 2026 12:16
d82c0ea to
42189c3Compareshai-almogforce-pushed
the
devguide-figure-harness
branch
3 times, most recently
from
September 2, 2026 13:06
335a54a to
f915089Compareshai-almogforce-pushed
the
devguide-guards
branch
from
September 2, 2026 13:21
b88150d to
5028cb2Compareshai-almogforce-pushed
the
devguide-figure-harness
branch
3 times, most recently
from
September 2, 2026 14:42
facd9ed to
15e0c1eCompareNone of the 24 generated figures could be reproduced outside CI. Running the generator on a Mac reproduces 0 of 24 byte-for-byte, with 3-20% of pixels differing. Two consecutive local runs are byte-identical, so the generator is deterministic on one host; the variable is the font. `PreAdvancedThemingScreenshots` styled with `Font.createSystemFont`, which resolves through `JavaSEPort.fontFaceSystem` -- "Arial" on macOS and Linux alike. Arial exists on a developer's Mac and not on a stock CI runner, so AWT silently substitutes and every glyph changes. The differences are exactly that: for `flow-layout.png` every differing pixel sits in y 10-130, the title and label rows, while the colored blocks below match to the pixel. So the byte-exact gate has only ever been green because CI both generates and verifies. A developer regenerating locally could not match it, and at the scale the guide's remaining ~260 app screenshots would need, that is untenable. The figures now load `native:MainRegular`, which `JavaSEPort.loadTrueTypeFont` reads from `/com/codename1/impl/javase/Roboto-Medium.ttf` on the classpath rather than from an installed-font lookup. This is also what the project's font rule requires everywhere: never `createSystemFont`, always the `native:` scheme. A null return refuses loudly rather than falling back to a host font, because a silent fallback would restore the exact non-determinism this removes. The committed baselines are regenerated from a Mac. CI byte-compares them on Linux, so the check either passes -- proving host independence rather than asserting it -- or fails and says so immediately. Composition, dimensions and colours are unchanged; only the typeface moves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ours CI reported one stale figure out of 24: `guibuilder-2-insets-3.png`, the only one containing a `TextField`. The other 23 matched Linux byte for byte, so the bundled-font change worked -- it just did not reach far enough. `applyBlockStyleToContent` styled `Label` and `Button`. Everything else kept the theme's default font, which resolves through the host, so the text field and its hint were still host-dependent. Enumerating the types that carry text would have left the next one added broken the same way, so the walk now pins the face on every component it visits, plus the hint label, which is painted by a `Label` that is not in the component tree and so is never reached by the walk. The first attempt used `BLOCK_FONT` for this and regressed the figure: at 29px against the theme default's 13px the field grew and squeezed "Submit" down to a clipped sliver. `FIELD_FONT` is sized to reproduce the original height, so the composition is unchanged and only the typeface moves. Verified locally by rendering twice, once with `JavaSEPort.setFontFaces` pointed at a family that does not exist -- which is what a machine without Arial looks like to the port. All 24 come out byte-identical, so nothing in these figures reads an installed font any more. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"Committed screenshot is stale: <name>" names the file and nothing else, so there is no way to tell a real regression from an environment difference without adding a debugging round trip to CI. The generated directory is now uploaded as an artifact when the step fails. Needed immediately: one figure still differs between a Mac and the runner after the font fix, and host fonts, JDK version and working directory have each been ruled out locally. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… edge
The artifact step paid for itself immediately. Downloading what the runner
rendered showed two figures differing, not the one the gate reported -- it exits
on the first mismatch -- and the two had nothing in common.
`mig-layout.png` differed by 12.8% of its pixels, a real layout shift.
MigLayout takes its default gaps from `PlatformDefaults`, which reads
`System.getProperty("os.name")` and answers MAC_OSX, GNOME or WINDOWS_XP, each
with different spacing. The figure was rendering with macOS gaps on a Mac and
GNOME gaps on the runner. Pinning the platform fixes it: with the pin, 23 of the
24 figures now match the runner's own output byte for byte.
`layered-layout.png` differed by 173 pixels, 0.113%, inside a 25x25 box. That
one is not fixable. Measured against the runner's bytes, the material glyph
lands at exactly the same size and the same origin -- a 55x49 bounding box --
and differs only in antialiased edge coverage, 946 fully-white pixels against
916. Java2D rasterizes the same glyph, from the same bundled font, at the same
size, slightly differently on the two platforms. Demanding byte equality there
would mean deleting legitimate content from the figure or carrying a
permanently red check.
So the comparison moves from `cmp -s` to a comparer that still requires byte
equality by default and accepts a bounded difference only where a figure carries
a `.tolerance` sidecar explaining itself, in the same key=value shape the CN1SS
suites already use. The area bound does the work: a per-pixel delta that large is
meaningless on its own, since a glyph edge flips between white and the block
behind it, but a regression that changed the icon would move far more than 0.3%
of the image.
Verified by running the comparer with the runner's own output against the
committed figures -- which is exactly what CI will do -- and by four probes: an
untoleranced figure that differs fails, the toleranced figure fails when changed
beyond its budget, a missing figure fails the count, and a clean run passes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>Two review findings on this PR. The first says `native:MainRegular` does not give host-independent output, because `loadTrueTypeFont` resolves native fonts to the first installed SF or Helvetica family before reaching the bundled Roboto. The branch is real, but it is reached only when `isIOS` is set, which `loadSkinFile` does for a skin whose systemFontFamily contains "helvetica" -- and this generator never loads a skin. The measurement agrees: figures rendered on a Mac match the Linux runner byte for byte, which could not happen if one side were resolving Helvetica Neue and the other Roboto. So the conclusion does not hold today, but the risk is real for tomorrow: a change that loads a skin here would put host fonts back into the output with no other symptom. The generator now refuses to run under an iOS platform, and says why, rather than leaving that to a comment nobody reads. The second finding is straightforwardly right and is the more serious of the two. `on.pull_request.paths` triggers on `scripts/developer-guide/**`, but the `Determine changed components` filter named only two scripts, so a pull request touching any other script here started the workflow with `docs` false and skipped the steps that script governs. A change to compare-screenshots.py could have merged without the screenshot check ever running it -- a gate that skips itself. The filter now covers the whole directory, which also removes the two-copies-drift the surrounding comment already warns about. Verified the guard changes no output: all 24 figures are unchanged, and the comparer still passes against the runner's own bytes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The comparator incremented its mismatch counter only for pixels whose channel delta EXCEEDED maxChannelDelta, which is what the CN1SS comparator does. With a sidecar written the way this one was -- a large delta bound paired with a small area bound -- that leaves an unbounded hole, and review gave the exploit in the figures' own palette: recolouring the green #06a806 to #a608a6 moves every channel by exactly 160, so with maxChannelDelta=160 not one pixel is counted and a dramatically different image reports zero mismatches. Reproduced it before fixing: the recolour changes 4191 pixels, 2.73% of the image, at a worst delta of exactly 160 -- and passed. The two bounds are now independent. maxMismatchPercent limits how much of the image may change at all, counting every differing pixel; maxChannelDelta caps how far any single pixel may move. The measured legitimate noise -- 173 pixels, 0.113%, worst delta 141 -- still passes, the recolour now fails on area, and a five-pixel solid overwrite fails on delta. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous commit changed the comparator so every differing pixel counts toward the area budget and the channel delta is a separate ceiling. The sidecar still explained the old behaviour -- that the area bound did the work and the delta was meaningless on its own -- which is now wrong in a file whose whole purpose is to justify the numbers beside it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The comparator converted both images to RGB before counting differences. The committed figures are genuine RGBA PNGs -- four channels, currently opaque everywhere -- so that dropped a real channel, and any change confined to alpha was invisible to the tolerance path. A regression that turned the whole figure transparent while leaving every colour channel intact reported zero changed pixels and passed. Byte equality, which every figure without a sidecar is still held to, always caught this. Only a figure carrying a sidecar could reach the weakened path, so today the exposure was one image -- but that image is exactly the one whose comparison is relaxed. Verified: making layered-layout.png fully transparent with its RGB channels untouched now reports 100% of pixels changed, the real runner output still passes, and the recolour and untoleranced-difference probes still fail as they did. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
shai-almogforce-pushed
the
devguide-figure-harness
branch
from
September 2, 2026 15:02
15e0c1e to
815afcfCompareUh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #5664. Review that one first; this diff is only the last commit.
The defect
None of the 24 generated figures can be reproduced outside CI. Running the generator on a Mac reproduces 0 of 24 byte-for-byte, with 3–20% of pixels differing.
Two consecutive local runs are byte-identical, so the generator is deterministic on one host. The variable is the font:
PreAdvancedThemingScreenshotsstyled withFont.createSystemFont, which resolves throughJavaSEPort.fontFaceSystem—"Arial"on macOS and Linux alike (JavaSEPort.java:765).flow-layout.pngevery differing pixel sits iny 10–130— the title and label rows — while the coloured blocks below match to the pixel.So the byte-exact gate in
developer-guide-docs.ymlhas only ever been green because CI both generates and verifies. A developer regenerating locally could not match it, and the guide has ~260 more app screenshots that want the same treatment.The fix
The figures load
native:MainRegular, whichJavaSEPort.loadTrueTypeFont(:13551) reads from/com/codename1/impl/javase/Roboto-Medium.ttfon the classpath rather than from an installed-font lookup. That cannot depend on what the machine has installed.This is also what the project's own font rule requires everywhere — never
createSystemFont, always thenative:scheme — so the violation and the non-determinism were the same bug.A
nullreturn refuses loudly rather than falling back to a host font: a silent fallback would restore precisely the non-determinism this removes.How this PR proves itself
The committed baselines are regenerated from a Mac, deliberately. CI byte-compares them on Linux. So the existing screenshot check either passes — demonstrating host independence rather than asserting it — or fails and says so immediately.
Composition, dimensions and colours are unchanged; only the typeface moves.
Scope note
These 24 are schematic layout diagrams — green labelled blocks (North/South/East/West/Center) on white under a blue title bar — whose job is to show where a layout manager puts things. They are deliberately not app screenshots, so they keep their hand-styling; rendering them under Material 3 or Liquid Glass would bury what they teach. The native-theme and light/dark machinery belongs with the app-UI figures in the Components, io and graphics chapters, and lands with those.