Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip failing records when rendering WMF images on 64-bit #8506

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

radarhere
Copy link
Member

Resolves #6980

The issue would like Pillow to be more lenient when rendering invalid EMF files. The API that raises the error is relatively simple

Pillow/src/display.c

Lines 799 to 800 in 81a3bf5

if (!PlayEnhMetaFile(dc, meta, &rect)) {
PyErr_SetString(PyExc_OSError, "cannot render metafile");

so I asked about it at https://learn.microsoft.com/en-us/answers/questions/2100252/playenhmetafile-fails-without-any-getlasterror-res, and the answer showed that PlayEnhMetaFileRecord can be used to render the image gradually, skipping failing records.

@petsuter
Copy link

Great! As mentioned in the ticket, the C# .NET implementation also uses such record-by-record API's internally and allows the files to be handled, so I wouldn't hesitate to do the same. Also programs like Windows Paint and IrfanView allow the files to be opened without error.👍

@radarhere radarhere force-pushed the emf_records branch 2 times, most recently from ee84978 to e4087f9 Compare October 26, 2024 11:33
@radarhere
Copy link
Member Author

It turns out my initial commit didn't work on 32-bit Windows - https://ci.appveyor.com/project/Python-pillow/pillow/builds/50869937/job/3jd42o23cjses7ms#L3048

src/display.c(807): error C2440: 'function': cannot convert from 'BOOL (__cdecl *)(HDC,HANDLETABLE *,const ENHMETARECORD *,int,LPARAM)' to 'ENHMFENUMPROC'

I've pushed a commit to only use the new behaviour on 64-bit. @petsuter is that sufficient to resolve your issue? Or would you like to see this work on 32-bit as well?

@radarhere radarhere changed the title Skip failing records when rendering WMF images Skip failing records when rendering WMF images on 64-bit Oct 26, 2024
@petsuter
Copy link

I don't know of any 32-bit Python usage anymore, thanks.

(Could it be the return value should be int not BOOL? https://learn.microsoft.com/en-us/windows/win32/api/wingdi/nc-wingdi-enhmfenumproc)

@radarhere
Copy link
Member Author

Thanks.

No, changing it to int still gives https://ci.appveyor.com/project/radarhere/pillow/builds/50870384/job/ji4kbtk94gl000q3#L3047

src/display.c(807): error C2440: 'function': cannot convert from 'int (__cdecl *)(HDC,HANDLETABLE *,const ENHMETARECORD *,int,LPARAM)' to 'ENHMFENUMPROC'

src/display.c Outdated
@@ -716,6 +716,14 @@ PyImaging_EventLoopWin32(PyObject *self, PyObject *args) {

#define GET32(p, o) ((DWORD *)(p + o))[0]

BOOL
enhMetaFileProc(
HDC hdc, HANDLETABLE FAR *lpht, const ENHMETARECORD *lpmr, int nHandles, LPARAM data
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why a FAR pointer?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -716,6 +716,14 @@ PyImaging_EventLoopWin32(PyObject *self, PyObject *args) {

#define GET32(p, o) ((DWORD *)(p + o))[0]

int
Copy link
Contributor

@nulano nulano Dec 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
int
static int CALLBACK

You need to specify the __stdcall call convention for 32-bit builds as the default is __cdecl.
It worked on 64-bit because there is only a single call convention used there.

I'm not sure why this is missing from the documentation, I had to look at wingdi.h to figure this out.
The real declaration is actually:

typedef int (CALLBACK* ENHMFENUMPROC)(_In_ HDC hdc, _In_reads_(nHandles) HANDLETABLE FAR* lpht, _In_ CONST ENHMETARECORD * lpmr, _In_ int nHandles, _In_opt_ LPARAM data);

I would also add static to match the other callback in this file.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the thoughts, but I can't find a way to put them together into something that passes on 32-bit. Would you be able to create a PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EMF files: OSError: cannot render metafile
4 participants