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

Synchronization between renderer and handlers doesn't work #2004

Closed
wants to merge 3 commits into from
Closed

Synchronization between renderer and handlers doesn't work #2004

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7901915
fix: Added synchronization outside of Render() (#1985)
v-ein Dec 29, 2022
a82f84c
fix: Replaced mutex with recursive_mutex (#1985)
v-ein Dec 30, 2022
9a0a60f
Merge branch 'master' into bugfix/1985-sync-issues
hoffstadt Mar 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
251 changes: 142 additions & 109 deletions src/dearpygui_commands.h
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/mvContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ Render()
mvToolManager::Draw();

{
std::lock_guard<std::mutex> lk(GContext->mutex);
std::lock_guard<std::recursive_mutex> lk(GContext->mutex);
if (GContext->resetTheme)
{
SetDefaultTheme();
Expand Down
3 changes: 1 addition & 2 deletions src/mvContext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,8 @@ struct mvIO
struct mvContext
{
std::atomic_bool waitOneFrame = false;
std::atomic_bool manualMutexControl = false;
std::atomic_bool started = false;
std::mutex mutex;
std::recursive_mutex mutex;
std::future<bool> future;
float deltaTime = 0.0f; // time since last frame
double time = 0.0; // total time since starting
Expand Down
2 changes: 1 addition & 1 deletion src/mvItemRegistry.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1365,7 +1365,7 @@ AddItemWithRuntimeChecks(mvItemRegistry& registry, std::shared_ptr<mvAppItem> it
};
AddTechnique technique = AddTechnique::NONE;

if (!GContext->manualMutexControl) std::lock_guard<std::mutex> lk(GContext->mutex);
std::lock_guard<std::recursive_mutex> lk(GContext->mutex);

//---------------------------------------------------------------------------
// STEP 2: handle root case
Expand Down
2 changes: 2 additions & 0 deletions src/mvLayoutWindow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ void mvLayoutWindow::renderTreeNode(std::shared_ptr<mvAppItem>& item)
void mvLayoutWindow::drawWidgets()
{

std::lock_guard<std::recursive_mutex> lk(GContext->mutex);

mvUUID parentName = 0;

if (_itemref == nullptr && GContext->itemRegistry->windowRoots.size() > 0)
Expand Down
2 changes: 2 additions & 0 deletions src/mvToolWindow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ void mvToolWindow::draw()
GContext->input.mousePos.x = (int)x;
GContext->input.mousePos.y = (int)y;

std::lock_guard<std::recursive_mutex> lk(GContext->mutex);

if (GContext->itemRegistry->activeWindow != getUUID())
GContext->itemRegistry->activeWindow = getUUID();

Expand Down
130 changes: 75 additions & 55 deletions src/mvViewport_win32.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,40 +44,45 @@ mvPrerender(mvViewport& viewport)
if (viewportData->msg.message == WM_QUIT)
viewport.running = false;

if (viewport.posDirty)
{
int horizontal_shift = get_horizontal_shift(viewportData->handle);
SetWindowPos(viewportData->handle, viewport.alwaysOnTop ? HWND_TOPMOST : HWND_TOP, viewport.xpos-horizontal_shift, viewport.ypos, 0, 0, SWP_SHOWWINDOW | SWP_NOSIZE);
viewport.posDirty = false;
}

if (viewport.sizeDirty)
{
SetWindowPos(viewportData->handle, viewport.alwaysOnTop ? HWND_TOPMOST : HWND_TOP, 0, 0, viewport.actualWidth, viewport.actualHeight, SWP_SHOWWINDOW | SWP_NOMOVE);
viewport.sizeDirty = false;
}
// TODO: we probably need a separate mutex for this
std::lock_guard<std::recursive_mutex> lk(GContext->mutex);

if (viewport.modesDirty)
{
viewportData->modes = WS_OVERLAPPED;

if (viewport.resizable && viewport.decorated) viewportData->modes |= WS_THICKFRAME;
if (viewport.decorated) {
viewportData->modes |= WS_CAPTION | WS_SYSMENU | WS_MINIMIZEBOX | WS_MAXIMIZEBOX;
if (viewport.posDirty)
{
int horizontal_shift = get_horizontal_shift(viewportData->handle);
SetWindowPos(viewportData->handle, viewport.alwaysOnTop ? HWND_TOPMOST : HWND_TOP, viewport.xpos-horizontal_shift, viewport.ypos, 0, 0, SWP_SHOWWINDOW | SWP_NOSIZE);
viewport.posDirty = false;
}
else {
viewportData->modes |= WS_POPUP;

if (viewport.sizeDirty)
{
SetWindowPos(viewportData->handle, viewport.alwaysOnTop ? HWND_TOPMOST : HWND_TOP, 0, 0, viewport.actualWidth, viewport.actualHeight, SWP_SHOWWINDOW | SWP_NOMOVE);
viewport.sizeDirty = false;
}

SetWindowLongPtr(viewportData->handle, GWL_STYLE, viewportData->modes);
SetWindowPos(viewportData->handle, viewport.alwaysOnTop ? HWND_TOPMOST : HWND_NOTOPMOST, 0, 0, 0, 0, SWP_NOMOVE | SWP_NOSIZE | SWP_SHOWWINDOW);
viewport.modesDirty = false;
}
if (viewport.modesDirty)
{
viewportData->modes = WS_OVERLAPPED;

if (viewport.titleDirty)
{
SetWindowTextA(viewportData->handle, viewport.title.c_str());
viewport.titleDirty = false;
if (viewport.resizable && viewport.decorated) viewportData->modes |= WS_THICKFRAME;
if (viewport.decorated) {
viewportData->modes |= WS_CAPTION | WS_SYSMENU | WS_MINIMIZEBOX | WS_MAXIMIZEBOX;
}
else {
viewportData->modes |= WS_POPUP;
}

SetWindowLongPtr(viewportData->handle, GWL_STYLE, viewportData->modes);
SetWindowPos(viewportData->handle, viewport.alwaysOnTop ? HWND_TOPMOST : HWND_NOTOPMOST, 0, 0, 0, 0, SWP_NOMOVE | SWP_NOSIZE | SWP_SHOWWINDOW);
viewport.modesDirty = false;
}

if (viewport.titleDirty)
{
SetWindowTextA(viewportData->handle, viewport.title.c_str());
viewport.titleDirty = false;
}
}

// Poll and handle messages (inputs, window resize, etc.)
Expand All @@ -96,11 +101,16 @@ mvPrerender(mvViewport& viewport)
//continue;
}

if (mvToolManager::GetFontManager().isInvalid())
{
mvToolManager::GetFontManager().rebuildAtlas();
ImGui_ImplDX11_InvalidateDeviceObjects();
mvToolManager::GetFontManager().updateAtlas();
// Font manager is thread-unsafe, so we'd better sync it
std::lock_guard<std::recursive_mutex> lk(GContext->mutex);

if (mvToolManager::GetFontManager().isInvalid())
{
mvToolManager::GetFontManager().rebuildAtlas();
ImGui_ImplDX11_InvalidateDeviceObjects();
mvToolManager::GetFontManager().updateAtlas();
}
}

// Start the Dear ImGui frame
Expand Down Expand Up @@ -146,30 +156,34 @@ mvHandleMsg(HWND hWnd, UINT msg, WPARAM wParam, LPARAM lParam) noexcept
cheight = crect.bottom - crect.top;
}

viewport->actualWidth = awidth;
viewport->actualHeight = aheight;


if (viewport->decorated)
{
GContext->viewport->clientHeight = cheight;
GContext->viewport->clientWidth = cwidth;
}
else
{
GContext->viewport->clientHeight = cheight;
GContext->viewport->clientWidth = cwidth;
}

//GContext->viewport->resized = true;
mvOnResize();
GContext->viewport->resized = false;

if (mvToolManager::GetFontManager().isInvalid())
{
mvToolManager::GetFontManager().rebuildAtlas();
ImGui_ImplDX11_InvalidateDeviceObjects();
mvToolManager::GetFontManager().updateAtlas();
std::lock_guard<std::recursive_mutex> lk(GContext->mutex);

viewport->actualWidth = awidth;
viewport->actualHeight = aheight;


if (viewport->decorated)
{
GContext->viewport->clientHeight = cheight;
GContext->viewport->clientWidth = cwidth;
}
else
{
GContext->viewport->clientHeight = cheight + 39;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't this break #1130 ? Not sure where these lines came from, I didn't have them in the original fix...

Copy link
Owner

Choose a reason for hiding this comment

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

Ooh. I believe so. Let me check (this was several fixes in a single commit).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have you had a chance to look at this? Any plans on how to handle it? Since this PR is closed and seems to have been included into 1.9, we probably need to open another PR to change the lines back, right?

I'm asking because I've started updating to 1.9 and would like to have a clean base upon which to cherry-pick my commits...

GContext->viewport->clientWidth = cwidth + 16;
}

//GContext->viewport->resized = true;
mvOnResize();
GContext->viewport->resized = false;

if (mvToolManager::GetFontManager().isInvalid())
{
mvToolManager::GetFontManager().rebuildAtlas();
ImGui_ImplDX11_InvalidateDeviceObjects();
mvToolManager::GetFontManager().updateAtlas();
}
}
// Start the Dear ImGui frame
ImGui_ImplDX11_NewFrame();
Expand All @@ -182,6 +196,8 @@ mvHandleMsg(HWND hWnd, UINT msg, WPARAM wParam, LPARAM lParam) noexcept

case WM_GETMINMAXINFO:
{
// TODO: lock the mutex?

LPMINMAXINFO lpMMI = (LPMINMAXINFO)lParam;
lpMMI->ptMinTrackSize.x = viewport->minwidth;
lpMMI->ptMinTrackSize.y = viewport->minheight;
Expand All @@ -192,6 +208,8 @@ mvHandleMsg(HWND hWnd, UINT msg, WPARAM wParam, LPARAM lParam) noexcept

case WM_MOVING:
{
std::lock_guard<std::recursive_mutex> lk(GContext->mutex);

int horizontal_shift = get_horizontal_shift(viewportData->handle);
RECT rect = *(RECT*)(lParam);
viewport->xpos = rect.left + horizontal_shift;
Expand Down Expand Up @@ -221,6 +239,8 @@ mvHandleMsg(HWND hWnd, UINT msg, WPARAM wParam, LPARAM lParam) noexcept
cheight = crect.bottom - crect.top;
}

std::lock_guard<std::recursive_mutex> lk(GContext->mutex);

viewport->actualWidth = awidth;
viewport->actualHeight = aheight;

Expand Down