diff --git a/RHI_Design_Issues.md b/RHI_Design_Issues.md index 3749ebda..d205b879 100644 --- a/RHI_Design_Issues.md +++ b/RHI_Design_Issues.md @@ -402,9 +402,40 @@ virtual void ResourceBarrier(uint32_t count, const ResourceBarrier* barriers) = 详见第一版文档，此处不再赘述。 -### 4.3 OpenGL 特有方法暴露 +### 4.3 OpenGL 特有方法暴露 ✅ 已完成 -详见第一版文档，此处不再赘述。 +**实现状态**：2026-03-25 已完成 + +**问题**：OpenGLDevice 和 OpenGLCommandList 暴露了 OpenGL 特有方法，违反"上层只调用抽象接口"原则。 + +**修复内容**： + +1. **OpenGLDevice 内部化方法**： + - 移除 `GetTextureUnitAllocator()` 和 `GetUniformBufferManager()` 公开访问（移到 private） + - 移除 `SwapBuffers()` 公开方法（OpenGLSwapChain 作为 friend 仍可访问） + - `GetPresentationDC()` 和 `GetGLContext()` 保留在 private，通过 friend 访问 + +2. **新增抽象接口**： + - `MakeContextCurrent()` - 封装 `wglMakeCurrent` 操作 + - `GetNativeContext()` - 返回 GL Context 供 RenderDoc 使用 + +3. **保留的逃生舱方法**： + - OpenGLCommandList 中的 `PrimitiveType` 相关方法是显式的 OpenGL 逃生舱 + - 这些方法使用 OpenGL 特有类型，文档化为"backend-specific escape hatch" + - 抽象层正确使用 `PrimitiveTopology` 枚举 + +**修改的文件**： +- `engine/include/XCEngine/RHI/OpenGL/OpenGLDevice.h` - 内部化特有方法，新增 MakeContextCurrent/GetNativeContext +- `engine/src/RHI/OpenGL/OpenGLDevice.cpp` - 实现 MakeContextCurrent +- `tests/RHI/OpenGL/unit/test_device.cpp` - 移除 SwapBuffers 测试 +- `tests/RHI/OpenGL/integration/minimal/main.cpp` - 使用 MakeContextCurrent +- `tests/RHI/OpenGL/integration/triangle/main.cpp` - 使用 MakeContextCurrent +- `tests/RHI/OpenGL/integration/quad/main.cpp` - 使用 MakeContextCurrent +- `tests/RHI/OpenGL/integration/sphere/main.cpp` - 使用 MakeContextCurrent + +**说明**： +- `PrimitiveType` 枚举在 OpenGL 专用逃生舱方法中使用是可接受的，因为这些是显式的后端特定接口 +- 抽象层正确使用 `PrimitiveTopology`，不存在类型泄漏问题 ### 4.4 缺少 Compute Pipeline 抽象 @@ -562,7 +593,7 @@ class RHITexture : public RHIResource { ... }; | 4 | ResourceView 类型不明确 | 🟡 中 | 中 | ✅ 基本完成 | | 5 | TransitionBarrier 针对 View 而非 Resource | 🟡 中 | 中 | ✅ 已完成 | | 6 | SetGlobal* 空操作 | 🟡 中 | 低 | ✅ 已完成 | -| 7 | OpenGL 特有方法暴露 | 🟡 中 | 高 | ❌ 未完成 | +| 7 | OpenGL 特有方法暴露 | 🟡 中 | 高 | ✅ 已完成 | | 8 | 缺少 Compute Pipeline 抽象 | 🟡 中 | 中 | ✅ 已完成 | --- diff --git a/engine/include/XCEngine/RHI/OpenGL/OpenGLDevice.h b/engine/include/XCEngine/RHI/OpenGL/OpenGLDevice.h index 9600299d..57c6b4a6 100644 --- a/engine/include/XCEngine/RHI/OpenGL/OpenGLDevice.h +++ b/engine/include/XCEngine/RHI/OpenGL/OpenGLDevice.h @@ -29,14 +29,10 @@ public: void Shutdown() override; bool InitializeWithExistingWindow(HWND hwnd); - HDC GetPresentationDC() const { return m_hdc; } - HGLRC GetGLContext() const { return m_hglrc; } const RHIDeviceInfo& GetDeviceInfoImpl() const { return m_deviceInfo; } - OpenGLTextureUnitAllocator* GetTextureUnitAllocator() { return m_textureUnitAllocator.get(); } - OpenGLUniformBufferManager* GetUniformBufferManager() { return m_uniformBufferManager.get(); } - - void SwapBuffers(); + bool MakeContextCurrent(); + void* GetNativeContext() const { return m_hglrc; } RHIBuffer* CreateBuffer(const BufferDesc& desc) override; RHITexture* CreateTexture(const TextureDesc& desc) override; @@ -66,6 +62,11 @@ public: private: friend class OpenGLSwapChain; + HDC GetPresentationDC() const { return m_hdc; } + OpenGLTextureUnitAllocator* GetTextureUnitAllocator() { return m_textureUnitAllocator.get(); } + OpenGLUniformBufferManager* GetUniformBufferManager() { return m_uniformBufferManager.get(); } + void SwapBuffers(); + HWND m_hwnd = nullptr; HDC m_hdc = nullptr; HGLRC m_hglrc = nullptr; diff --git a/engine/src/RHI/OpenGL/OpenGLDevice.cpp b/engine/src/RHI/OpenGL/OpenGLDevice.cpp index 0c2499c3..41ca7473 100644 --- a/engine/src/RHI/OpenGL/OpenGLDevice.cpp +++ b/engine/src/RHI/OpenGL/OpenGLDevice.cpp @@ -259,6 +259,13 @@ void OpenGLDevice::Shutdown() { m_initialized = false; } +bool OpenGLDevice::MakeContextCurrent() { + if (m_hdc && m_hglrc) { + return ::wglMakeCurrent(m_hdc, m_hglrc) == TRUE; + } + return false; +} + void OpenGLDevice::SwapBuffers() { if (m_hdc) { ::SwapBuffers(m_hdc); diff --git a/tests/RHI/OpenGL/integration/minimal/main.cpp b/tests/RHI/OpenGL/integration/minimal/main.cpp index 7871429c..7581d9f9 100644 --- a/tests/RHI/OpenGL/integration/minimal/main.cpp +++ b/tests/RHI/OpenGL/integration/minimal/main.cpp @@ -91,7 +91,7 @@ int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine return -1; } - RenderDocCapture::Get().SetDevice(device.GetGLContext()); + RenderDocCapture::Get().SetDevice(device.GetNativeContext()); ShowWindow(hwnd, nShowCmd); UpdateWindow(hwnd); @@ -117,7 +117,7 @@ int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine TranslateMessage(&msg); DispatchMessageW(&msg); } else { - wglMakeCurrent(device.GetPresentationDC(), device.GetGLContext()); + device.MakeContextCurrent(); commandList.SetViewport(0, 0, gWidth, gHeight); commandList.Clear(1.0f, 0.0f, 0.0f, 1.0f, 1 | 2); diff --git a/tests/RHI/OpenGL/integration/quad/main.cpp b/tests/RHI/OpenGL/integration/quad/main.cpp index 6fd673ca..c7c93c99 100644 --- a/tests/RHI/OpenGL/integration/quad/main.cpp +++ b/tests/RHI/OpenGL/integration/quad/main.cpp @@ -95,7 +95,7 @@ int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine return -1; } - RenderDocCapture::Get().SetDevice(device.GetGLContext()); + RenderDocCapture::Get().SetDevice(device.GetNativeContext()); ShowWindow(hwnd, nShowCmd); UpdateWindow(hwnd); @@ -202,7 +202,7 @@ int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine TranslateMessage(&msg); DispatchMessageW(&msg); } else { - wglMakeCurrent(device.GetPresentationDC(), device.GetGLContext()); + device.MakeContextCurrent(); commandList.SetViewport(0, 0, gWidth, gHeight); commandList.Clear(0.0f, 0.0f, 1.0f, 1.0f, 1); diff --git a/tests/RHI/OpenGL/integration/sphere/main.cpp b/tests/RHI/OpenGL/integration/sphere/main.cpp index 1e03a400..c64b58bf 100644 --- a/tests/RHI/OpenGL/integration/sphere/main.cpp +++ b/tests/RHI/OpenGL/integration/sphere/main.cpp @@ -174,7 +174,7 @@ int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine return -1; } - RenderDocCapture::Get().SetDevice(device.GetGLContext()); + RenderDocCapture::Get().SetDevice(device.GetNativeContext()); ShowWindow(hwnd, nShowCmd); UpdateWindow(hwnd); @@ -312,7 +312,7 @@ int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine TranslateMessage(&msg); DispatchMessageW(&msg); } else { - wglMakeCurrent(device.GetPresentationDC(), device.GetGLContext()); + device.MakeContextCurrent(); renderCount++; GLenum err = glGetError(); diff --git a/tests/RHI/OpenGL/integration/triangle/main.cpp b/tests/RHI/OpenGL/integration/triangle/main.cpp index 35d9d1bf..ac87d1dc 100644 --- a/tests/RHI/OpenGL/integration/triangle/main.cpp +++ b/tests/RHI/OpenGL/integration/triangle/main.cpp @@ -91,7 +91,7 @@ int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine return -1; } - RenderDocCapture::Get().SetDevice(device.GetGLContext()); + RenderDocCapture::Get().SetDevice(device.GetNativeContext()); ShowWindow(hwnd, nShowCmd); UpdateWindow(hwnd); @@ -179,7 +179,7 @@ int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine TranslateMessage(&msg); DispatchMessageW(&msg); } else { - wglMakeCurrent(device.GetPresentationDC(), device.GetGLContext()); + device.MakeContextCurrent(); commandList.SetViewport(0, 0, gWidth, gHeight); commandList.Clear(0.0f, 0.0f, 1.0f, 1.0f, 1); diff --git a/tests/RHI/OpenGL/unit/test_device.cpp b/tests/RHI/OpenGL/unit/test_device.cpp index 1985259e..926f8e5f 100644 --- a/tests/RHI/OpenGL/unit/test_device.cpp +++ b/tests/RHI/OpenGL/unit/test_device.cpp @@ -22,14 +22,4 @@ TEST_F(OpenGLTestFixture, Device_GetDeviceInfo_ReturnsValid) { EXPECT_FALSE(info.renderer.empty()); EXPECT_GE(info.majorVersion, 3); EXPECT_GE(info.minorVersion, 0); -} - -TEST_F(OpenGLTestFixture, Device_SwapBuffers_NoErrors) { - OpenGLDevice device; - device.InitializeWithExistingWindow(GetWindow()); - - device.SwapBuffers(); - - GLenum error = glGetError(); - EXPECT_EQ(error, GL_NO_ERROR); } \ No newline at end of file