[Spice-devel] [vdagent-win PATCH 3/7] Move image handling to a separate file
Frediano Ziglio
fziglio at redhat.com
Fri Jul 14 12:45:37 UTC 2017
>
> On Thu, 2017-07-06 at 14:32 +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 | 46 ++++++++++++++++++++++++++++
> > vdagent/vdagent.cpp | 57 +++++------------------------------
> > 4 files changed, 142 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..fb19dbc
> > --- /dev/null
> > +++ b/vdagent/image.cpp
> > @@ -0,0 +1,86 @@
> > +/*
> > + Copyright (C) 2013-2016 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"
>
>
> (it is going to be removed, but it is a system header, per our style the
> order
> should be different)
>
> > +
> > +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;
> > +
> > + 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..9ba6003
> > --- /dev/null
> > +++ b/vdagent/image.h
> > @@ -0,0 +1,46 @@
> > +/*
> > + Copyright (C) 2013-2016 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 format format decided. This can be changed by the function
> > to
> > + * reflect a better format
> > + */
>
> so doxygen. Doesn't it require an empty line between the description(s) and
> the
> list of parameters? Maybe it is not required, but in my opinion is more
> readable
> :)
>
Done
> I don't understand the description of "@param format". Is it input or output
> parameter?
>
Both, updated the description.
> > +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
> > + */
> > +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 6cb889f..cd49755 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"
> ok, i see, it's copied from here
>
> > #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) {
> > 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
>
> Pavel
>
>
More information about the Spice-devel
mailing list