kinogaki-platform

kinogaki-platform — audit issues

Audited 2026-06-20. Repo: libraries/kinogaki-platform. Criteria: completeness · tests · separation of concerns · verb-named functions · file size (<1000 lines) · organization.

Health summary

This is a small, well-disciplined library: 8 public headers + a thin Cocoa/Metal backend, the largest source file is 286 lines, and the build is clean (-Wall -Wextra, 35 test cases / 105 checks all passing). Separation of concerns is genuinely good — windowing, RHI, and the two buses are cleanly layered and no Cocoa/Metal type leaks into a public header. The real gaps are in test coverage of the OS-adjacent-but-testable surfaces (the Cocoa keyOf/modifier mapping, the null backend, the RHI blend/scissor/texture paths) and a couple of documentation drifts (a stale ctx->clear() example, lingering PRISMAPLATFORM_/prisma naming in the CMake build). Nothing is broken or stubbed beyond the intentional null backend.

Issues

[MEDIUM] tests — Cocoa event-translation logic is untested though it is pure and testable

• Where: src/cocoa/CocoaWindow.mm:23 (keyOf), :13 (modifiersOf), :218-228 (modifier+Left button emulation)
• Problem: keyOf (a 60-case scan-code → Key table), modifiersOf, and the Ctrl/Alt+Left → Right/Middle emulation with modifier consumption are pure functions of their inputs — exactly the kind of fiddly mapping that rots silently (one transposed virtual keycode and ⌘Z binds to the wrong physical key). They live inside a .mm TU and are file-local, so nothing exercises them. The window/NSEvent plumbing itself is legitimately GUI-only, but this translation core is not.
• Fix: Hoist keyOf / modifiersOf into a small free-function header (taking a unsigned short keyCode / a modifier-flag bitmask, not an NSEvent) so a host test can feed raw codes and assert the Key/Modifier results. Add a few cases covering letters, digits, arrows, and Minus/Equal.

[MEDIUM] tests — null backend and RHI blend/scissor/texture/window paths have no coverage

• Where: src/null/NullSystem.cpp:7; src/cocoa/MetalRhi.mm blend setup :169-178, setScissor :89, newTextureRGBA/setFragmentTexture :120-131/:86
• Problem: rhi_test.cpp covers clear+readback and an unblended triangle, but never exercises a textured draw, a blended (Alpha/Premultiplied) draw, scissor clipping, or newTextureRGBA seeding — all headlessly testable via offscreen render + readbackRGBA. The null backend's createSystem()==nullptr contract is asserted nowhere. The event model (Event POD defaults) is only lightly touched in types_test.cpp.
• Fix: Add RHI tests: (a) seed an RGBA texture, sample it in a fragment shader, readback and assert the sampled colour; (b) draw two overlapping quads with Blend::Alpha and assert a blended pixel; (c) set a scissor and assert pixels outside it are unchanged. Add a one-line null-backend test (guarded for non-Apple) asserting createSystem() is null. These all run on the existing offscreen path.

[LOW] organization — stale prisma/PRISMAPLATFORM naming survives in the build system

• Where: CMakeLists.txt:12-13,17-18,41 (PRISMAPLATFORM_BUILD_TESTS, PRISMAPLATFORM_BUILD_DEMO, PRISMAPLATFORM_WARNINGS); local build dir holds untracked build/prismaplatform_tests and build/prismplatform_tests
• Problem: Per the project's own rename history (Prisma → Prism → kinogaki), the option/variable names in CMake still read PRISMAPLATFORM_* even though the project, target, and namespace are all Kinogaki/kinogaki. The two stale prisma*_tests binaries in build/ are leftovers from prior names; they are gitignored (build/) so they are not committed, but they clutter local trees.
• Fix: Rename the CMake options/vars to KINOGAKIPLATFORM_* (or KGP_*) for consistency with the target and namespace. Optionally rm build/prismaplatform_tests build/prismplatform_tests locally; no repo change needed since build/ is ignored.

[LOW] completeness — docstring example calls a method that does not exist on IContext

• Where: include/kinogaki/platform/System.h:12
• Problem: The usage example shows if (ctx->beginFrame()) { ctx->clear(0.1f, 0.1f, 0.12f, 1.0f); ctx->endFrame(); }, but IContext (Context.h) has no clear() — clearing moved to the RHI (Device::beginWindow(ctx, clear=true, r,g,b,a)), as Context.h's own header comment correctly documents. A reader copying this example won't compile. Minor, but it is the first thing a consumer reads.
• Fix: Update the example to the real frame shape: auto enc = device->beginWindow(ctx, true, r,g,b,a); device->submit(enc.get(), false); ctx->endFrame(); (mirroring the correct sequence already in Context.h:6-12).

[LOW] completeness — quietly-swallowed writes are intentional but undocumented at one call site

• Where: src/cocoa/MetalRhi.mm:43-45 (MetalBuffer::update)
• Problem: update() silently no-ops when offset + bytes > size_ (an out-of-bounds buffer write is dropped with no signal). This is a defensible safety choice, but unlike the shader/pipeline failures (which NSLog), it is silent — a caller passing a too-large update gets no buffer change and no diagnostic, which is a hard bug to chase.
• Fix: Either NSLog/assert on the overflow case for parity with the other failure paths, or add a one-line comment stating the clamp is deliberate. Low severity since the header documents resource lifetimes but not this clamp.

[LOW] separation — clean

• Where: whole tree
• Problem: None. Windowing (ISystem/IWindow), drawing context (IContext), RHI (rhi::Device), and the two buses are cleanly separated interfaces; the Cocoa/Metal specifics are isolated entirely in src/cocoa/*.mm behind internal factory headers (makeCocoaWindow, makeMetalContext), and no public header imports AppKit/Metal (the MetalHandles/drawable handles are deliberately opaque void*). The buses depend on nothing. No god files.

[LOW] naming — clean

• Where: function/method names across the tree
• Problem: None worth flagging. Public methods read as verbs (createWindow, processEvents, nextEvent, beginFrame, setPipeline, subscribe, publish, run, undo). The few noun-ish names are correct conventions, not violations: now() (clock accessor), effect()/describe() (Command queries), context()/metalHandles() (accessors — exempt as getters), and the factory createSystem()/fromContext(). EventSynth::feed is a verb. No ambiguous or noun-named action functions found.

[LOW] filesize — clean

• Where: all source files
• Problem: None. Largest files: src/cocoa/CocoaWindow.mm (286), include/kinogaki/command/CommandBus.h (182), src/events/Bus.cpp (175), src/cocoa/MetalRhi.mm (266). Nothing approaches the 800-line caution line, let alone 1000.