> > 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@xxxxxxxxxx> > > --- > > 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 > > _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel