gdiplus: Reimplement metafile loading using gdi32 instead of IPicture.

Sebastian Lackner sebastian at fds-team.de
Wed Aug 17 14:28:39 CDT 2016


On 17.08.2016 18:42, Vincent Povirk wrote:
> I'd probably be OK with this as-is, but since you asked for feedback:
> 
>> @@ -2219,12 +2155,6 @@ GpStatus WINGDIPAPI GdipGetImageBounds(GpImage *image, GpRectF *srcRect,
>>          srcRect->Height = (REAL) ((GpBitmap*)image)->height;
>>          *srcUnit = UnitPixel;
>>      }
>> -    else{
>> -        srcRect->X = srcRect->Y = 0.0;
>> -        srcRect->Width = ipicture_pixel_width(image->picture);
>> -        srcRect->Height = ipicture_pixel_height(image->picture);
>> -        *srcUnit = UnitPixel;
>> -    }
>>
>>      TRACE("returning (%f, %f) (%f, %f) unit type %d\n", srcRect->X, srcRect->Y,
>>            srcRect->Width, srcRect->Height, *srcUnit);
> 
> Even for ipicture metafiles, this case wasn't really used before,
> except when given an invalid image object (type isn't bitmap or
> metafile) where it would likely crash. We should probably return
> InvalidParameter here.
> 
> Same with the other GdipGetImage* functions where we're removing else clauses.
> 
> I am OK with the simplified error handling from IStream_Read.
> 
>> +    hr = IStream_Read(stream, buf, emh.nBytes, &size);
>> +    if (hr == S_OK && size == emh.nBytes)
>> +    {
>> +        hemf = SetEnhMetaFileBits(emh.nBytes, buf);
>> +        if (hemf)
>> +        {
>> +            status = GdipCreateMetafileFromEmf(hemf, TRUE, metafile);
>> +            if (status != Ok)
>> +                DeleteEnhMetaFile(hemf);
>> +        }
>>      }
>>
>> +    heap_free(buf);
>> +    return status;
>> +}
> 
> I guess this works, but it's kind of confusing how status gets set
> here when IStream_Read or SetEnhMetaFileBits fails. I'd rather it be
> set closer to the failure.
> 

Thanks for the feedback, I've just sent a new version. Please let me know if
it addresses all your concerns. In case you are planning to implement other
metafile gdiplus functions, you might also want to take a look here to check
on what else Dmitry has been working on in the past months:
https://github.com/wine-compholio/wine-staging/tree/master/patches/gdiplus-GdipCreateMetafileFromStream
I can try to clean them up when I have some time, however I also don't mind
if you take a look yourself and refactor / rewrite them (if necessary) for
upstream inclusion.

Best regards,
Sebastian




More information about the wine-devel mailing list