summaryrefslogtreecommitdiff
path: root/PLATFORM_ANALYSIS.md
blob: eefbd424a6137af258cd81bce241dc5d112ce02f (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
# 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)