[prev in list] [next in list] [prev in thread] [next in thread]
List: vbox-dev
Subject: Re: [vbox-dev] [PATCH] VirtualBox Additions DIB Patch
From: Sergey Staroletov <sergey.staroletov () sibers ! com>
Date: 2013-09-30 10:53:14
Message-ID: 91797e7ce68fb3b70e0aa3314ee1a86a () sibers ! com
[Download RAW message or body]
Hi, we improved the patch a little bit, please see comments.
On 16.09.2013 14:12, Klaus Espenlaub wrote:
> Hi Sergey,
> On 12.09.2013 13:39, Sergey Staroletov wrote:
>> Hello, did anybody found our patch useful? Why not to include it to
>> the
>> trunk?
>
> The patch would be trivial to integrate if its code wouldn't
> contradict
> its comments and would be more complete/correct :)
>
> For example the comments hint that BI_BITFIELDS are not handled, but
> the
> check in the code accepts both BI_RGB and BI_BITFIELDS and proceeds
> with
> them. For BI_BITFIELDS it will not cause trouble if height is
> positive
> (or if it is already DIBv1 - but still the code has to be more
> consistent. I suspect it shreds the data in the code path which
> changes
> orientation or header size.
>
Now the code matches this comment, images with BI_BITFIELDS are passing
and aren't processed with our function.
Also we are skipping compressed images, images with palette and images
with null height or width.
> Also, it's unacceptable that the code malfunctions (for upside down
> images or if the code recreates the header which actually shouldn't
> ever
> happen) if there is a color table (8bpp or less) or is BI_BITFIELDS.
> It
> silently drops the color table, making the DIB violate the spec. The
> result will be unpredictable, ranging from image corruption to
> crashes.
> Depending on what images the user is dealing with I have the
> impression
> that there can be more crashes than before.
8bpp checking was in the old code. Now all images with palette are
skipping, that's why copying is
processing right. If we required to flip image with DIB header version
1, we don't change the header but just copy it, and put the flipped data
behind it.
> Can you provide a patch with those issues addressed? Would speed up
> integration, as we'd first have to dig into the subtleties of the DIB
> format before we can efficiently bring your contribution to
> production
> quality.
>
> Thanks for trying to improve VirtualBox, we appreciate any
> contributions,
>
> Klaus
>
>>
>> Thanks, Sergey
>>
>> On 19.06.2013 10:49, Sergey Staroletov wrote:
>>> Hi François,
>>>
>>> On 19.06.2013 09:33, François Revol wrote:
>>>> Thanks for working on this, I knew it wasn't fully working when I
>>>> wrote
>>>> the initial code, but it worked for me, and I don't have a Mac
>>>> anymore
>>>> anyway.
>>>
>>>> Of course best would be to settle on a really standardized (PNG?)
>>>> format
>>>> for interchange instead of a "mostly documented" one, but well...
>>>>
>>>
>>> Yes may be it a better solution, but we worked on it for one client
>>> and we can release only this patch as he wants it to be opensource.
>>>
>>>>>
>>>>> We modified VBoxClipboard.cpp. We were used VirtualBox-4.2.12
>>>>> guests for
>>>>> tests.
>>>>
>>>> It's usually best to submit diffs in unified format (diff -u) so
>>>> that
>>>> they contain context lines around for patch to make sure it
>>>> doesn't
>>>> break anything.
>>>>
>>>> Also, your patch has much higher chances of getting in if you diff
>>>> against the current svn trunk, in which case "svn diff" will
>>>> produce
>>>> the
>>>> unified format automatically.
>>>
>>>
>>> no problem, here is diff -u for the current trunk.
>>>
>>>
>>>> François.
--
Sergey Staroletov
Senior UNIX C++ developer
Sibers (HireRussians/former Key-Soft Ltd.)
E-mail: sergey.staroletov@sibers.com
["VBoxClipboard.diff" (text/x-diff)]
--- VBoxClipboard_trunk.cpp 2013-09-30 17:27:28.348731129 +0700
+++ VBoxClipboard_v2.cpp 2013-09-30 17:27:29.060731131 +0700
@@ -129,6 +129,135 @@
pCtx->fCBChainPingInProcess = FALSE;
}
+static HANDLE vboxRgbDibToDibV1(HANDLE dibHandle)
+{
+ PBITMAPINFOHEADER dibHeaderData = NULL;
+ void *dibRawData = 0;
+
+ bool changeOrientation = false;
+ bool changeHeader = false;
+
+ HANDLE dibv1Handle = NULL;
+ void *dibv1Data = NULL;
+ PBITMAPINFOHEADER dibv1Header = NULL;
+
+ DWORD oldHeaderSize = 0;
+ LONG width;
+ LONG height;
+ SIZE_T rawDataLineSize = 0;
+ SIZE_T rawDataLinePadding = 0;
+
+ if (dibHandle == NULL)
+ {
+ return dibHandle;
+ }
+
+ dibHeaderData = static_cast<PBITMAPINFOHEADER>(GlobalLock(dibHandle));
+
+ if (dibHeaderData == NULL)
+ {
+ /* Can't lock dibHandle */
+ return dibHandle;
+ }
+
+ if (dibHeaderData->biCompression != BI_RGB)
+ {
+ /* Leave compressed images or image with pallete as is */
+ return dibHandle;
+ }
+
+ width = dibHeaderData->biWidth;
+ height = dibHeaderData->biHeight;
+
+ if (dibHeaderData->biBitCount < 8 ||
+ width <= 0 ||
+ dibHeaderData->biHeight == 0)
+ {
+ /* Leave 8bpp or empty image as it */
+ return dibHandle;
+ }
+
+ if (height < 0)
+ {
+ /* Upside down image */
+ changeOrientation = true;
+ height *= -1;
+ }
+
+ if (dibHeaderData->biSize != sizeof(BITMAPINFOHEADER))
+ {
+ /* Header version is not equal 1 */
+ changeHeader = true;
+ }
+
+ if (!changeOrientation && !changeHeader)
+ {
+ /* Nothing changed */
+ return dibHandle;
+ }
+
+ rawDataLineSize = (dibHeaderData->biBitCount/8)*(dibHeaderData->biWidth);
+
+ /* The size of each row is rounded up to a multiple of 4 bytes (a 32-bit DWORD) \
by padding. */ + rawDataLinePadding = rawDataLineSize%4;
+ if (rawDataLinePadding > 0)
+ {
+ rawDataLineSize += 4 - rawDataLinePadding;
+ }
+
+ /* Create new handle */
+ dibv1Handle = GlobalAlloc(GMEM_DDESHARE | GMEM_MOVEABLE, \
sizeof(BITMAPINFOHEADER) + rawDataLineSize*height); +
+ if (dibv1Handle == NULL)
+ {
+ /* Can't alloc handle */
+ return dibHandle;
+ }
+
+ dibv1Data = GlobalLock(dibv1Handle);
+
+ if (dibv1Data == NULL)
+ {
+ /* Can't lock handle */
+ GlobalFree(dibv1Handle);
+ return dibHandle;
+ }
+
+ dibv1Header = static_cast<PBITMAPINFOHEADER>(dibv1Data);
+
+ /* Copy header anyway */
+ memcpy((void*)dibv1Header, (void*)dibHeaderData, sizeof(BITMAPINFOHEADER));
+ dibv1Header->biSize = sizeof(BITMAPINFOHEADER);
+ dibv1Header->biHeight = height;
+
+ if (changeOrientation)
+ {
+ /* Copy data with correct orientation.
+ * Start from last row in source data and first row in distance.
+ */
+ char *psIter = (char*)dibHeaderData + dibHeaderData->biSize + \
rawDataLineSize*(height - 1) ; + char *pdIter = (char*)dibv1Data + \
sizeof(BITMAPINFOHEADER); + for (int i = 0; i < height; ++i)
+ {
+ memcpy(pdIter, psIter, rawDataLineSize);
+ psIter -= rawDataLineSize;
+ pdIter += rawDataLineSize;
+ }
+ }
+ else
+ {
+ /* Copy data */
+ memcpy((void*)((char*)dibv1Data + sizeof(BITMAPINFOHEADER)),
+ (void*)((char*)dibHeaderData + dibHeaderData->biSize),
+ rawDataLineSize*height);
+ }
+
+ GlobalUnlock(dibHandle);
+ GlobalUnlock(dibv1Handle);
+
+ return dibv1Handle;
+}
+
static LRESULT vboxClipboardProcessMsg(VBOXCLIPBOARDCONTEXT *pCtx, HWND hwnd, UINT \
msg, WPARAM wParam, LPARAM lParam) {
LRESULT rc = 0;
@@ -349,15 +478,39 @@
if (hMem)
{
/* 'hMem' contains the host clipboard data.
- * size is 'cb' and format is 'format'. */
- HANDLE hClip = SetClipboardData(format, hMem);
+ * size is 'cb' and format is 'format'.
+ * 'dataMem' contains data converted to dibV1
+ */
+ HANDLE dataMem = hMem;
+ if (format == CF_DIB)
+ {
+ dataMem = vboxRgbDibToDibV1(hMem);
+ if (dataMem == NULL)
+ {
+ dataMem = hMem;
+ }
+ }
+ HANDLE hClip = SetClipboardData(format, \
dataMem);
Log(("VBoxTray: vboxClipboardProcessMsg: \
WM_RENDERFORMAT hClip = %p\n", hClip));
if (hClip)
{
- /* The hMem ownership has gone to the \
system. Finish the processing. */ + /* The \
dataMem ownership has gone to the system. Finish the processing. */ + \
if (hMem != dataMem) + {
+ /* Cleanup hMem if new Dib data created \
*/ + GlobalFree(hMem);
+ }
break;
}
+ else
+ {
+ /* Clean up dib dataMem if needed */
+ if (hMem != dataMem)
+ {
+ GlobalFree(dataMem);
+ }
+ }
/* Cleanup follows. */
}
@@ -725,4 +878,3 @@
memset (pCtx, 0, sizeof (*pCtx));
return;
}
-
_______________________________________________
vbox-dev mailing list
vbox-dev@virtualbox.org
https://www.virtualbox.org/mailman/listinfo/vbox-dev
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic