Hi,
This is my first wine patch series. I've tried to follow any instructions I could find on the wiki, but it's probably a good idea to review this thoroughly.
The new test looks a lot like spaghetti, but it would be difficult to split up, and since other tests are even longer, I hope that's ok.
Cheers,
Florian Will (5): gdiplus: Refactor to split up encode_image_wic(). include: Add enums and GUID required for GdipSaveAdd(). gdiplus: Add GdipSaveAddImage() stub. gdiplus/tests: Add multi-page tiff file saving test. gdiplus: Implement GdipSaveAddImage() and GdipSaveAdd().
dlls/gdiplus/gdiplus.spec | 2 +- dlls/gdiplus/gdiplus_private.h | 2 + dlls/gdiplus/image.c | 160 +++++++++++++++++---- dlls/gdiplus/tests/image.c | 244 +++++++++++++++++++++++++++++---- include/gdiplusenums.h | 41 ++++++ include/gdiplusflat.h | 2 + include/gdiplusimaging.h | 2 + 7 files changed, 398 insertions(+), 55 deletions(-)
New functions initialize_encoder_wic(), encode_frame_wic() and terminate_encoder_wic() are useful for implementing GdipSaveAdd() and GdipSaveAddImage() later.
Signed-off-by: Florian Will florian.will@gmail.com --- dlls/gdiplus/gdiplus_private.h | 1 + dlls/gdiplus/image.c | 79 ++++++++++++++++++++++++---------- 2 files changed, 58 insertions(+), 22 deletions(-)
diff --git a/dlls/gdiplus/gdiplus_private.h b/dlls/gdiplus/gdiplus_private.h index 8c4fcceded..36e79e29eb 100644 --- a/dlls/gdiplus/gdiplus_private.h +++ b/dlls/gdiplus/gdiplus_private.h @@ -72,6 +72,7 @@ extern GpStatus gdip_transform_points(GpGraphics *graphics, GpCoordinateSpace ds
extern GpStatus graphics_from_image(GpImage *image, GpGraphics **graphics) DECLSPEC_HIDDEN; extern GpStatus encode_image_png(GpImage *image, IStream* stream, GDIPCONST EncoderParameters* params) DECLSPEC_HIDDEN; +extern GpStatus terminate_encoder_wic(IWICBitmapEncoder *encoder) DECLSPEC_HIDDEN;
extern GpStatus METAFILE_GetGraphicsContext(GpMetafile* metafile, GpGraphics **result) DECLSPEC_HIDDEN; extern GpStatus METAFILE_GetDC(GpMetafile* metafile, HDC *hdc) DECLSPEC_HIDDEN; diff --git a/dlls/gdiplus/image.c b/dlls/gdiplus/image.c index dcdc49c6d6..daf3169bd1 100644 --- a/dlls/gdiplus/image.c +++ b/dlls/gdiplus/image.c @@ -4435,13 +4435,41 @@ GpStatus WINGDIPAPI GdipSaveImageToFile(GpImage *image, GDIPCONST WCHAR* filenam * These functions encode an image in different image file formats. */
-static GpStatus encode_image_wic(GpImage *image, IStream* stream, - REFGUID container, GDIPCONST EncoderParameters* params) +static GpStatus initialize_encoder_wic(IStream *stream, REFGUID container, + IWICBitmapEncoder **encoder) +{ + IWICImagingFactory *factory; + HRESULT hr; + + TRACE("%p,%s\n", stream, wine_dbgstr_guid(container)); + + hr = WICCreateImagingFactory_Proxy(WINCODEC_SDK_VERSION, &factory); + if (FAILED(hr)) return hresult_to_status(hr); + hr = IWICImagingFactory_CreateEncoder(factory, container, NULL, encoder); + IWICImagingFactory_Release(factory); + if (FAILED(hr)) return hresult_to_status(hr); + + hr = IWICBitmapEncoder_Initialize(*encoder, stream, WICBitmapEncoderNoCache); + if (FAILED(hr)) + { + IWICBitmapEncoder_Release(*encoder); + *encoder = NULL; + return hresult_to_status(hr); + } + return Ok; +} + +GpStatus terminate_encoder_wic(IWICBitmapEncoder *encoder) +{ + HRESULT hr = IWICBitmapEncoder_Commit(encoder); + IWICBitmapEncoder_Release(encoder); + return hresult_to_status(hr); +} + +static GpStatus encode_frame_wic(IWICBitmapEncoder *encoder, GpImage *image) { GpStatus stat; GpBitmap *bitmap; - IWICImagingFactory *factory; - IWICBitmapEncoder *encoder; IWICBitmapFrameEncode *frameencode; IPropertyBag2 *encoderoptions; HRESULT hr; @@ -4466,20 +4494,7 @@ static GpStatus encode_image_wic(GpImage *image, IStream* stream, rc.Width = width; rc.Height = height;
- hr = WICCreateImagingFactory_Proxy(WINCODEC_SDK_VERSION, &factory); - if (FAILED(hr)) - return hresult_to_status(hr); - hr = IWICImagingFactory_CreateEncoder(factory, container, NULL, &encoder); - IWICImagingFactory_Release(factory); - if (FAILED(hr)) - return hresult_to_status(hr); - - hr = IWICBitmapEncoder_Initialize(encoder, stream, WICBitmapEncoderNoCache); - - if (SUCCEEDED(hr)) - { - hr = IWICBitmapEncoder_CreateNewFrame(encoder, &frameencode, &encoderoptions); - } + hr = IWICBitmapEncoder_CreateNewFrame(encoder, &frameencode, &encoderoptions);
if (SUCCEEDED(hr)) /* created frame */ { @@ -4563,13 +4578,33 @@ static GpStatus encode_image_wic(GpImage *image, IStream* stream, IPropertyBag2_Release(encoderoptions); }
- if (SUCCEEDED(hr)) - hr = IWICBitmapEncoder_Commit(encoder); - - IWICBitmapEncoder_Release(encoder); return hresult_to_status(hr); }
+static GpStatus encode_image_wic(GpImage *image, IStream *stream, + REFGUID container, GDIPCONST EncoderParameters *params) +{ + IWICBitmapEncoder *encoder = NULL; + GpStatus status; + + if (image->type != ImageTypeBitmap) + return GenericError; + + status = initialize_encoder_wic(stream, container, &encoder); + + if (status == Ok) + status = encode_frame_wic(encoder, image); + + if (encoder) + { + GpStatus terminate_status = terminate_encoder_wic(encoder); + if (status == Ok) + status = terminate_status; + } + + return status; +} + static GpStatus encode_image_BMP(GpImage *image, IStream* stream, GDIPCONST EncoderParameters* params) {
+GpStatus terminate_encoder_wic(IWICBitmapEncoder *encoder) +{
- HRESULT hr = IWICBitmapEncoder_Commit(encoder);
- IWICBitmapEncoder_Release(encoder);
- return hresult_to_status(hr);
+}
I don't like having the Release call separated from clearing or freeing the stored reference to the interface. I think we're likely to miss a use-after-free this way.
I'd suggest either making this function take a GpImage* so we can clear the reference, or moving the Release call out of it.
Signed-off-by: Florian Will florian.will@gmail.com --- include/gdiplusenums.h | 41 ++++++++++++++++++++++++++++++++++++++++ include/gdiplusimaging.h | 2 ++ 2 files changed, 43 insertions(+)
diff --git a/include/gdiplusenums.h b/include/gdiplusenums.h index 905a87a451..0fcc11d4b9 100644 --- a/include/gdiplusenums.h +++ b/include/gdiplusenums.h @@ -336,6 +336,45 @@ enum ImageFlags ImageFlagsCaching = 0x00020000 };
+enum EncoderParameterValueType { + EncoderParameterValueTypeByte = 1, + EncoderParameterValueTypeASCII = 2, + EncoderParameterValueTypeShort = 3, + EncoderParameterValueTypeLong = 4, + EncoderParameterValueTypeRational = 5, + EncoderParameterValueTypeLongRange = 6, + EncoderParameterValueTypeUndefined = 7, + EncoderParameterValueTypeRationalRange = 8, + EncoderParameterValueTypePointer = 9 +}; + +enum EncoderValue { + EncoderValueColorTypeCMYK = 0, + EncoderValueColorTypeYCCK = 1, + EncoderValueCompressionLZW = 2, + EncoderValueCompressionCCITT3 = 3, + EncoderValueCompressionCCITT4 = 4, + EncoderValueCompressionRle = 5, + EncoderValueCompressionNone = 6, + EncoderValueScanMethodInterlaced = 7, + EncoderValueScanMethodNonInterlaced = 8, + EncoderValueVersionGif87 = 9, + EncoderValueVersionGif89 = 10, + EncoderValueRenderProgressive = 11, + EncoderValueRenderNonProgressive = 12, + EncoderValueTransformRotate90 = 13, + EncoderValueTransformRotate180 = 14, + EncoderValueTransformRotate270 = 15, + EncoderValueTransformFlipHorizontal = 16, + EncoderValueTransformFlipVertical = 17, + EncoderValueMultiFrame = 18, + EncoderValueLastFrame = 19, + EncoderValueFlush = 20, + EncoderValueFrameDimensionTime = 21, + EncoderValueFrameDimensionResolution = 22, + EncoderValueFrameDimensionPage = 23 +}; + enum CombineMode { CombineModeReplace, @@ -747,6 +786,8 @@ typedef enum HotkeyPrefix HotkeyPrefix; typedef enum PenAlignment PenAlignment; typedef enum PaletteFlags PaletteFlags; typedef enum ImageCodecFlags ImageCodecFlags; +typedef enum EncoderParameterValueType EncoderParameterValueType; +typedef enum EncoderValue EncoderValue; typedef enum CombineMode CombineMode; typedef enum FlushIntention FlushIntention; typedef enum CoordinateSpace CoordinateSpace; diff --git a/include/gdiplusimaging.h b/include/gdiplusimaging.h index 93114d95c4..9e6b58283f 100644 --- a/include/gdiplusimaging.h +++ b/include/gdiplusimaging.h @@ -35,6 +35,8 @@ DEFINE_GUID(FrameDimensionTime, 0x6aedbd6d, 0x3fb5, 0x418a, 0x83, 0xa6, 0x DEFINE_GUID(FrameDimensionPage, 0x7462dc86, 0x6180, 0x4c7e, 0x8e, 0x3f, 0xee, 0x73, 0x33, 0xa7, 0xa4, 0x83); DEFINE_GUID(FrameDimensionResolution, 0x84236f7b, 0x3bd3, 0x428f, 0x8d, 0xab, 0x4e, 0xa1, 0x43, 0x9c, 0xa3, 0x15);
+DEFINE_GUID(EncoderSaveFlag, 0x292266fc, 0xac40, 0x47bf, 0x8c, 0xfc, 0xa8, 0x5b, 0x89, 0xa6, 0x55, 0xde); + enum ImageLockMode { ImageLockModeRead = 1,
Also update the GdipSaveAdd() stub to return NotImplemented instead of silently failing.
Signed-off-by: Florian Will florian.will@gmail.com --- dlls/gdiplus/gdiplus.spec | 2 +- dlls/gdiplus/image.c | 22 +++++++++++++++++++++- include/gdiplusflat.h | 2 ++ 3 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/dlls/gdiplus/gdiplus.spec b/dlls/gdiplus/gdiplus.spec index 11866633b3..43dc0a82d2 100644 --- a/dlls/gdiplus/gdiplus.spec +++ b/dlls/gdiplus/gdiplus.spec @@ -472,7 +472,7 @@ 472 stdcall GdipRotateTextureTransform(ptr float long) 473 stdcall GdipRotateWorldTransform(ptr float long) 474 stdcall GdipSaveAdd(ptr ptr) -475 stub GdipSaveAddImage +475 stdcall GdipSaveAddImage(ptr ptr ptr) 476 stdcall GdipSaveGraphics(ptr ptr) 477 stdcall GdipSaveImageToFile(ptr wstr ptr ptr) 478 stdcall GdipSaveImageToStream(ptr ptr ptr ptr) diff --git a/dlls/gdiplus/image.c b/dlls/gdiplus/image.c index daf3169bd1..94d9344082 100644 --- a/dlls/gdiplus/image.c +++ b/dlls/gdiplus/image.c @@ -4667,11 +4667,31 @@ GpStatus WINGDIPAPI GdipSaveImageToStream(GpImage *image, IStream* stream,
/***************************************************************************** * GdipSaveAdd [GDIPLUS.@] + * + * Like GdipSaveAddImage(), but encode the currently active frame of the given image into the file + * or stream that is currently being encoded. */ GpStatus WINGDIPAPI GdipSaveAdd(GpImage *image, GDIPCONST EncoderParameters *params) { FIXME("(%p,%p): stub\n", image, params); - return Ok; + return NotImplemented; +} + +/***************************************************************************** + * GdipSaveAddImage [GDIPLUS.@] + * + * Encode the currently active frame of additional_image into the file or stream that is currently + * being encoded by the image given in the image parameter. The first frame of a multi-frame image + * must be encoded using the normal GdipSaveImageToStream() or GdipSaveImageToFile() functions, + * but with the "MultiFrame" encoding parameter set. The multi-frame encoding process must be + * finished after adding the last frame by calling GdipSaveAdd() with the "Flush" encoding parameter + * set. + */ +GpStatus WINGDIPAPI GdipSaveAddImage(GpImage *image, GpImage *additional_image, + GDIPCONST EncoderParameters *params) +{ + FIXME("(%p,%p,%p): stub\n", image, additional_image, params); + return NotImplemented; }
/***************************************************************************** diff --git a/include/gdiplusflat.h b/include/gdiplusflat.h index 1e26c0a9d5..ed0f498483 100644 --- a/include/gdiplusflat.h +++ b/include/gdiplusflat.h @@ -433,6 +433,8 @@ GpStatus WINGDIPAPI GdipRemovePropertyItem(GpImage*,PROPID); GpStatus WINGDIPAPI GdipSaveImageToFile(GpImage*,GDIPCONST WCHAR*,GDIPCONST CLSID*,GDIPCONST EncoderParameters*); GpStatus WINGDIPAPI GdipSaveImageToStream(GpImage*,IStream*, GDIPCONST CLSID*,GDIPCONST EncoderParameters*); +GpStatus WINGDIPAPI GdipSaveAdd(GpImage*,GDIPCONST EncoderParameters*); +GpStatus WINGDIPAPI GdipSaveAddImage(GpImage*,GpImage*,GDIPCONST EncoderParameters*); GpStatus WINGDIPAPI GdipSetImagePalette(GpImage*,GDIPCONST ColorPalette*); GpStatus WINGDIPAPI GdipSetPropertyItem(GpImage*,GDIPCONST PropertyItem*);
Also move the get_encoder_clsid() helper to the top of the file so it can be used by the new test.
Signed-off-by: Florian Will florian.will@gmail.com --- dlls/gdiplus/tests/image.c | 244 ++++++++++++++++++++++++++++++++----- 1 file changed, 214 insertions(+), 30 deletions(-)
diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c index d6f74050c0..01d79f08c4 100644 --- a/dlls/gdiplus/tests/image.c +++ b/dlls/gdiplus/tests/image.c @@ -76,6 +76,36 @@ static void expect_rawformat(REFGUID expected, GpImage *img, int line, BOOL todo expect_guid(expected, &raw, line, todo); }
+static BOOL get_encoder_clsid(LPCWSTR mime, GUID *format, CLSID *clsid) +{ + GpStatus status; + UINT n_codecs, info_size, i; + ImageCodecInfo *info; + BOOL ret = FALSE; + + status = GdipGetImageEncodersSize(&n_codecs, &info_size); + expect(Ok, status); + + info = GdipAlloc(info_size); + + status = GdipGetImageEncoders(n_codecs, info_size, info); + expect(Ok, status); + + for (i = 0; i < n_codecs; i++) + { + if (!lstrcmpW(info[i].MimeType, mime)) + { + *format = info[i].FormatID; + *clsid = info[i].Clsid; + ret = TRUE; + break; + } + } + + GdipFree(info); + return ret; +} + static void test_bufferrawformat(void* buff, int size, REFGUID expected, int line, BOOL todo) { LPSTREAM stream; @@ -487,6 +517,189 @@ static void test_SavingImages(void) ok(DeleteFileA(filenameA), "Delete failed.\n"); }
+static void test_SavingMultiPageTiff(void) +{ + GpStatus stat; + BOOL result; + GpBitmap *bm1 = NULL, *bm2 = NULL, *check_bm = NULL; + const REAL WIDTH = 10.0, HEIGHT = 20.0; + REAL w, h; + GUID format, tiff_clsid; + EncoderParameters params; + ULONG32 paramValue = EncoderValueFrameDimensionPage; + UINT frame_count; + static const CHAR filename1A[] = "1.tif"; + static const CHAR filename2A[] = "2.tif"; + static const WCHAR filename1[] = { '1','.','t','i','f',0 }; + static const WCHAR filename2[] = { '2','.','t','i','f',0 }; + static const WCHAR tiff_mimetype[] = { 'i','m','a','g','e','/','t','i','f','f',0 }; + + params.Count = 1; + params.Parameter[0].Guid = EncoderSaveFlag; + params.Parameter[0].Type = EncoderParameterValueTypeLong; + params.Parameter[0].NumberOfValues = 1; + params.Parameter[0].Value = ¶mValue; + + stat = GdipCreateBitmapFromScan0(WIDTH, HEIGHT, 0, PixelFormat24bppRGB, NULL, &bm1); + expect(Ok, stat); + stat = GdipCreateBitmapFromScan0(2 * WIDTH, 2 * HEIGHT, 0, PixelFormat24bppRGB, NULL, &bm2); + expect(Ok, stat); + result = get_encoder_clsid(tiff_mimetype, &format, &tiff_clsid); + ok(result, "getting TIFF encoding clsid failed"); + + if (!bm1 || !bm2 || !result) + return; + + /* invalid params: NULL */ + stat = GdipSaveAdd(0, ¶ms); + todo_wine expect(InvalidParameter, stat); + stat = GdipSaveAdd((GpImage*)bm1, 0); + todo_wine expect(InvalidParameter, stat); + + stat = GdipSaveAddImage((GpImage*)bm1, (GpImage*)bm2, 0); + todo_wine expect(InvalidParameter, stat); + stat = GdipSaveAddImage((GpImage*)bm1, 0, ¶ms); + todo_wine expect(InvalidParameter, stat); + stat = GdipSaveAddImage(0, (GpImage*)bm2, ¶ms); + todo_wine expect(InvalidParameter, stat); + + /* win32 error: SaveAdd() can only be called after Save() with the MultiFrame param */ + stat = GdipSaveAdd((GpImage*)bm1, ¶ms); + todo_wine expect(Win32Error, stat); + stat = GdipSaveAddImage((GpImage*)bm1, (GpImage*)bm2, ¶ms); + todo_wine expect(Win32Error, stat); + + stat = GdipSaveImageToFile((GpImage*)bm1, filename1, &tiff_clsid, 0); /* param not set! */ + expect(Ok, stat); + if (stat != Ok) goto cleanup; + + stat = GdipSaveAdd((GpImage*)bm1, ¶ms); + todo_wine expect(Win32Error, stat); + stat = GdipSaveAddImage((GpImage*)bm1, (GpImage*)bm2, ¶ms); + todo_wine expect(Win32Error, stat); + + /* win32 error: can't flush before starting the encoding process */ + paramValue = EncoderValueFlush; + stat = GdipSaveAdd((GpImage*)bm1, ¶ms); + todo_wine expect(Win32Error, stat); + + /* win32 error: can't start encoding process through SaveAdd(), only Save() */ + paramValue = EncoderValueMultiFrame; + stat = GdipSaveAdd((GpImage*)bm1, ¶ms); + todo_wine expect(Win32Error, stat); + + /* start encoding process: add first frame (bm1) */ + paramValue = EncoderValueMultiFrame; + stat = GdipSaveImageToFile((GpImage*)bm1, filename1, &tiff_clsid, ¶ms); + expect(Ok, stat); + + /* re-start encoding process: add first frame (bm1), should re-create the file */ + stat = GdipSaveImageToFile((GpImage*)bm1, filename1, &tiff_clsid, ¶ms); + expect(Ok, stat); + + /* add second frame (bm2) */ + paramValue = EncoderValueFrameDimensionPage; + stat = GdipSaveAddImage((GpImage*)bm1, (GpImage*)bm2, ¶ms); + todo_wine expect(Ok, stat); + if (stat != Ok) goto cleanup; + + /* finish encoding process */ + paramValue = EncoderValueFlush; + stat = GdipSaveAdd((GpImage*)bm1, ¶ms); + expect(Ok, stat); + + /* bm1 should be unchanged, only the saved file on disk has multiple frames */ + stat = GdipImageGetFrameCount((GpImage*)bm1, &FrameDimensionPage, &frame_count); + expect(Ok, stat); + expect(1, frame_count); + + /* win32 error: encoding process already finished */ + paramValue = EncoderValueFrameDimensionPage; + stat = GdipSaveAddImage((GpImage*)bm1, (GpImage*)bm2, ¶ms); + expect(Win32Error, stat); + + stat = GdipSaveAdd((GpImage*)bm1, ¶ms); + expect(Win32Error, stat); + + GdipDisposeImage((GpImage*)bm1); + bm1 = 0; + GdipDisposeImage((GpImage*)bm2); + bm2 = 0; + + /* re-load and check image stats */ + stat = GdipLoadImageFromFile(filename1, (GpImage**)&check_bm); + expect(Ok, stat); + + stat = GdipImageGetFrameCount((GpImage*)check_bm, &FrameDimensionPage, &frame_count); + expect(Ok, stat); + expect(2, frame_count); + if (stat != Ok || frame_count != 2) goto cleanup; + + stat = GdipGetImageDimension((GpImage*)check_bm, &w, &h); + expect(Ok, stat); + expectf(WIDTH, w); /* frame index 0: bm1 stats */ + expectf(HEIGHT, h); + + stat = GdipImageSelectActiveFrame((GpImage*)check_bm, &FrameDimensionPage, 1); + expect(Ok, stat); + + stat = GdipGetImageDimension((GpImage*)check_bm, &w, &h); + expectf(2 * WIDTH, w); /* frame index 1: bm2 stats */ + expectf(2 * HEIGHT, h); + + /* now proper API use for SaveAdd() to swap the frames in check_bm */ + paramValue = EncoderValueMultiFrame; + stat = GdipSaveImageToFile((GpImage*)check_bm, filename2, &tiff_clsid, ¶ms); + expect(Ok, stat); /* second frame is active: bm2 */ + + stat = GdipImageSelectActiveFrame((GpImage*)check_bm, &FrameDimensionPage, 0); + expect(Ok, stat); + + paramValue = EncoderValueFrameDimensionPage; + stat = GdipSaveAdd((GpImage*)check_bm, ¶ms); + expect(Ok, stat); /* first frame is active: bm1 */ + + paramValue = EncoderValueFlush; + stat = GdipSaveAdd((GpImage*)check_bm, ¶ms); + expect(Ok, stat); /* flushed encoder (finished encoding process) */ + + GdipDisposeImage((GpImage*)check_bm); + check_bm = 0; + + /* re-load and check image stats */ + stat = GdipLoadImageFromFile(filename2, (GpImage**)&check_bm); + expect(Ok, stat); + + stat = GdipImageGetFrameCount((GpImage*)check_bm, &FrameDimensionPage, &frame_count); + expect(Ok, stat); + expect(2, frame_count); + + stat = GdipGetImageDimension((GpImage*)check_bm, &w, &h); + expect(Ok, stat); + expectf(2 * WIDTH, w); /* frame index 0: bm2 stats */ + expectf(2 * HEIGHT, h); + + stat = GdipImageSelectActiveFrame((GpImage*)check_bm, &FrameDimensionPage, 1); + expect(Ok, stat); + + stat = GdipGetImageDimension((GpImage*)check_bm, &w, &h); + expectf(WIDTH, w); /* frame index 1: bm1 stats */ + expectf(HEIGHT, h); + + cleanup: + if (bm1) + GdipDisposeImage((GpImage*)bm1); + if (bm2) + GdipDisposeImage((GpImage*)bm2); + ok(DeleteFileA(filename1A), "Delete 1.tif failed.\n"); + + if (check_bm) + { + GdipDisposeImage((GpImage*)check_bm); + ok(DeleteFileA(filename2A), "Delete 2.tif failed.\n"); + } +} + static void test_encoders(void) { GpStatus stat; @@ -4888,36 +5101,6 @@ static void test_CloneBitmapArea(void) GdipDisposeImage((GpImage *)bitmap); }
-static BOOL get_encoder_clsid(LPCWSTR mime, GUID *format, CLSID *clsid) -{ - GpStatus status; - UINT n_codecs, info_size, i; - ImageCodecInfo *info; - BOOL ret = FALSE; - - status = GdipGetImageEncodersSize(&n_codecs, &info_size); - expect(Ok, status); - - info = GdipAlloc(info_size); - - status = GdipGetImageEncoders(n_codecs, info_size, info); - expect(Ok, status); - - for (i = 0; i < n_codecs; i++) - { - if (!lstrcmpW(info[i].MimeType, mime)) - { - *format = info[i].FormatID; - *clsid = info[i].Clsid; - ret = TRUE; - break; - } - } - - GdipFree(info); - return ret; -} - static void test_supported_encoders(void) { static const WCHAR bmp_mimetype[] = { 'i', 'm', 'a','g', 'e', '/', 'b', 'm', 'p',0 }; @@ -5700,6 +5883,7 @@ START_TEST(image) test_GdipImageGetFrameDimensionsCount(); test_LoadingImages(); test_SavingImages(); + test_SavingMultiPageTiff(); test_encoders(); test_LockBits(); test_LockBits_UserBuf();
Signed-off-by: Florian Will florian.will@gmail.com --- dlls/gdiplus/gdiplus_private.h | 1 + dlls/gdiplus/image.c | 67 +++++++++++++++++++++++++++++++--- dlls/gdiplus/tests/image.c | 24 ++++++------ 3 files changed, 75 insertions(+), 17 deletions(-)
diff --git a/dlls/gdiplus/gdiplus_private.h b/dlls/gdiplus/gdiplus_private.h index 36e79e29eb..12d7e32dda 100644 --- a/dlls/gdiplus/gdiplus_private.h +++ b/dlls/gdiplus/gdiplus_private.h @@ -347,6 +347,7 @@ struct GpAdjustableArrowCap{
struct GpImage{ IWICBitmapDecoder *decoder; + IWICBitmapEncoder *encoder; /* set during multi-frame save (GdipSaveAdd functions) */ ImageType type; GUID format; UINT flags; diff --git a/dlls/gdiplus/image.c b/dlls/gdiplus/image.c index 94d9344082..263b796b18 100644 --- a/dlls/gdiplus/image.c +++ b/dlls/gdiplus/image.c @@ -1827,6 +1827,7 @@ GpStatus WINGDIPAPI GdipCreateBitmapFromScan0(INT width, INT height, INT stride, (*bitmap)->height = height; (*bitmap)->format = format; (*bitmap)->image.decoder = NULL; + (*bitmap)->image.encoder = NULL; (*bitmap)->hbitmap = hbitmap; (*bitmap)->hdc = NULL; (*bitmap)->bits = bits; @@ -2046,6 +2047,9 @@ static void move_bitmap(GpBitmap *dst, GpBitmap *src, BOOL clobber_palette) if (dst->image.decoder) IWICBitmapDecoder_Release(dst->image.decoder); dst->image.decoder = src->image.decoder; + if (dst->image.encoder) + terminate_encoder_wic(dst->image.encoder); + dst->image.encoder = src->image.encoder; dst->image.frame_count = src->image.frame_count; dst->image.current_frame = src->image.current_frame; dst->image.format = src->image.format; @@ -2078,6 +2082,8 @@ static GpStatus free_image_data(GpImage *image) } if (image->decoder) IWICBitmapDecoder_Release(image->decoder); + if (image->encoder) + terminate_encoder_wic(image->encoder); heap_free(image->palette);
return Ok; @@ -3744,6 +3750,8 @@ static GpStatus select_frame_wic(GpImage *image, UINT active_frame)
new_image->busy = image->busy; memcpy(&new_image->format, &image->format, sizeof(GUID)); + new_image->encoder = image->encoder; + image->encoder = NULL; free_image_data(image); if (image->type == ImageTypeBitmap) *(GpBitmap *)image = *(GpBitmap *)new_image; @@ -4420,6 +4428,13 @@ GpStatus WINGDIPAPI GdipSaveImageToFile(GpImage *image, GDIPCONST WCHAR* filenam if (!image || !filename|| !clsidEncoder) return InvalidParameter;
+ if (image->encoder) + { + /* this might release an old file stream held by the encoder so we can re-create it below */ + terminate_encoder_wic(image->encoder); + image->encoder = NULL; + } + stat = GdipCreateStreamOnFile(filename, GENERIC_WRITE, &stream); if (stat != Ok) return GenericError; @@ -4581,21 +4596,48 @@ static GpStatus encode_frame_wic(IWICBitmapEncoder *encoder, GpImage *image) return hresult_to_status(hr); }
+static BOOL has_encoder_param_long(GDIPCONST EncoderParameters *params, GUID param_guid, ULONG val) +{ + if (!params) + return FALSE; + + for (int param_idx = 0; param_idx < params->Count; param_idx++) + { + EncoderParameter param = params->Parameter[param_idx]; + if (param.Type == EncoderParameterValueTypeLong && IsEqualCLSID(¶m.Guid, ¶m_guid)) + { + ULONG *value_array = (ULONG*) param.Value; + for (int value_idx = 0; value_idx < param.NumberOfValues; value_idx++) + { + if (value_array[value_idx] == val) + return TRUE; + } + } + } + return FALSE; +} + static GpStatus encode_image_wic(GpImage *image, IStream *stream, REFGUID container, GDIPCONST EncoderParameters *params) { IWICBitmapEncoder *encoder = NULL; GpStatus status; + BOOL is_multi_frame = has_encoder_param_long(params, EncoderSaveFlag, EncoderValueMultiFrame);
if (image->type != ImageTypeBitmap) return GenericError;
+ if (is_multi_frame && image->encoder != NULL) + terminate_encoder_wic(image->encoder); + status = initialize_encoder_wic(stream, container, &encoder);
if (status == Ok) status = encode_frame_wic(encoder, image);
- if (encoder) + if (is_multi_frame) + image->encoder = encoder; + else if (encoder) { GpStatus terminate_status = terminate_encoder_wic(encoder); if (status == Ok) @@ -4673,8 +4715,7 @@ GpStatus WINGDIPAPI GdipSaveImageToStream(GpImage *image, IStream* stream, */ GpStatus WINGDIPAPI GdipSaveAdd(GpImage *image, GDIPCONST EncoderParameters *params) { - FIXME("(%p,%p): stub\n", image, params); - return NotImplemented; + return GdipSaveAddImage(image, image, params); }
/***************************************************************************** @@ -4690,8 +4731,24 @@ GpStatus WINGDIPAPI GdipSaveAdd(GpImage *image, GDIPCONST EncoderParameters *par GpStatus WINGDIPAPI GdipSaveAddImage(GpImage *image, GpImage *additional_image, GDIPCONST EncoderParameters *params) { - FIXME("(%p,%p,%p): stub\n", image, additional_image, params); - return NotImplemented; + TRACE("%p, %p, %p\n", image, additional_image, params); + + if (!image || !additional_image || !params) + return InvalidParameter; + + if (!image->encoder) + return Win32Error; + + if (has_encoder_param_long(params, EncoderSaveFlag, EncoderValueFlush)) + { + GpStatus status = terminate_encoder_wic(image->encoder); + image->encoder = NULL; + return status; + } + else if (has_encoder_param_long(params, EncoderSaveFlag, EncoderValueFrameDimensionPage)) + return encode_frame_wic(image->encoder, additional_image); + else + return InvalidParameter; }
/***************************************************************************** diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c index 01d79f08c4..bbb9365edb 100644 --- a/dlls/gdiplus/tests/image.c +++ b/dlls/gdiplus/tests/image.c @@ -552,41 +552,41 @@ static void test_SavingMultiPageTiff(void)
/* invalid params: NULL */ stat = GdipSaveAdd(0, ¶ms); - todo_wine expect(InvalidParameter, stat); + expect(InvalidParameter, stat); stat = GdipSaveAdd((GpImage*)bm1, 0); - todo_wine expect(InvalidParameter, stat); + expect(InvalidParameter, stat);
stat = GdipSaveAddImage((GpImage*)bm1, (GpImage*)bm2, 0); - todo_wine expect(InvalidParameter, stat); + expect(InvalidParameter, stat); stat = GdipSaveAddImage((GpImage*)bm1, 0, ¶ms); - todo_wine expect(InvalidParameter, stat); + expect(InvalidParameter, stat); stat = GdipSaveAddImage(0, (GpImage*)bm2, ¶ms); - todo_wine expect(InvalidParameter, stat); + expect(InvalidParameter, stat);
/* win32 error: SaveAdd() can only be called after Save() with the MultiFrame param */ stat = GdipSaveAdd((GpImage*)bm1, ¶ms); - todo_wine expect(Win32Error, stat); + expect(Win32Error, stat); stat = GdipSaveAddImage((GpImage*)bm1, (GpImage*)bm2, ¶ms); - todo_wine expect(Win32Error, stat); + expect(Win32Error, stat);
stat = GdipSaveImageToFile((GpImage*)bm1, filename1, &tiff_clsid, 0); /* param not set! */ expect(Ok, stat); if (stat != Ok) goto cleanup;
stat = GdipSaveAdd((GpImage*)bm1, ¶ms); - todo_wine expect(Win32Error, stat); + expect(Win32Error, stat); stat = GdipSaveAddImage((GpImage*)bm1, (GpImage*)bm2, ¶ms); - todo_wine expect(Win32Error, stat); + expect(Win32Error, stat);
/* win32 error: can't flush before starting the encoding process */ paramValue = EncoderValueFlush; stat = GdipSaveAdd((GpImage*)bm1, ¶ms); - todo_wine expect(Win32Error, stat); + expect(Win32Error, stat);
/* win32 error: can't start encoding process through SaveAdd(), only Save() */ paramValue = EncoderValueMultiFrame; stat = GdipSaveAdd((GpImage*)bm1, ¶ms); - todo_wine expect(Win32Error, stat); + expect(Win32Error, stat);
/* start encoding process: add first frame (bm1) */ paramValue = EncoderValueMultiFrame; @@ -600,7 +600,7 @@ static void test_SavingMultiPageTiff(void) /* add second frame (bm2) */ paramValue = EncoderValueFrameDimensionPage; stat = GdipSaveAddImage((GpImage*)bm1, (GpImage*)bm2, ¶ms); - todo_wine expect(Ok, stat); + expect(Ok, stat); if (stat != Ok) goto cleanup;
/* finish encoding process */
-
Florian Will -
Vincent Povirk (they/them)