Dmitry Timoshkov dmitry@baikal.ru writes:
Based on OLEPictureImpl_Save implementation.
Surely you don't need to duplicate all of it.
Alexandre Julliard julliard@winehq.org wrote:
Based on OLEPictureImpl_Save implementation.
Surely you don't need to duplicate all of it.
Otherwise I would need to rewrite both SaveAsFile() and Save(), which could be inappropriate for the code freeze, so I decided to just fix SaveAsFile() which is clearly broken at the moment. It should be possible to reduce the duplication, but only after 1.8 is released.
Dmitry Timoshkov dmitry@baikal.ru writes:
Alexandre Julliard julliard@winehq.org wrote:
Based on OLEPictureImpl_Save implementation.
Surely you don't need to duplicate all of it.
Otherwise I would need to rewrite both SaveAsFile() and Save(), which could be inappropriate for the code freeze, so I decided to just fix SaveAsFile() which is clearly broken at the moment. It should be possible to reduce the duplication, but only after 1.8 is released.
OK, if you promise to do that after 1.8, I'll put it in ;-)
Alexandre Julliard julliard@winehq.org wrote:
Based on OLEPictureImpl_Save implementation.
Surely you don't need to duplicate all of it.
Otherwise I would need to rewrite both SaveAsFile() and Save(), which could be inappropriate for the code freeze, so I decided to just fix SaveAsFile() which is clearly broken at the moment. It should be possible to reduce the duplication, but only after 1.8 is released.
OK, if you promise to do that after 1.8, I'll put it in ;-)
It should be pretty easy to make Save() call SaveAsFile() as a backend.
Dmitry Timoshkov dmitry@baikal.ru writes:
Alexandre Julliard julliard@winehq.org wrote:
Based on OLEPictureImpl_Save implementation.
Surely you don't need to duplicate all of it.
Otherwise I would need to rewrite both SaveAsFile() and Save(), which could be inappropriate for the code freeze, so I decided to just fix SaveAsFile() which is clearly broken at the moment. It should be possible to reduce the duplication, but only after 1.8 is released.
OK, if you promise to do that after 1.8, I'll put it in ;-)
It should be pretty easy to make Save() call SaveAsFile() as a backend.
I failed to notice in my test run yesterday, but this is causing test failures in gdiplus. It's possible that this would have far-reaching consequences, so I'm going to revert the patch for now. It will have to be redone properly after code freeze. Sorry about that.
Alexandre Julliard julliard@winehq.org wrote:
Based on OLEPictureImpl_Save implementation.
Surely you don't need to duplicate all of it.
Otherwise I would need to rewrite both SaveAsFile() and Save(), which could be inappropriate for the code freeze, so I decided to just fix SaveAsFile() which is clearly broken at the moment. It should be possible to reduce the duplication, but only after 1.8 is released.
OK, if you promise to do that after 1.8, I'll put it in ;-)
It should be pretty easy to make Save() call SaveAsFile() as a backend.
I failed to notice in my test run yesterday, but this is causing test failures in gdiplus. It's possible that this would have far-reaching consequences, so I'm going to revert the patch for now. It will have to be redone properly after code freeze. Sorry about that.
Failures in gdiplus tests are caused by broken code in gdiplus, in particular GdipCloneImage() is broken. First, it calls IPicture_SaveAsFile() with 'mem_copy' parameter set to FALSE, and according to the tests IPicture_SaveAsFile() always returns E_FAIL in that case. Second, IPicture_SaveAsFile() has never been writing correct data to the provided stream, so despite that it hasn't returned an error the result just couldn't be correct. It looks like whoever has written that code hasn't tested it at all.
Regardless of the above, I'm fine with the revert, it seems like I no longer have obligations to fix the code duplication :)
Dmitry Timoshkov dmitry@baikal.ru writes:
Alexandre Julliard julliard@winehq.org wrote:
> Based on OLEPictureImpl_Save implementation.
Surely you don't need to duplicate all of it.
Otherwise I would need to rewrite both SaveAsFile() and Save(), which could be inappropriate for the code freeze, so I decided to just fix SaveAsFile() which is clearly broken at the moment. It should be possible to reduce the duplication, but only after 1.8 is released.
OK, if you promise to do that after 1.8, I'll put it in ;-)
It should be pretty easy to make Save() call SaveAsFile() as a backend.
I failed to notice in my test run yesterday, but this is causing test failures in gdiplus. It's possible that this would have far-reaching consequences, so I'm going to revert the patch for now. It will have to be redone properly after code freeze. Sorry about that.
Failures in gdiplus tests are caused by broken code in gdiplus, in particular GdipCloneImage() is broken. First, it calls IPicture_SaveAsFile() with 'mem_copy' parameter set to FALSE, and according to the tests IPicture_SaveAsFile() always returns E_FAIL in that case. Second, IPicture_SaveAsFile() has never been writing correct data to the provided stream, so despite that it hasn't returned an error the result just couldn't be correct. It looks like whoever has written that code hasn't tested it at all.
Sure, this will all have to be fixed, and breaking it by making IPicture_SaveAsFile more correct is actually a good thing since it reveals problems. Only not during code freeze...
Failures in gdiplus tests are caused by broken code in gdiplus, in particular GdipCloneImage() is broken. First, it calls IPicture_SaveAsFile() with 'mem_copy' parameter set to FALSE, and according to the tests IPicture_SaveAsFile() always returns E_FAIL in that case. Second, IPicture_SaveAsFile() has never been writing correct data to the provided stream, so despite that it hasn't returned an error the result just couldn't be correct. It looks like whoever has written that code hasn't tested it at all.
Really, gdiplus code shouldn't be using IPicture at all. We should instead use gdi32 metafile handles directly.
-
Alexandre Julliard -
Dmitry Timoshkov -
Vincent Povirk