From 191b8f950fcf77aa8c90b819f3d5b2dc3958ecfa Mon Sep 17 00:00:00 2001 From: David Fort Date: Sat, 23 Dec 2017 13:50:54 +0100 Subject: [PATCH] Fix for #4330 Since ec027bf dynamic resolution is broken when used with egfx. Before that commit we were tracking a server sent resize by setting a DesktopResize callback. This callback is called when the desktop is resized by the server. Anyway the problem was that when this callback is called, the activation sequence is not always completed, which were leading to some freeze with 2012r2 servers (sending packets before the sequence is finished). So with the faulty commit, we are tracking server resizes by subscribing to the Actived event, that is called at the end of a reactivation sequence, so we're sure to not send packets when not fully activated. Anyway the issue that shows on (#4330) is that when you use egfx, no reactivation sequence happens, the server only sends a ResetGraphics message with the new size, and so we miss the resized event. This fix introduces a new GraphicsReset event, makes the display channel subscribe to that event, and react accordingly. --- channels/rdpgfx/client/rdpgfx_codec.c | 3 +-- channels/rdpgfx/client/rdpgfx_main.c | 12 ++++++++++-- client/X11/xf_disp.c | 27 +++++++++++++++++++++++++++ include/freerdp/event.h | 5 +++++ libfreerdp/core/freerdp.c | 1 + 5 files changed, 44 insertions(+), 4 deletions(-) diff --git a/channels/rdpgfx/client/rdpgfx_codec.c b/channels/rdpgfx/client/rdpgfx_codec.c index 0ea15a394..83b27018f 100644 --- a/channels/rdpgfx/client/rdpgfx_codec.c +++ b/channels/rdpgfx/client/rdpgfx_codec.c @@ -139,8 +139,8 @@ static UINT rdpgfx_decode_AVC420(RDPGFX_PLUGIN* gfx, RDPGFX_SURFACE_COMMAND* cmd wStream* s; RDPGFX_AVC420_BITMAP_STREAM h264; RdpgfxClientContext* context = (RdpgfxClientContext*) gfx->iface.pInterface; - s = Stream_New(cmd->data, cmd->length); + s = Stream_New(cmd->data, cmd->length); if (!s) { WLog_ERR(TAG, "Stream_New failed!"); @@ -186,7 +186,6 @@ static UINT rdpgfx_decode_AVC444(RDPGFX_PLUGIN* gfx, RDPGFX_SURFACE_COMMAND* cmd RdpgfxClientContext* context = (RdpgfxClientContext*) gfx->iface.pInterface; s = Stream_New(cmd->data, cmd->length); - if (!s) { WLog_ERR(TAG, "Stream_New failed!"); diff --git a/channels/rdpgfx/client/rdpgfx_main.c b/channels/rdpgfx/client/rdpgfx_main.c index 0279ce5d2..d1e6cffd7 100644 --- a/channels/rdpgfx/client/rdpgfx_main.c +++ b/channels/rdpgfx/client/rdpgfx_main.c @@ -272,6 +272,7 @@ static UINT rdpgfx_recv_reset_graphics_pdu(RDPGFX_CHANNEL_CALLBACK* callback, RDPGFX_PLUGIN* gfx = (RDPGFX_PLUGIN*) callback->plugin; RdpgfxClientContext* context = (RdpgfxClientContext*) gfx->iface.pInterface; UINT error = CHANNEL_RC_OK; + GraphicsResetEventArgs graphicsReset; if (Stream_GetRemainingLength(s) < 12) { @@ -335,6 +336,12 @@ static UINT rdpgfx_recv_reset_graphics_pdu(RDPGFX_CHANNEL_CALLBACK* callback, WLog_Print(gfx->log, WLOG_ERROR, "context->ResetGraphics failed with error %"PRIu32"", error); } + /* some listeners may be interested (namely the display channel) */ + EventArgsInit(&graphicsReset, "xfreerdp"); + graphicsReset.width = pdu.width; + graphicsReset.height = pdu.height; + PubSub_OnGraphicsReset(gfx->rdpcontext->pubSub, gfx->rdpcontext, &graphicsReset); + free(pdu.monitorDefArray); return error; } @@ -1691,8 +1698,9 @@ UINT DVCPluginEntry(IDRDYNVC_ENTRY_POINTS* pEntryPoints) gfx->iface.Connected = NULL; gfx->iface.Disconnected = NULL; gfx->iface.Terminated = rdpgfx_plugin_terminated; - gfx->SurfaceTable = HashTable_New(TRUE); + gfx->rdpcontext = ((freerdp *)gfx->settings->instance)->context; + gfx->SurfaceTable = HashTable_New(TRUE); if (!gfx->SurfaceTable) { free(gfx); @@ -1712,8 +1720,8 @@ UINT DVCPluginEntry(IDRDYNVC_ENTRY_POINTS* pEntryPoints) gfx->SmallCache = TRUE; gfx->MaxCacheSlot = gfx->SmallCache ? 4096 : 25600; - context = (RdpgfxClientContext *)calloc(1, sizeof(RdpgfxClientContext)); + context = (RdpgfxClientContext *)calloc(1, sizeof(RdpgfxClientContext)); if (!context) { free(gfx); diff --git a/client/X11/xf_disp.c b/client/X11/xf_disp.c index a21e7f85c..835f56e4b 100644 --- a/client/X11/xf_disp.c +++ b/client/X11/xf_disp.c @@ -120,6 +120,32 @@ static void xf_disp_OnActivated(rdpContext* context, ActivatedEventArgs* e) } } + +static void xf_disp_OnGraphicsReset(rdpContext* context, GraphicsResetEventArgs* e) +{ + xfContext *xfc = (xfContext *)context; + xfDispContext *xfDisp = xfc->xfDisp; + rdpSettings *settings = context->settings; + + xfDisp->waitingResize = FALSE; + + if (xfDisp->activated && !settings->Fullscreen) + { + xf_disp_set_window_resizable(xfDisp); + + /* if a resize has been done recently don't do anything and let the timer + * perform the resize */ + if (GetTickCount64() - xfDisp->lastSentDate < RESIZE_MIN_DELAY) + return; + + if ((xfDisp->lastSentWidth != xfDisp->targetWidth) || (xfDisp->lastSentHeight != xfDisp->targetHeight)) + { + WLog_DBG(TAG, "performing delayed resize to %dx%d", xfDisp->targetWidth, xfDisp->targetHeight); + xf_disp_sendResize(xfDisp, xfDisp->targetWidth, xfDisp->targetHeight); + } + } +} + static void xf_disp_OnTimer(rdpContext* context, TimerEventArgs* e) { xfContext *xfc = (xfContext *)context; @@ -156,6 +182,7 @@ xfDispContext *xf_disp_new(xfContext* xfc) ret->lastSentHeight = ret->targetHeight = xfc->context.settings->DesktopHeight; PubSub_SubscribeActivated(xfc->context.pubSub, (pActivatedEventHandler)xf_disp_OnActivated); + PubSub_SubscribeGraphicsReset(xfc->context.pubSub, (pGraphicsResetEventHandler)xf_disp_OnGraphicsReset); PubSub_SubscribeTimer(xfc->context.pubSub, (pTimerEventHandler)xf_disp_OnTimer); return ret; } diff --git a/include/freerdp/event.h b/include/freerdp/event.h index 947349db6..de76afad0 100644 --- a/include/freerdp/event.h +++ b/include/freerdp/event.h @@ -108,6 +108,11 @@ DEFINE_EVENT_BEGIN(Timer) UINT64 now; DEFINE_EVENT_END(Timer) +DEFINE_EVENT_BEGIN(GraphicsReset) +UINT32 width; +UINT32 height; +DEFINE_EVENT_END(GraphicsReset) + #ifdef __cplusplus } #endif diff --git a/libfreerdp/core/freerdp.c b/libfreerdp/core/freerdp.c index 052ba904b..a2d949fbc 100644 --- a/libfreerdp/core/freerdp.c +++ b/libfreerdp/core/freerdp.c @@ -606,6 +606,7 @@ static wEventType FreeRDP_Events[] = DEFINE_EVENT_ENTRY(MouseEvent) DEFINE_EVENT_ENTRY(Activated) DEFINE_EVENT_ENTRY(Timer) + DEFINE_EVENT_ENTRY(GraphicsReset) }; /** Allocator function for a rdp context.