Hello, have anyone looked to this patch?:) Or it was interesting to us only? Our client wants it to be opensource.
On 30.09.2013 11:53, Sergey Staroletov wrote: > 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: [email protected] _______________________________________________ vbox-dev mailing list [email protected] https://www.virtualbox.org/mailman/listinfo/vbox-dev
