-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Fast filters #2679
Fast filters #2679
Conversation
homm
commented
Aug 12, 2017
•
edited
Loading
edited
# Conflicts: # libImaging/Filter.c
# Conflicts: # libImaging/Filter.c
PIL/Image.py
Outdated
@@ -1112,7 +1112,9 @@ def filter(self, filter): | |||
raise TypeError("filter argument should be ImageFilter.Filter " + | |||
"instance or class") | |||
|
|||
if self.im.bands == 1: | |||
multiband = getattr(filter, 'is_multiband', False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All filters used to work only with single-channel images. As this is public API and there are can be custom filters in a wild, we can't just send multi-channel images to all filters. So is_multiband
flag indicates that the filter is ready for multi-channel images.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wiredfool As of naming things is one of the hard problems in programming, I need your help there )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's a reasonable name, at least, I haven't thought of anything better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The another option is subclass MultibandFilter
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's probably cleaner, especially with an eye towards the typing PR.
@@ -852,8 +852,12 @@ _filter(ImagingObject* self, PyObject* args) | |||
return ImagingError_ValueError("bad kernel size"); | |||
} | |||
|
|||
for (i = 0; i < kernelsize; ++i) { | |||
kerneldata[i] /= divisor; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Precompute by dividing coefficients to divisor rather than each result pixel.
Oh my god! I clash with the same bug in Intel x64 CPUs with int to float conversion, as I did when was working on resampling for Pillow 2.7. In essence: GCC version < 4.9 (at least Ubuntu 14.04, Debian 7 Wheezy, Red Hat Linux 7) generates code which runs about 2.5 times slower than GCC >= 4.9. This can be fixed using ugly hack with assembler. We decided to include this hack in Pillow 2.7 (released Jan 1, 2015). After rewriting the resampling using fixed point arithmetics, this hack become obsolete and was removed in Pillow 3.3 (Jul 1, 2016). Unlike for resampling, I don't see a way to implement kernel filters on FPA. So, I suggest to include this hack again. |
I've just tried SSE4 for multiband images, with
|
@wiredfool implemented through MultibandFilter |