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
Fix: regression failure with Pillow 8.1.0 #182
Conversation
I'm not sure I like changing the tests to fix a bug :) |
I'm not sure the sizes are "broken" as it worked perfectly before Pillow update. |
No, as far as NML is concerned they are not. But the images (assuming they're not actually invalid) should not have to be changed to work around Pillow bugs/changed behaviour And also, NML should handle such "broken" images better than outputting a python stacktrace |
I think the best for now is to not use Pillow >=8.1.0 |
Opened an issue on pillow |
Got a reply. The failing PCX are indeed malformed (according to spec linked in the issue comment). But they opened a PR that may allow loading, as some image editors (like gimp) can create malformed PCX. |
Given that it's out of spec, it's probably fine to change the images, but fixing the CI is more important right now, imo |
This fix is a bit problematic for the Debian package: We don't have the luxury of using an older pillow version, so this bug now breaks building in unstable, which is a release-critical bug. Any plans to also fix the PCX images? Would that be a matter of re-saving them with a program that saves them properly? |
As said in PR on pillow,
So the solution would be my first idea, ie resize the images. |
"Fixing" the tests isn't really a full solution -- many grfs are made using GIMP, so this issue is likely to hurt actual users. The test images are typical of grf source files in the wild and indeed taken from real projects. |
Can you open an issue on GIMP's issue tracker linking this then I'll try to look at what we can do on our end. |
In the issue I opened on pillow there's a link to pcx spec, and it seems many tools don't follow |
Do NewGRF people still use PCX? I would think everyone learnt about PNG by now. |
Actually this was already fixed in GIMP 2 years ago, so if you have a recent enough GIMP it should not save with odd values of |
Indeed re exporting files with GIMP seem to be enough. |
A change in Pillow 8.1.0 (release 4 days ago) triggers
OSError: buffer overrun when reading image file
for opengfx_generic_trams1.pcx (371x150) andOSError: image file is truncated (0 bytes not processed)
for opengfx_trains_start.pcx (409x210)I don't really know if it's a bug in Pillow, but I fixed the issue by croping first file to 370x150 and second file to 408x210.