371 lines
9.4 KiB
Markdown
371 lines
9.4 KiB
Markdown
|
|
# Editor Core Refactor Plan
|
||
|
|
|
||
|
|
## Goal
|
||
|
|
|
||
|
|
This refactor turns the current `editor/app` directory map from a documented
|
||
|
|
convention into a build-checked architecture.
|
||
|
|
|
||
|
|
The desired production shape is:
|
||
|
|
|
||
|
|
```text
|
||
|
|
XCUIEditor
|
||
|
|
reusable editor UI framework:
|
||
|
|
widgets, docking, shell, workspace, menu, viewport slots, field controls
|
||
|
|
|
||
|
|
XCEditorCore
|
||
|
|
editor product core:
|
||
|
|
composition, commands, state, project and scene services, feature panels,
|
||
|
|
window/workspace core, and host-facing contracts
|
||
|
|
|
||
|
|
XCEditor
|
||
|
|
thin executable host:
|
||
|
|
main, Win32 process/window host, D3D12 runtime host, resources, startup,
|
||
|
|
final object wiring
|
||
|
|
```
|
||
|
|
|
||
|
|
`XCEditor` may keep the output name `XCEngine.exe`; the target name should
|
||
|
|
remain `XCEditor`.
|
||
|
|
|
||
|
|
## Current Problem
|
||
|
|
|
||
|
|
The reusable layer is mostly healthy:
|
||
|
|
|
||
|
|
- `editor/include/XCEditor/**` and `editor/src/**` form `XCUIEditor`.
|
||
|
|
- `XCUIEditor` is testable and mostly free of app, Win32, and D3D12 concerns.
|
||
|
|
|
||
|
|
The app layer has a good directory vocabulary but weak enforcement:
|
||
|
|
|
||
|
|
- `editor/app/Composition` directly includes concrete feature-panel contracts
|
||
|
|
from `editor/app/Features`.
|
||
|
|
- `editor/app/Features` also includes `Composition/EditorContext.h` and
|
||
|
|
`Composition/EditorPanelIds.h`, creating a composition/feature cycle.
|
||
|
|
- `editor/app/Project` depends on `Features/Project/ProjectBrowserModel.h`,
|
||
|
|
which makes a service layer depend on a feature UI model.
|
||
|
|
- Win32 and D3D12 currently know about each other through concrete headers.
|
||
|
|
- App-side tests exist but are not consistently wired into CMake; some are
|
||
|
|
stale and reference removed headers.
|
||
|
|
|
||
|
|
The root issue is not the existence of a single executable target by itself.
|
||
|
|
The root issue is that shared app contracts are stored inside concrete layer
|
||
|
|
directories, and CMake exposes the whole `editor/app` include root to all app
|
||
|
|
code. That makes the directory map advisory rather than enforceable.
|
||
|
|
|
||
|
|
## Target Directory Shape
|
||
|
|
|
||
|
|
The refactor should converge on this shape:
|
||
|
|
|
||
|
|
```text
|
||
|
|
editor/app/
|
||
|
|
Core/
|
||
|
|
Commands/
|
||
|
|
Panels/
|
||
|
|
State/
|
||
|
|
UtilityWindows/
|
||
|
|
WorkspacePanels/
|
||
|
|
|
||
|
|
Services/
|
||
|
|
Project/
|
||
|
|
Scene/
|
||
|
|
|
||
|
|
Features/
|
||
|
|
Console/
|
||
|
|
ColorPicker/
|
||
|
|
Hierarchy/
|
||
|
|
Inspector/
|
||
|
|
Project/
|
||
|
|
Scene/
|
||
|
|
|
||
|
|
Windowing/
|
||
|
|
Content/
|
||
|
|
Coordinator/
|
||
|
|
Frame/
|
||
|
|
Host/
|
||
|
|
Runtime/
|
||
|
|
|
||
|
|
Host/
|
||
|
|
Interfaces/
|
||
|
|
Win32/
|
||
|
|
D3D12/
|
||
|
|
|
||
|
|
Bootstrap/
|
||
|
|
Support/
|
||
|
|
```
|
||
|
|
|
||
|
|
This is a convergence target, not a requirement to move every file at once.
|
||
|
|
The first cut should only move contracts that are already acting as global app
|
||
|
|
interfaces.
|
||
|
|
|
||
|
|
## Dependency Rules
|
||
|
|
|
||
|
|
The final direction should be:
|
||
|
|
|
||
|
|
```text
|
||
|
|
XCEditor -> XCEditorCore -> XCUIEditor -> XCEngine
|
||
|
|
```
|
||
|
|
|
||
|
|
Within `XCEditorCore`:
|
||
|
|
|
||
|
|
- `Composition` may depend on `Core`, `Services`, `Windowing` contracts, and
|
||
|
|
`XCUIEditor`.
|
||
|
|
- `Composition` must not include concrete feature panel headers.
|
||
|
|
- `Features` may depend on `Core`, `Services`, and `XCUIEditor`.
|
||
|
|
- `Services` must not depend on `Features`.
|
||
|
|
- `State` must not depend on `Composition`.
|
||
|
|
- `Windowing` may depend on `Core`, `Composition` interfaces, and
|
||
|
|
`XCUIEditor`, but must not include Win32 or D3D12 concrete host headers.
|
||
|
|
- Host contracts may live in `Core` or `Host/Interfaces`.
|
||
|
|
- Win32 and D3D12 concrete implementations live outside `XCEditorCore` unless
|
||
|
|
they are purely abstract host contracts.
|
||
|
|
|
||
|
|
## Phase 1: Fix Shared Contract Placement
|
||
|
|
|
||
|
|
Move the most obviously misplaced shared contracts first.
|
||
|
|
|
||
|
|
### 1. Panel IDs
|
||
|
|
|
||
|
|
Move:
|
||
|
|
|
||
|
|
```text
|
||
|
|
editor/app/Composition/EditorPanelIds.h
|
||
|
|
```
|
||
|
|
|
||
|
|
to:
|
||
|
|
|
||
|
|
```text
|
||
|
|
editor/app/Core/Panels/EditorPanelIds.h
|
||
|
|
```
|
||
|
|
|
||
|
|
Then update all includes to use `Core/Panels/EditorPanelIds.h`.
|
||
|
|
|
||
|
|
Reason: panel IDs are global app model constants, not composition-private
|
||
|
|
implementation details.
|
||
|
|
|
||
|
|
### 2. Workspace Panel Runtime Contract
|
||
|
|
|
||
|
|
Split the existing workspace panel registry header:
|
||
|
|
|
||
|
|
Current:
|
||
|
|
|
||
|
|
```text
|
||
|
|
editor/app/Features/EditorWorkspacePanelRegistry.h
|
||
|
|
```
|
||
|
|
|
||
|
|
Target:
|
||
|
|
|
||
|
|
```text
|
||
|
|
editor/app/Core/WorkspacePanels/EditorWorkspacePanelRuntime.h
|
||
|
|
editor/app/Features/EditorWorkspacePanelRegistry.h
|
||
|
|
```
|
||
|
|
|
||
|
|
The new core header owns:
|
||
|
|
|
||
|
|
- `EditorWorkspacePanelCursorKind`
|
||
|
|
- `EditorWorkspacePanelUpdatePhase`
|
||
|
|
- `EditorWorkspacePanelFrameEvent`
|
||
|
|
- `EditorWorkspacePanelInitializationContext`
|
||
|
|
- `EditorWorkspacePanelShutdownContext`
|
||
|
|
- `EditorWorkspacePanelUpdateContext`
|
||
|
|
- `EditorWorkspacePanel`
|
||
|
|
- `EditorWorkspacePanelRuntimeSet`
|
||
|
|
|
||
|
|
The feature registry header owns only:
|
||
|
|
|
||
|
|
- `CreateEditorWorkspacePanelRuntimeSet()`
|
||
|
|
|
||
|
|
Reason: composition should depend on the panel runtime interface, not on the
|
||
|
|
feature registry. Concrete feature construction belongs to `Features`.
|
||
|
|
|
||
|
|
### 3. Keep the First Cut Buildable
|
||
|
|
|
||
|
|
This phase should not change behavior.
|
||
|
|
|
||
|
|
Expected behavioral diff: none.
|
||
|
|
|
||
|
|
Expected dependency improvement:
|
||
|
|
|
||
|
|
```text
|
||
|
|
Composition -> Core/WorkspacePanels
|
||
|
|
Features -> Core/WorkspacePanels
|
||
|
|
Features -> Composition only where concrete panels still need EditorContext
|
||
|
|
```
|
||
|
|
|
||
|
|
This does not fully solve all cycles, but it removes the worst misplaced public
|
||
|
|
contract and creates the landing zone for the next cut.
|
||
|
|
|
||
|
|
## Phase 2: Introduce XCEditorCore
|
||
|
|
|
||
|
|
Create:
|
||
|
|
|
||
|
|
```cmake
|
||
|
|
add_library(XCEditorCore STATIC ...)
|
||
|
|
```
|
||
|
|
|
||
|
|
Initial contents:
|
||
|
|
|
||
|
|
- `app/Core/**`
|
||
|
|
- `app/Commands/**`
|
||
|
|
- `app/Composition/**`
|
||
|
|
- `app/Features/**`
|
||
|
|
- `app/Project/**` or `app/Services/Project/**`
|
||
|
|
- `app/Scene/**` or `app/Services/Scene/**`
|
||
|
|
- `app/State/**`
|
||
|
|
- `app/UtilityWindows/**`
|
||
|
|
- `app/Windowing/**` core files that do not require Win32 or D3D12
|
||
|
|
- host-facing abstract interfaces
|
||
|
|
|
||
|
|
Keep in `XCEditor` executable:
|
||
|
|
|
||
|
|
- `app/main.cpp`
|
||
|
|
- `app/Bootstrap/Application.*`
|
||
|
|
- `app/Bootstrap/EditorApp.rc`
|
||
|
|
- `app/Platform/Win32/**`
|
||
|
|
- `app/Rendering/D3D12/**`
|
||
|
|
- any concrete host glue that includes `windows.h`
|
||
|
|
|
||
|
|
`XCEditor` links `XCEditorCore`, `XCUIEditor`, and concrete platform/rendering
|
||
|
|
libraries.
|
||
|
|
|
||
|
|
Important: do not hide Win32/D3D12 in `XCEditorCore` just to make the first
|
||
|
|
CMake edit easier. If a source file needs `windows.h`, it belongs in the host
|
||
|
|
side until a neutral interface exists.
|
||
|
|
|
||
|
|
## Phase 3: Restore App-Core Tests
|
||
|
|
|
||
|
|
Create or restore:
|
||
|
|
|
||
|
|
```cmake
|
||
|
|
editor_app_core_tests
|
||
|
|
```
|
||
|
|
|
||
|
|
This target should link:
|
||
|
|
|
||
|
|
```text
|
||
|
|
XCEditorCore
|
||
|
|
GTest::gtest_main
|
||
|
|
```
|
||
|
|
|
||
|
|
Start with tests that should not need Win32/D3D12:
|
||
|
|
|
||
|
|
- `test_editor_host_command_bridge.cpp`
|
||
|
|
- `test_editor_project_runtime.cpp`
|
||
|
|
- `test_editor_shell_asset_validation.cpp`
|
||
|
|
- `test_project_browser_model.cpp`
|
||
|
|
- `test_hierarchy_scene_binding.cpp`
|
||
|
|
- `test_inspector_presentation.cpp`
|
||
|
|
|
||
|
|
Then either fix or remove stale test references:
|
||
|
|
|
||
|
|
- `Ports/SystemInteractionPort.h`
|
||
|
|
- `Rendering/Viewport/ViewportRenderTargetInternal.h`
|
||
|
|
|
||
|
|
The test target is part of the architecture. If app core cannot be tested
|
||
|
|
without starting the executable host, the boundary is not real.
|
||
|
|
|
||
|
|
## Phase 4: Service and Feature Boundary Cleanup
|
||
|
|
|
||
|
|
After `XCEditorCore` exists, remove remaining service-to-feature dependencies.
|
||
|
|
|
||
|
|
### Project
|
||
|
|
|
||
|
|
Current problem:
|
||
|
|
|
||
|
|
```text
|
||
|
|
Project/EditorProjectRuntime.h -> Features/Project/ProjectBrowserModel.h
|
||
|
|
```
|
||
|
|
|
||
|
|
Target:
|
||
|
|
|
||
|
|
```text
|
||
|
|
Services/Project/ProjectBrowserModel.h
|
||
|
|
Services/Project/EditorProjectRuntime.h
|
||
|
|
Features/Project/ProjectPanel.h
|
||
|
|
```
|
||
|
|
|
||
|
|
The browser model can be owned by project services if it is truly the domain
|
||
|
|
model. If it is panel-specific presentation state, split it into a service
|
||
|
|
model and a panel view model.
|
||
|
|
|
||
|
|
### Scene
|
||
|
|
|
||
|
|
Keep scene runtime as a service layer. Scene viewport feature code may use
|
||
|
|
scene services, but scene services should not depend on concrete feature UI.
|
||
|
|
|
||
|
|
### Utility Windows
|
||
|
|
|
||
|
|
Move utility descriptors and kinds to `Core/UtilityWindows` if they are shared
|
||
|
|
by windowing and features. Concrete utility panel creation remains in
|
||
|
|
`Features`.
|
||
|
|
|
||
|
|
## Phase 5: Host Boundary Cleanup
|
||
|
|
|
||
|
|
Split neutral host contracts from concrete implementations.
|
||
|
|
|
||
|
|
Target:
|
||
|
|
|
||
|
|
```text
|
||
|
|
app/Host/Interfaces/
|
||
|
|
EditorWindowHostInterfaces.h
|
||
|
|
EditorWindowRenderRuntime.h
|
||
|
|
UiTextureHost.h
|
||
|
|
ViewportRenderHost.h
|
||
|
|
SystemInteractionService.h
|
||
|
|
|
||
|
|
app/Host/Win32/
|
||
|
|
Win32SystemInteractionHost.*
|
||
|
|
EditorWindow.*
|
||
|
|
message dispatch, DPI, chrome, pointer capture
|
||
|
|
|
||
|
|
app/Host/D3D12/
|
||
|
|
D3D12EditorWindowRenderRuntime.*
|
||
|
|
D3D12 UI renderer, texture host, text system, swap chain presenter
|
||
|
|
```
|
||
|
|
|
||
|
|
Then replace concrete Win32/D3D12 cross-includes with a neutral surface or
|
||
|
|
factory contract.
|
||
|
|
|
||
|
|
## Phase 6: Documentation Update
|
||
|
|
|
||
|
|
Update `editor/AGENTS.md` after each completed boundary cut.
|
||
|
|
|
||
|
|
Required changes:
|
||
|
|
|
||
|
|
- production target shape becomes `XCUIEditor`, `XCEditorCore`, `XCEditor`
|
||
|
|
- `Core` owns shared app contracts
|
||
|
|
- `Features/EditorWorkspacePanelRegistry.*` owns concrete feature factories
|
||
|
|
- app tests link `XCEditorCore`
|
||
|
|
- no new app code should add direct `Composition <-> Features` cycles
|
||
|
|
|
||
|
|
## Validation
|
||
|
|
|
||
|
|
Run these after each phase where possible:
|
||
|
|
|
||
|
|
```powershell
|
||
|
|
cmake --build build --config Debug --target XCUIEditor
|
||
|
|
cmake --build build --config Debug --target XCEditorCore
|
||
|
|
cmake --build build --config Debug --target XCEditor
|
||
|
|
cmake --build build --config Debug --target editor_ui_tests
|
||
|
|
cmake --build build --config Debug --target editor_app_core_tests
|
||
|
|
```
|
||
|
|
|
||
|
|
When app smoke is available:
|
||
|
|
|
||
|
|
```powershell
|
||
|
|
ctest -C Debug -R xceditor_smoke --output-on-failure
|
||
|
|
```
|
||
|
|
|
||
|
|
If `XCEditorCore` does not exist yet, skip that target until Phase 2 lands.
|
||
|
|
|
||
|
|
## Done Criteria
|
||
|
|
|
||
|
|
The refactor is complete when:
|
||
|
|
|
||
|
|
- `XCEditorCore.lib` exists and is linked by `XCEditor`.
|
||
|
|
- `XCEditor` executable source is limited to host startup and concrete
|
||
|
|
platform/render backend wiring.
|
||
|
|
- `Composition` no longer includes concrete feature panel headers.
|
||
|
|
- `Services` no longer include `Features/**`.
|
||
|
|
- Win32 and D3D12 communicate through neutral host/render contracts.
|
||
|
|
- app-core tests are wired into CMake and build without running the executable.
|
||
|
|
- `editor/AGENTS.md` describes the new target shape and directory rules.
|
||
|
|
|