Ensure page rotation 0 for non-PDF documents #2946
Conversation
src/__init__.py
Outdated
| if not self.parent.is_pdf: | ||
| return 0 | ||
| page = self.this if isinstance(self.this, mupdf.PdfPage) else mupdf.pdf_page_from_fz_page(self.this) | ||
| if not page: | ||
| return 0 |
There was a problem hiding this comment.
I think a simpler fix might be to change if not page: to if not page.m_internal. [I thought i'd fixed all of these mistakes but i missed this one.]
[Actually we could write a function _as_fz_page() similar to as_pdf_page() and as_pdf_document(), which does the down-cast and returns with .m_internal None if not a pdf page, but that can be done separately.]
There was a problem hiding this comment.
let me try this one then.
There was a problem hiding this comment.
Indeed, this check resolves the problem.
Is there any documentation about what property m_internal does? Feels like I should have known this ...
There was a problem hiding this comment.
Apart from POD types (e.g. fz_rect and FzRect), all C++ wrapper classes have an m_internal member which is a pointer to the underlying MuPDF C struct. So for example mupdf::PdfPage has a pdf_page* m_internal member.
If a MuPDF function returns a NULL pdf_page*, then when wraped in C++ or Python, it will return a PdfPage instance with a NULL or None m_internal. This unfortunately breaks simple translation of code that checks whether a pointer is non-NULL, as in this PR.
[I've often wondered about adding an operator bool() member function to the C++ wrapper classes, to generically avoid the problem, but have never got round to trying it out.]
There was a problem hiding this comment.
Ah, ok, thanks for the explanation!
There was a problem hiding this comment.
There is very little information about m_internal at https://mupdf.readthedocs.io/en/latest/language-bindings.html. I've added a todo item to improve this.
Use success of fz_page conversion to pdf_page as an indicator for rotation = 0.
JorjMcKie
force-pushed
the
worng-rotation-evaluation
branch
from
ef2042c to
20e7f19
Compare
Accessing page.rotation for non-PDF leads to an interpreter crash. Return zero as rotation for non-PDF documents.