diff --git a/editor/src/Viewport/ViewportHostService.h b/editor/src/Viewport/ViewportHostService.h index a3b02ef3..503e86d1 100644 --- a/editor/src/Viewport/ViewportHostService.h +++ b/editor/src/Viewport/ViewportHostService.h @@ -6,6 +6,7 @@ #include "IViewportHostService.h" #include "SceneViewportPicker.h" #include "SceneViewportCameraController.h" +#include "ViewportObjectIdPicker.h" #include "ViewportHostSurfaceUtils.h" #include "UI/ImGuiBackendBridge.h" @@ -15,7 +16,6 @@ #include #include #include -#include #include #include #include @@ -876,46 +876,34 @@ private: const ImVec2& viewportSize, const ImVec2& viewportMousePosition, uint64_t& outEntityId) { - outEntityId = 0; - - if (m_device == nullptr || - m_sceneViewLastRenderContext.commandQueue == nullptr || - entry.objectIdTexture == nullptr || - !entry.hasValidObjectIdFrame || - viewportSize.x <= 1.0f || - viewportSize.y <= 1.0f || - viewportMousePosition.x < 0.0f || - viewportMousePosition.y < 0.0f || - viewportMousePosition.x > viewportSize.x || - viewportMousePosition.y > viewportSize.y) { + if (m_device == nullptr) { + outEntityId = 0; return false; } - auto* commandQueue = - m_sceneViewLastRenderContext.commandQueue; - if (commandQueue == nullptr) { - return false; - } + ViewportObjectIdPickContext pickContext = {}; + pickContext.commandQueue = m_sceneViewLastRenderContext.commandQueue; + pickContext.texture = entry.objectIdTexture; + pickContext.textureState = entry.objectIdState; + pickContext.textureWidth = entry.width; + pickContext.textureHeight = entry.height; + pickContext.hasValidFrame = entry.hasValidObjectIdFrame; + pickContext.viewportSize = viewportSize; + pickContext.viewportMousePosition = viewportMousePosition; - const uint32_t pixelX = ClampViewportPixelCoordinate(viewportMousePosition.x, entry.width); - const uint32_t pixelY = ClampViewportPixelCoordinate(viewportMousePosition.y, entry.height); - std::array rgba = {}; - if (!m_device->ReadTexturePixelRGBA8( - commandQueue, - entry.objectIdTexture, - entry.objectIdState, - pixelX, - pixelY, - rgba)) { - return false; - } - - outEntityId = static_cast(Rendering::DecodeObjectIdFromColor( - rgba[0], - rgba[1], - rgba[2], - rgba[3])); - return true; + return TryPickViewportObjectIdEntity( + pickContext, + [this](const ViewportObjectIdReadbackRequest& request, std::array& outRgba) { + return m_device != nullptr && + m_device->ReadTexturePixelRGBA8( + request.commandQueue, + request.texture, + request.textureState, + request.pixelX, + request.pixelY, + outRgba); + }, + outEntityId); } void DestroyViewportResources(ViewportEntry& entry) { diff --git a/editor/src/Viewport/ViewportObjectIdPicker.h b/editor/src/Viewport/ViewportObjectIdPicker.h new file mode 100644 index 00000000..4f497449 --- /dev/null +++ b/editor/src/Viewport/ViewportObjectIdPicker.h @@ -0,0 +1,95 @@ +#pragma once + +#include "ViewportHostSurfaceUtils.h" + +#include +#include +#include +#include + +#include +#include + +namespace XCEngine { +namespace Editor { + +struct ViewportObjectIdPickContext { + RHI::RHICommandQueue* commandQueue = nullptr; + RHI::RHITexture* texture = nullptr; + RHI::ResourceStates textureState = RHI::ResourceStates::Common; + uint32_t textureWidth = 0; + uint32_t textureHeight = 0; + bool hasValidFrame = false; + ImVec2 viewportSize = ImVec2(0.0f, 0.0f); + ImVec2 viewportMousePosition = ImVec2(0.0f, 0.0f); +}; + +struct ViewportObjectIdReadbackRequest { + RHI::RHICommandQueue* commandQueue = nullptr; + RHI::RHITexture* texture = nullptr; + RHI::ResourceStates textureState = RHI::ResourceStates::Common; + uint32_t pixelX = 0; + uint32_t pixelY = 0; +}; + +inline bool CanPickViewportObjectId(const ViewportObjectIdPickContext& context) { + return context.commandQueue != nullptr && + context.texture != nullptr && + context.textureWidth > 0 && + context.textureHeight > 0 && + context.hasValidFrame && + context.viewportSize.x > 1.0f && + context.viewportSize.y > 1.0f && + context.viewportMousePosition.x >= 0.0f && + context.viewportMousePosition.y >= 0.0f && + context.viewportMousePosition.x <= context.viewportSize.x && + context.viewportMousePosition.y <= context.viewportSize.y; +} + +inline bool BuildViewportObjectIdReadbackRequest( + const ViewportObjectIdPickContext& context, + ViewportObjectIdReadbackRequest& outRequest) { + outRequest = {}; + if (!CanPickViewportObjectId(context)) { + return false; + } + + outRequest.commandQueue = context.commandQueue; + outRequest.texture = context.texture; + outRequest.textureState = context.textureState; + outRequest.pixelX = ClampViewportPixelCoordinate( + context.viewportMousePosition.x, + context.textureWidth); + outRequest.pixelY = ClampViewportPixelCoordinate( + context.viewportMousePosition.y, + context.textureHeight); + return true; +} + +template +bool TryPickViewportObjectIdEntity( + const ViewportObjectIdPickContext& context, + ReadPixelFn&& readPixel, + uint64_t& outEntityId) { + outEntityId = 0; + + ViewportObjectIdReadbackRequest request = {}; + if (!BuildViewportObjectIdReadbackRequest(context, request)) { + return false; + } + + std::array rgba = {}; + if (!readPixel(request, rgba)) { + return false; + } + + outEntityId = static_cast(Rendering::DecodeObjectIdFromColor( + rgba[0], + rgba[1], + rgba[2], + rgba[3])); + return true; +} + +} // namespace Editor +} // namespace XCEngine diff --git a/tests/editor/CMakeLists.txt b/tests/editor/CMakeLists.txt index f937d953..7496e91b 100644 --- a/tests/editor/CMakeLists.txt +++ b/tests/editor/CMakeLists.txt @@ -11,6 +11,7 @@ set(EDITOR_TEST_SOURCES test_scene_viewport_picker.cpp test_scene_viewport_overlay_renderer.cpp test_viewport_host_surface_utils.cpp + test_viewport_object_id_picker.cpp test_builtin_icon_layout_utils.cpp ${CMAKE_SOURCE_DIR}/editor/src/Core/UndoManager.cpp ${CMAKE_SOURCE_DIR}/editor/src/Managers/SceneManager.cpp diff --git a/tests/editor/test_viewport_object_id_picker.cpp b/tests/editor/test_viewport_object_id_picker.cpp new file mode 100644 index 00000000..d7b2713d --- /dev/null +++ b/tests/editor/test_viewport_object_id_picker.cpp @@ -0,0 +1,115 @@ +#include + +#include "Viewport/ViewportObjectIdPicker.h" + +#include + +namespace { + +using XCEngine::Editor::BuildViewportObjectIdReadbackRequest; +using XCEngine::Editor::CanPickViewportObjectId; +using XCEngine::Editor::TryPickViewportObjectIdEntity; +using XCEngine::Editor::ViewportObjectIdPickContext; +using XCEngine::Editor::ViewportObjectIdReadbackRequest; +using XCEngine::RHI::RHICommandQueue; +using XCEngine::RHI::RHITexture; +using XCEngine::RHI::ResourceStates; + +RHICommandQueue* MakeDummyQueue() { + return reinterpret_cast(static_cast(0x1)); +} + +RHITexture* MakeDummyTexture() { + return reinterpret_cast(static_cast(0x2)); +} + +ViewportObjectIdPickContext CreateValidContext() { + ViewportObjectIdPickContext context = {}; + context.commandQueue = MakeDummyQueue(); + context.texture = MakeDummyTexture(); + context.textureState = ResourceStates::PixelShaderResource; + context.textureWidth = 1280; + context.textureHeight = 720; + context.hasValidFrame = true; + context.viewportSize = ImVec2(1280.0f, 720.0f); + context.viewportMousePosition = ImVec2(640.0f, 360.0f); + return context; +} + +TEST(ViewportObjectIdPickerTest, CanPickRejectsMissingOrOutOfBoundsInputs) { + ViewportObjectIdPickContext context = CreateValidContext(); + EXPECT_TRUE(CanPickViewportObjectId(context)); + + context.commandQueue = nullptr; + EXPECT_FALSE(CanPickViewportObjectId(context)); + + context = CreateValidContext(); + context.texture = nullptr; + EXPECT_FALSE(CanPickViewportObjectId(context)); + + context = CreateValidContext(); + context.hasValidFrame = false; + EXPECT_FALSE(CanPickViewportObjectId(context)); + + context = CreateValidContext(); + context.textureWidth = 0; + EXPECT_FALSE(CanPickViewportObjectId(context)); + + context = CreateValidContext(); + context.viewportMousePosition = ImVec2(-1.0f, 10.0f); + EXPECT_FALSE(CanPickViewportObjectId(context)); + + context = CreateValidContext(); + context.viewportMousePosition = ImVec2(10.0f, 721.0f); + EXPECT_FALSE(CanPickViewportObjectId(context)); +} + +TEST(ViewportObjectIdPickerTest, BuildReadbackRequestMapsViewportCoordinatesToTexturePixels) { + ViewportObjectIdPickContext context = CreateValidContext(); + context.viewportMousePosition = ImVec2(1280.0f, 720.0f); + + ViewportObjectIdReadbackRequest request = {}; + ASSERT_TRUE(BuildViewportObjectIdReadbackRequest(context, request)); + EXPECT_EQ(request.commandQueue, context.commandQueue); + EXPECT_EQ(request.texture, context.texture); + EXPECT_EQ(request.textureState, ResourceStates::PixelShaderResource); + EXPECT_EQ(request.pixelX, 1279u); + EXPECT_EQ(request.pixelY, 719u); +} + +TEST(ViewportObjectIdPickerTest, TryPickViewportObjectIdEntityDecodesReadbackColor) { + const ViewportObjectIdPickContext context = CreateValidContext(); + + uint64_t entityId = 0; + ViewportObjectIdReadbackRequest capturedRequest = {}; + const bool picked = TryPickViewportObjectIdEntity( + context, + [&capturedRequest](const ViewportObjectIdReadbackRequest& request, std::array& outRgba) { + capturedRequest = request; + outRgba = { 0x78, 0x56, 0x34, 0x12 }; + return true; + }, + entityId); + + EXPECT_TRUE(picked); + EXPECT_EQ(entityId, 0x12345678ull); + EXPECT_EQ(capturedRequest.pixelX, 640u); + EXPECT_EQ(capturedRequest.pixelY, 360u); +} + +TEST(ViewportObjectIdPickerTest, TryPickViewportObjectIdEntityFailsWhenReadbackFails) { + const ViewportObjectIdPickContext context = CreateValidContext(); + + uint64_t entityId = 99; + const bool picked = TryPickViewportObjectIdEntity( + context, + [](const ViewportObjectIdReadbackRequest&, std::array&) { + return false; + }, + entityId); + + EXPECT_FALSE(picked); + EXPECT_EQ(entityId, 0u); +} + +} // namespace