diff options
Diffstat (limited to 'PLATFORM_ANALYSIS.md')
| -rw-r--r-- | PLATFORM_ANALYSIS.md | 312 |
1 files changed, 0 insertions, 312 deletions
diff --git a/PLATFORM_ANALYSIS.md b/PLATFORM_ANALYSIS.md deleted file mode 100644 index eefbd42..0000000 --- a/PLATFORM_ANALYSIS.md +++ /dev/null @@ -1,312 +0,0 @@ -# Platform Code Analysis & Reorganization Plan - -## Current State - -### Files Location -- `src/platform.h` (100 lines) - Platform API header with WebGPU shims -- `src/platform.cc` (131 lines) - GLFW-based windowing implementation - -### Coverage Status -**Coverage: 0%** - No tests currently exist for platform code - -### Platform API Surface -**Core Functions** (all untested): -```cpp -PlatformState platform_init(bool fullscreen, int width, int height); -void platform_shutdown(PlatformState* state); -void platform_poll(PlatformState* state); -bool platform_should_close(PlatformState* state); -void platform_toggle_fullscreen(PlatformState* state); -WGPUSurface platform_create_wgpu_surface(WGPUInstance instance, PlatformState* state); -double platform_get_time(); -``` - -**Platform Shims** (in platform.h): -```cpp -// Win32 vs Native API differences -str_view() // String view wrapper -label_view() // Label wrapper (respects STRIP_ALL) -platform_wgpu_wait_any() -platform_wgpu_set_error_callback() -``` - -### Platform-Specific Code Distribution - -#### In `src/platform.h` (60% of file) -- Lines 10-72: WebGPU API shims for Win32 vs native -- `DEMO_CROSS_COMPILE_WIN32` conditional compilation -- String view helpers - -#### In `src/gpu/` (scattered across 4 files) -1. **gpu/gpu.h** (line 60): - - Win32 vs native device descriptor differences - -2. **gpu/gpu.cc** (12 blocks): - - Lines 214-266: Device creation descriptor differences - - Lines 269-301: Adapter request differences - - Lines 312-321: Instance creation differences - - Lines 327-329: Error callback setup - - Lines 332-341: Error handling differences - - Lines 345-347: Device loss callback - -3. **gpu/effect.cc** (3 blocks): - - Lines 268-270: Shader composition (WGPUShaderSourceWGSL) - - Lines 310-312: Compute pipeline creation - - Lines 346-348: Render pipeline creation - -4. **gpu/texture_manager.cc** (2 blocks): - - Lines 8+: Texture format differences - - Line 63+: Copy texture descriptor differences - -**Total**: ~150 lines of platform-specific code across 5 files - -### Current Includes (10 files) -- Production: `main.cc`, `test_demo.cc` -- GPU: `gpu/gpu.cc`, `gpu/gpu.h` -- Tests: `test_texture_manager.cc`, `test_3d_render.cc`, `test_mesh.cc`, `test_shader_compilation.cc`, `test_3d_physics.cc` -- Self: `platform.cc` - ---- - -## Proposed Reorganization - -### ✅ Option A: Minimal Move (COMPLETED - February 7, 2026) -**Goal**: Move platform files to subdirectory, keep GPU platform code in place - -**Changes**: -1. ✅ Create `src/platform/` directory -2. ✅ Move `src/platform.{h,cc}` → `src/platform/platform.{h,cc}` -3. ✅ Update all includes: `#include "platform.h"` → `#include "platform/platform.h"` -4. ✅ Leave GPU platform-specific code in `src/gpu/*` (it's WebGPU API abstraction, not OS abstraction) - -**Results**: -- Clean separation: platform windowing in `platform/`, GPU API shims in `gpu/` -- 11 files updated (include paths) -- Follows existing pattern (`src/audio/`, `src/3d/`, `src/gpu/`) -- All builds pass, tests pass, no functional changes - -**Commit**: `17b8ffa - refactor: Move platform files to src/platform/ subdirectory` - ---- - -### Option B: Aggressive Consolidation (Not Recommended) -**Goal**: Centralize ALL platform-specific code in one place - -**Changes**: -1. Create `src/platform/` directory structure: - ``` - src/platform/ - ├── platform.h/cc # Windowing API - ├── webgpu_shims.h # WebGPU compatibility layer - └── webgpu_platform.cc # Extracted from gpu.cc - ``` -2. Extract platform-specific blocks from `gpu/*.cc` into new files -3. Create abstraction layer for WebGPU platform differences - -**Pros**: -- All platform code in one place -- Easier to add new platforms (e.g., bare Win32) - -**Cons**: -- Major refactor (~20 files touched) -- Mixes OS-level platform code with WebGPU API abstraction (different concerns) -- GPU code becomes harder to read (indirection through platform layer) -- Risk of regressions - -**Impact**: 20+ files, ~500 lines of changes - ---- - -## Test Coverage Plan - -### Test File: `src/tests/test_platform.cc` - -#### Testable Functions (without GUI) -1. ✅ **platform_get_time()** - - Call and verify returns valid timestamp - - Call twice, verify time advances - -2. ⚠️ **platform_init()** (requires GLFW context) - - Test windowed mode initialization - - Test fullscreen mode initialization - - Verify default dimensions - - Check aspect ratio calculation - -3. ⚠️ **platform_poll()** (requires GLFW context) - - Verify time updates - - Verify aspect ratio updates - -4. ⚠️ **platform_should_close()** (requires GLFW context) - - Initially returns false - - Returns true after close signal - -5. ⚠️ **platform_toggle_fullscreen()** (requires GLFW context) - - Toggle state change - - Windowed geometry preservation - -6. ❌ **platform_create_wgpu_surface()** (requires WebGPU instance) - - Hard to test without full GPU initialization - - Covered implicitly by existing GPU tests - -7. ⚠️ **platform_shutdown()** (requires GLFW context) - - Cleanup without crash - -#### Testing Strategy - -**Approach 1: Headless GLFW** (Recommended) -- Use `GLFW_VISIBLE` window hint set to false -- Test basic lifecycle without visible window -- Limited: Cannot test actual fullscreen behavior - -**Approach 2: Mock Platform** (Overkill) -- Create `MockPlatformState` for testing -- Requires significant refactoring -- Not worth effort for simple windowing code - -**Approach 3: Minimal Coverage** (Pragmatic) -- Test `platform_get_time()` only (no GLFW dependency) -- Accept low coverage for GLFW wrapper code -- Platform code is thin wrapper, not complex logic - -### Recommended Test Implementation - -```cpp -// src/tests/test_platform.cc -#include "platform/platform.h" -#include <cassert> -#include <cmath> -#include <unistd.h> - -// Test 1: Time query -void test_platform_get_time() { - const double t1 = platform_get_time(); - assert(t1 >= 0.0); - - usleep(10000); // Sleep 10ms - - const double t2 = platform_get_time(); - assert(t2 > t1); - assert(t2 - t1 >= 0.01 && t2 - t1 < 0.1); // ~10ms tolerance -} - -// Test 2: Basic lifecycle (headless window) -void test_platform_lifecycle() { - PlatformState state = platform_init(false, 640, 480); - - assert(state.window != nullptr); - assert(state.width > 0); - assert(state.height > 0); - assert(state.aspect_ratio > 0.0f); - assert(state.time >= 0.0); - assert(!state.is_fullscreen); - - // Poll should update time - const double t1 = state.time; - usleep(10000); - platform_poll(&state); - assert(state.time > t1); - - // Should not close initially - assert(!platform_should_close(&state)); - - platform_shutdown(&state); -} - -// Test 3: String view helpers (Win32 vs native) -void test_string_views() { - const char* test_str = "test"; - -#if defined(DEMO_CROSS_COMPILE_WIN32) - // Win32: returns const char* - assert(str_view(test_str) == test_str); - assert(label_view(test_str) == test_str); -#else - // Native: returns WGPUStringView - WGPUStringView sv = str_view(test_str); - assert(sv.data == test_str); - assert(sv.length == 4); - -#if !defined(STRIP_ALL) - WGPUStringView lv = label_view(test_str); - assert(lv.data == test_str); - assert(lv.length == 4); -#else - WGPUStringView lv = label_view(test_str); - assert(lv.data == nullptr); - assert(lv.length == 0); -#endif -#endif -} - -int main() { - test_platform_get_time(); - test_string_views(); - test_platform_lifecycle(); // Requires GLFW, may fail in CI - - return 0; -} -``` - -**Expected Coverage Increase**: 60-70% (time functions, string views, basic init/shutdown) - ---- - -## Recommendation - -### Phase 1: Create Test (Immediate) -1. Create `src/tests/test_platform.cc` with basic coverage -2. Add to CMakeLists.txt under `DEMO_BUILD_TESTS` -3. Test platform_get_time() and string view helpers -4. Optionally test basic lifecycle with headless GLFW window - -**Expected outcome**: Coverage 60-70% (from 0%) - -### Phase 2: Move Files (Low Priority) -1. Create `src/platform/` directory -2. Move `platform.{h,cc}` to `src/platform/` -3. Update 10 include paths -4. Verify build on all platforms (macOS, Linux, Win32) - -**Expected outcome**: Better organization, no functional change - -### Phase 3: Task B Cleanup (Future) -- Once platform code is in `platform/`, revisit Task B -- Consider extracting common WebGPU shims to `platform/webgpu_shims.h` -- Leave GPU-specific platform code in `gpu/` (it's WebGPU API abstraction) - ---- - -## Platform-Specific Code Summary - -| Location | Lines | Purpose | Should Move? | -|----------|-------|---------|--------------| -| `src/platform.h` | 60 | WebGPU API shims, string views | ✅ Yes → `platform/` | -| `src/platform.cc` | 131 | GLFW windowing | ✅ Yes → `platform/` | -| `src/gpu/gpu.h` | 2 | Device descriptor diffs | ❌ No (GPU API) | -| `src/gpu/gpu.cc` | ~80 | WebGPU init differences | ❌ No (GPU API) | -| `src/gpu/effect.cc` | ~15 | Shader/pipeline creation | ❌ No (GPU API) | -| `src/gpu/texture_manager.cc` | ~10 | Texture format diffs | ❌ No (GPU API) | - -**Total**: ~298 lines of platform-specific code -- **Windowing layer** (191 lines): Should move to `platform/` -- **WebGPU API layer** (107 lines): Should stay in `gpu/` - ---- - -## Conclusion - -**Recommended Action**: -1. ✅ Create `test_platform.cc` now (boosts coverage 0% → 60%) -2. ✅ Move `platform.{h,cc}` to `src/platform/` subdirectory -3. ❌ Do NOT extract GPU platform code (different concern - API abstraction, not OS abstraction) - -**Rationale**: -- Platform windowing (GLFW) vs WebGPU API differences are separate concerns -- Moving only windowing code is clean, low-risk -- GPU code is already well-organized and platform-specific blocks are minimal -- Consolidating everything would create unnecessary indirection - -**Task B Implication**: -- After move, `platform/platform.h` centralizes OS windowing abstractions -- GPU platform-specific code remains in `gpu/` (it's WebGPU API compatibility, not OS code) -- Task B can be updated: "Move platform windowing to `platform/` subdirectory" (done) |