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
Added reduce operation #4251
Added reduce operation #4251
Conversation
ImagingReduce1x2, ImagingReduce1x3, ImagingReduce2x1, ImagingReduce3x1
@@ -1821,6 +1821,39 @@ def resize(self, size, resample=NEAREST, box=None): | |||
|
|||
return self._new(self.im.resize(size, resample, box)) | |||
|
|||
def reduce(self, factor, box=None): |
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 name of the operation and argument could be discussed.
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.
@python-pillow/pillow-team any objections to naming?
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.
Well, I can't think of a better name :)
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.
thumbs up for the current naming!
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 suggested some small corrections. On the whole, everything is well written :)
@@ -1821,6 +1821,39 @@ def resize(self, size, resample=NEAREST, box=None): | |||
|
|||
return self._new(self.im.resize(size, resample, box)) | |||
|
|||
def reduce(self, factor, box=None): |
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.
thumbs up for the current naming!
What is it
A new operation that reduces an image size in
N×M
integer times. Each pixel gets an average value ofN×M
pixels of the source image. WithN == M
andN == {2, 4, 8}
this operation perfectly match JPEG DCT Scaling. When the image width is divisible byN
and image height is divisible byM
, this operation is perfectly match.resize((im.width / N, im.height / M), Image.BOX)
.Why is it
This will be used in the next Pull request in
.resize()
operation (through new argument) and in.thumbnail()
by default. In a nutshell, it allows speed up resizing in times in some cases.Why not use resize with BOX filter?
Two reasons:
N
or image height is not divisible byM
. This increases the error in the next steps.Why so much code?
The good news is you don't have to review it all. There are:
ImagingReduce1x2
,ImagingReduce1x3
,ImagingReduce2x1
,ImagingReduce3x1
,ImagingReduce2x2
,ImagingReduce3x3
,ImagingReduce4x4
andImagingReduce5x5
for predefined scales.ImagingReduce1xN
andImagingReduceNx1
for xscale == 1 and yscale == 1.ImagingReduceNxN
for any other scales.ImagingReduceCorners
for the last row and last column if any, which is shared across all scales.INT32
andFLOAT32
data types:ImagingReduceNxN_32bpc
andImagingReduceCorners_32bpc
respectively.All of them are:
ImagingReduceNxN
andImagingReduce2x2
with very little changes in logic (more/less columns/rows).TestImageReduce.compare_reduce_with_reference
) and error threshold only 1 tier per channel.So basically, all you need is to carefully check the tests.
Performance
bench_reduce.py
Reduce in x•y times, time in ms.