Skip to content

Commit

Permalink
Merge ddnet#4989
Browse files Browse the repository at this point in the history
4989: Add null backend r=def- a=ChillerDragon

Thanks to `@Jupeyy` for the idea.

Replacing graphics_threaded_null.h with a null backend
makes code maintenance and memory cleanup easier.

Fixes these memory leaks in headless client (ddnet#4970)

```
Direct leak of 1048576 byte(s) in 1 object(s) allocated from:
    #0 0x4b6f6d in malloc (/home/runner/work/ddnet/ddnet/san/DDNet+0x4b6f6d)
    #1 0xd92ffb in CTextRender::InitTextures(int, int, IGraphics::CTextureHandle (&) [2], unsigned char* (&) [2]) /home/runner/work/ddnet/ddnet/src/engine/client/text.cpp:325:24
    #2 0xd53795 in CTextRender::LoadFont(char const*, unsigned char const*, unsigned long) /home/runner/work/ddnet/ddnet/src/engine/client/text.cpp:729:3
    #3 0xade5cd in CClient::LoadFont() /home/runner/work/ddnet/ddnet/src/engine/client/client.cpp:4070:32
    #4 0x155b3de in CGameClient::OnInit() /home/runner/work/ddnet/ddnet/src/game/client/gameclient.cpp:246:12
    #5 0xa8ecfe in CClient::Run() /home/runner/work/ddnet/ddnet/src/engine/client/client.cpp:2933:16
    #6 0xaf6726 in main /home/runner/work/ddnet/ddnet/src/engine/client/client.cpp:4458:11
    #7 0x7fadd9e610b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x240b2)
```

## Checklist

- [x] Tested the change ingame
- [ ] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test if it works standalone, system.c especially
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [x] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)


Co-authored-by: ChillerDrgon <ChillerDragon@gmail.com>
  • Loading branch information
bors[bot] and ChillerDragon authored Apr 16, 2022
2 parents 2825ceb + 9ee3534 commit d4dcaa2
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 222 deletions.
3 changes: 2 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1824,6 +1824,8 @@ if(CLIENT)
backend/backend_base.h
backend/glsl_shader_compiler.cpp
backend/glsl_shader_compiler.h
backend/null/backend_null.cpp
backend/null/backend_null.h
backend/opengl/backend_opengl.cpp
backend/opengl/backend_opengl.h
backend/opengl/backend_opengl3.cpp
Expand Down Expand Up @@ -1858,7 +1860,6 @@ if(CLIENT)
graphics_defines.h
graphics_threaded.cpp
graphics_threaded.h
graphics_threaded_null.h
http.cpp
http.h
input.cpp
Expand Down
59 changes: 59 additions & 0 deletions src/engine/client/backend/null/backend_null.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
#include "backend_null.h"

bool CCommandProcessorFragment_Null::RunCommand(const CCommandBuffer::SCommand *pBaseCommand)
{
switch(pBaseCommand->m_Cmd)
{
case CCommandProcessorFragment_Null::CMD_INIT:
Cmd_Init(static_cast<const SCommand_Init *>(pBaseCommand));
break;
case CCommandBuffer::CMD_TEXTURE_CREATE:
Cmd_Texture_Create(static_cast<const CCommandBuffer::SCommand_Texture_Create *>(pBaseCommand));
break;
case CCommandBuffer::CMD_TEXT_TEXTURES_CREATE:
Cmd_TextTextures_Create(static_cast<const CCommandBuffer::SCommand_TextTextures_Create *>(pBaseCommand));
break;
case CCommandBuffer::CMD_TEXT_TEXTURE_UPDATE:
Cmd_TextTexture_Update(static_cast<const CCommandBuffer::SCommand_TextTexture_Update *>(pBaseCommand));
break;
}
return true;
}

bool CCommandProcessorFragment_Null::Cmd_Init(const SCommand_Init *pCommand)
{
pCommand->m_pCapabilities->m_TileBuffering = false;
pCommand->m_pCapabilities->m_QuadBuffering = false;
pCommand->m_pCapabilities->m_TextBuffering = false;
pCommand->m_pCapabilities->m_QuadContainerBuffering = false;

pCommand->m_pCapabilities->m_MipMapping = false;
pCommand->m_pCapabilities->m_NPOTTextures = false;
pCommand->m_pCapabilities->m_3DTextures = false;
pCommand->m_pCapabilities->m_2DArrayTextures = false;
pCommand->m_pCapabilities->m_2DArrayTexturesAsExtension = false;
pCommand->m_pCapabilities->m_ShaderSupport = false;

pCommand->m_pCapabilities->m_TrianglesAsQuads = false;

pCommand->m_pCapabilities->m_ContextMajor = 0;
pCommand->m_pCapabilities->m_ContextMinor = 0;
pCommand->m_pCapabilities->m_ContextPatch = 0;
return false;
}

void CCommandProcessorFragment_Null::Cmd_Texture_Create(const CCommandBuffer::SCommand_Texture_Create *pCommand)
{
free(pCommand->m_pData);
}

void CCommandProcessorFragment_Null::Cmd_TextTextures_Create(const CCommandBuffer::SCommand_TextTextures_Create *pCommand)
{
free(pCommand->m_pTextData);
free(pCommand->m_pTextOutlineData);
}

void CCommandProcessorFragment_Null::Cmd_TextTexture_Update(const CCommandBuffer::SCommand_TextTexture_Update *pCommand)
{
free(pCommand->m_pData);
}
16 changes: 16 additions & 0 deletions src/engine/client/backend/null/backend_null.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#ifndef ENGINE_CLIENT_BACKEND_NULL_BACKEND_NULL_H
#define ENGINE_CLIENT_BACKEND_NULL_BACKEND_NULL_H

#include <engine/client/backend/backend_base.h>

class CCommandProcessorFragment_Null : public CCommandProcessorFragment_GLBase
{
virtual bool GetPresentedImageData(uint32_t &Width, uint32_t &Height, uint32_t &Format, std::vector<uint8_t> &DstData) { return false; };
virtual bool RunCommand(const CCommandBuffer::SCommand *pBaseCommand);
bool Cmd_Init(const SCommand_Init *pCommand);
virtual void Cmd_Texture_Create(const CCommandBuffer::SCommand_Texture_Create *pCommand);
virtual void Cmd_TextTextures_Create(const CCommandBuffer::SCommand_TextTextures_Create *pCommand);
virtual void Cmd_TextTexture_Update(const CCommandBuffer::SCommand_TextTexture_Update *pCommand);
};

#endif
16 changes: 16 additions & 0 deletions src/engine/client/backend_sdl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@

#include "backend_sdl.h"

#include "backend/null/backend_null.h"

#if !defined(CONF_BACKEND_OPENGL_ES)
#include "backend/opengl/backend_opengl3.h"
#endif
Expand Down Expand Up @@ -266,6 +268,9 @@ CCommandProcessor_SDL_GL::CCommandProcessor_SDL_GL(EBackendType BackendType, int
{
m_BackendType = BackendType;

#if defined(CONF_HEADLESS_CLIENT)
m_pGLBackend = new CCommandProcessorFragment_Null();
#else
if(BackendType == BACKEND_TYPE_OPENGL_ES)
{
#if defined(CONF_BACKEND_OPENGL_ES) || defined(CONF_BACKEND_OPENGL_ES3)
Expand Down Expand Up @@ -306,6 +311,7 @@ CCommandProcessor_SDL_GL::CCommandProcessor_SDL_GL(EBackendType BackendType, int
m_pGLBackend = CreateVulkanCommandProcessorFragment();
#endif
}
#endif
}

CCommandProcessor_SDL_GL::~CCommandProcessor_SDL_GL()
Expand Down Expand Up @@ -858,6 +864,15 @@ CGraphicsBackend_SDL_GL::CGraphicsBackend_SDL_GL()

int CGraphicsBackend_SDL_GL::Init(const char *pName, int *pScreen, int *pWidth, int *pHeight, int *pRefreshRate, int FsaaSamples, int Flags, int *pDesktopWidth, int *pDesktopHeight, int *pCurrentWidth, int *pCurrentHeight, IStorage *pStorage)
{
#if defined(CONF_HEADLESS_CLIENT)
int InitError = 0;
const char *pErrorStr = NULL;
int GlewMajor = 0;
int GlewMinor = 0;
int GlewPatch = 0;
IsVersionSupportedGlew(m_BackendType, g_Config.m_GfxGLMajor, g_Config.m_GfxGLMinor, g_Config.m_GfxGLPatch, GlewMajor, GlewMinor, GlewPatch);
BackendInitGlew(m_BackendType, GlewMajor, GlewMinor, GlewPatch);
#else
// print sdl version
{
SDL_version Compiled;
Expand Down Expand Up @@ -1102,6 +1117,7 @@ int CGraphicsBackend_SDL_GL::Init(const char *pName, int *pScreen, int *pWidth,

return EGraphicsBackendErrorCodes::GRAPHICS_BACKEND_ERROR_CODE_GL_VERSION_FAILED;
}
#endif // CONF_HEADLESS_CLIENT

// start the command processor
dbg_assert(m_pProcessor == nullptr, "Processor was not cleaned up properly.");
Expand Down
5 changes: 0 additions & 5 deletions src/engine/client/graphics_threaded.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
#endif

#include "graphics_threaded.h"
#include "graphics_threaded_null.h"

static CVideoMode g_aFakeModes[] = {
{8192, 4320, 8192, 4320, 0, 8, 8, 8, 0}, {7680, 4320, 7680, 4320, 0, 8, 8, 8, 0}, {5120, 2880, 5120, 2880, 0, 8, 8, 8, 0},
Expand Down Expand Up @@ -2842,9 +2841,5 @@ int CGraphics_Threaded::GetVideoModes(CVideoMode *pModes, int MaxModes, int Scre

extern IEngineGraphics *CreateEngineGraphicsThreaded()
{
#ifdef CONF_HEADLESS_CLIENT
return new CGraphics_ThreadedNull();
#else
return new CGraphics_Threaded();
#endif
}
216 changes: 0 additions & 216 deletions src/engine/client/graphics_threaded_null.h

This file was deleted.

0 comments on commit d4dcaa2

Please sign in to comment.