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.