Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

GBM/X11/Wayland: Implement asynchronous texture uploads #25139

Merged
merged 7 commits into from
Jun 2, 2024
Merged

GBM/X11/Wayland: Implement asynchronous texture uploads #25139

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
162bf61
GL/GLES prepare for async texture uploads
sarbes May 5, 2024
3bb94d0
X11: Implement async texture uploads
sarbes May 5, 2024
b96997d
GBM: Implement async texture uploads
sarbes May 5, 2024
8722b68
Wayland: Implement async texture uploads
sarbes May 5, 2024
1b4d3ce
Make upload call explicit
sarbes May 8, 2024
b818c0a
Cleanup EGLUtils
sarbes May 8, 2024
84625af
Cleanup GLContextEGL
sarbes May 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
72 changes: 54 additions & 18 deletions xbmc/guilib/Texture.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@
#include "guilib/TextureBase.h"
#include "guilib/iimage.h"
#include "guilib/imagefactory.h"
#include "settings/AdvancedSettings.h"
#include "settings/SettingsComponent.h"
#include "utils/URIUtils.h"
#include "utils/log.h"
#include "windowing/WinSystem.h"
#if defined(TARGET_DARWIN_EMBEDDED)
#include <ImageIO/ImageIO.h>
#include "filesystem/File.h"
Expand Down Expand Up @@ -236,26 +239,59 @@ bool CTexture::LoadIImage(IImage* pImage,
unsigned int width,
unsigned int height)
{
if(pImage != NULL && pImage->LoadImageFromMemory(buffer, bufSize, width, height))
if (pImage == nullptr)
return false;

if (!pImage->LoadImageFromMemory(buffer, bufSize, width, height))
return false;

if (pImage->Width() == 0 || pImage->Height() == 0)
return false;

Allocate(pImage->Width(), pImage->Height(), XB_FMT_A8R8G8B8);

if (m_pixels == nullptr)
return false;

if (!pImage->Decode(m_pixels, GetTextureWidth(), GetRows(), GetPitch(), XB_FMT_A8R8G8B8))
return false;

if (pImage->Orientation())
m_orientation = pImage->Orientation() - 1;

m_hasAlpha = pImage->hasAlpha();
m_originalWidth = pImage->originalWidth();
m_originalHeight = pImage->originalHeight();
m_imageWidth = pImage->Width();
m_imageHeight = pImage->Height();

ClampToEdge();

LoadToGPUAsync();

return true;
}

void CTexture::LoadToGPUAsync()
{
if (!CServiceBroker::GetSettingsComponent()->GetAdvancedSettings()->m_guiAsyncTextureUpload)
sarbes marked this conversation as resolved.
Show resolved Hide resolved
return;

// Already in main context?
if (CServiceBroker::GetWinSystem()->HasContext())
{
if (pImage->Width() > 0 && pImage->Height() > 0)
{
Allocate(pImage->Width(), pImage->Height(), XB_FMT_A8R8G8B8);
if (m_pixels != nullptr && pImage->Decode(m_pixels, GetTextureWidth(), GetRows(), GetPitch(), XB_FMT_A8R8G8B8))
{
if (pImage->Orientation())
m_orientation = pImage->Orientation() - 1;
m_hasAlpha = pImage->hasAlpha();
m_originalWidth = pImage->originalWidth();
m_originalHeight = pImage->originalHeight();
m_imageWidth = pImage->Width();
m_imageHeight = pImage->Height();
ClampToEdge();
return true;
}
}
LoadToGPU();
return;
}
return false;

if (!CServiceBroker::GetWinSystem()->BindTextureUploadContext())
return;

LoadToGPU();

SyncGPU();

CServiceBroker::GetWinSystem()->UnbindTextureUploadContext();
}

bool CTexture::LoadFromMemory(unsigned int width,
Expand Down
8 changes: 8 additions & 0 deletions xbmc/guilib/Texture.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ class CTexture : public CTextureBase
virtual void CreateTextureObject() = 0;
virtual void DestroyTextureObject() = 0;
virtual void LoadToGPU() = 0;
/*!
* \brief Blocks execution until the previous GFX commands have been processed.
*/
virtual void SyncGPU(){};
virtual void BindToUnit(unsigned int unit) = 0;

private:
Expand All @@ -106,4 +110,8 @@ class CTexture : public CTextureBase
unsigned int bufSize,
unsigned int width,
unsigned int height);
/*!
* \brief Uploads the texture to the GPU.
*/
void LoadToGPUAsync();
};
5 changes: 5 additions & 0 deletions xbmc/guilib/TextureGL.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,11 @@ void CGLTexture::LoadToGPU()
m_loadedToGPU = true;
}

void CGLTexture::SyncGPU()
{
glFinish();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This effectively blocks no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's desired. It blocks the upload thread, and waits until the texture is visible for the GPU.

}

void CGLTexture::BindToUnit(unsigned int unit)
{
glActiveTexture(GL_TEXTURE0 + unit);
Expand Down
1 change: 1 addition & 0 deletions xbmc/guilib/TextureGL.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class CGLTexture : public CTexture
void CreateTextureObject() override;
void DestroyTextureObject() override;
void LoadToGPU() override;
void SyncGPU() override;
void BindToUnit(unsigned int unit) override;

protected:
Expand Down
1 change: 1 addition & 0 deletions xbmc/settings/AdvancedSettings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1226,6 +1226,7 @@ void CAdvancedSettings::ParseSettingsFile(const std::string &file)
XMLUtils::GetInt(pElement, "anisotropicfiltering", m_guiAnisotropicFiltering);
XMLUtils::GetBoolean(pElement, "fronttobackrendering", m_guiFrontToBackRendering);
XMLUtils::GetBoolean(pElement, "geometryclear", m_guiGeometryClear);
XMLUtils::GetBoolean(pElement, "asynctextureupload", m_guiAsyncTextureUpload);
}

std::string seekSteps;
Expand Down
2 changes: 2 additions & 0 deletions xbmc/settings/AdvancedSettings.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,8 @@ class CAdvancedSettings : public ISettingCallback, public ISettingsHandler
int32_t m_guiAnisotropicFiltering{0};
bool m_guiFrontToBackRendering{false};
bool m_guiGeometryClear{true};
bool m_guiAsyncTextureUpload{false};

unsigned int m_addonPackageFolderSize;

bool m_jsonOutputCompact;
Expand Down
58 changes: 58 additions & 0 deletions xbmc/utils/EGLUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,12 @@ bool CEGLContextUtils::CreateContext(CEGLAttributesVec contextAttribs)
return false;
}

m_eglUploadContext =
eglCreateContext(m_eglDisplay, m_eglConfig, m_eglContext, contextAttribs.Get());

if (m_eglUploadContext == EGL_NO_CONTEXT)
CLog::Log(LOGWARNING, "Failed to create EGL upload context");

return true;
}

Expand Down Expand Up @@ -565,6 +571,12 @@ void CEGLContextUtils::DestroyContext()
eglDestroyContext(m_eglDisplay, m_eglContext);
m_eglContext = EGL_NO_CONTEXT;
}

if (m_eglUploadContext)
{
eglDestroyContext(m_eglDisplay, m_eglUploadContext);
m_eglUploadContext = EGL_NO_CONTEXT;
}
}

void CEGLContextUtils::DestroySurface()
Expand Down Expand Up @@ -597,3 +609,49 @@ bool CEGLContextUtils::TrySwapBuffers()

return (eglSwapBuffers(m_eglDisplay, m_eglSurface) == EGL_TRUE);
}

bool CEGLContextUtils::BindTextureUploadContext()
{
if (m_eglDisplay == EGL_NO_DISPLAY || m_eglUploadContext == EGL_NO_CONTEXT)
{
CLog::LogF(LOGERROR, "No texture upload context found.");
return false;
}

m_textureUploadLock.lock();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love the fact of locking in the middle of these functions. I assume because the texture uploads may happen from other threads? It would be nice if there was the ability to add texture uploads to a queue to avoid needing the lock.

Copy link
Member Author

@sarbes sarbes May 6, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there can be multiple jobs in parallel. Only one can have the upload context.


if (eglMakeCurrent(m_eglDisplay, EGL_NO_SURFACE, EGL_NO_SURFACE, m_eglUploadContext))
sarbes marked this conversation as resolved.
Show resolved Hide resolved
{
return true;
}
else
{
m_textureUploadLock.unlock();
CLog::LogF(LOGERROR, "Couldn't bind texture upload context.");
return false;
}
}

bool CEGLContextUtils::UnbindTextureUploadContext()
{
if (m_eglDisplay == EGL_NO_DISPLAY || m_eglUploadContext == EGL_NO_CONTEXT)
{
CLog::LogF(LOGERROR, "No texture upload context found.");
m_textureUploadLock.unlock();
return false;
}

bool success = eglMakeCurrent(m_eglDisplay, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this not bind back to the other context?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The job has no context to begin with.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this effectively unbind the main context if called from the main thread? I don't think we bind the main thread context unless windowing is being inited

Copy link
Member Author

@sarbes sarbes May 11, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it checks if there is a context bound, and bails if so.

And in its current state, it is only called in the threaded texture loader.


if (!success)
sarbes marked this conversation as resolved.
Show resolved Hide resolved
CLog::Log(LOGERROR, "Couldn't release texture upload context");

m_textureUploadLock.unlock();

return success;
}

bool CEGLContextUtils::HasContext()
{
return eglGetCurrentContext() != EGL_NO_CONTEXT;
}
8 changes: 8 additions & 0 deletions xbmc/utils/EGLUtils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

#pragma once

#include "threads/CriticalSection.h"

#include <array>
#include <set>
#include <stdexcept>
Expand Down Expand Up @@ -219,6 +221,10 @@ class CEGLContextUtils final
return m_eglConfig;
}

bool BindTextureUploadContext();
bool UnbindTextureUploadContext();
bool HasContext();

private:
void SurfaceAttrib();

Expand All @@ -229,4 +235,6 @@ class CEGLContextUtils final
EGLSurface m_eglSurface{EGL_NO_SURFACE};
EGLContext m_eglContext{EGL_NO_CONTEXT};
EGLConfig m_eglConfig{}, m_eglHDRConfig{};
EGLContext m_eglUploadContext{EGL_NO_CONTEXT};
mutable CCriticalSection m_textureUploadLock;
};
18 changes: 18 additions & 0 deletions xbmc/windowing/WinSystem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,24 @@ class CWinSystemBase
*/
std::pair<bool, int> GetDitherSettings();

/*!
* \brief Binds a shared context to the current thread, in order to upload textures asynchronously.
* \return Return true if a texture upload context exists and the binding succeeds.
*/
virtual bool BindTextureUploadContext() { return false; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to avoid polluting CWinSystemBase with more stuff. Can this be moved somewhere else? Possibly the caller can cast the window system to the EGL intermediate class?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've experimented with that, but I couldn't come up with an universal solution which does not look hacky. If you could give me some pointers, I'd appreciate it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See here:

xbmc/xbmc/cores/VideoPlayer/VideoRenderers/HwDecRender/RendererDRMPRIMEGLES.cpp

Line 59 in 64e0ecf

auto winSystemEGL =

The problem is that x11 uses a different EGL context class so that might be annoying. It was always the intention to get x11 to use the same EGL class that wayland and gbm are using, it just hasn't happened yet.

Copy link
Member Author

@sarbes sarbes May 11, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's why I shelved the branch for a long time. I'm still running X11. I'll see what I can do.

On the flip side, I believe that GLX and DX11 can have a very similar implementations. It is not EGL specific, if that's your concern.


/*!
* \brief Unbinds the shared context.
* \return Return true if the texture upload context has been unbound.
*/
virtual bool UnbindTextureUploadContext() { return false; }

/*!
* \brief Checks if a graphics context is already bound to the current thread.
* \return Return true if so.
*/
virtual bool HasContext() { return false; }

protected:
void UpdateDesktopResolution(RESOLUTION_INFO& newRes, const std::string &output, int width, int height, float refreshRate, uint32_t dwFlags);
void UpdateDesktopResolution(RESOLUTION_INFO& newRes,
Expand Down
64 changes: 63 additions & 1 deletion xbmc/windowing/X11/GLContextEGL.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,21 @@
#include "GLContextEGL.h"

#include "ServiceBroker.h"
#include "commons/ilog.h"
#include "settings/AdvancedSettings.h"
#include "settings/SettingsComponent.h"
#include "utils/log.h"

#include <clocale>
#include <mutex>

#include <EGL/eglext.h>
#include <unistd.h>

#include "PlatformDefs.h"
#include "system_gl.h"

#include <EGL/eglext.h>

#define EGL_NO_CONFIG (EGLConfig)0

CGLContextEGL::CGLContextEGL(Display* dpy, EGLint renderingApi)
Expand Down Expand Up @@ -160,6 +162,8 @@ bool CGLContextEGL::Refresh(bool force, int screen, Window glWindow, bool &newCo
EGL_NONE
};
m_eglContext = eglCreateContext(m_eglDisplay, m_eglConfig, EGL_NO_CONTEXT, contextAttributes);
m_eglUploadContext = eglCreateContext(m_eglDisplay, m_eglConfig, m_eglContext, contextAttributes);

if (m_eglContext == EGL_NO_CONTEXT)
{
EGLint contextAttributes[] =
Expand All @@ -168,6 +172,8 @@ bool CGLContextEGL::Refresh(bool force, int screen, Window glWindow, bool &newCo
EGL_NONE
};
m_eglContext = eglCreateContext(m_eglDisplay, m_eglConfig, EGL_NO_CONTEXT, contextAttributes);
m_eglUploadContext =
eglCreateContext(m_eglDisplay, m_eglConfig, m_eglContext, contextAttributes);

if (m_eglContext == EGL_NO_CONTEXT)
{
Expand All @@ -190,6 +196,9 @@ bool CGLContextEGL::Refresh(bool force, int screen, Window glWindow, bool &newCo

m_eglGetSyncValuesCHROMIUM = (PFNEGLGETSYNCVALUESCHROMIUMPROC)eglGetProcAddress("eglGetSyncValuesCHROMIUM");

if (m_eglUploadContext == EGL_NO_CONTEXT)
CLog::Log(LOGWARNING, "failed to create EGL upload context");

m_usePB = false;
return true;
}
Expand Down Expand Up @@ -286,6 +295,11 @@ bool CGLContextEGL::CreatePB()
return false;
}

m_eglUploadContext = eglCreateContext(m_eglDisplay, m_eglConfig, m_eglContext, contextAttributes);

if (m_eglUploadContext == EGL_NO_CONTEXT)
CLog::Log(LOGWARNING, "Failed to create EGL upload context");

m_usePB = true;
return true;
}
Expand All @@ -300,6 +314,12 @@ void CGLContextEGL::Destroy()
m_eglContext = EGL_NO_CONTEXT;
}

if (m_eglUploadContext)
{
eglDestroyContext(m_eglDisplay, m_eglUploadContext);
m_eglUploadContext = EGL_NO_CONTEXT;
}

if (m_eglSurface)
{
eglDestroySurface(m_eglDisplay, m_eglSurface);
Expand Down Expand Up @@ -534,6 +554,48 @@ uint64_t CGLContextEGL::GetVblankTiming(uint64_t &msc, uint64_t &interval)
return ret;
}

bool CGLContextEGL::BindTextureUploadContext()
{
if (m_eglDisplay == EGL_NO_DISPLAY || m_eglUploadContext == EGL_NO_CONTEXT)
{
CLog::LogF(LOGERROR, "No texture upload context found.");
return false;
}

m_textureUploadLock.lock();

if (eglMakeCurrent(m_eglDisplay, EGL_NO_SURFACE, EGL_NO_SURFACE, m_eglUploadContext))
{
return true;
}
else
{
m_textureUploadLock.unlock();
CLog::LogF(LOGERROR, "Couldn't bind texture upload context.");
return false;
}
}

bool CGLContextEGL::UnbindTextureUploadContext()
{
if (m_eglDisplay == EGL_NO_DISPLAY || m_eglUploadContext == EGL_NO_CONTEXT)
{
CLog::LogF(LOGERROR, "No texture upload context found.");
m_textureUploadLock.unlock();
return false;
}
bool success = eglMakeCurrent(m_eglDisplay, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT);
if (!success)
CLog::LogF(LOGERROR, "Couldn't release texture upload context.");
m_textureUploadLock.unlock();
return success;
}

bool CGLContextEGL::HasContext()
{
return eglGetCurrentContext() != EGL_NO_CONTEXT;
}

void CGLContextEGL::QueryExtensions()
{
std::string extensions = eglQueryString(m_eglDisplay, EGL_EXTENSIONS);
Expand Down