From aee622da6c83fb5ae8dc611aca0e48a74f7f6780 Mon Sep 17 00:00:00 2001 From: Almamu Date: Tue, 1 Apr 2025 02:43:13 +0200 Subject: [PATCH] fix: use glReadnPixels for X11 copying to be more memory safe --- .../Application/CWallpaperApplication.cpp | 3 +-- .../Application/CWallpaperApplication.h | 2 +- .../Render/Drivers/CGLFWOpenGLDriver.cpp | 13 ++++++++++--- .../Render/Drivers/Output/CGLFWWindowOutput.cpp | 4 ++++ .../Render/Drivers/Output/CGLFWWindowOutput.h | 1 + src/WallpaperEngine/Render/Drivers/Output/COutput.h | 1 + .../Render/Drivers/Output/CWaylandOutput.cpp | 4 ++++ .../Render/Drivers/Output/CWaylandOutput.h | 1 + .../Render/Drivers/Output/CX11Output.cpp | 5 +++++ .../Render/Drivers/Output/CX11Output.h | 2 ++ 10 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/WallpaperEngine/Application/CWallpaperApplication.cpp b/src/WallpaperEngine/Application/CWallpaperApplication.cpp index 9ce850e..acf7b83 100644 --- a/src/WallpaperEngine/Application/CWallpaperApplication.cpp +++ b/src/WallpaperEngine/Application/CWallpaperApplication.cpp @@ -219,7 +219,7 @@ void CWallpaperApplication::setupProperties () { this->setupPropertiesForProject (info); } -void CWallpaperApplication::takeScreenshot (const std::filesystem::path& filename) { +void CWallpaperApplication::takeScreenshot (const std::filesystem::path& filename) const { // this should be getting called at the end of the frame, so the right thing should be bound already const int width = this->m_renderContext->getOutput ().getFullWidth (); const int height = this->m_renderContext->getOutput ().getFullHeight (); @@ -238,7 +238,6 @@ void CWallpaperApplication::takeScreenshot (const std::filesystem::path& filenam const uint8_t* pixel = buffer; // read the viewport data into the pixel buffer - glPixelStorei (GL_PACK_ALIGNMENT, 1); glReadnPixels (viewport->viewport.x, viewport->viewport.y, viewport->viewport.z, viewport->viewport.w, GL_RGB, GL_UNSIGNED_BYTE, bufferSize, buffer); diff --git a/src/WallpaperEngine/Application/CWallpaperApplication.h b/src/WallpaperEngine/Application/CWallpaperApplication.h index e8e62ab..94a02fc 100644 --- a/src/WallpaperEngine/Application/CWallpaperApplication.h +++ b/src/WallpaperEngine/Application/CWallpaperApplication.h @@ -106,7 +106,7 @@ class CWallpaperApplication { * * @param filename */ - void takeScreenshot (const std::filesystem::path& filename); + void takeScreenshot (const std::filesystem::path& filename) const; /** The application context that contains the current app settings */ CApplicationContext& m_context; diff --git a/src/WallpaperEngine/Render/Drivers/CGLFWOpenGLDriver.cpp b/src/WallpaperEngine/Render/Drivers/CGLFWOpenGLDriver.cpp index 4cd821e..48fffb1 100644 --- a/src/WallpaperEngine/Render/Drivers/CGLFWOpenGLDriver.cpp +++ b/src/WallpaperEngine/Render/Drivers/CGLFWOpenGLDriver.cpp @@ -132,9 +132,16 @@ void CGLFWOpenGLDriver::dispatchEventQueue () { this->getApp ().update (viewport); // read the full texture into the image - if (this->m_output->haveImageBuffer ()) - glReadPixels (0, 0, this->m_output->getFullWidth (), this->m_output->getFullHeight (), GL_BGRA, - GL_UNSIGNED_BYTE, this->m_output->getImageBuffer ()); + if (this->m_output->haveImageBuffer ()) { + glReadnPixels (0, 0, this->m_output->getFullWidth (), this->m_output->getFullHeight (), GL_BGRA, + GL_UNSIGNED_BYTE, this->m_output->getImageBufferSize(), this->m_output->getImageBuffer ()); + + GLenum error = glGetError(); + + if (error != GL_NO_ERROR) { + sLog.exception("OpenGL error when reading texture ", error); + } + } // TODO: FRAMETIME CONTROL SHOULD GO BACK TO THE CWALLPAPAERAPPLICATION ONCE ACTUAL PARTICLES ARE IMPLEMENTED // TODO: AS THOSE, MORE THAN LIKELY, WILL REQUIRE OF A DIFFERENT PROCESSING RATE diff --git a/src/WallpaperEngine/Render/Drivers/Output/CGLFWWindowOutput.cpp b/src/WallpaperEngine/Render/Drivers/Output/CGLFWWindowOutput.cpp index e566564..f4b116d 100644 --- a/src/WallpaperEngine/Render/Drivers/Output/CGLFWWindowOutput.cpp +++ b/src/WallpaperEngine/Render/Drivers/Output/CGLFWWindowOutput.cpp @@ -56,6 +56,10 @@ void* CGLFWWindowOutput::getImageBuffer () const { return nullptr; } +uint32_t CGLFWWindowOutput::getImageBufferSize () const { + return 0; +} + void CGLFWWindowOutput::updateRender () const { if (this->m_context.settings.render.mode != Application::CApplicationContext::NORMAL_WINDOW) return; diff --git a/src/WallpaperEngine/Render/Drivers/Output/CGLFWWindowOutput.h b/src/WallpaperEngine/Render/Drivers/Output/CGLFWWindowOutput.h index f7d7582..b8817f5 100644 --- a/src/WallpaperEngine/Render/Drivers/Output/CGLFWWindowOutput.h +++ b/src/WallpaperEngine/Render/Drivers/Output/CGLFWWindowOutput.h @@ -13,6 +13,7 @@ class CGLFWWindowOutput final : public COutput { bool renderMultiple () const override; bool haveImageBuffer () const override; void* getImageBuffer () const override; + uint32_t getImageBufferSize () const override; void updateRender () const override; private: diff --git a/src/WallpaperEngine/Render/Drivers/Output/COutput.h b/src/WallpaperEngine/Render/Drivers/Output/COutput.h index 934f411..57b90e1 100644 --- a/src/WallpaperEngine/Render/Drivers/Output/COutput.h +++ b/src/WallpaperEngine/Render/Drivers/Output/COutput.h @@ -38,6 +38,7 @@ class COutput { virtual bool haveImageBuffer () const = 0; const std::map& getViewports () const; virtual void* getImageBuffer () const = 0; + virtual uint32_t getImageBufferSize () const = 0; virtual void updateRender () const = 0; protected: diff --git a/src/WallpaperEngine/Render/Drivers/Output/CWaylandOutput.cpp b/src/WallpaperEngine/Render/Drivers/Output/CWaylandOutput.cpp index 824b46e..0a32196 100644 --- a/src/WallpaperEngine/Render/Drivers/Output/CWaylandOutput.cpp +++ b/src/WallpaperEngine/Render/Drivers/Output/CWaylandOutput.cpp @@ -51,4 +51,8 @@ void* CWaylandOutput::getImageBuffer () const { return nullptr; } +uint32_t CWaylandOutput::getImageBufferSize () const { + return 0; +} + void CWaylandOutput::updateRender () const {} \ No newline at end of file diff --git a/src/WallpaperEngine/Render/Drivers/Output/CWaylandOutput.h b/src/WallpaperEngine/Render/Drivers/Output/CWaylandOutput.h index 52b840b..22398b3 100644 --- a/src/WallpaperEngine/Render/Drivers/Output/CWaylandOutput.h +++ b/src/WallpaperEngine/Render/Drivers/Output/CWaylandOutput.h @@ -24,6 +24,7 @@ class CWaylandOutput final : public COutput { bool renderMultiple () const override; bool haveImageBuffer () const override; void* getImageBuffer () const override; + uint32_t getImageBufferSize () const override; void updateRender () const override; private: diff --git a/src/WallpaperEngine/Render/Drivers/Output/CX11Output.cpp b/src/WallpaperEngine/Render/Drivers/Output/CX11Output.cpp index 74d7778..845d0b1 100644 --- a/src/WallpaperEngine/Render/Drivers/Output/CX11Output.cpp +++ b/src/WallpaperEngine/Render/Drivers/Output/CX11Output.cpp @@ -83,6 +83,10 @@ bool CX11Output::haveImageBuffer () const { return true; } +uint32_t CX11Output::getImageBufferSize () const { + return this->m_imageSize; +} + void CX11Output::loadScreenInfo () { // reset the viewports this->m_viewports.clear (); @@ -163,6 +167,7 @@ void CX11Output::loadScreenInfo () { // set the window background as our pixmap XSetWindowBackgroundPixmap (this->m_display, this->m_root, this->m_pixmap); // allocate space for the image's data + this->m_imageSize = this->m_fullWidth * this->m_fullHeight * 4; this->m_imageData = new char [this->m_fullWidth * this->m_fullHeight * 4]; // create an image so we can copy it over this->m_image = XCreateImage (this->m_display, CopyFromParent, 24, ZPixmap, 0, this->m_imageData, this->m_fullWidth, diff --git a/src/WallpaperEngine/Render/Drivers/Output/CX11Output.h b/src/WallpaperEngine/Render/Drivers/Output/CX11Output.h index 6e255c8..81eb108 100644 --- a/src/WallpaperEngine/Render/Drivers/Output/CX11Output.h +++ b/src/WallpaperEngine/Render/Drivers/Output/CX11Output.h @@ -21,6 +21,7 @@ class CX11Output final : public COutput { bool renderMultiple () const override; bool haveImageBuffer () const override; void* getImageBuffer () const override; + uint32_t getImageBufferSize () const override; void updateRender () const override; private: @@ -32,6 +33,7 @@ class CX11Output final : public COutput { Window m_root; GC m_gc; char* m_imageData; + uint32_t m_imageSize; XImage* m_image; std::vector m_screens; };