fix: Ensure safe ID management in WidgetIdScope during ImGui frame initialization

This commit is contained in:
scawful
2025-10-02 11:01:30 -04:00
parent b77bd201e2
commit 3c9669d062
2 changed files with 59 additions and 24 deletions

View File

@@ -34,6 +34,24 @@ bool IsTestCompleted(ImGuiTest* test) {
return test->Output.Status != ImGuiTestStatus_Queued && return test->Output.Status != ImGuiTestStatus_Queued &&
test->Output.Status != ImGuiTestStatus_Running; test->Output.Status != ImGuiTestStatus_Running;
} }
// Thread-safe state for Wait RPC communication
struct WaitState {
std::atomic<bool> condition_met{false};
std::mutex message_mutex;
std::string message;
void SetMessage(const std::string& msg) {
std::lock_guard<std::mutex> lock(message_mutex);
message = msg;
}
std::string GetMessage() {
std::lock_guard<std::mutex> lock(message_mutex);
return message;
}
};
} // namespace } // namespace
#endif #endif
@@ -444,41 +462,47 @@ absl::Status ImGuiTestHarnessServiceImpl::Wait(const WaitRequest* request,
int timeout_ms = request->timeout_ms() > 0 ? request->timeout_ms() : 5000; // Default 5s int timeout_ms = request->timeout_ms() > 0 ? request->timeout_ms() : 5000; // Default 5s
int poll_interval_ms = request->poll_interval_ms() > 0 ? request->poll_interval_ms() : 100; // Default 100ms int poll_interval_ms = request->poll_interval_ms() > 0 ? request->poll_interval_ms() : 100; // Default 100ms
// Create a dynamic test to poll the condition // Create thread-safe shared state for communication
bool condition_met = false; auto wait_state = std::make_shared<WaitState>();
std::string message;
auto test_data = std::make_shared<DynamicTestData>(); auto test_data = std::make_shared<DynamicTestData>();
test_data->test_func = [=, &condition_met, &message](ImGuiTestContext* ctx) { test_data->test_func = [wait_state, condition_type, condition_target,
timeout_ms, poll_interval_ms](ImGuiTestContext* ctx) {
try { try {
auto poll_start = std::chrono::steady_clock::now(); auto poll_start = std::chrono::steady_clock::now();
auto timeout = std::chrono::milliseconds(timeout_ms); auto timeout = std::chrono::milliseconds(timeout_ms);
// Give ImGui one frame to process the menu click and create windows
ctx->Yield();
while (std::chrono::steady_clock::now() - poll_start < timeout) { while (std::chrono::steady_clock::now() - poll_start < timeout) {
bool current_state = false; bool current_state = false;
// Check the condition type // Check the condition type using thread-safe ctx methods
if (condition_type == "window_visible") { if (condition_type == "window_visible") {
ImGuiWindow* window = ImGui::FindWindowByName(condition_target.c_str()); // Use ctx->WindowInfo instead of ImGui::FindWindowByName for thread safety
current_state = (window != nullptr && !window->Hidden); ImGuiTestItemInfo window_info = ctx->WindowInfo(condition_target.c_str(),
ImGuiTestOpFlags_NoError);
current_state = (window_info.ID != 0);
} else if (condition_type == "element_visible") { } else if (condition_type == "element_visible") {
ImGuiTestItemInfo item = ctx->ItemInfo(condition_target.c_str()); ImGuiTestItemInfo item = ctx->ItemInfo(condition_target.c_str());
current_state = (item.ID != 0 && item.RectClipped.GetWidth() > 0 && item.RectClipped.GetHeight() > 0); current_state = (item.ID != 0 && item.RectClipped.GetWidth() > 0 &&
item.RectClipped.GetHeight() > 0);
} else if (condition_type == "element_enabled") { } else if (condition_type == "element_enabled") {
ImGuiTestItemInfo item = ctx->ItemInfo(condition_target.c_str()); ImGuiTestItemInfo item = ctx->ItemInfo(condition_target.c_str());
current_state = (item.ID != 0 && !(item.ItemFlags & ImGuiItemFlags_Disabled)); current_state = (item.ID != 0 && !(item.ItemFlags & ImGuiItemFlags_Disabled));
} else { } else {
message = absl::StrFormat("Unknown condition type: %s", condition_type); wait_state->SetMessage(absl::StrFormat("Unknown condition type: %s", condition_type));
condition_met = false; wait_state->condition_met = false;
return; return;
} }
if (current_state) { if (current_state) {
condition_met = true; wait_state->condition_met = true;
message = absl::StrFormat("Condition '%s:%s' met after %lld ms", wait_state->SetMessage(absl::StrFormat("Condition '%s:%s' met after %lld ms",
condition_type, condition_target, condition_type, condition_target,
std::chrono::duration_cast<std::chrono::milliseconds>( std::chrono::duration_cast<std::chrono::milliseconds>(
std::chrono::steady_clock::now() - poll_start).count()); std::chrono::steady_clock::now() - poll_start).count()));
return; return;
} }
@@ -488,12 +512,12 @@ absl::Status ImGuiTestHarnessServiceImpl::Wait(const WaitRequest* request,
} }
// Timeout reached // Timeout reached
condition_met = false; wait_state->condition_met = false;
message = absl::StrFormat("Condition '%s:%s' not met after %d ms timeout", wait_state->SetMessage(absl::StrFormat("Condition '%s:%s' not met after %d ms timeout",
condition_type, condition_target, timeout_ms); condition_type, condition_target, timeout_ms));
} catch (const std::exception& e) { } catch (const std::exception& e) {
condition_met = false; wait_state->condition_met = false;
message = absl::StrFormat("Wait failed: %s", e.what()); wait_state->SetMessage(absl::StrFormat("Wait failed: %s", e.what()));
} }
}; };
@@ -513,14 +537,18 @@ absl::Status ImGuiTestHarnessServiceImpl::Wait(const WaitRequest* request,
auto wait_start = std::chrono::steady_clock::now(); auto wait_start = std::chrono::steady_clock::now();
while (!IsTestCompleted(test)) { while (!IsTestCompleted(test)) {
if (std::chrono::steady_clock::now() - wait_start > extended_timeout) { if (std::chrono::steady_clock::now() - wait_start > extended_timeout) {
condition_met = false; wait_state->condition_met = false;
message = "Test execution timeout"; wait_state->SetMessage("Test execution timeout");
break; break;
} }
// Yield to allow ImGui event processing // Yield to allow ImGui event processing
std::this_thread::sleep_for(std::chrono::milliseconds(100)); std::this_thread::sleep_for(std::chrono::milliseconds(100));
} }
// Read final state from thread-safe shared state
bool condition_met = wait_state->condition_met.load();
std::string message = wait_state->GetMessage();
// Check final test status // Check final test status
if (IsTestCompleted(test)) { if (IsTestCompleted(test)) {
if (test->Output.Status == ImGuiTestStatus_Success) { if (test->Output.Status == ImGuiTestStatus_Success) {

View File

@@ -8,6 +8,7 @@
#include "absl/strings/str_format.h" #include "absl/strings/str_format.h"
#include "absl/strings/str_join.h" #include "absl/strings/str_join.h"
#include "absl/strings/str_split.h" #include "absl/strings/str_split.h"
#include "imgui/imgui_internal.h" // For ImGuiContext internals
namespace yaze { namespace yaze {
namespace gui { namespace gui {
@@ -16,13 +17,19 @@ namespace gui {
thread_local std::vector<std::string> WidgetIdScope::id_stack_; thread_local std::vector<std::string> WidgetIdScope::id_stack_;
WidgetIdScope::WidgetIdScope(const std::string& name) : name_(name) { WidgetIdScope::WidgetIdScope(const std::string& name) : name_(name) {
ImGui::PushID(name.c_str()); // Only push ID if we're in an active ImGui frame with a valid window
id_stack_.push_back(name); // This prevents crashes during editor initialization before ImGui begins its frame
ImGuiContext* ctx = ImGui::GetCurrentContext();
if (ctx && ctx->CurrentWindow && !ctx->Windows.empty()) {
ImGui::PushID(name.c_str());
id_stack_.push_back(name);
}
} }
WidgetIdScope::~WidgetIdScope() { WidgetIdScope::~WidgetIdScope() {
ImGui::PopID(); // Only pop if we successfully pushed
if (!id_stack_.empty()) { if (!id_stack_.empty() && id_stack_.back() == name_) {
ImGui::PopID();
id_stack_.pop_back(); id_stack_.pop_back();
} }
} }