From afde5271207e5ab83dab81fcb06ebd334d1d1aeb Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 23 May 2024 09:10:59 +0200 Subject: [PATCH 1/5] [gdi,gfx] unify updatesurfaces calls --- libfreerdp/gdi/gfx.c | 93 +++++++++++++------------------------------- 1 file changed, 28 insertions(+), 65 deletions(-) diff --git a/libfreerdp/gdi/gfx.c b/libfreerdp/gdi/gfx.c index 9d82ea4a1..03b931a1e 100644 --- a/libfreerdp/gdi/gfx.c +++ b/libfreerdp/gdi/gfx.c @@ -305,6 +305,12 @@ static UINT gdi_StartFrame(RdpgfxClientContext* context, const RDPGFX_START_FRAM return CHANNEL_RC_OK; } +static UINT gdi_call_update_surfaces(RdpgfxClientContext* context) +{ + WINPR_ASSERT(context); + return IFCALLRESULT(CHANNEL_RC_OK, context->UpdateSurfaces, context); +} + /** * Function description * @@ -312,19 +318,25 @@ static UINT gdi_StartFrame(RdpgfxClientContext* context, const RDPGFX_START_FRAM */ static UINT gdi_EndFrame(RdpgfxClientContext* context, const RDPGFX_END_FRAME_PDU* endFrame) { - UINT status = CHANNEL_RC_OK; - rdpGdi* gdi = NULL; - WINPR_ASSERT(context); WINPR_ASSERT(endFrame); - gdi = (rdpGdi*)context->custom; + rdpGdi* gdi = (rdpGdi*)context->custom; WINPR_ASSERT(gdi); - IFCALLRET(context->UpdateSurfaces, status, context); + const UINT status = gdi_call_update_surfaces(context); gdi->inGfxFrame = FALSE; return status; } +static UINT gdi_interFrameUpdate(rdpGdi* gdi, RdpgfxClientContext* context) +{ + WINPR_ASSERT(gdi); + UINT status = CHANNEL_RC_OK; + if (!gdi->inGfxFrame) + status = gdi_call_update_surfaces(context); + return status; +} + /** * Function description * @@ -379,11 +391,7 @@ static UINT gdi_SurfaceCommand_Uncompressed(rdpGdi* gdi, RdpgfxClientContext* co if (status != CHANNEL_RC_OK) goto fail; - if (!gdi->inGfxFrame) - { - status = CHANNEL_RC_NOT_INITIALIZED; - IFCALLRET(context->UpdateSurfaces, status, context); - } + status = gdi_interFrameUpdate(gdi, context); fail: return status; @@ -438,11 +446,7 @@ static UINT gdi_SurfaceCommand_RemoteFX(rdpGdi* gdi, RdpgfxClientContext* contex for (UINT32 x = 0; x < nrRects; x++) region16_union_rect(&surface->invalidRegion, &surface->invalidRegion, &rects[x]); - if (!gdi->inGfxFrame) - { - status = CHANNEL_RC_NOT_INITIALIZED; - IFCALLRET(context->UpdateSurfaces, status, context); - } + status = gdi_interFrameUpdate(gdi, context); fail: region16_uninit(&invalidRegion); @@ -497,11 +501,7 @@ static UINT gdi_SurfaceCommand_ClearCodec(rdpGdi* gdi, RdpgfxClientContext* cont if (status != CHANNEL_RC_OK) goto fail; - if (!gdi->inGfxFrame) - { - status = CHANNEL_RC_NOT_INITIALIZED; - IFCALLRET(context->UpdateSurfaces, status, context); - } + status = gdi_interFrameUpdate(gdi, context); fail: return status; @@ -554,11 +554,7 @@ static UINT gdi_SurfaceCommand_Planar(rdpGdi* gdi, RdpgfxClientContext* context, if (status != CHANNEL_RC_OK) goto fail; - if (!gdi->inGfxFrame) - { - status = CHANNEL_RC_NOT_INITIALIZED; - IFCALLRET(context->UpdateSurfaces, status, context); - } + status = gdi_interFrameUpdate(gdi, context); fail: return status; @@ -637,11 +633,7 @@ static UINT gdi_SurfaceCommand_AVC420(rdpGdi* gdi, RdpgfxClientContext* context, if (status != CHANNEL_RC_OK) goto fail; - if (!gdi->inGfxFrame) - { - status = CHANNEL_RC_NOT_INITIALIZED; - IFCALLRET(context->UpdateSurfaces, status, context); - } + status = gdi_interFrameUpdate(gdi, context); fail: return status; @@ -742,11 +734,7 @@ static UINT gdi_SurfaceCommand_AVC444(rdpGdi* gdi, RdpgfxClientContext* context, if (status != CHANNEL_RC_OK) goto fail; - if (!gdi->inGfxFrame) - { - status = CHANNEL_RC_NOT_INITIALIZED; - IFCALLRET(context->UpdateSurfaces, status, context); - } + status = gdi_interFrameUpdate(gdi, context); fail: return status; @@ -918,11 +906,7 @@ static UINT gdi_SurfaceCommand_Alpha(rdpGdi* gdi, RdpgfxClientContext* context, if (status != CHANNEL_RC_OK) goto fail; - if (!gdi->inGfxFrame) - { - status = CHANNEL_RC_NOT_INITIALIZED; - IFCALLRET(context->UpdateSurfaces, status, context); - } + status = gdi_interFrameUpdate(gdi, context); fail: return status; @@ -999,11 +983,7 @@ static UINT gdi_SurfaceCommand_Progressive(rdpGdi* gdi, RdpgfxClientContext* con region16_uninit(&invalidRegion); - if (!gdi->inGfxFrame) - { - status = CHANNEL_RC_NOT_INITIALIZED; - IFCALLRET(context->UpdateSurfaces, status, context); - } + status = gdi_interFrameUpdate(gdi, context); fail: return status; @@ -1297,13 +1277,7 @@ static UINT gdi_SolidFill(RdpgfxClientContext* context, const RDPGFX_SOLID_FILL_ LeaveCriticalSection(&context->mux); - if (!gdi->inGfxFrame) - { - status = CHANNEL_RC_NOT_INITIALIZED; - IFCALLRET(context->UpdateSurfaces, status, context); - } - - return status; + return gdi_interFrameUpdate(gdi, context); fail: LeaveCriticalSection(&context->mux); return status; @@ -1375,13 +1349,7 @@ static UINT gdi_SurfaceToSurface(RdpgfxClientContext* context, LeaveCriticalSection(&context->mux); - if (!gdi->inGfxFrame) - { - status = CHANNEL_RC_NOT_INITIALIZED; - IFCALLRET(context->UpdateSurfaces, status, context); - } - - return status; + return gdi_interFrameUpdate(gdi, context); fail: LeaveCriticalSection(&context->mux); return status; @@ -1527,12 +1495,7 @@ static UINT gdi_CacheToSurface(RdpgfxClientContext* context, LeaveCriticalSection(&context->mux); - if (!gdi->inGfxFrame) - status = IFCALLRESULT(CHANNEL_RC_NOT_INITIALIZED, context->UpdateSurfaces, context); - else - status = CHANNEL_RC_OK; - - return status; + return gdi_interFrameUpdate(gdi, context); fail: LeaveCriticalSection(&context->mux); From 1a58e74c17a367dbbdd7038ded8ecd03eeddbb5b Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 23 May 2024 10:00:02 +0200 Subject: [PATCH 2/5] [codec,color] freerdp_image_copy_no_overlap use single memcopy if possible to speed up copy --- libfreerdp/codec/color.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/libfreerdp/codec/color.c b/libfreerdp/codec/color.c index 2c3d08a36..eeb7fbe6a 100644 --- a/libfreerdp/codec/color.c +++ b/libfreerdp/codec/color.c @@ -645,13 +645,22 @@ static BOOL freerdp_image_copy_no_overlap(BYTE* WINPR_RESTRICT pDstData, DWORD D } else if (FreeRDPAreColorFormatsEqualNoAlpha(SrcFormat, DstFormat)) { - for (SSIZE_T y = 0; y < nHeight; y++) + if (!vSrcVFlip && (nDstStep == nSrcStep) && (xSrcOffset == 0) && (xDstOffset == 0)) { - const BYTE* WINPR_RESTRICT srcLine = - &pSrcData[srcVMultiplier * (y + nYSrc) * nSrcStep + srcVOffset]; - BYTE* WINPR_RESTRICT dstLine = - &pDstData[dstVMultiplier * (y + nYDst) * nDstStep + dstVOffset]; - memcpy(&dstLine[xDstOffset], &srcLine[xSrcOffset], copyDstWidth); + const void* src = &pSrcData[1ull * nYSrc * nSrcStep]; + void* dst = &pDstData[1ull * nYDst * nDstStep]; + memcpy(dst, src, 1ull * nDstStep * nHeight); + } + else + { + for (SSIZE_T y = 0; y < nHeight; y++) + { + const BYTE* WINPR_RESTRICT srcLine = + &pSrcData[srcVMultiplier * (y + nYSrc) * nSrcStep + srcVOffset]; + BYTE* WINPR_RESTRICT dstLine = + &pDstData[dstVMultiplier * (y + nYDst) * nDstStep + dstVOffset]; + memcpy(&dstLine[xDstOffset], &srcLine[xSrcOffset], copyDstWidth); + } } } else From 1b3f3a0408ba19894e85958ddb2ebb720d4730b8 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 23 May 2024 10:07:14 +0200 Subject: [PATCH 3/5] [codec,color] expose freerdp_image_copy (no)overlap expose functions for overlapping and non overlapping images to use directly --- include/freerdp/codec/color.h | 18 ++++++++++++- libfreerdp/codec/clear.c | 10 ++++--- libfreerdp/codec/color.c | 24 ++++++++--------- libfreerdp/codec/interleaved.c | 11 ++++---- libfreerdp/codec/nsc.c | 5 ++-- libfreerdp/codec/planar.c | 5 ++-- libfreerdp/codec/progressive.c | 6 ++--- libfreerdp/codec/rfx.c | 6 ++--- libfreerdp/gdi/gdi.c | 18 +++++++------ libfreerdp/gdi/gfx.c | 49 +++++++++++++++++++++------------- libfreerdp/gdi/graphics.c | 16 ++++++----- server/shadow/Mac/mac_shadow.c | 6 ++--- server/shadow/Win/win_shadow.c | 5 ++-- server/shadow/X11/x11_shadow.c | 8 +++--- server/shadow/shadow_client.c | 4 +-- 15 files changed, 114 insertions(+), 77 deletions(-) diff --git a/include/freerdp/codec/color.h b/include/freerdp/codec/color.h index 10b35ef9d..f96a8fd75 100644 --- a/include/freerdp/codec/color.h +++ b/include/freerdp/codec/color.h @@ -352,7 +352,8 @@ typedef struct gdi_palette gdiPalette; UINT32 xorMaskLength, const BYTE* WINPR_RESTRICT andMask, UINT32 andMaskLength, UINT32 xorBpp, const gdiPalette* WINPR_RESTRICT palette); - /*** + /*** Copies an image from source to destination, converting if necessary. + * Source and destination may overlap. * * @param pDstData destination buffer * @param DstFormat destination buffer format @@ -377,6 +378,21 @@ typedef struct gdi_palette gdiPalette; UINT32 nXSrc, UINT32 nYSrc, const gdiPalette* palette, UINT32 flags); + /*** Same as freerdp_image_copy() but only for overlapping source and destination + */ + FREERDP_API BOOL freerdp_image_copy_overlap( + BYTE* WINPR_RESTRICT pDstData, DWORD DstFormat, UINT32 nDstStep, UINT32 nXDst, UINT32 nYDst, + UINT32 nWidth, UINT32 nHeight, const BYTE* WINPR_RESTRICT pSrcData, DWORD SrcFormat, + UINT32 nSrcStep, UINT32 nXSrc, UINT32 nYSrc, const gdiPalette* WINPR_RESTRICT palette, + UINT32 flags); + + /*** Same as freerdp_image_copy() but only for non overlapping source and destination + */ + FREERDP_API BOOL freerdp_image_copy_no_overlap( + BYTE* WINPR_RESTRICT pDstData, DWORD DstFormat, UINT32 nDstStep, UINT32 nXDst, UINT32 nYDst, + UINT32 nWidth, UINT32 nHeight, const BYTE* WINPR_RESTRICT pSrcData, DWORD SrcFormat, + UINT32 nSrcStep, UINT32 nXSrc, UINT32 nYSrc, const gdiPalette* WINPR_RESTRICT palette, + UINT32 flags); /*** * * @param pDstData destination buffer diff --git a/libfreerdp/codec/clear.c b/libfreerdp/codec/clear.c index 551e93b08..664b11c3e 100644 --- a/libfreerdp/codec/clear.c +++ b/libfreerdp/codec/clear.c @@ -124,8 +124,9 @@ static BOOL convert_color(BYTE* dst, UINT32 nDstStep, UINT32 DstFormat, UINT32 n if (nHeight + nYDst > nDstHeight) nHeight = nDstHeight - nYDst; - return freerdp_image_copy(dst, DstFormat, nDstStep, nXDst, nYDst, nWidth, nHeight, src, - SrcFormat, nSrcStep, 0, 0, palette, FREERDP_KEEP_DST_ALPHA); + return freerdp_image_copy_no_overlap(dst, DstFormat, nDstStep, nXDst, nYDst, nWidth, nHeight, + src, SrcFormat, nSrcStep, 0, 0, palette, + FREERDP_KEEP_DST_ALPHA); } static BOOL clear_decompress_nscodec(NSC_CONTEXT* nsc, UINT32 width, UINT32 height, wStream* s, @@ -1118,8 +1119,9 @@ INT32 clear_decompress(CLEAR_CONTEXT* clear, const BYTE* pSrcData, UINT32 SrcSiz if (glyphData) { - if (!freerdp_image_copy(glyphData, clear->format, 0, 0, 0, nWidth, nHeight, pDstData, - DstFormat, nDstStep, nXDst, nYDst, palette, FREERDP_KEEP_DST_ALPHA)) + if (!freerdp_image_copy_no_overlap(glyphData, clear->format, 0, 0, 0, nWidth, nHeight, + pDstData, DstFormat, nDstStep, nXDst, nYDst, palette, + FREERDP_KEEP_DST_ALPHA)) goto fail; } diff --git a/libfreerdp/codec/color.c b/libfreerdp/codec/color.c index eeb7fbe6a..7f26ad9a7 100644 --- a/libfreerdp/codec/color.c +++ b/libfreerdp/codec/color.c @@ -241,8 +241,8 @@ BOOL freerdp_image_copy_from_icon_data(BYTE* WINPR_RESTRICT pDstData, UINT32 Dst return FALSE; fill_gdi_palette_for_icon(colorTable, cbColorTable, &palette); - if (!freerdp_image_copy(pDstData, DstFormat, nDstStep, nXDst, nYDst, nWidth, nHeight, bitsColor, - format, 0, 0, 0, &palette, FREERDP_FLIP_VERTICAL)) + if (!freerdp_image_copy_no_overlap(pDstData, DstFormat, nDstStep, nXDst, nYDst, nWidth, nHeight, + bitsColor, format, 0, 0, 0, &palette, FREERDP_FLIP_VERTICAL)) return FALSE; /* apply alpha mask */ @@ -576,12 +576,11 @@ static INLINE BOOL overlapping(const BYTE* pDstData, UINT32 nXDst, UINT32 nYDst, return FALSE; } -static BOOL freerdp_image_copy_no_overlap(BYTE* WINPR_RESTRICT pDstData, DWORD DstFormat, - UINT32 nDstStep, UINT32 nXDst, UINT32 nYDst, - UINT32 nWidth, UINT32 nHeight, - const BYTE* WINPR_RESTRICT pSrcData, DWORD SrcFormat, - UINT32 nSrcStep, UINT32 nXSrc, UINT32 nYSrc, - const gdiPalette* WINPR_RESTRICT palette, UINT32 flags) +BOOL freerdp_image_copy_no_overlap(BYTE* WINPR_RESTRICT pDstData, DWORD DstFormat, UINT32 nDstStep, + UINT32 nXDst, UINT32 nYDst, UINT32 nWidth, UINT32 nHeight, + const BYTE* WINPR_RESTRICT pSrcData, DWORD SrcFormat, + UINT32 nSrcStep, UINT32 nXSrc, UINT32 nYSrc, + const gdiPalette* WINPR_RESTRICT palette, UINT32 flags) { const SSIZE_T dstByte = FreeRDPGetBytesPerPixel(DstFormat); const SSIZE_T srcByte = FreeRDPGetBytesPerPixel(SrcFormat); @@ -696,11 +695,10 @@ static BOOL freerdp_image_copy_no_overlap(BYTE* WINPR_RESTRICT pDstData, DWORD D return TRUE; } -static BOOL freerdp_image_copy_overlap(BYTE* pDstData, DWORD DstFormat, UINT32 nDstStep, - UINT32 nXDst, UINT32 nYDst, UINT32 nWidth, UINT32 nHeight, - const BYTE* pSrcData, DWORD SrcFormat, UINT32 nSrcStep, - UINT32 nXSrc, UINT32 nYSrc, const gdiPalette* palette, - UINT32 flags) +BOOL freerdp_image_copy_overlap(BYTE* pDstData, DWORD DstFormat, UINT32 nDstStep, UINT32 nXDst, + UINT32 nYDst, UINT32 nWidth, UINT32 nHeight, const BYTE* pSrcData, + DWORD SrcFormat, UINT32 nSrcStep, UINT32 nXSrc, UINT32 nYSrc, + const gdiPalette* palette, UINT32 flags) { const UINT32 dstByte = FreeRDPGetBytesPerPixel(DstFormat); const UINT32 srcByte = FreeRDPGetBytesPerPixel(SrcFormat); diff --git a/libfreerdp/codec/interleaved.c b/libfreerdp/codec/interleaved.c index 262895d02..af32cab46 100644 --- a/libfreerdp/codec/interleaved.c +++ b/libfreerdp/codec/interleaved.c @@ -622,9 +622,9 @@ BOOL interleaved_decompress(BITMAP_INTERLEAVED_CONTEXT* interleaved, const BYTE* return FALSE; } - if (!freerdp_image_copy(pDstData, DstFormat, nDstStep, nXDst, nYDst, nDstWidth, nDstHeight, - interleaved->TempBuffer, SrcFormat, scanline, 0, 0, palette, - FREERDP_FLIP_VERTICAL | FREERDP_KEEP_DST_ALPHA)) + if (!freerdp_image_copy_no_overlap(pDstData, DstFormat, nDstStep, nXDst, nYDst, nDstWidth, + nDstHeight, interleaved->TempBuffer, SrcFormat, scanline, 0, + 0, palette, FREERDP_FLIP_VERTICAL | FREERDP_KEEP_DST_ALPHA)) { WLog_ERR(TAG, "freerdp_image_copy failed"); return FALSE; @@ -681,8 +681,9 @@ BOOL interleaved_compress(BITMAP_INTERLEAVED_CONTEXT* interleaved, BYTE* pDstDat return FALSE; } - if (!freerdp_image_copy(interleaved->TempBuffer, DstFormat, 0, 0, 0, nWidth, nHeight, pSrcData, - SrcFormat, nSrcStep, nXSrc, nYSrc, palette, FREERDP_KEEP_DST_ALPHA)) + if (!freerdp_image_copy_no_overlap(interleaved->TempBuffer, DstFormat, 0, 0, 0, nWidth, nHeight, + pSrcData, SrcFormat, nSrcStep, nXSrc, nYSrc, palette, + FREERDP_KEEP_DST_ALPHA)) return FALSE; s = Stream_New(pDstData, *pDstSize); diff --git a/libfreerdp/codec/nsc.c b/libfreerdp/codec/nsc.c index 4b8709f12..8bf3da88a 100644 --- a/libfreerdp/codec/nsc.c +++ b/libfreerdp/codec/nsc.c @@ -505,8 +505,9 @@ BOOL nsc_process_message(NSC_CONTEXT* context, UINT16 bpp, UINT32 width, UINT32 return FALSE; } - if (!freerdp_image_copy(pDstData, DstFormat, nDstStride, nXDst, nYDst, width, height, - context->BitmapData, PIXEL_FORMAT_BGRA32, 0, 0, 0, NULL, flip)) + if (!freerdp_image_copy_no_overlap(pDstData, DstFormat, nDstStride, nXDst, nYDst, width, height, + context->BitmapData, PIXEL_FORMAT_BGRA32, 0, 0, 0, NULL, + flip)) return FALSE; return TRUE; diff --git a/libfreerdp/codec/planar.c b/libfreerdp/codec/planar.c index d63cf06f0..2b6d04eee 100644 --- a/libfreerdp/codec/planar.c +++ b/libfreerdp/codec/planar.c @@ -978,8 +978,9 @@ BOOL planar_decompress(BITMAP_PLANAR_CONTEXT* planar, const BYTE* pSrcData, UINT if (pTempData != pDstData) { - if (!freerdp_image_copy(pDstData, DstFormat, nDstStep, nXDst, nYDst, w, h, pTempData, - TempFormat, nTempStep, nXDst, nYDst, NULL, FREERDP_FLIP_NONE)) + if (!freerdp_image_copy_no_overlap(pDstData, DstFormat, nDstStep, nXDst, nYDst, w, h, + pTempData, TempFormat, nTempStep, nXDst, nYDst, NULL, + FREERDP_FLIP_NONE)) { WLog_ERR(TAG, "planar image copy failed"); return FALSE; diff --git a/libfreerdp/codec/progressive.c b/libfreerdp/codec/progressive.c index 563731e47..9c45e7d20 100644 --- a/libfreerdp/codec/progressive.c +++ b/libfreerdp/codec/progressive.c @@ -2365,9 +2365,9 @@ static BOOL update_tiles(PROGRESSIVE_CONTEXT* progressive, PROGRESSIVE_SURFACE_C goto fail; if (rect->top + height > surface->height) goto fail; - rc = freerdp_image_copy(pDstData, DstFormat, nDstStep, rect->left, rect->top, width, - height, tile->data, progressive->format, tile->stride, nXSrc, - nYSrc, NULL, FREERDP_KEEP_DST_ALPHA); + rc = freerdp_image_copy_no_overlap( + pDstData, DstFormat, nDstStep, rect->left, rect->top, width, height, tile->data, + progressive->format, tile->stride, nXSrc, nYSrc, NULL, FREERDP_KEEP_DST_ALPHA); if (!rc) break; diff --git a/libfreerdp/codec/rfx.c b/libfreerdp/codec/rfx.c index c12f6d493..30cc2dba4 100644 --- a/libfreerdp/codec/rfx.c +++ b/libfreerdp/codec/rfx.c @@ -1327,9 +1327,9 @@ BOOL rfx_process_message(RFX_CONTEXT* context, const BYTE* data, UINT32 length, const UINT32 nWidth = updateRects[j].right - updateRects[j].left; const UINT32 nHeight = updateRects[j].bottom - updateRects[j].top; - if (!freerdp_image_copy(dst, dstFormat, dstStride, nXDst, nYDst, nWidth, nHeight, - tile->data, context->pixel_format, stride, nXSrc, nYSrc, - NULL, FREERDP_FLIP_NONE)) + if (!freerdp_image_copy_no_overlap(dst, dstFormat, dstStride, nXDst, nYDst, nWidth, + nHeight, tile->data, context->pixel_format, + stride, nXSrc, nYSrc, NULL, FREERDP_FLIP_NONE)) { region16_uninit(&updateRegion); WLog_Print(context->priv->log, WLOG_ERROR, diff --git a/libfreerdp/gdi/gdi.c b/libfreerdp/gdi/gdi.c index 832ae24e6..2ef01f2e7 100644 --- a/libfreerdp/gdi/gdi.c +++ b/libfreerdp/gdi/gdi.c @@ -637,8 +637,9 @@ static BOOL gdi_patblt(rdpContext* context, PATBLT_ORDER* patblt) brushFormat = gdi_get_pixel_format(bpp); - if (!freerdp_image_copy(data, gdi->drawing->hdc->format, 0, 0, 0, 8, 8, brush->data, - brushFormat, 0, 0, 0, &gdi->palette, FREERDP_FLIP_NONE)) + if (!freerdp_image_copy_no_overlap(data, gdi->drawing->hdc->format, 0, 0, 0, 8, 8, + brush->data, brushFormat, 0, 0, 0, &gdi->palette, + FREERDP_FLIP_NONE)) goto out_error; } else @@ -898,8 +899,9 @@ static BOOL gdi_mem3blt(rdpContext* context, MEM3BLT_ORDER* mem3blt) brushFormat = gdi_get_pixel_format(bpp); - if (!freerdp_image_copy(data, gdi->drawing->hdc->format, 0, 0, 0, 8, 8, brush->data, - brushFormat, 0, 0, 0, &gdi->palette, FREERDP_FLIP_NONE)) + if (!freerdp_image_copy_no_overlap(data, gdi->drawing->hdc->format, 0, 0, 0, 8, 8, + brush->data, brushFormat, 0, 0, 0, &gdi->palette, + FREERDP_FLIP_NONE)) { ret = FALSE; winpr_aligned_free(data); @@ -1108,10 +1110,10 @@ static BOOL gdi_surface_bits(rdpContext* context, const SURFACE_BITS_COMMAND* cm goto out; } - if (!freerdp_image_copy(gdi->primary_buffer, gdi->dstFormat, gdi->stride, cmdRect.left, - cmdRect.top, cmdRect.right - cmdRect.left, - cmdRect.bottom - cmdRect.top, cmd->bmp.bitmapData, format, 0, 0, - 0, &gdi->palette, FREERDP_FLIP_VERTICAL)) + if (!freerdp_image_copy_no_overlap( + gdi->primary_buffer, gdi->dstFormat, gdi->stride, cmdRect.left, cmdRect.top, + cmdRect.right - cmdRect.left, cmdRect.bottom - cmdRect.top, cmd->bmp.bitmapData, + format, 0, 0, 0, &gdi->palette, FREERDP_FLIP_VERTICAL)) { WLog_ERR(TAG, "Failed to process nocodec message"); goto out; diff --git a/libfreerdp/gdi/gfx.c b/libfreerdp/gdi/gfx.c index 03b931a1e..c35734301 100644 --- a/libfreerdp/gdi/gfx.c +++ b/libfreerdp/gdi/gfx.c @@ -375,9 +375,9 @@ static UINT gdi_SurfaceCommand_Uncompressed(rdpGdi* gdi, RdpgfxClientContext* co return ERROR_INVALID_DATA; } - if (!freerdp_image_copy(surface->data, surface->format, surface->scanline, cmd->left, cmd->top, - cmd->width, cmd->height, cmd->data, cmd->format, 0, 0, 0, NULL, - FREERDP_FLIP_NONE)) + if (!freerdp_image_copy_no_overlap(surface->data, surface->format, surface->scanline, cmd->left, + cmd->top, cmd->width, cmd->height, cmd->data, cmd->format, 0, + 0, 0, NULL, FREERDP_FLIP_NONE)) return ERROR_INTERNAL_ERROR; invalidRect.left = (UINT16)MIN(UINT16_MAX, cmd->left); @@ -1332,11 +1332,22 @@ static UINT gdi_SurfaceToSurface(RdpgfxClientContext* context, if (!is_rect_valid(&rect, surfaceDst->width, surfaceDst->height)) goto fail; - if (!freerdp_image_copy(surfaceDst->data, surfaceDst->format, surfaceDst->scanline, - destPt->x, destPt->y, nWidth, nHeight, surfaceSrc->data, - surfaceSrc->format, surfaceSrc->scanline, rectSrc->left, - rectSrc->top, NULL, FREERDP_FLIP_NONE)) - goto fail; + if (surfaceDst == surfaceSrc) + { + if (!freerdp_image_copy_overlap( + surfaceDst->data, surfaceDst->format, surfaceDst->scanline, destPt->x, + destPt->y, nWidth, nHeight, surfaceSrc->data, surfaceSrc->format, + surfaceSrc->scanline, rectSrc->left, rectSrc->top, NULL, FREERDP_FLIP_NONE)) + goto fail; + } + else + { + if (!freerdp_image_copy_no_overlap( + surfaceDst->data, surfaceDst->format, surfaceDst->scanline, destPt->x, + destPt->y, nWidth, nHeight, surfaceSrc->data, surfaceSrc->format, + surfaceSrc->scanline, rectSrc->left, rectSrc->top, NULL, FREERDP_FLIP_NONE)) + goto fail; + } invalidRect = rect; region16_union_rect(&surfaceDst->invalidRegion, &surfaceDst->invalidRegion, &invalidRect); @@ -1422,9 +1433,10 @@ static UINT gdi_SurfaceToCache(RdpgfxClientContext* context, if (!cacheEntry->data) goto fail; - if (!freerdp_image_copy(cacheEntry->data, cacheEntry->format, cacheEntry->scanline, 0, 0, - cacheEntry->width, cacheEntry->height, surface->data, surface->format, - surface->scanline, rect->left, rect->top, NULL, FREERDP_FLIP_NONE)) + if (!freerdp_image_copy_no_overlap(cacheEntry->data, cacheEntry->format, cacheEntry->scanline, + 0, 0, cacheEntry->width, cacheEntry->height, surface->data, + surface->format, surface->scanline, rect->left, rect->top, + NULL, FREERDP_FLIP_NONE)) goto fail; RDPGFX_EVICT_CACHE_ENTRY_PDU evict = { surfaceToCache->cacheSlot }; @@ -1478,10 +1490,10 @@ static UINT gdi_CacheToSurface(RdpgfxClientContext* context, if (!is_rect_valid(&rect, surface->width, surface->height)) goto fail; - if (!freerdp_image_copy(surface->data, surface->format, surface->scanline, destPt->x, - destPt->y, cacheEntry->width, cacheEntry->height, cacheEntry->data, - cacheEntry->format, cacheEntry->scanline, 0, 0, NULL, - FREERDP_FLIP_NONE)) + if (!freerdp_image_copy_no_overlap(surface->data, surface->format, surface->scanline, + destPt->x, destPt->y, cacheEntry->width, + cacheEntry->height, cacheEntry->data, cacheEntry->format, + cacheEntry->scanline, 0, 0, NULL, FREERDP_FLIP_NONE)) goto fail; invalidRect = rect; @@ -1566,9 +1578,10 @@ static UINT gdi_ImportCacheEntry(RdpgfxClientContext* context, UINT16 cacheSlot, if (!cacheEntry) goto fail; - if (!freerdp_image_copy(cacheEntry->data, cacheEntry->format, cacheEntry->scanline, 0, 0, - cacheEntry->width, cacheEntry->height, importCacheEntry->data, - PIXEL_FORMAT_BGRX32, 0, 0, 0, NULL, FREERDP_FLIP_NONE)) + if (!freerdp_image_copy_no_overlap(cacheEntry->data, cacheEntry->format, cacheEntry->scanline, + 0, 0, cacheEntry->width, cacheEntry->height, + importCacheEntry->data, PIXEL_FORMAT_BGRX32, 0, 0, 0, NULL, + FREERDP_FLIP_NONE)) goto fail; RDPGFX_EVICT_CACHE_ENTRY_PDU evict = { cacheSlot }; diff --git a/libfreerdp/gdi/graphics.c b/libfreerdp/gdi/graphics.c index 28860cf12..5c1409fc1 100644 --- a/libfreerdp/gdi/graphics.c +++ b/libfreerdp/gdi/graphics.c @@ -59,8 +59,9 @@ HGDI_BITMAP gdi_create_bitmap(rdpGdi* gdi, UINT32 nWidth, UINT32 nHeight, UINT32 pSrcData = data; nSrcStep = nWidth * FreeRDPGetBytesPerPixel(SrcFormat); - if (!freerdp_image_copy(pDstData, gdi->dstFormat, nDstStep, 0, 0, nWidth, nHeight, pSrcData, - SrcFormat, nSrcStep, 0, 0, &gdi->palette, FREERDP_FLIP_NONE)) + if (!freerdp_image_copy_no_overlap(pDstData, gdi->dstFormat, nDstStep, 0, 0, nWidth, nHeight, + pSrcData, SrcFormat, nSrcStep, 0, 0, &gdi->palette, + FREERDP_FLIP_NONE)) { winpr_aligned_free(pDstData); return NULL; @@ -183,9 +184,9 @@ static BOOL gdi_Bitmap_Decompress(rdpContext* context, rdpBitmap* bitmap, const return FALSE; } - return freerdp_image_copy(bitmap->data, bitmap->format, 0, 0, 0, DstWidth, DstHeight, - pSrcData, PIXEL_FORMAT_XRGB32, 0, 0, 0, &gdi->palette, - FREERDP_FLIP_VERTICAL); + return freerdp_image_copy_no_overlap(bitmap->data, bitmap->format, 0, 0, 0, DstWidth, + DstHeight, pSrcData, PIXEL_FORMAT_XRGB32, 0, 0, 0, + &gdi->palette, FREERDP_FLIP_VERTICAL); } else if (bpp < 32) { @@ -231,8 +232,9 @@ static BOOL gdi_Bitmap_Decompress(rdpContext* context, rdpBitmap* bitmap, const } } - if (!freerdp_image_copy(bitmap->data, bitmap->format, 0, 0, 0, DstWidth, DstHeight, - pSrcData, SrcFormat, 0, 0, 0, &gdi->palette, FREERDP_FLIP_VERTICAL)) + if (!freerdp_image_copy_no_overlap(bitmap->data, bitmap->format, 0, 0, 0, DstWidth, + DstHeight, pSrcData, SrcFormat, 0, 0, 0, &gdi->palette, + FREERDP_FLIP_VERTICAL)) { WLog_ERR(TAG, "freerdp_image_copy failed"); return FALSE; diff --git a/server/shadow/Mac/mac_shadow.c b/server/shadow/Mac/mac_shadow.c index ba6d2b05f..54384d022 100644 --- a/server/shadow/Mac/mac_shadow.c +++ b/server/shadow/Mac/mac_shadow.c @@ -405,9 +405,9 @@ static void (^mac_capture_stream_handler)( } else { - freerdp_image_copy(surface->data, surface->format, surface->scanline, x, y, width, height, - pSrcData, PIXEL_FORMAT_BGRX32, nSrcStep, x, y, NULL, - FREERDP_FLIP_NONE); + freerdp_image_copy_no_overlap(surface->data, surface->format, surface->scanline, x, y, + width, height, pSrcData, PIXEL_FORMAT_BGRX32, nSrcStep, x, + y, NULL, FREERDP_FLIP_NONE); } LeaveCriticalSection(&(surface->lock)); diff --git a/server/shadow/Win/win_shadow.c b/server/shadow/Win/win_shadow.c index d3d5a177e..f05a55cf2 100644 --- a/server/shadow/Win/win_shadow.c +++ b/server/shadow/Win/win_shadow.c @@ -314,8 +314,9 @@ static int win_shadow_surface_copy(winShadowSubsystem* subsystem) if (status <= 0) return status; - if (!freerdp_image_copy(surface->data, surface->format, surface->scanline, x, y, width, height, - pDstData, DstFormat, nDstStep, x, y, NULL, FREERDP_FLIP_NONE)) + if (!freerdp_image_copy_no_overlap(surface->data, surface->format, surface->scanline, x, y, + width, height, pDstData, DstFormat, nDstStep, x, y, NULL, + FREERDP_FLIP_NONE)) return ERROR_INTERNAL_ERROR; ArrayList_Lock(server->clients); diff --git a/server/shadow/X11/x11_shadow.c b/server/shadow/X11/x11_shadow.c index 1e4f945dd..2649b0af3 100644 --- a/server/shadow/X11/x11_shadow.c +++ b/server/shadow/X11/x11_shadow.c @@ -853,10 +853,10 @@ static int x11_shadow_screen_grab(x11ShadowSubsystem* subsystem) WINPR_ASSERT(image->bytes_per_line >= 0); WINPR_ASSERT(width >= 0); WINPR_ASSERT(height >= 0); - success = freerdp_image_copy(surface->data, surface->format, surface->scanline, x, y, - (UINT32)width, (UINT32)height, (BYTE*)image->data, - subsystem->format, (UINT32)image->bytes_per_line, x, y, - NULL, FREERDP_FLIP_NONE); + success = freerdp_image_copy_no_overlap( + surface->data, surface->format, surface->scanline, x, y, (UINT32)width, + (UINT32)height, (BYTE*)image->data, subsystem->format, + (UINT32)image->bytes_per_line, x, y, NULL, FREERDP_FLIP_NONE); LeaveCriticalSection(&surface->lock); if (!success) goto fail_capture; diff --git a/server/shadow/shadow_client.c b/server/shadow/shadow_client.c index f0b4dc457..b5aa8c243 100644 --- a/server/shadow/shadow_client.c +++ b/server/shadow/shadow_client.c @@ -1335,8 +1335,8 @@ static BOOL shadow_client_send_surface_gfx(rdpShadowClient* client, const BYTE* WINPR_ASSERT(data); - rc = freerdp_image_copy(data, PIXEL_FORMAT_BGRA32, 0, 0, 0, w, h, pSrcData, SrcFormat, - nSrcStep, cmd.left, cmd.top, NULL, 0); + rc = freerdp_image_copy_no_overlap(data, PIXEL_FORMAT_BGRA32, 0, 0, 0, w, h, pSrcData, + SrcFormat, nSrcStep, cmd.left, cmd.top, NULL, 0); WINPR_ASSERT(rc); cmd.data = data; From ef86df9a26557fb4aacdec7beb74eb5f6936974c Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 23 May 2024 11:02:34 +0200 Subject: [PATCH 4/5] [crypto,tls] log BIO_do_handshake errors add proper logging to make details of failures auditable --- libfreerdp/crypto/tls.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/libfreerdp/crypto/tls.c b/libfreerdp/crypto/tls.c index 2d400389b..906e2501c 100644 --- a/libfreerdp/crypto/tls.c +++ b/libfreerdp/crypto/tls.c @@ -898,6 +898,12 @@ TlsHandshakeResult freerdp_tls_connect_ex(rdpTls* tls, BIO* underlying, const SS return freerdp_tls_handshake(tls); } +static int bio_err_print(const char* str, size_t len, void* u) +{ + wLog* log = u; + WLog_Print(log, WLOG_ERROR, "[BIO_do_handshake] %s [%" PRIuz "]", str, len); +} + TlsHandshakeResult freerdp_tls_handshake(rdpTls* tls) { TlsHandshakeResult ret = TLS_HANDSHAKE_ERROR; @@ -907,7 +913,12 @@ TlsHandshakeResult freerdp_tls_handshake(rdpTls* tls) if (status != 1) { if (!BIO_should_retry(tls->bio)) + { + wLog* log = WLog_Get(TAG); + WLog_Print(log, WLOG_ERROR, "BIO_do_handshake failed"); + ERR_print_errors_cb(bio_err_print, log); return TLS_HANDSHAKE_ERROR; + } return TLS_HANDSHAKE_CONTINUE; } From 776a7b727f6c8528dcc2bd55da8bd21121e8ae69 Mon Sep 17 00:00:00 2001 From: akallabeth Date: Thu, 23 May 2024 11:48:11 +0200 Subject: [PATCH 5/5] [core,gcc] clear multitransport if the client does not send a multitransport capability clear the flags. --- libfreerdp/core/gcc.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/libfreerdp/core/gcc.c b/libfreerdp/core/gcc.c index f2d035a9c..1e3b32632 100644 --- a/libfreerdp/core/gcc.c +++ b/libfreerdp/core/gcc.c @@ -572,6 +572,9 @@ BOOL gcc_read_client_data_blocks(wStream* s, rdpMcs* mcs, UINT16 length) { WINPR_ASSERT(s); WINPR_ASSERT(mcs); + + BOOL gotMultitransport = FALSE; + while (length > 0) { wStream sbuffer = { 0 }; @@ -641,6 +644,7 @@ BOOL gcc_read_client_data_blocks(wStream* s, rdpMcs* mcs, UINT16 length) case 0xC009: case CS_MULTITRANSPORT: + gotMultitransport = TRUE; if (!gcc_read_client_multitransport_channel_data(sub, mcs)) return FALSE; @@ -676,6 +680,14 @@ BOOL gcc_read_client_data_blocks(wStream* s, rdpMcs* mcs, UINT16 length) length -= blockLength; } + if (!gotMultitransport) + { + rdpSettings* settings = mcs_get_settings(mcs); + if (!freerdp_settings_set_bool(settings, FreeRDP_SupportMultitransport, FALSE)) + return FALSE; + if (!freerdp_settings_set_uint32(settings, FreeRDP_MultitransportFlags, 0)) + return FALSE; + } return TRUE; }