Skip to content

Commit

Permalink
Fixed communication issue
Browse files Browse the repository at this point in the history
Now handle partial communication instead of waiting for completion. Was causing issue when both client/server were waiting for complete command reception at the same time, causing deadlock.
Now read/send as much data of a command as possible and continue the loop, tracking were the command transmission was at.
  • Loading branch information
sammyfreg committed Dec 2, 2024
1 parent bd23d27 commit 237dc78
Show file tree
Hide file tree
Showing 19 changed files with 715 additions and 571 deletions.
8 changes: 4 additions & 4 deletions Code/Client/NetImgui_Api.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@
//! @Name : NetImgui
//=================================================================================================
//! @author : Sammy Fatnassi
//! @date : 2024/11/19
//! @version : v1.11.3
//! @date : 2024/12/01
//! @version : v1.11.4
//! @Details : For integration info : https://github.com/sammyfreg/netImgui/wiki
//=================================================================================================
#define NETIMGUI_VERSION "1.11.3" // Upgrade to Dear ImGui 1.Improved connection speed
#define NETIMGUI_VERSION_NUM 11103
#define NETIMGUI_VERSION "1.11.4" // Handle partial com instead of blocking
#define NETIMGUI_VERSION_NUM 11104



Expand Down
4 changes: 2 additions & 2 deletions Code/Client/Private/NetImgui_Api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ void Disconnect(void)
// Attempt fake connection on local socket waiting for a Server connection,
// so the blocking operation can terminate and release the communication thread
Client::ClientInfo& client = *gpClientInfo;
client.mbDisconnectRequest = true;
client.mbDisconnectPending = true;
client.mbDisconnectListen = true;
if( client.mpSocketListen.load() != nullptr )
{
Expand Down Expand Up @@ -335,7 +335,7 @@ void SendDataTexture(ImTextureID textureId, void* pData, uint16_t width, uint16_
pCmdTexture->mpTextureData.SetPtr(reinterpret_cast<uint8_t*>(&pCmdTexture[1]));
memcpy(pCmdTexture->mpTextureData.Get(), pData, dataSize);

pCmdTexture->mHeader.mSize = SizeNeeded;
pCmdTexture->mSize = SizeNeeded;
pCmdTexture->mWidth = width;
pCmdTexture->mHeight = height;
pCmdTexture->mTextureId = texId64;
Expand Down
265 changes: 157 additions & 108 deletions Code/Client/Private/NetImgui_Client.cpp

Large diffs are not rendered by default.

15 changes: 9 additions & 6 deletions Code/Client/Private/NetImgui_Client.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ struct ClientInfo
std::atomic<Network::SocketInfo*> mpSocketPending; // Hold socket info until communication is established
std::atomic<Network::SocketInfo*> mpSocketComs; // Socket used for communications with server
std::atomic<Network::SocketInfo*> mpSocketListen; // Socket used to wait for communication request from server
std::atomic_bool mbDisconnectPending; // Terminate Client/Server coms
std::atomic_bool mbDisconnectListen; // Terminate waiting connection from Server
uint32_t mSocketListenPort = 0; // Socket Port number used to wait for communication request from server
VecTexture mTextures; // List if textures created by this client (used un main thread)
char mName[64] = {};
Expand All @@ -93,6 +95,10 @@ struct ClientInfo
ExchangePtr<CmdClipboard> mPendingClipboardIn; // Clipboard content received from Server and waiting to be taken by client
ExchangePtr<CmdClipboard> mPendingClipboardOut; // Clipboard content copied on Client and waiting to be sent to Server
ImGuiContext* mpContext = nullptr; // Context that the remote drawing should use (either the one active when connection request happened, or a clone)
PendingCom mPendingRcv; // Data being currently received from Server
PendingCom mPendingSend; // Data being currently sent to Server
uint32_t mPendingSendNext = 0; // Type of Cmd to next attempt sending, when channel is available
CmdPendingRead mCmdPendingRead; // Used to get info on the next incoming command from Server
CmdInput* mpCmdInputPending = nullptr; // Last Input Command from server, waiting to be processed by client
CmdClipboard* mpCmdClipboard = nullptr; // Last received clipboad command
CmdDrawFrame* mpCmdDrawLast = nullptr; // Last sent Draw Command. Used by data compression, to generate delta between previous and current frame
Expand All @@ -107,12 +113,9 @@ struct ClientInfo
SavedImguiContext mSavedContextValues;
std::atomic_uint32_t mTexturesPendingSent;
std::atomic_uint32_t mTexturesPendingCreated;

bool mbDisconnectListen = false; // Waiting to Stop listening to connection request
bool mbDisconnectRequest = false; // Waiting to Disconnect
bool mbDisconnectProcessed = false; // Disconnect command sent to server, ready to disconnect
bool mbClientThreadActive = false;
bool mbListenThreadActive = false;

bool mbClientThreadActive = false; // True when connected and communicating with Server
bool mbListenThreadActive = false; // True when listening from connection request from Server
bool mbHasTextureUpdate = false;
bool mbIsDrawing = false; // We are inside a 'NetImgui::NewFrame' / 'NetImgui::EndFrame' (even if not for a remote draw)
bool mbIsRemoteDrawing = false; // True if the rendering it meant for the remote netImgui server
Expand Down
49 changes: 32 additions & 17 deletions Code/Client/Private/NetImgui_CmdPackets.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,22 @@ namespace NetImgui { namespace Internal
{

//Note: If updating any of these commands data structure, increase 'CmdVersion::eVersion'

struct alignas(8) CmdHeader
{
enum class eCommands : uint8_t { Disconnect, Version, Texture, Input, DrawFrame, Background, Clipboard, Count };
CmdHeader(){}
enum class eCommands : uint8_t { Version, Texture, Input, DrawFrame, Background, Clipboard, Count };
CmdHeader(eCommands CmdType, uint16_t Size) : mSize(Size), mType(CmdType){}
uint32_t mSize = 0;
eCommands mType = eCommands::Count;
uint8_t mPadding[3] = {};
};

struct alignas(8) CmdDisconnect
// Used as step 1 of 2 of reading incoming transmission between Client/Server, to get header whose size we know
struct alignas(8) CmdPendingRead : public CmdHeader
{
CmdHeader mHeader = CmdHeader(CmdHeader::eCommands::Disconnect, sizeof(CmdDisconnect));
CmdPendingRead() : CmdHeader(eCommands::Count, sizeof(CmdPendingRead) ){}
};

struct alignas(8) CmdVersion
struct alignas(8) CmdVersion : public CmdHeader
{
enum class eVersion : uint32_t
{
Expand All @@ -43,6 +42,7 @@ struct alignas(8) CmdVersion
Clipboard = 14, // Added clipboard support between server/client
ForceReconnect = 15, // Server can now take over the connection from another server
UpdatedComs = 16, // Faster protocol by removing blocking coms
RemDisconnect = 17, // Removed Disconnect command
// Insert new version here

//--------------------------------
Expand All @@ -56,7 +56,7 @@ struct alignas(8) CmdVersion
ConnectForce = 0x04, // Server telling Client it want to take over connection if there's already one
ConnectExclusive = 0x08, // Server telling Client that once connected, others servers should be denied access
};
CmdHeader mHeader = CmdHeader(CmdHeader::eCommands::Version, sizeof(CmdVersion));
CmdVersion() : CmdHeader(CmdHeader::eCommands::Version, sizeof(CmdVersion)){}
char mClientName[64] = {};
char mImguiVerName[16] = {IMGUI_VERSION};
char mNetImguiVerName[16] = {NETIMGUI_VERSION};
Expand All @@ -68,7 +68,7 @@ struct alignas(8) CmdVersion
char PADDING[2];
};

struct alignas(8) CmdInput
struct alignas(8) CmdInput : public CmdHeader
{
// Identify a mouse button.
// Those values are guaranteed to be stable and we frequently use 0/1 directly. Named enums provided for convenience.
Expand Down Expand Up @@ -188,7 +188,7 @@ struct alignas(8) CmdInput
static constexpr uint32_t kAnalog_Last = ImGuiKey_GamepadRStickDown;
static constexpr uint32_t kAnalog_Count = kAnalog_Last - kAnalog_First + 1;

CmdHeader mHeader = CmdHeader(CmdHeader::eCommands::Input, sizeof(CmdInput));
CmdInput() : CmdHeader(CmdHeader::eCommands::Input, sizeof(CmdInput)){}
uint16_t mScreenSize[2] = {};
int16_t mMousePos[2] = {};
float mMouseWheelVert = 0.f;
Expand All @@ -205,9 +205,9 @@ struct alignas(8) CmdInput
inline bool IsKeyDown(NetImguiKeys netimguiKey) const;
};

struct alignas(8) CmdTexture
struct alignas(8) CmdTexture : public CmdHeader
{
CmdHeader mHeader = CmdHeader(CmdHeader::eCommands::Texture, sizeof(CmdTexture));
CmdTexture() : CmdHeader(CmdHeader::eCommands::Texture, sizeof(CmdTexture)){}
OffsetPointer<uint8_t> mpTextureData;
uint64_t mTextureId = 0;
uint16_t mWidth = 0;
Expand All @@ -216,9 +216,9 @@ struct alignas(8) CmdTexture
uint8_t PADDING[3] = {};
};

struct alignas(8) CmdDrawFrame
struct alignas(8) CmdDrawFrame : public CmdHeader
{
CmdHeader mHeader = CmdHeader(CmdHeader::eCommands::DrawFrame, sizeof(CmdDrawFrame));
CmdDrawFrame() : CmdHeader(CmdHeader::eCommands::DrawFrame, sizeof(CmdDrawFrame)){}
uint64_t mFrameIndex = 0;
uint32_t mMouseCursor = 0; // ImGuiMouseCursor value
float mDisplayArea[4] = {};
Expand All @@ -235,27 +235,42 @@ struct alignas(8) CmdDrawFrame
inline void ToOffsets();
};

struct alignas(8) CmdBackground
struct alignas(8) CmdBackground : public CmdHeader
{
CmdBackground() : CmdHeader(CmdHeader::eCommands::Background, sizeof(CmdBackground)){}
static constexpr uint64_t kDefaultTexture = ~0u;
CmdHeader mHeader = CmdHeader(CmdHeader::eCommands::Background, sizeof(CmdBackground));
float mClearColor[4] = {0.2f, 0.2f, 0.2f, 1.f}; // Background color
float mTextureTint[4] = {1.f, 1.f, 1.f, 0.5f}; // Tint/alpha applied to texture
uint64_t mTextureId = kDefaultTexture; // Texture rendered in background, use server texture by default
inline bool operator==(const CmdBackground& cmp)const;
inline bool operator!=(const CmdBackground& cmp)const;
};

struct alignas(8) CmdClipboard
struct alignas(8) CmdClipboard : public CmdHeader
{
CmdHeader mHeader = CmdHeader(CmdHeader::eCommands::Clipboard, sizeof(CmdClipboard));
CmdClipboard() : CmdHeader(CmdHeader::eCommands::Clipboard, sizeof(CmdClipboard)){}
size_t mByteSize = 0;
OffsetPointer<char> mContentUTF8;
inline void ToPointers();
inline void ToOffsets();
inline static CmdClipboard* Create(const char* clipboard);
};

//=============================================================================
// Keeping track of partial incoming/outgoing transmissions
//=============================================================================
struct PendingCom
{
size_t SizeCurrent = 0; // Amount of data sent or received so far
bool bAutoFree = false; // Need to free data buffer at the end of processing
bool bError = false; // If an error occurs during coms
CmdHeader* pCommand = nullptr; // Where to store incoming data or read to send data
inline bool IsError()const{ return bError; }
inline bool IsDone()const { return IsError() || (pCommand && pCommand->mSize == SizeCurrent); }
inline bool IsReady()const{ return !IsError() && pCommand == nullptr; }
inline bool IsPending()const{ return !IsError() && !IsDone() && !IsReady(); }
};

}} // namespace NetImgui::Internal

#include "NetImgui_CmdPackets.inl"
3 changes: 1 addition & 2 deletions Code/Client/Private/NetImgui_CmdPackets.inl
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,10 @@ CmdClipboard* CmdClipboard::Create(const char* clipboard)
while(clipboard[clipboardByteSize++] != 0);
size_t totalDataCount = sizeof(CmdClipboard) + DivUp<size_t>(clipboardByteSize, ComDataSize);
auto pNewClipboard = NetImgui::Internal::netImguiSizedNew<CmdClipboard>(totalDataCount*ComDataSize);
pNewClipboard->mHeader.mSize = static_cast<uint32_t>(totalDataCount*ComDataSize);
pNewClipboard->mSize = static_cast<uint32_t>(totalDataCount*ComDataSize);
pNewClipboard->mByteSize = clipboardByteSize;
pNewClipboard->mContentUTF8.SetPtr(reinterpret_cast<char*>(&pNewClipboard[1]));
memcpy(pNewClipboard->mContentUTF8.Get(), clipboard, clipboardByteSize);

return pNewClipboard;
}
return nullptr;
Expand Down
8 changes: 4 additions & 4 deletions Code/Client/Private/NetImgui_CmdPackets_DrawFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ CmdDrawFrame* CompressCmdDrawFrame(const CmdDrawFrame* pDrawFramePrev, const Cmd
//-----------------------------------------------------------------------------------------
// Allocate memory for worst case scenario (no compression possible)
// New DrawFrame size + 2 'compression block info' per data stream
size_t neededDataCount = DivUp<size_t>(pDrawFrameNew->mHeader.mSize, ComDataSize) + 6*static_cast<size_t>(pDrawFrameNew->mDrawGroupCount);
size_t neededDataCount = DivUp<size_t>(pDrawFrameNew->mSize, ComDataSize) + 6*static_cast<size_t>(pDrawFrameNew->mDrawGroupCount);
CmdDrawFrame* pDrawFramePacked = netImguiSizedNew<CmdDrawFrame>(neededDataCount*ComDataSize);
*pDrawFramePacked = *pDrawFrameNew;
pDrawFramePacked->mCompressed = true;
Expand Down Expand Up @@ -259,7 +259,7 @@ CmdDrawFrame* CompressCmdDrawFrame(const CmdDrawFrame* pDrawFramePrev, const Cmd
}

// Adjust data transfert amount to memory that has been actually needed
pDrawFramePacked->mHeader.mSize = static_cast<uint32_t>((pDataOutput - reinterpret_cast<ComDataType*>(pDrawFramePacked)))*static_cast<uint32_t>(sizeof(uint64_t));
pDrawFramePacked->mSize = static_cast<uint32_t>((pDataOutput - reinterpret_cast<ComDataType*>(pDrawFramePacked)))*static_cast<uint32_t>(sizeof(uint64_t));
return pDrawFramePacked;
}

Expand Down Expand Up @@ -370,8 +370,8 @@ CmdDrawFrame* ConvertToCmdDrawFrame(const ImDrawData* pDearImguiData, ImGuiMouse
pDrawFrame->mTotalDrawCount += drawGroup.mDrawCount;
}

pDrawFrame->mHeader.mSize = static_cast<uint32_t>(pDataOutput - reinterpret_cast<const ComDataType*>(pDrawFrame)) * ComDataSize;
pDrawFrame->mUncompressedSize = pDrawFrame->mHeader.mSize; // No compression with this item, so same value
pDrawFrame->mSize = static_cast<uint32_t>(pDataOutput - reinterpret_cast<const ComDataType*>(pDrawFrame)) * ComDataSize;
pDrawFrame->mUncompressedSize = pDrawFrame->mSize; // No compression with this item, so same value
return pDrawFrame;
}

Expand Down
8 changes: 5 additions & 3 deletions Code/Client/Private/NetImgui_Network.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#pragma once

namespace NetImgui { namespace Internal { namespace Network
namespace NetImgui { namespace Internal { struct PendingCom; }}

namespace NetImgui { namespace Internal { namespace Network
{

struct SocketInfo;
Expand All @@ -14,7 +16,7 @@ SocketInfo* ListenStart (uint32_t ListenPort); // Listening Socket expec
void Disconnect (SocketInfo* pClientSocket);

bool DataReceivePending (SocketInfo* pClientSocket); // True if some new data if waiting to be processed from remote connection
bool DataReceive (SocketInfo* pClientSocket, void* pDataIn, size_t Size); // Read X amount of bytes to remote connection. Will idle until size request is fullfilled.
bool DataSend (SocketInfo* pClientSocket, void* pDataOut, size_t Size); // Send x amount of bytes to remote connection. Will idle until size request is fullfilled.
void DataReceive (SocketInfo* pClientSocket, PendingCom& PendingComRcv); // Try reading X amount of bytes from remote connection, but can fall short.
void DataSend (SocketInfo* pClientSocket, PendingCom& PendingComSend); // Try sending X amount of bytes to remote connection, but can fall short.

}}} //namespace NetImgui::Internal::Network
83 changes: 37 additions & 46 deletions Code/Client/Private/NetImgui_NetworkWin32.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#pragma comment(lib, "ws2_32")
#endif

#include "NetImgui_CmdPackets.h"

namespace NetImgui { namespace Internal { namespace Network
{
Expand All @@ -31,12 +32,6 @@ struct SocketInfo

//constexpr int kComsRcvBuffer = 1014*1024;
//setsockopt(mSocket, SOL_SOCKET, SO_RCVBUF, reinterpret_cast<const char*>(&kComsRcvBuffer), sizeof(kComsRcvBuffer));

#if 0 // @sammyfreg : No timeout useful when debugging, to keep connection alive while code breakpoint
constexpr DWORD kComsTimeoutMs = 10000;
setsockopt(mSocket, SOL_SOCKET, SO_RCVTIMEO, reinterpret_cast<const char*>(&kComsTimeoutMs), sizeof(kComsTimeoutMs));
setsockopt(mSocket, SOL_SOCKET, SO_SNDTIMEO, reinterpret_cast<const char*>(&kComsTimeoutMs), sizeof(kComsTimeoutMs));
#endif
}

SOCKET mSocket;
Expand Down Expand Up @@ -178,57 +173,53 @@ bool DataReceivePending(SocketInfo* pClientSocket)
}

//=================================================================================================
// Block until all requested data has been received from the remote connection
// Try receiving data from remote connection
//=================================================================================================
bool DataReceive(SocketInfo* pClientSocket, void* pDataIn, size_t Size)
void DataReceive(SocketInfo* pClientSocket, NetImgui::Internal::PendingCom& PendingComRcv)
{
if( !pClientSocket ) return false;
// Invalid command
if( !pClientSocket || !PendingComRcv.pCommand ){
PendingComRcv.bError = true;
}

int totalRcv(0);
while( totalRcv < static_cast<int>(Size) )
{
int resultRcv = recv(pClientSocket->mSocket, &reinterpret_cast<char*>(pDataIn)[totalRcv], static_cast<int>(Size)-totalRcv, 0);
if( resultRcv != SOCKET_ERROR )
{
totalRcv += resultRcv;
}
else
{
if( WSAGetLastError() != WSAEWOULDBLOCK )
{
return false; // Connection error, abort transmission
}
std::this_thread::yield();
}
// Receive data from remote connection
int resultRcv = recv( pClientSocket->mSocket,
&reinterpret_cast<char*>(PendingComRcv.pCommand)[PendingComRcv.SizeCurrent],
static_cast<int>(PendingComRcv.pCommand->mSize-PendingComRcv.SizeCurrent),
0);

if( resultRcv != SOCKET_ERROR ){
PendingComRcv.SizeCurrent += static_cast<size_t>(resultRcv);
}
// Connection error, abort transmission
else if( WSAGetLastError() != WSAEWOULDBLOCK ){
PendingComRcv.bError = true;
}
return totalRcv == static_cast<int>(Size);
}

//=================================================================================================
// Block until all requested data has been sent to remote connection
//=================================================================================================
bool DataSend(SocketInfo* pClientSocket, void* pDataOut, size_t Size)
void DataSend(SocketInfo* pClientSocket, NetImgui::Internal::PendingCom& PendingComSend)
{
if( !pClientSocket ) return false;

int totalSent(0);
while( totalSent < static_cast<int>(Size) )
{
int resultSent = send(pClientSocket->mSocket, &reinterpret_cast<char*>(pDataOut)[totalSent], static_cast<int>(Size)-totalSent, 0);
if( resultSent != SOCKET_ERROR )
{
totalSent += resultSent;
}
else
{
if( WSAGetLastError() != WSAEWOULDBLOCK )
{
return false; // Connection error, abort transmission
}
std::this_thread::yield();
}
// Invalid command
if( !pClientSocket || !PendingComSend.pCommand ){
PendingComSend.bError = true;
}

// Send data to remote connection
int resultSent = send( pClientSocket->mSocket,
&reinterpret_cast<char*>(PendingComSend.pCommand)[PendingComSend.SizeCurrent],
static_cast<int>(PendingComSend.pCommand->mSize-PendingComSend.SizeCurrent),
0);

if( resultSent != SOCKET_ERROR ){
PendingComSend.SizeCurrent += static_cast<size_t>(resultSent);
}
// Connection error, abort transmission
else if( WSAGetLastError() != WSAEWOULDBLOCK ){
PendingComSend.bError = true;
}
return totalSent == static_cast<int>(Size);
}

}}} // namespace NetImgui::Internal::Network
Expand Down
4 changes: 2 additions & 2 deletions Code/Sample/Common/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,8 @@ int main(int, char**)
//=========================================================================================

// Present
HRESULT hr = g_pSwapChain->Present(1, 0); // Present with vsync
//HRESULT hr = g_pSwapChain->Present(0, 0); // Present without vsync
//HRESULT hr = g_pSwapChain->Present(1, 0); // Present with vsync
HRESULT hr = g_pSwapChain->Present(0, 0); // Present without vsync
g_SwapChainOccluded = (hr == DXGI_STATUS_OCCLUDED);
}

Expand Down
Loading

0 comments on commit 237dc78

Please sign in to comment.