From 6e87302d5e6f0a7d1fcc27cc806b92afc406f290 Mon Sep 17 00:00:00 2001 From: Clinton Stimpson Date: Wed, 18 Sep 2024 16:43:44 -0600 Subject: [PATCH 1/4] Add vtkWindow::EnsureDisplay for checking display connections For X: - vtkXRenderWindowInteractor no longer owns a Display - vtkXOpenGLRenderWindow implements EnsureDisplay --- Common/Core/vtkWindow.h | 2 + Rendering/OpenGL2/vtkXOpenGLRenderWindow.cxx | 103 ++++++------------- Rendering/OpenGL2/vtkXOpenGLRenderWindow.h | 5 + Rendering/UI/vtkXRenderWindowInteractor.cxx | 18 +--- Rendering/UI/vtkXRenderWindowInteractor.h | 1 - 5 files changed, 42 insertions(+), 87 deletions(-) diff --git a/Common/Core/vtkWindow.h b/Common/Core/vtkWindow.h index fdf016fff79..5c83dc49eb8 100644 --- a/Common/Core/vtkWindow.h +++ b/Common/Core/vtkWindow.h @@ -43,6 +43,8 @@ public: virtual void* GetGenericDrawable() { return nullptr; } virtual void SetWindowInfo(const char*) {} virtual void SetParentInfo(const char*) {} + virtual bool EnsureDisplay() { return true; } + ///@} ///@{ diff --git a/Rendering/OpenGL2/vtkXOpenGLRenderWindow.cxx b/Rendering/OpenGL2/vtkXOpenGLRenderWindow.cxx index ca6c8c456e7..ada7f72a057 100644 --- a/Rendering/OpenGL2/vtkXOpenGLRenderWindow.cxx +++ b/Rendering/OpenGL2/vtkXOpenGLRenderWindow.cxx @@ -208,18 +208,9 @@ vtkXVisualInfo* vtkXOpenGLRenderWindow::GetDesiredVisualInfo() XVisualInfo* v = nullptr; // get the default display connection - if (!this->DisplayId) + if (!this->EnsureDisplay()) { - this->DisplayId = XOpenDisplay(static_cast(nullptr)); - - if (this->DisplayId == nullptr) - { - vtkWarningMacro(<< "bad X server connection. DISPLAY=" - << vtksys::SystemTools::GetEnv("DISPLAY")); - return nullptr; - } - - this->OwnDisplay = 1; + return nullptr; } this->Internal->FBConfig = vtkXOpenGLRenderWindowGetDesiredFBConfig(this->DisplayId, this->StereoCapableWindow, @@ -434,16 +425,9 @@ void vtkXOpenGLRenderWindow::CreateAWindow() xsh.height = height; // get the default display connection - if (!this->DisplayId) + if (!this->EnsureDisplay()) { - this->DisplayId = XOpenDisplay(static_cast(nullptr)); - if (this->DisplayId == nullptr) - { - vtkWarningMacro(<< "bad X server connection. DISPLAY=" - << vtksys::SystemTools::GetEnv("DISPLAY")); - return; - } - this->OwnDisplay = 1; + return; } attr.override_redirect = False; @@ -1250,21 +1234,11 @@ vtkTypeBool vtkXOpenGLRenderWindow::GetEventPending() int* vtkXOpenGLRenderWindow::GetScreenSize() { // get the default display connection - if (!this->DisplayId) + if (!this->EnsureDisplay()) { - this->DisplayId = XOpenDisplay(static_cast(nullptr)); - if (this->DisplayId == nullptr) - { - vtkWarningMacro(<< "bad X server connection. DISPLAY=" - << vtksys::SystemTools::GetEnv("DISPLAY")); - this->ScreenSize[0] = 0; - this->ScreenSize[1] = 0; - return this->ScreenSize; - } - else - { - this->OwnDisplay = 1; - } + this->ScreenSize[0] = 0; + this->ScreenSize[1] = 0; + return this->ScreenSize; } this->ScreenSize[0] = XDisplayWidth(this->DisplayId, XDefaultScreen(this->DisplayId)); @@ -1305,6 +1279,24 @@ Display* vtkXOpenGLRenderWindow::GetDisplayId() return this->DisplayId; } +bool vtkXOpenGLRenderWindow::EnsureDisplay() +{ + if (!this->DisplayId) + { + this->DisplayId = XOpenDisplay(static_cast(nullptr)); + if (this->DisplayId == nullptr) + { + vtkWarningMacro(<< "bad X server connection. DISPLAY=" + << vtksys::SystemTools::GetEnv("DISPLAY")); + } + else + { + this->OwnDisplay = 1; + } + } + return this->DisplayId != nullptr; +} + // Get this RenderWindow's parent X window id. Window vtkXOpenGLRenderWindow::GetParentId() { @@ -1369,26 +1361,11 @@ void vtkXOpenGLRenderWindow::SetWindowId(Window arg) // Set this RenderWindow's X window id to a pre-existing window. void vtkXOpenGLRenderWindow::SetWindowInfo(const char* info) { - int tmp; - - // get the default display connection - if (!this->DisplayId) - { - this->DisplayId = XOpenDisplay(static_cast(nullptr)); - if (this->DisplayId == nullptr) - { - vtkWarningMacro(<< "bad X server connection. DISPLAY=" - << vtksys::SystemTools::GetEnv("DISPLAY")); - return; - } - else - { - this->OwnDisplay = 1; - } - } + // note: potential Display/Window mismatch here + this->EnsureDisplay(); + int tmp; sscanf(info, "%i", &tmp); - this->SetWindowId(static_cast(tmp)); } @@ -1397,33 +1374,17 @@ void vtkXOpenGLRenderWindow::SetNextWindowInfo(const char* info) { int tmp; sscanf(info, "%i", &tmp); - this->SetNextWindowId(static_cast(tmp)); } // Sets the X window id of the window that WILL BE created. void vtkXOpenGLRenderWindow::SetParentInfo(const char* info) { - int tmp; - - // get the default display connection - if (!this->DisplayId) - { - this->DisplayId = XOpenDisplay(static_cast(nullptr)); - if (this->DisplayId == nullptr) - { - vtkWarningMacro(<< "bad X server connection. DISPLAY=" - << vtksys::SystemTools::GetEnv("DISPLAY")); - return; - } - else - { - this->OwnDisplay = 1; - } - } + // note: potential Display/Window mismatch here + this->EnsureDisplay(); + int tmp; sscanf(info, "%i", &tmp); - this->SetParentId(static_cast(tmp)); } diff --git a/Rendering/OpenGL2/vtkXOpenGLRenderWindow.h b/Rendering/OpenGL2/vtkXOpenGLRenderWindow.h index c594a075c00..501ffdb0f59 100644 --- a/Rendering/OpenGL2/vtkXOpenGLRenderWindow.h +++ b/Rendering/OpenGL2/vtkXOpenGLRenderWindow.h @@ -184,6 +184,11 @@ public: VTK_MARSHALEXCLUDE(VTK_MARSHAL_EXCLUDE_REASON_NOT_SUPPORTED) Display* GetDisplayId(); + /** + * Ensure RenderWindow's X display is opened + */ + bool EnsureDisplay() override; + ///@{ /** * Set the X display id for this RenderWindow to use to a pre-existing diff --git a/Rendering/UI/vtkXRenderWindowInteractor.cxx b/Rendering/UI/vtkXRenderWindowInteractor.cxx index bff1d8c6c7a..878a5662d79 100644 --- a/Rendering/UI/vtkXRenderWindowInteractor.cxx +++ b/Rendering/UI/vtkXRenderWindowInteractor.cxx @@ -187,6 +187,8 @@ vtkXRenderWindowInteractor::vtkXRenderWindowInteractor() //------------------------------------------------------------------------------ vtkXRenderWindowInteractor::~vtkXRenderWindowInteractor() { + this->Finalize(); + this->Disable(); delete this->Internal; @@ -368,15 +370,8 @@ void vtkXRenderWindowInteractor::Initialize() this->Initialized = 1; ren = this->RenderWindow; + ren->EnsureDisplay(); this->DisplayId = static_cast(ren->GetGenericDisplayId()); - if (!this->DisplayId) - { - vtkDebugMacro("opening display"); - this->DisplayId = XOpenDisplay(nullptr); - this->OwnDisplay = true; - vtkDebugMacro("opened display"); - ren->SetDisplayId(this->DisplayId); - } vtkXRenderWindowInteractorInternals::Instances.insert(this); @@ -425,15 +420,8 @@ void vtkXRenderWindowInteractor::Finalize() this->RenderWindow->Finalize(); } - // if we create the display, we'll delete it - if (this->OwnDisplay && this->DisplayId) - { - XCloseDisplay(this->DisplayId); - } - // disconnect from the display, even if we didn't own it this->DisplayId = nullptr; - this->OwnDisplay = false; // revert to uninitialized state this->Initialized = false; diff --git a/Rendering/UI/vtkXRenderWindowInteractor.h b/Rendering/UI/vtkXRenderWindowInteractor.h index a1b0d8cad08..7a4cfae8314 100644 --- a/Rendering/UI/vtkXRenderWindowInteractor.h +++ b/Rendering/UI/vtkXRenderWindowInteractor.h @@ -102,7 +102,6 @@ protected: static int NumAppInitialized; Display* DisplayId; - bool OwnDisplay = false; Window WindowId; Atom KillAtom; int PositionBeforeStereo[2]; -- GitLab From da36c14c2eefc31c71cb3eed6b0c8c1f13476c10 Mon Sep 17 00:00:00 2001 From: Clinton Stimpson Date: Thu, 19 Sep 2024 11:33:24 -0600 Subject: [PATCH 2/4] Change management of X Display file descriptors collecting multiple file descriptors is now done in WaitForEvents --- Rendering/UI/vtkXRenderWindowInteractor.cxx | 35 ++++++++++----------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/Rendering/UI/vtkXRenderWindowInteractor.cxx b/Rendering/UI/vtkXRenderWindowInteractor.cxx index 878a5662d79..055f406c98c 100644 --- a/Rendering/UI/vtkXRenderWindowInteractor.cxx +++ b/Rendering/UI/vtkXRenderWindowInteractor.cxx @@ -56,7 +56,11 @@ constexpr unsigned char XDND_VERSION = 5; class vtkXRenderWindowInteractorInternals { public: - vtkXRenderWindowInteractorInternals() { this->TimerIdCount = 1; } + vtkXRenderWindowInteractorInternals() + { + this->DisplayConnection = -1; + this->TimerIdCount = 1; + } ~vtkXRenderWindowInteractorInternals() = default; // duration is in milliseconds @@ -150,9 +154,8 @@ public: uint64_t NumEventsDispatched = 0; // the timeout value provided to `select`. waits until an event occurs or this interval expires. timeval WaitInterval; - // file descriptors that are monitored for activity in `WaitForEvents` - std::vector RwiFileDescriptors; } FDWaitInfo; + int DisplayConnection; private: int TimerIdCount; @@ -232,23 +235,13 @@ void vtkXRenderWindowInteractor::ProcessEvents() auto& done = internals.FDWaitInfo.Done; auto& evCount = internals.FDWaitInfo.NumEventsDispatched; auto& waitTv = internals.FDWaitInfo.WaitInterval; - auto& rwiFileDescriptors = internals.FDWaitInfo.RwiFileDescriptors; auto& useTimeout = internals.FDWaitInfo.UseTimeout; // reset vars which help VTK wait for new events or timer timeouts. done = true; evCount = 0; - rwiFileDescriptors.clear(); - rwiFileDescriptors.reserve(vtkXRenderWindowInteractorInternals::Instances.size()); useTimeout = false; - // these file descriptors will be polled for new events. after pending events are processed, if - // any. - for (auto rwi : vtkXRenderWindowInteractorInternals::Instances) - { - rwiFileDescriptors.push_back(ConnectionNumber(rwi->DisplayId)); - } - for (auto rwi = vtkXRenderWindowInteractorInternals::Instances.begin(); rwi != vtkXRenderWindowInteractorInternals::Instances.end();) { @@ -284,10 +277,8 @@ void vtkXRenderWindowInteractor::ProcessEvents() // Finalize the rwi (*rwi)->Finalize(); - // Adjust the file descriptors vector - int rwiPosition = std::distance(vtkXRenderWindowInteractorInternals::Instances.begin(), rwi); + // Adjust the Instances vector rwi = vtkXRenderWindowInteractorInternals::Instances.erase(rwi); - rwiFileDescriptors.erase(rwiFileDescriptors.begin() + rwiPosition); } else { @@ -308,10 +299,14 @@ void vtkXRenderWindowInteractor::WaitForEvents() FD_ZERO(&in_fds); int maxFd = -1; timeval* timeout = fdWaitInfo.UseTimeout ? &fdWaitInfo.WaitInterval : nullptr; - for (const auto& rwiFileDescriptor : fdWaitInfo.RwiFileDescriptors) + for (auto rwi : vtkXRenderWindowInteractorInternals::Instances) { - FD_SET(rwiFileDescriptor, &in_fds); - maxFd = std::max(maxFd, rwiFileDescriptor); + if (!rwi->Done) + { + int rwi_fd = rwi->Internal->DisplayConnection; + FD_SET(rwi_fd, &in_fds); + maxFd = std::max(maxFd, rwi_fd); + } } vtkDebugMacro(<< "wait"); select(maxFd + 1, &in_fds, nullptr, nullptr, timeout); @@ -380,6 +375,7 @@ void vtkXRenderWindowInteractor::Initialize() size[1] = ((size[1] > 0) ? size[1] : 300); if (this->DisplayId) { + this->Internal->DisplayConnection = ConnectionNumber(this->DisplayId); XSync(this->DisplayId, False); } @@ -422,6 +418,7 @@ void vtkXRenderWindowInteractor::Finalize() // disconnect from the display, even if we didn't own it this->DisplayId = nullptr; + this->Internal->DisplayConnection = -1; // revert to uninitialized state this->Initialized = false; -- GitLab From 36c83d188199ca9343885cb6dbac403412d8edff Mon Sep 17 00:00:00 2001 From: Clinton Stimpson Date: Thu, 19 Sep 2024 11:59:58 -0600 Subject: [PATCH 3/4] Handle X loop timeout in WaitForEvents() Also improve handling of timeouts by looking through all interactors and choosing the soonest one for the wait loop. --- Rendering/UI/vtkXRenderWindowInteractor.cxx | 54 +++++++++++++-------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/Rendering/UI/vtkXRenderWindowInteractor.cxx b/Rendering/UI/vtkXRenderWindowInteractor.cxx index 055f406c98c..f78301f348a 100644 --- a/Rendering/UI/vtkXRenderWindowInteractor.cxx +++ b/Rendering/UI/vtkXRenderWindowInteractor.cxx @@ -144,17 +144,13 @@ public: static std::set Instances; - struct FDWaitInformation + struct LoopInformation { // whether application was terminated bool Done = false; - // whether `WaitForEvents` invokes `select` with a timeout argument. - bool UseTimeout = false; // the number of events dispatched by `ProcessEvents()` uint64_t NumEventsDispatched = 0; - // the timeout value provided to `select`. waits until an event occurs or this interval expires. - timeval WaitInterval; - } FDWaitInfo; + } LoopInfo; int DisplayConnection; private: @@ -232,25 +228,17 @@ void vtkXRenderWindowInteractor::TerminateApp() void vtkXRenderWindowInteractor::ProcessEvents() { auto& internals = (*this->Internal); - auto& done = internals.FDWaitInfo.Done; - auto& evCount = internals.FDWaitInfo.NumEventsDispatched; - auto& waitTv = internals.FDWaitInfo.WaitInterval; - auto& useTimeout = internals.FDWaitInfo.UseTimeout; + auto& done = internals.LoopInfo.Done; + auto& evCount = internals.LoopInfo.NumEventsDispatched; // reset vars which help VTK wait for new events or timer timeouts. done = true; evCount = 0; - useTimeout = false; for (auto rwi = vtkXRenderWindowInteractorInternals::Instances.begin(); rwi != vtkXRenderWindowInteractorInternals::Instances.end();) { XEvent event; - if (XPending((*rwi)->DisplayId) == 0) - { - // get how long to wait for the next timer - useTimeout = (*rwi)->Internal->GetTimeToNextTimer(waitTv); - } while (XPending((*rwi)->DisplayId) != 0) { // If events are pending, dispatch them to the right RenderWindowInteractor @@ -285,20 +273,44 @@ void vtkXRenderWindowInteractor::ProcessEvents() ++rwi; } } + this->Done = done; } //------------------------------------------------------------------------------ void vtkXRenderWindowInteractor::WaitForEvents() { - auto& internals = (*this->Internal); - auto& fdWaitInfo = (internals.FDWaitInfo); + bool useTimeout = false; + timeval soonestTimer; + + // check to see how long we wait for the next timer + for (auto rwi : vtkXRenderWindowInteractorInternals::Instances) + { + if (rwi->Done) + continue; + + timeval t; + bool haveTimer = rwi->Internal->GetTimeToNextTimer(t); + if (haveTimer) + { + if (!useTimeout) + { + useTimeout = true; + soonestTimer = t; + } + else if (timercmp(&t, &soonestTimer, <)) + { + soonestTimer = t; + } + } + } + fd_set in_fds; // select will wait until 'tv' elapses or something else wakes us FD_ZERO(&in_fds); int maxFd = -1; - timeval* timeout = fdWaitInfo.UseTimeout ? &fdWaitInfo.WaitInterval : nullptr; + timeval* timeout = useTimeout ? &soonestTimer : nullptr; for (auto rwi : vtkXRenderWindowInteractorInternals::Instances) { if (!rwi->Done) @@ -330,11 +342,11 @@ void vtkXRenderWindowInteractor::StartEventLoop() do { auto& internals = (*this->Internal); - auto& fdWaitInfo = (internals.FDWaitInfo); + auto& loopInfo = (internals.LoopInfo); // process pending events. this->ProcessEvents(); // wait for events only if no events were dispatched and application is not yet terminated. - if (!fdWaitInfo.NumEventsDispatched && !fdWaitInfo.Done) + if (!loopInfo.NumEventsDispatched && !loopInfo.Done) { this->WaitForEvents(); } -- GitLab From 7185fc1f95975008b8cda4b480a3e847ae37fd70 Mon Sep 17 00:00:00 2001 From: Clinton Stimpson Date: Thu, 19 Sep 2024 13:02:22 -0600 Subject: [PATCH 4/4] For X, use app-wide done flag for event loop. --- Rendering/UI/vtkXRenderWindowInteractor.cxx | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Rendering/UI/vtkXRenderWindowInteractor.cxx b/Rendering/UI/vtkXRenderWindowInteractor.cxx index f78301f348a..0a3e07b4b0b 100644 --- a/Rendering/UI/vtkXRenderWindowInteractor.cxx +++ b/Rendering/UI/vtkXRenderWindowInteractor.cxx @@ -273,8 +273,6 @@ void vtkXRenderWindowInteractor::ProcessEvents() ++rwi; } } - - this->Done = done; } //------------------------------------------------------------------------------ @@ -339,10 +337,10 @@ void vtkXRenderWindowInteractor::StartEventLoop() { rwi->Done = false; } + + auto& loopInfo = this->Internal->LoopInfo; do { - auto& internals = (*this->Internal); - auto& loopInfo = (internals.LoopInfo); // process pending events. this->ProcessEvents(); // wait for events only if no events were dispatched and application is not yet terminated. @@ -350,7 +348,7 @@ void vtkXRenderWindowInteractor::StartEventLoop() { this->WaitForEvents(); } - } while (!this->Done); + } while (!loopInfo.Done); } //------------------------------------------------------------------------------ -- GitLab