[Spice-devel] [vdagent-win PATCH v3 1/5] Move image handling to a separate file
Pavel Grunt
pgrunt at redhat.com
Fri Jul 14 13:32:46 UTC 2017
On Fri, 2017-07-14 at 13:57 +0100, Frediano Ziglio wrote:
> This will make easier to change code that handle images.
>
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
> Makefile.am | 2 ++
> vdagent/image.cpp | 86
> +++++++++++++++++++++++++++++++++++++++++++++++++++++
> vdagent/image.h | 48 ++++++++++++++++++++++++++++++
> vdagent/vdagent.cpp | 57 +++++------------------------------
> 4 files changed, 144 insertions(+), 49 deletions(-)
> create mode 100644 vdagent/image.cpp
> create mode 100644 vdagent/image.h
>
> diff --git a/Makefile.am b/Makefile.am
> index b60a718..868199e 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -42,6 +42,8 @@ vdagent_SOURCES = \
> vdagent/vdagent.cpp \
> vdagent/as_user.cpp \
> vdagent/as_user.h \
> + vdagent/image.cpp \
> + vdagent/image.h \
> $(NULL)
>
> vdagent_rc.$(OBJEXT): vdagent/vdagent.rc
> diff --git a/vdagent/image.cpp b/vdagent/image.cpp
> new file mode 100644
> index 0000000..598a742
> --- /dev/null
> +++ b/vdagent/image.cpp
> @@ -0,0 +1,86 @@
> +/*
> + Copyright (C) 2013-2017 Red Hat, Inc.
> +
> + This program is free software; you can redistribute it and/or
> + modify it under the terms of the GNU General Public License as
> + published by the Free Software Foundation; either version 2 of
> + the License, or (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>.
> +*/
> +
> +#include <spice/macros.h>
> +
> +#include "vdcommon.h"
> +#include "image.h"
> +
> +#include "ximage.h"
> +
> +typedef struct ImageType {
> + uint32_t type;
> + DWORD cximage_format;
> +} ImageType;
> +
> +static const ImageType image_types[] = {
> + {VD_AGENT_CLIPBOARD_IMAGE_PNG, CXIMAGE_FORMAT_PNG},
> + {VD_AGENT_CLIPBOARD_IMAGE_BMP, CXIMAGE_FORMAT_BMP},
> +};
> +
> +static DWORD get_cximage_format(uint32_t type)
> +{
> + for (unsigned int i = 0; i < SPICE_N_ELEMENTS(image_types); i++) {
> + if (image_types[i].type == type) {
> + return image_types[i].cximage_format;
> + }
> + }
> + return 0;
> +}
> +
> +HANDLE get_image_handle(const VDAgentClipboard& clipboard, uint32_t size,
> UINT &format)
> +{
> + HANDLE clip_data;
> + DWORD cximage_format = get_cximage_format(clipboard.type);
> + ASSERT(cximage_format);
> + CxImage image((BYTE*)clipboard.data, size, cximage_format);
> + clip_data = image.CopyToHandle();
> + return clip_data;
> +}
> +
> +uint8_t* get_raw_clipboard_image(const VDAgentClipboardRequest&
> clipboard_request,
> + HANDLE clip_data, long& new_size)
> +{
> + CxImage image;
> + uint8_t *new_data = NULL;
> + DWORD cximage_format = get_cximage_format(clipboard_request.type);
> + HPALETTE pal = 0;
I'd suggest to set new_size to zero to match the documentation (size of returned
data)
> +
> + ASSERT(cximage_format);
> + if (IsClipboardFormatAvailable(CF_PALETTE)) {
> + pal = (HPALETTE)GetClipboardData(CF_PALETTE);
> + }
> + if (!image.CreateFromHBITMAP((HBITMAP)clip_data, pal)) {
> + vd_printf("Image create from handle failed");
> + return NULL;
> + }
> + if (!image.Encode(new_data, new_size, cximage_format)) {
> + vd_printf("Image encode to type %u failed", clipboard_request.type);
> + return NULL;
> + }
> + vd_printf("Image encoded to %lu bytes", new_size);
> + return new_data;
> +}
> +
> +void free_raw_clipboard_image(uint8_t *data)
> +{
> + // this is really just a free however is better to make
> + // the free from CxImage code as on Windows the free
> + // can be different between libraries
> + CxImage image;
> + image.FreeMemory(data);
> +}
> diff --git a/vdagent/image.h b/vdagent/image.h
> new file mode 100644
> index 0000000..379c5cc
> --- /dev/null
> +++ b/vdagent/image.h
> @@ -0,0 +1,48 @@
> +/*
> + Copyright (C) 2013-2017 Red Hat, Inc.
> +
> + This program is free software; you can redistribute it and/or
> + modify it under the terms of the GNU General Public License as
> + published by the Free Software Foundation; either version 2 of
> + the License, or (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>.
> +*/
> +
> +#ifndef VDAGENT_IMAGE_H_
> +#define VDAGENT_IMAGE_H_
> +
> +/**
> + * Returns image to put in the clipboard.
> + *
> + * @param clipboard data to write in the clipboard
> + * @param size size of data
> + * @param[in,out] format suggested clipboard format. This can be changed
> by
> + * the function to reflect a better format
> + */
> +HANDLE get_image_handle(const VDAgentClipboard& clipboard, uint32_t size,
> UINT& format);
> +
> +/**
> + * Return raw data got from the clipboard.
> + *
> + * Function could use clip_data or get new data from the clipboard.
> + * You should free data returned with free_raw_clipboard_image.
> + * @param clipboard_request request
> + * @param clip_data clipboard data
> + * @param new_size size of returned data
[out]
> + */
> +uint8_t* get_raw_clipboard_image(const VDAgentClipboardRequest&
> clipboard_request,
> + HANDLE clip_data, long& new_size);
> +
> +/**
> + * Free data returned by get_raw_clipboard_image
> + */
> +void free_raw_clipboard_image(uint8_t *data);
> +
> +#endif
> diff --git a/vdagent/vdagent.cpp b/vdagent/vdagent.cpp
> index bb07e1d..f00fbf5 100644
> --- a/vdagent/vdagent.cpp
> +++ b/vdagent/vdagent.cpp
> @@ -19,7 +19,7 @@
> #include "desktop_layout.h"
> #include "display_setting.h"
> #include "file_xfer.h"
> -#include "ximage.h"
> +#include "image.h"
> #undef max
> #undef min
> #include <spice/macros.h>
> @@ -55,16 +55,6 @@ static const VDClipboardFormat clipboard_formats[] = {
>
> #define clipboard_formats_count SPICE_N_ELEMENTS(clipboard_formats)
>
> -typedef struct ImageType {
> - uint32_t type;
> - DWORD cximage_format;
> -} ImageType;
> -
> -static const ImageType image_types[] = {
> - {VD_AGENT_CLIPBOARD_IMAGE_PNG, CXIMAGE_FORMAT_PNG},
> - {VD_AGENT_CLIPBOARD_IMAGE_BMP, CXIMAGE_FORMAT_BMP},
> -};
> -
> typedef struct ALIGN_VC VDIChunk {
> VDIChunkHeader hdr;
> uint8_t data[0];
> @@ -725,23 +715,19 @@ bool VDAgent::handle_clipboard(VDAgentClipboard*
> clipboard, uint32_t size)
> if (clipboard->type == VD_AGENT_CLIPBOARD_NONE) {
> goto fin;
> }
> + format = get_clipboard_format(clipboard->type);
> switch (clipboard->type) {
> case VD_AGENT_CLIPBOARD_UTF8_TEXT:
> clip_data = utf8_alloc((LPCSTR)clipboard->data, size);
> break;
> case VD_AGENT_CLIPBOARD_IMAGE_PNG:
> - case VD_AGENT_CLIPBOARD_IMAGE_BMP: {
> - DWORD cximage_format = get_cximage_format(clipboard->type);
> - ASSERT(cximage_format);
> - CxImage image(clipboard->data, size, cximage_format);
> - clip_data = image.CopyToHandle();
> + case VD_AGENT_CLIPBOARD_IMAGE_BMP:
> + clip_data = get_image_handle(*clipboard, size, format);
> break;
> - }
> default:
> vd_printf("Unsupported clipboard type %u", clipboard->type);
> goto fin;
> }
> - format = get_clipboard_format(clipboard->type);
> if (format == 0) {
> vd_printf("Unknown clipboard format, type %u", clipboard->type);
> goto fin;
> @@ -1104,7 +1090,6 @@ bool
> VDAgent::handle_clipboard_request(VDAgentClipboardRequest* clipboard_reques
> uint8_t* new_data = NULL;
> long new_size = 0;
> size_t len = 0;
> - CxImage image;
> VDAgentClipboard* clipboard = NULL;
>
> if (_clipboard_owner != owner_guest) {
> @@ -1135,28 +1120,12 @@ bool
> VDAgent::handle_clipboard_request(VDAgentClipboardRequest* clipboard_reques
> new_size = WideCharToMultiByte(CP_UTF8, 0, (LPCWSTR)new_data,
> (int)len, NULL, 0, NULL, NULL);
> break;
> case VD_AGENT_CLIPBOARD_IMAGE_PNG:
> - case VD_AGENT_CLIPBOARD_IMAGE_BMP: {
> - DWORD cximage_format = get_cximage_format(clipboard_request->type);
> - HPALETTE pal = 0;
> -
> - ASSERT(cximage_format);
> - if (IsClipboardFormatAvailable(CF_PALETTE)) {
> - pal = (HPALETTE)GetClipboardData(CF_PALETTE);
> - }
> - if (!image.CreateFromHBITMAP((HBITMAP)clip_data, pal)) {
> - vd_printf("Image create from handle failed");
> - break;
> - }
> - if (!image.Encode(new_data, new_size, cximage_format)) {
> - vd_printf("Image encode to type %u failed", clipboard_request-
> >type);
> - break;
> - }
> - vd_printf("Image encoded to %lu bytes", new_size);
> + case VD_AGENT_CLIPBOARD_IMAGE_BMP:
> + new_data = get_raw_clipboard_image(*clipboard_request, clip_data,
> new_size);
> break;
> }
> - }
>
> - if (!new_size) {
> + if (!new_size || !new_data) {
imho it should not happen that size > 0 when data = NULL.
Ack,
Pavel
> vd_printf("clipboard is empty");
> goto handle_clipboard_request_fail;
> }
> @@ -1184,7 +1153,7 @@ bool
> VDAgent::handle_clipboard_request(VDAgentClipboardRequest* clipboard_reques
> case VD_AGENT_CLIPBOARD_IMAGE_PNG:
> case VD_AGENT_CLIPBOARD_IMAGE_BMP:
> memcpy(clipboard->data, new_data, new_size);
> - image.FreeMemory(new_data);
> + free_raw_clipboard_image(new_data);
> break;
> }
> CloseClipboard();
> @@ -1242,16 +1211,6 @@ uint32_t VDAgent::get_clipboard_type(uint32_t format)
> const
> return 0;
> }
>
> -DWORD VDAgent::get_cximage_format(uint32_t type) const
> -{
> - for (unsigned int i = 0; i < SPICE_N_ELEMENTS(image_types); i++) {
> - if (image_types[i].type == type) {
> - return image_types[i].cximage_format;
> - }
> - }
> - return 0;
> -}
> -
> void VDAgent::set_clipboard_owner(int new_owner)
> {
> // FIXME: Clear requests, clipboard data and state
More information about the Spice-devel
mailing list