added review
This commit is contained in:
parent
3f646ec42b
commit
005b04b183
GPG key ID:
2937E59CDCC1BCFB
6 changed files with 222 additions and 1 deletions
2
.gitignore
vendored
2
.gitignore
vendored
|
|
@ -2,3 +2,5 @@ build/
|
|||
bin/
|
||||
pipeline_cache.bin
|
||||
/AGENTS.md
|
||||
.review-gates/push/
|
||||
.review-gates/trees/
|
||||
|
|
|
|||
39
.review-gates/config.json
Normal file
39
.review-gates/config.json
Normal file
|
|
@ -0,0 +1,39 @@
|
|||
{
|
||||
"base": "origin/master",
|
||||
"rangeFrom": "upstream",
|
||||
"authors": [
|
||||
"jorijnvdgraaf@catcrafts.net"
|
||||
],
|
||||
"aiTrailers": "forbid",
|
||||
"requireSigned": true,
|
||||
"modes": {
|
||||
"commits": {
|
||||
"kind": "commits",
|
||||
"lenses": [
|
||||
"repo-better-solution",
|
||||
"repo-bug-prover",
|
||||
"repo-facts"
|
||||
],
|
||||
"effort": "xhigh"
|
||||
}
|
||||
},
|
||||
"gate0": [
|
||||
"CRAFTER_GRAPHICS_VALIDATION=1 crafter-build test --jobs=8",
|
||||
"crafter-build --target=wasm32-wasip1",
|
||||
"crafter-build test --target=x86_64-w64-mingw32 --runner=cmd:true Win32Link",
|
||||
"ok=1; for d in examples/*/; do (cd \"$d\" && crafter-build) || ok=0; done; [ $ok = 1 ]",
|
||||
"node --check additional/dom-env.js",
|
||||
"node --check additional/dom-webgpu.js"
|
||||
],
|
||||
"lenses": {
|
||||
"repo-better-solution": {
|
||||
"section": ".review-gates/lenses/repo-better-solution.md"
|
||||
},
|
||||
"repo-bug-prover": {
|
||||
"section": ".review-gates/lenses/repo-bug-prover.md"
|
||||
},
|
||||
"repo-facts": {
|
||||
"section": ".review-gates/lenses/repo-facts.md"
|
||||
}
|
||||
}
|
||||
}
|
||||
67
.review-gates/lenses/repo-better-solution.md
Normal file
67
.review-gates/lenses/repo-better-solution.md
Normal file
|
|
@ -0,0 +1,67 @@
|
|||
## This repository: Crafter.Graphics
|
||||
|
||||
The Crafter engine's graphics library: one C++ module, `Crafter.Graphics`,
|
||||
one partition per file. On native targets it is a Vulkan 1.3+ renderer
|
||||
built around `VK_EXT_descriptor_heap`, with a bindless descriptor-set
|
||||
fallback, hardware ray tracing, a compute-shader UI and a Wayland or Win32
|
||||
window; on `wasm32-*` the same C++ surface runs in a browser over WebGPU,
|
||||
through the JS bridge in `additional/`. The stated design is `README.md`
|
||||
(the manual), `AGENTS.template.md` (the contributor guide; `AGENTS.md` is
|
||||
written from it and the dependencies' notes by every crafter-build run,
|
||||
and is not in the tree), `AGENTS.consumer.md` (the notes consumers' agents
|
||||
get) and the driver investigations in `examples/VulkanTriangle/README.md`
|
||||
and `examples/Sponza/README.md`. The guide's "Design rules" are the
|
||||
owner's decisions: a range that departs from one without saying so is a
|
||||
finding; whether one should change is the owner's call, not a defect of
|
||||
the range.
|
||||
|
||||
The layers: the partitions (`interfaces/`, `implementations/`), where the
|
||||
target picks the backend in `project.cpp` and there is no runtime switch
|
||||
and no abstraction layer: where the backends' APIs differ, the call site
|
||||
`#ifdef`s. Every partition exists on every target, its body gated. The UI
|
||||
kernels in `shaders/`, each compiled twice from one source, for the
|
||||
descriptor heap and for the descriptor-set build (`.sets.spv`), which
|
||||
`UIFallbackParity` holds pixel-identical, with their WGSL counterparts
|
||||
baked into `additional/dom-webgpu.js`. `project.cpp`: the dependencies, the partition
|
||||
and implementation lists, the shader builds and every test. The examples:
|
||||
the in-tree consumers, GLSL and WGSL side by side. `UIDispatchHeader`
|
||||
and the item structs are frozen and grow only through their reserved
|
||||
`flags` bits and `_pad`, and `UIFusedHeader` is a separate contract, for
|
||||
`DispatchFused`; a component is a stateless function over
|
||||
buffers the user owns, never a hook or a virtual; the library enables only
|
||||
the device features its own shaders use.
|
||||
|
||||
Search first: `Device::EnqueueDeletion` for a handle a frame in flight may
|
||||
still use; `VulkanBuffer::Stage` for a staged re-upload;
|
||||
`DescriptorHeapVulkan`'s slots (`AllocateBufferSlots`, `BufferSlot`,
|
||||
`AddSwapchainSlot`); the barrier builders (`SwapchainStageUnion`,
|
||||
`SwapchainWriterAccess`, `BuildSwapchainInterPassBarrier`,
|
||||
`BuildTlasWriteBarrier`, `BuildTlasReadBarrier`) and `Device::stageMask`;
|
||||
`ImageVulkan::Create` and `VulkanTransition`;
|
||||
`Device::Initialize(required, optional)` for a feature an application's
|
||||
shader needs; `UIRenderer`'s helpers, `Rect::SubRect` and `Inset`; the
|
||||
decode helpers in `shaders/ui-shared.glsl`; the vendored `lib/` (stb,
|
||||
libevdev, the generated Wayland protocol code); Crafter.Event,
|
||||
Crafter.Math and Crafter.Asset as cloned under
|
||||
`~/.cache/crafter.build/external/` or checked out beside this repository,
|
||||
and the Vulkan headers and Vulkan-Utility-Libraries cloned there too.
|
||||
What belongs in one of the three dependencies lands there first (the
|
||||
commit guidelines), not as a local copy here. A driver workaround takes
|
||||
the shape of the one there is: a `Device` flag, one fenced block, a kill
|
||||
switch, a probe test and a status note.
|
||||
|
||||
Commit shape here, as the history shows and design rule 11 asks for the
|
||||
examples and tests: a change travels in one commit with its test, the
|
||||
examples and tests it breaks, and the README, `AGENTS.template.md` and
|
||||
`AGENTS.consumer.md` text it makes untrue; a bug fix ships with a test
|
||||
that fails before the fix (the testing instructions).
|
||||
|
||||
Compatibility: the consumers are the examples, each a `LocalProject` of
|
||||
`../../project.cpp`, and the owner's applications, which pull `master`
|
||||
unpinned and get `AGENTS.consumer.md` on their next build. Design rule 11
|
||||
prefers the clean breaking change to a compatibility shim, so a break is
|
||||
no finding by itself; a break the range does not carry through every
|
||||
example, test and consumer-facing document in this tree is. The
|
||||
environment switches `AGENTS.consumer.md` lists, the GLSL contract user
|
||||
shaders `#include` from `ui-shared.glsl` and the shader file names
|
||||
`ComputeShader::Load` resolves are contracts too.
|
||||
75
.review-gates/lenses/repo-bug-prover.md
Normal file
75
.review-gates/lenses/repo-bug-prover.md
Normal file
|
|
@ -0,0 +1,75 @@
|
|||
## This repository: Crafter.Graphics
|
||||
|
||||
The execution contexts to enumerate, beside the charter's:
|
||||
- One source, several builds. `project.cpp` defines exactly one of
|
||||
`CRAFTER_GRAPHICS_WINDOW_WAYLAND`, `CRAFTER_GRAPHICS_WINDOW_WIN32` and
|
||||
`CRAFTER_GRAPHICS_WINDOW_DOM` from the target: native Linux (Wayland and
|
||||
Vulkan), `x86_64-w64-mingw32` (Win32 and Vulkan, gamepads through
|
||||
Windows.Gaming.Input), `wasm32-wasip1` (the DOM and WebGPU, through
|
||||
`additional/dom-env.js` and `additional/dom-webgpu.js`) and
|
||||
`aarch64-alpine-linux-musl` (Mesa Turnip: no descriptor heap, no ray
|
||||
tracing). Code behind one backend's `#ifdef` is still the range's code,
|
||||
and a change to one backend's half can leave the other's behind.
|
||||
- One device, several paths, chosen at start-up: the descriptor heap or
|
||||
the bindless descriptor-set fallback (`CRAFTER_GRAPHICS_NO_DESCRIPTOR_HEAP`
|
||||
forces it), with each UI kernel's two SPIR-V builds; ray tracing or none
|
||||
(heap-only; `Device::stageMask` drops its stage without it); Vulkan 1.4
|
||||
or 1.3 (`CRAFTER_GRAPHICS_NO_VULKAN_1_4`); `swapchain_maintenance1` or
|
||||
not; GPU memory decompression or the CPU path
|
||||
(`Device::memoryDecompressionSupported`); the NVIDIA AS workaround on or
|
||||
off (`Device::workaroundDescriptorHeapAS`,
|
||||
`CRAFTER_GRAPHICS_NO_AS_WORKAROUND`); llvmpipe on a machine without a
|
||||
GPU.
|
||||
- The GPU timeline. Three frames in flight, a fence each: what lives per
|
||||
frame is keyed by `Window::currentFrame`, what lives per swapchain image
|
||||
by `currentImage`, and the image count follows the present mode. A
|
||||
handle a frame in flight may still use is released through the deletion
|
||||
queue (freed `framesInFlight` frames later) or after a wait for the GPU;
|
||||
a freed descriptor slot is handed out again at once; a swapchain rebuild
|
||||
asked for from a listener mid-frame (`Resize`, `SetPresentMode`) waits
|
||||
for that frame's present, and `onResize` runs after it; a static owner
|
||||
is destroyed after the driver is gone. Within one dispatch, Vulkan
|
||||
orders nothing between workgroups.
|
||||
- Re-entry from the platform. The Wayland listeners run inside
|
||||
`wl_display_dispatch`, `wl_display_dispatch_pending` and
|
||||
`wl_display_roundtrip`, which the frame loop, window creation,
|
||||
`Device::Initialize` and the clipboard's reads call. Windows calls the
|
||||
window procedure from the message loop and also synchronously from
|
||||
inside `CreateWindowEx`, `SendMessage` and `DefWindowProc`. The DOM's
|
||||
listeners in `dom-env.js` call the `__crafterDom_*` exports. A listener
|
||||
can resize, close or destroy the window it was called for.
|
||||
Windows.Gaming.Input reports gamepad connects and disconnects on threads
|
||||
of its own, queued under a lock until the event loop takes them.
|
||||
- Shared state: `Device`'s statics are process-wide; `Device::frameCounter`
|
||||
is one clock for every window, which the deletion queue, the staging
|
||||
rings and the font atlas's eviction run on; the deletion queue is shared
|
||||
under `deletionMutex`. The caches keyed on a `Font*` alias a new font at
|
||||
a freed one's address unless `UIRenderer::InvalidateFont` ran.
|
||||
- Data the GPU and JavaScript read: an item or header struct is mirrored
|
||||
byte for byte by a `std430` block in `shaders/ui-shared.glsl`, by the
|
||||
WGSL in `dom-webgpu.js` and by every shader that includes it, the
|
||||
examples' among them; one push block serves both builds, through
|
||||
`vkCmdPushDataEXT` on the heap and `vkCmdPushConstants` in the set
|
||||
build, within 256 bytes.
|
||||
- Tests: each its own process, in parallel, with a 60 s timeout; every
|
||||
test that calls `Device::Initialize` needs a Wayland display, and one
|
||||
that lacks a capability exits 77 and pins nothing on that machine. A
|
||||
validation check counts only in a run where the layer loaded.
|
||||
|
||||
Semantics to read on disk rather than recall: the Vulkan headers and
|
||||
registry (`include/vulkan/`, `registry/vk.xml`) and Vulkan-Utility-Libraries
|
||||
as cloned (`~/.cache/crafter.build/external/Vulkan-Headers-<hash>/`,
|
||||
`Vulkan-Utility-Libraries-<hash>/`; several clones may sit side by side);
|
||||
the Wayland protocols (`/run/host/usr/share/wayland/wayland.xml`,
|
||||
`/run/host/usr/share/wayland-protocols/`, the generated code in `lib/`) and
|
||||
the client headers (`/run/host/usr/include/wayland-client*.h`,
|
||||
`xkbcommon/`); the MinGW headers (`/run/host/usr/x86_64-w64-mingw32/include/`);
|
||||
libc++ and clang as the host installs them (`/run/host/usr/include/c++/v1`,
|
||||
`/run/host/usr/lib/clang/<major>/include`); the vendored stb and libevdev
|
||||
in `lib/`; Crafter.Event, Crafter.Math and Crafter.Asset as cloned
|
||||
(`~/.cache/crafter.build/external/<Name>-<hash>/`). The crafter-build
|
||||
Flatpak compiles with an LLVM of its own, which is not readable from
|
||||
here: where a semantic may differ between LLVM releases, the host's
|
||||
headers settle it for the host's build only. The Vulkan specification's
|
||||
valid-usage rules, the WebGPU and WGSL specifications and Microsoft's API
|
||||
documentation are not on disk: a semantic only they settle is unverified.
|
||||
35
.review-gates/lenses/repo-facts.md
Normal file
35
.review-gates/lenses/repo-facts.md
Normal file
|
|
@ -0,0 +1,35 @@
|
|||
## This repository: Crafter.Graphics
|
||||
|
||||
- A claim that a tree passes the test suite, builds for a target, links
|
||||
`Win32Link`, builds an example or passes `node --check` traces to the
|
||||
Gate 0 log among the evidence paths INPUTS names: each command with its
|
||||
exit code and the sha it ran at. A test the log shows as skipped, by
|
||||
exit 77 or a missing tool, is not a pass. A claim of a run anywhere else
|
||||
(the aarch64 phone target, the Win32 tests under Wine, a named GPU or
|
||||
driver version, a browser, Deno's WebGPU, an example run by hand, a
|
||||
frame judged by eye) traces to a capture of that run that names the
|
||||
commit, or is unverifiable.
|
||||
- A validation claim ("clean under the validation layer", "no validation
|
||||
errors") holds only for a run in which the layer loaded. Read
|
||||
`Device::Initialize` at HEAD for what a run with
|
||||
`CRAFTER_GRAPHICS_VALIDATION` set does when `VK_LAYER_KHRONOS_validation`
|
||||
is not installed: where it creates the instance without the layer and
|
||||
says nothing, every validation check passes with nothing seen, and since
|
||||
crafter-build prints only the name and time of a passing test, the Gate
|
||||
0 log alone does not show that the layer ran.
|
||||
- A test a message names exists as a `LibraryTest("<Name>")` in
|
||||
`project.cpp` and a `tests/<Name>/main.cpp` whose checks pin what the
|
||||
message says.
|
||||
- `AGENTS.md` is generated from `AGENTS.template.md` and the dependencies'
|
||||
`AGENTS.consumer.md` and is not in the tree: a claim about it is checked
|
||||
against those sources. `AGENTS.consumer.md` reaches consumers' agents on
|
||||
their next build: every sentence in it is a claim about the code at the
|
||||
commit that writes it.
|
||||
- A statement of where a driver stands (the NVIDIA notes in `README.md`,
|
||||
`examples/VulkanTriangle/README.md`, `examples/Sponza/README.md` and the
|
||||
guide's gotchas) names the driver version and traces to a run of the
|
||||
probe on it (`HeapASRead --no-workaround`, `HeapDynamicIndexHit`), or is
|
||||
unverifiable.
|
||||
- A size or layout claim about a struct the GPU reads is checked against
|
||||
the `static_assert` beside the C++ struct, the `std430` block in
|
||||
`shaders/ui-shared.glsl` and the WGSL in `additional/dom-webgpu.js`.
|
||||
|
|
@ -20,6 +20,7 @@ Crafter.Graphics is the Crafter engine's graphics library: one C++ module, `Craf
|
|||
| `README.md`, `examples/*/README.md` | The manual, and the NVIDIA driver investigations (`VulkanTriangle`, `Sponza`). |
|
||||
| `AGENTS.consumer.md` | Notes for consumers' agents; crafter-build injects them into consumers' `AGENTS.md`. |
|
||||
| `bin/`, `build/`, `pipeline_cache.bin` | Generated and gitignored. `build/test-logs/` keeps the output of failed tests; `pipeline_cache.bin` is written by any example or test run from here. |
|
||||
| `.review-gates/` | The pre-push review's `config.json` and this repository's lens sections (`lenses/`), read by Crafter.Review. Its `push/` and `trees/` are generated and gitignored. |
|
||||
|
||||
Partitions, by what they are for. Every partition file exists on every target; one that does not apply compiles to an empty module behind `#ifdef CRAFTER_GRAPHICS_WINDOW_DOM` (or `#ifndef`), and several hold both halves in one file (`Window`, `Device`, `UI`, `Mesh`, `RenderingElement3D`, `RTPass`, `RenderPass`, `Image2D`, `FontAtlas`, `RT`).
|
||||
|
||||
|
|
@ -120,9 +121,11 @@ These were decided deliberately. Do not undo them without the maintainer's say-s
|
|||
## Commit and PR guidelines
|
||||
|
||||
- Branch from `master`; PRs go to `master` on `forgejo.catcrafts.net/Catcrafts/Crafter.Graphics`. Agent branches are `claude/<topic>`, issue work `claude/issue-<n>`. The fetch URL is HTTPS and the push URL SSH (`forgejo@forgejo.catcrafts.net:Catcrafts/Crafter.Graphics.git`); the server refuses HTTPS pushes.
|
||||
- Subject: `type(scope): imperative summary`, with `feat`, `fix`, `perf`, `refactor`, `docs` or `test` and the area as the scope (`device`, `rt`, `ui`, `window`, `wayland`, `win32`, `webgpu`, `font-atlas`, `compute`, `gamepad`). The body is grouped under the components it touches (`Device`, `Shaders`, `Window`, `Tests`) and says what was wrong, why, and why the fix has this shape; `git log -3` shows the voice. Add a `Co-Authored-By:` trailer when an agent wrote it.
|
||||
- Subject: `type(scope): imperative summary`, with `feat`, `fix`, `perf`, `refactor`, `docs` or `test` and the area as the scope (`device`, `rt`, `ui`, `window`, `wayland`, `win32`, `webgpu`, `font-atlas`, `compute`, `gamepad`). The body is grouped under the components it touches (`Device`, `Shaders`, `Window`, `Tests`) and says what was wrong, why, and why the fix has this shape; `git log -3` shows the voice.
|
||||
- Update the README (and the example README that holds the investigation) in the same change when user-visible behaviour changes, and `AGENTS.consumer.md` when a convention, a signature, a contract or an environment variable consumers lean on changes. Consumers pick the new notes up on their next build.
|
||||
- Crafter.Event, Crafter.Math and Crafter.Asset are pulled unpinned, so a change that needs one of them lands there first and is built here with `--local` meanwhile. Pushing is per repository: land the sibling, then this one.
|
||||
- Every push of new commits goes through Crafter.Review's pre-push review first, and goes out only once the owner approves the frozen hash. Run `flatpak run net.catcrafts.CrafterReview init` once in a fresh checkout or worktree: it writes the `/prepush` skill that drives the review, and the skill stays untracked. There is no CI, so the review's gate0, the commands in `.review-gates/config.json`, is the check before the push: the "Before a change is done" checks above, with every example built, and `node --check` on the two JS files in `additional/`. `freeze` refuses merge commits, so the merge of a reviewed branch into `master` is pushed without a freeze of its own.
|
||||
- Every commit of a pushed range is GPG-signed, has `jorijnvdgraaf@catcrafts.net` as its author and committer, and carries no AI attribution trailer in its message, neither `Co-Authored-By:` nor `Assisted-by:`. `freeze` refuses a range with a commit that breaks any of these (`requireSigned`, `authors` and `aiTrailers` in `.review-gates/config.json`).
|
||||
- Never commit `bin/`, `build/` or `pipeline_cache.bin`, and never edit `lib/`.
|
||||
- A second worktree of this repository may sit beside it (`git worktree list`); its branch shows with a `+` in `git branch`. Leave it to whoever opened it.
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue