Skip to content

Commit 968a953

Browse files
githubawngithubawn
committed
docs: Track upstream candidates as notes rather than separate PRs
The branch carries a mix of general fixes, backports and web-only scaffolding, and it goes up as one PR rather than a stream of small ones. Record which is which, why each fix is guarded the way it is, and what is settled so it does not get chased a second time.
1 parent 7bce115 commit 968a953

1 file changed

Lines changed: 144 additions & 0 deletions

File tree

‎UPSTREAM-NOTES.md‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
# Upstream notes — ci/wasm-build
2+
3+
Working notes for what on this branch is a general fix versus web-port scaffolding.
4+
The intent is that `ci/wasm-build` eventually goes up as **one** PR, so nothing here
5+
should be sent piecemeal.
6+
7+
---
8+
9+
## Already open against bobtista
10+
11+
Raised as individual PRs before the one-big-PR plan was settled. Left open as-is —
12+
they are not the route this work is going up by, and nothing here depends on them.
13+
The commit hashes are the ones on the bobtista branches; the equivalent work on this
14+
branch is listed in the next section.
15+
16+
| PR | What |
17+
|----|------|
18+
| [#3](https://github.com/bobtista/GeneralsGameCode/pull/3) | GLES/WebGL scene composite sampled upside down |
19+
| [#4](https://github.com/bobtista/GeneralsGameCode/pull/4) | A1R5G5B5 terrain atlas expanded to BGRA8 off Windows |
20+
| [#5](https://github.com/bobtista/GeneralsGameCode/pull/5) | `%hs` kept printable when rewriting wide formats for libc |
21+
22+
All three cherry-picked cleanly onto `bobtista/topic/trunk` and had the `githubawn`
23+
handle stripped from code comments.
24+
25+
---
26+
27+
## General fixes on this branch (not web-specific)
28+
29+
Candidates for the eventual PR. Each is self-contained and guarded.
30+
31+
### `755d205cc` — GLES composite V-flip
32+
Render targets are bottom-left origin on GLES/WebGL, top-left on DX11/Metal. The
33+
composite vertex shader built one fixed set of texcoords and the backend passed a
34+
hardcoded `0` where the flip flag belongs, so the whole 3D scene came out mirrored
35+
while the UI stayed upright. Cap-driven (`originBottomLeft`), so DX11/Metal pass `0.0`
36+
and are byte-for-byte unaffected.
37+
38+
Trunk still has the bug — `vs_scene_composite.sc` there is byte-identical to the
39+
pre-fix parent, and `BgfxBackend.cpp` still has the hardcoded `0.0f`.
40+
41+
### `ebd422631` — A1R5G5B5 terrain atlas
42+
`BGR5A1` is not a native GLES format, so bgfx converted on upload, over-read the
43+
source buffer and painted coloured speckle across the terrain. Three linked places
44+
all need changing (`GetBgfxTextureUploadFormat`, `IsTerrainAtlasTexture`,
45+
`UploadTerrainAtlasMips`), which is why partial fixes appear to do nothing.
46+
47+
Trunk still maps `A1R5G5B5 -> BGR5A1`. The Switch branches were stripped for upstream:
48+
trunk has zero `__SWITCH__` references, so they would have been unbuildable dead code,
49+
and dropping them also removed three now-unreachable `|| RGBA8` acceptances.
50+
51+
Vibecode frees its intermediate buffer with `bgfx::release()`, which does not exist in
52+
this bgfx revision. `CopyTextureLevel` is asked for the final upload format instead — it
53+
already knows how to widen `A1R5G5B5` — so there is no second allocation.
54+
55+
### `4c121ad92` — `%hs` wide-format rewrite
56+
`translateWideFormat` emitted length modifiers as it read them, so `%hs` reached musl,
57+
which rejects it: `vswprintf` returns -1 and the string comes out empty. `GameText`
58+
formats its missing-label placeholder as `MISSING: '%hs'`, so any INI naming an absent
59+
label produced an empty string and `INI::parseAndTranslateLabel` threw — killing
60+
startup. Reachable with retail data: `CommandMap.ini` asks for `GUI:SaveView5`, which
61+
`generals.csf` does not define. Windows keeps its own `vswprintf` path.
62+
63+
Trunk does not have this.
64+
65+
---
66+
67+
## Backported FROM trunk into this branch
68+
69+
### `45a413a82` — case-correct the leading path component
70+
Backported from `bobtista/topic/trunk` (fixed there 29/07/2026 on Linux); this branch
71+
forked earlier and missed it. `fixFilenameFromWindowsPath` copied the first component
72+
of a relative path through verbatim before its case-insensitive traversal, so a
73+
mis-cased leading directory could never be repaired.
74+
75+
`SidesList` spells the skirmish scripts path `"data\Scripts\SkirmishScripts.scb"` with
76+
a lowercase `d` while it is `Data` on disk. On a case-sensitive filesystem the open
77+
failed, and since the skirmish team records are cleared immediately before that parse,
78+
they stayed empty → the skirmish AI got no teams → null default team → its starting
79+
base belonged to no team → `hasAnyObjects()` false → defeated on frame 0 → **instant
80+
win**. The score screen still showed 1 unit / 1 building because `onUnitCreated` /
81+
`onStructureCreated` are called explicitly, independent of team membership.
82+
83+
The path literal is deliberately left lowercase: the filesystem layer is the single
84+
point of tolerance, and the function is byte-identical to trunk's.
85+
86+
Reproduced on wasm in **both** this port and the vibecode port; not a lost re-port fix.
87+
Hidden on Windows, macOS (case-insensitive APFS) and WSL over `/mnt/c`.
88+
Skirmish plays normally since this landed.
89+
90+
---
91+
92+
## Web-only (only meaningful if the Emscripten target goes up too)
93+
94+
- `53da6ba0b` — texture loader with no loader thread. Guarded to `__EMSCRIPTEN__` and
95+
staying that way: it travels with the web target in the single PR, so it is not dead
96+
code there. Trunk's `ThreadClass::Execute()` also returns immediately under `_UNIX`,
97+
so Linux/macOS/Android may share the latent problem — widening the guard is a
98+
separate change that needs the failure demonstrated on one of those first.
99+
- `7bce11538` — disable the automatic fullscreen on the web. Startup asked for
100+
fullscreen unless `-win`, and the web build has no command line. The browser never
101+
granted it, but the request sized the drawing buffer to the desktop mode while the
102+
page presented the canvas at its `100vw/100vh` CSS box, so pointer coordinates were
103+
scaled against the wrong extent and clicks missed. Guarded to `__EMSCRIPTEN__`.
104+
105+
Known limit, deliberately not addressed: this covers startup only.
106+
`W3DDisplay::setDisplayMode` is passed `TheDisplay->getWindowed()`, which is still
107+
false on the web, so changing resolution from the Options menu takes the `else`
108+
branch and calls `SDL_SetWindowFullscreen(true)` plus a mode switch again. Doing that
109+
properly means deciding whether the web target should own the container outright
110+
rather than pretending to have a window - a separate piece of work, not a patch.
111+
- `cede40a5b` — manual reference build of the branch the port came from, artifact only,
112+
no deploy. Scaffolding for comparing the two ports; goes or stays with the web target.
113+
- All `ci(web)` / `build(web)` / `feat(web)` commits: Emscripten target, CI workflow,
114+
Vercel deploy, OPFS data picker, LAN-over-WebSocket relay, `serve.py`, `shell.html`.
115+
116+
---
117+
118+
## Open
119+
120+
- **Hosted data whitelist coverage.** The hosted build only reaches four `Data\`
121+
subfolders (Cursors, WaterPlane, Scripts, INI). The local `serve.py` serves the whole
122+
`Data\` tree minus `Movies\`. Anything a real install keeps elsewhere under `Data\` is
123+
silently absent from the hosted build.
124+
- **`GgcRuntimeFlags` scaffolding is still live** (`66985ba06`, April). `GGC_TRACE`
125+
breadcrumbs, the `GGC_BGFX_RENDERER` override and `GGC_POINTGROUP_DIAG` reach four
126+
files — `GgcRuntimeFlags.h`, `BgfxBackend.cpp`, `W3DVolumetricShadow.cpp`,
127+
`SDL3Main.cpp`. Env-var gated and off by default, but development instrumentation
128+
that should not go up. Deliberately not removed here: the same commit also flipped
129+
prewarm and mouse-grab defaults, so stripping it is a behaviour change to make on
130+
purpose, not a history rewrite three months back.
131+
132+
---
133+
134+
## Settled — do not reopen
135+
136+
- **Magenta missing-texture placeholder.** An attempt to bind the white fallback
137+
instead of the magenta placeholder on the web, and a second attempt to do the same
138+
where the pixels are read, were both chasing the pink terrain. The cause was the
139+
A1R5G5B5 atlas above. Both are removed from this branch; the placeholder behaves as
140+
it does everywhere else, and a genuinely absent asset shows magenta plus its
141+
`Missing texture <reason>: <file>` line in the browser console.
142+
- **Skirmish instant win.** Fixed by `45a413a82`, not a lost re-port fix.
143+
- **`Data/Scripts/Scripts.ini` never copying into OPFS.** Whitelisted, never lands, and
144+
no engine reference to it exists — it is a WorldBuilder file. Harmless.

0 commit comments

Comments
 (0)