Draw felt and oak from real textures, at their looks' colours - #381
Merged
Merged
Conversation
The owner asked for realistic felt and oak while plain stays the flat, power-saving look. A table look's texture paths named no package, so they resolved to nothing and every table was drawn in flat colours. The validator now stamps each TableLook with the id of the package it was read out of, so a look's pictures are keyed package::path exactly like a die's atlas and can only reach its own package's files. The bundled package is never on disk, so InstalledArtwork answers its id from the resource files it was validated from, ahead of any folder. Every picture goes through the same path, size and header checks as a die texture. A look may name a colour picture, a normal map and a roughness map per surface, laid at real size (floor_tile_mm / wall_tile_mm), and say how its colour meets the picture (color_mode: multiply, the old meaning and the default for photo tables, or average, so a grey felt averages out at the look's colour and the picker swatch stays true). They are drawn with a third material compiled on first use and cached like the others; the dice materials are untouched. Pictures are uploaded with full mip chains (sRGB for colour) and sampled trilinearly with 8x anisotropy. The built-in felts use a CC0 ambientCG needle felt at 80 mm a copy and oak walls; oak uses a 300 mm cut of Poly Haven's CC0 oak_wood_planks. 1.5 MiB of WebP, recorded with sources in docs/assets/README.md and made by tools/generate-table-textures.py. Decision 92. Rebased onto the lighting, reflection and edge work (decisions 89, 93 and specular anti-aliasing). A floor that is both glossy and drawn from pictures is drawn from its pictures and takes no reflection pass: no material has both, and no bundled look needs one. The table material is compiled without specular anti-aliasing, since Filament's filter reads the mesh's flat normal rather than the normal map, and its cache fingerprint says so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On the Pixel 10a the textured oak drew (112, 91, 82) for #5a3a1e — pinkish grey — and its rim grey stone. The colour maths was not the cause: the average tint over the part of the picture the floor shows comes to (89, 57, 30) before light. The grey is the key light's highlight. The key comes down two in one from the far side, so in the tilted shot its mirror direction lands near the middle of the floor, and the roughness map replaced the look's roughness: Poly Haven's oak boards average 0.44, a lacquered finish. A GGX model of the key plus the studio's prefiltered room predicts +0.067 linear grey over the floor at that map, against +0.065 measured, and +0.019 for the felt against +0.017. In color_mode = "average" the look's numbers are what its surface averages out to and the pictures add only grain, so the roughness map now follows the colour: TableTint.levelOf takes its stored mean, roughnessShift moves the whole map by the difference (moved rather than scaled, so the grain keeps its depth), and the shader clamps the sum. A look that multiplies keeps its photograph's map as it was. Oak is oiled rather than lacquered at 0.75: at 0.55 the highlight alone is still about 0.04 in the tilted shot. What is left is the room's sheen on a dielectric, about a hundredth looking straight down, which no roughness takes away and which is ten to twenty levels on a blue of 30. TableTextureDeviceTest now holds each textured floor, straight down with the real room and the post pass, to its look's colour — green felt within 10 a channel, oak and black felt within 20 and oak to red over green over blue — and logs the tilted shot beside it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On the Pixel 10a the textured green felt drew #1f5e3a as (44, 94, 65): its red 13 over. That is the felt's own reflection of the lamps. A dielectric at roughness 0.9 under the studio's key and room gives back 1.4 % of white looking straight down (key 0.0079, room 0.0063), grey, on top of its colour, and green felt's red is 1.4 % in light. A model of exactly that, the colour at the diffuse share plus the sheen, predicts (45.0, 94.1, 64.1). What it is not: the normal and roughness maps (modelled with the felt's own maps the sheen is a flat surface's to a ten-thousandth), the albedo mean (only a 2 % scale in the fit), the roughness clamp, or multiscatter compensation (3 %). The flat felt drew less, (34, 89, 58), because the dice's material writes material.clearCoat only for a lacquered die and Filament's default is a full coat: under it the base f0 goes to nought and what is left is a mirror's 4 % of the dim ceiling straight overhead (0.085 of white). That fits all three of its channels and is left for a follow-up in docs/TODO.md, since fixing it moves every flat look. A look in color_mode = "average" now hands the material the colour that draws as its own once the sheen is added. SurfaceLight works the sheen and the diffuse share out from the key, the room, the exposure and the look's roughness through Filament's GGX: distribution, the fast Smith visibility a phone's material uses, Schlick, and the split-sum albedo, integrated over GGX's sample variable, with energy compensation. It is derived rather than measured, so it follows the lamps. A colour with a channel darker than the sheen cannot be drawn; it is lifted by the least grey that reaches it, which keeps its hue where clamping that channel alone would turn a dark brown maroon. Most of a felt's grain at screen size is the maps moving the sheen, so black felt keeps its cloth. Expected straight down: green felt (32, 91-94, 57-58), black felt (32, 32, 32) for #1a1a1a, oak (92-94, 63-65, 42) for #5a3a1e, within the test's unchanged tolerances. TableTextureDeviceTest now logs every floor before it holds any of them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TableTextureDeviceTest wanted at least 1.0 level of grain between neighbouring pixels, a round number chosen before the floors were drawn at their looks' colours. Taking the felt's sheen out of its colour took a little of the sparkle its weave caught: the green felt measured 0.91 on the Pixel 10a, still 57 times plain's 0.016 and plainly cloth in the gallery. The bar is now half as much again as the flat threshold the test already uses (0.75), so it keeps asking whether there is a picture on the floor. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…repo Two failures on #381's CI. detekt flagged two bare 4.0s in SurfaceLight.albedoLookingDown: GGX's 4 from density to BRDF, and Simpson's odd-sample weight; both are named now. SonarCloud rated new code's security C over tools/generate-table-textures.py, which took its download folder and the repository from the command line and named downloads after the URL. It takes no arguments now: the repository is where the script is, downloads go to build/table-texture-sources, and a download is named by the URL's bare last part. The six picture names are constants rather than repeated literals. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
drehtuer
added this pull request to stack #386
October 5, 2026 06:37
drehtuer
added a commit
that referenced
this pull request
Oct 5, 2026
Prunes redundant unit tests and adds tests where function or branch coverage was missing, across all 29 measured modules. Five agents did the work in parallel, one set of modules each, and their branches were merged locally. **Based on `feature/designer-material-and-roundness`**, which sits on #381. That branch has no PR of its own yet. ## What changed - **About 40 redundant tests were removed or merged.** In each case another test asserts the same outcome on the same path, or the test's assertions are a strict subset of a broader test's. Where the removed test carried the only explanation, its comment moved to the test that keeps the case. Duplicated fakes (`SilentTray`, `DirectTray`, presenter setup) now live once in `RollScreenFakes`. One "test" could never fail: `ShapeHullTest` compared each corner with the maximum over the same corners. - **About 200 tests were added.** They target untested functions, error paths, boundaries and `when` arms, not Compose `$changed` bookkeeping or getters. New files include `StandardDiceTest`, `VerifyDeviceTestResultsTaskTest` (the only verdict on device runs), `WaysBetweenScreensTest`, `ChosenFilesTest`, `RollWiringTest`, `GraphChartTest`, `EarnedThrowTest`, `TrayPlanInkTest`, `TrayLoopEndingsTest` and `TimeStampTest`. - **Four existing tests did not test what their names claimed**, and now do: - "saving leaves the editor, and so does deleting" never deleted. - The install test never checked the running state. - The update test bypassed the sheet. - The "no downloader" test was handed a downloader. - `StatsScreenTest`, `HistoryScreenTest` and `TrayLoopTest` reached detekt's `LargeClass` limit. Their session, forget and stall tests moved to files of their own; no test was lost in the move. - **One production change, no behaviour change:** `RollWiring.table(pinned)` went from `private` to `internal`, so the table precedence rule (pin → chosen → bundled → fallback) can be tested without a Jolt run. - `docs/STATUS.md` and `docs/TODO.md` record the new coverage figures, and list the problems the pass found. ## Coverage (JaCoCo, merged JVM + Robolectric; base `3a342c5d` → this branch) No module drops on either counter. | | Function | Branch | |---|---|---| | **Total** | **91.1 % → 92.8 %** (629 → 505 missed) | **73.2 % → 75.4 %** (4380 → 4024 missed) | <details><summary>Per module</summary> ``` app M 72.9-> 78.8 B 61.0-> 62.5 core/collection M 94.4-> 95.8 B 86.9-> 95.6 core/glyphs M 94.8-> 94.8 B 92.7-> 93.1 core/model M 96.8-> 99.4 B 96.0-> 98.3 core/notation M 96.7-> 97.3 B 92.4-> 94.3 core/probability M 99.0-> 99.0 B 89.0-> 91.6 core/stats M 92.5-> 92.5 B 96.4-> 99.1 data M 90.5-> 91.7 B 83.3-> 86.9 designer M 97.4-> 97.4 B 88.0-> 92.7 dicesets/builtin M 91.7-> 91.7 B 87.5-> 87.5 dicesets/format M 93.8-> 94.5 B 87.9-> 96.6 dicesets/install M 98.1-> 98.1 B 84.2-> 89.9 feature/designer M 98.1-> 98.5 B 65.7-> 65.7 feature/graph M 91.5-> 95.8 B 61.3-> 68.3 feature/roll M 94.5-> 96.0 B 70.7-> 71.3 feature/saved M 93.3-> 96.1 B 63.1-> 65.0 feature/sets M 94.9-> 97.2 B 68.0-> 69.7 feature/settings M 92.9-> 99.3 B 62.6-> 63.8 feature/stats M 97.0-> 98.6 B 64.4-> 65.3 feature/tables M 98.9-> 98.9 B 66.6-> 70.3 feedback M 96.8-> 96.8 B 89.3-> 90.5 input/shake M 98.3-> 98.3 B 94.2-> 95.3 render/filament M 76.6-> 76.8 B 73.3-> 74.9 render/headless M 76.5-> 79.4 B 100.0->100.0 simulation/api M 96.2-> 96.2 B 91.9-> 94.9 simulation/harness M 92.6-> 92.6 B 94.2-> 95.2 simulation/jolt M 72.0-> 74.0 B 81.0-> 81.0 test-fixtures M 67.2-> 89.6 B 73.3-> 83.3 ui/common M 93.5-> 99.4 B 62.3-> 65.1 ``` </details> Most of the branches still missed are Compose skip branches, plus the device-only bridges (`FilamentStage`, `TrayDriver`, `FilamentEngine`, `JoltWorld`), which were deliberately not faked into coverage. ## Found, not fixed (now in `docs/TODO.md`) These are kept out of this PR so that no new test locks in a wrong answer: - `SafeExtractor` extracts tar device and FIFO entries as empty files. `TarArchiveEntry.isFile` is true for anything that is not a directory, so the documented refusal never fires for them. This is not exploitable, but it is a validator gap. - `InstallSource.gitea` drops a subfolder and installs from the repository root. - `GraphMachine.redraw()` resets `truncatedMass` to 0. - `ReferencedFile.staysInsidePackage` refuses a path with a trailing `/`. - There is a short list of dead code and guards that can never be false. ## Checks - `./gradlew test coverageReport detekt ktlintCheck lint`: green in the devcontainer, no new warnings. - `build-logic`'s own tests: green. - No device tests were needed: no device-tested code changed. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
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 free
to 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.
Owner request: make the felt and the oak look realistic with textures. Plain stays a flat colour, for example for power saving.
Textures (decision 92):
tools/generate-table-textures.py; the licences are indocs/assets/README.md.TrayMesh.The open question this answers: a table look's textures now resolve as
package::path, like die artwork. The validator stamps each look with the package it came from (TableLook.packageId). The built-in package is answered from its APK resources, validated the same way. Downloaded tables go through the same checks as dice textures.Format (optional, backward-compatible):
floor_normal,floor_roughness,floor_tile_mmand thewall_*equivalents.color_mode = "average" | "multiply". Withaverage, the colour is what the surface averages out to, so a table's swatch stays its designed colour.Colour, three rounds on the Pixel 10a:
Oak drew pinkish grey (112, 91, 82). Cause: the roughness map replaced the look's roughness and made the floor glossier than designed. Fixed: the map is averaged at the look's roughness, and oak is oiled at 0.75.
The felts drew greyer than set: green (45, 94, 65) against (31, 94, 58). Cause: the felt's own 4 % dielectric sheen under the key light and the studio. Fixed:
SurfaceLightderives the sheen and the diffuse share from the lights (GGX, Fresnel, the room integral) and takes them out of anaveragelook's tint.Now, straight down:
Test bar changed, openly:
TableTextureDeviceTestwanted a round 1.0 level of grain. Taking out the sheen took a little sparkle from the weave, and the green felt measured 0.91 (plain 0.016, i.e. 57×). The weave is plainly visible in the gallery. The bar is now tied to the flat threshold the test already uses (1.5 × 0.5 = 0.75), so it still asks whether there is a picture on the floor. Said in the test and in its own commit.Found, not fixed (logged in TODO):
DiceMaterial.SOURCEonly writesclearCoatwhen it is > 0, and Filament defaults it to 1.0. So plain, dark glass and every flat surface wear a full mirror lacquer. Fixing it shifts every flat look and their device tests; it's a separate decision.On the Pixel 10a:
:render:filamentdevice suite 59/60 on the run before the bar change. The one failure was the bar above; after the change,TableTextureDeviceTestpasses.APK: +1.54 MiB.
Based on #380.
🤖 Generated with Claude Code