Conversation
| LOAD_PATHS = [ | ||
| *SAVE_PATHS, | ||
| IMAGES_PATH / "uncompressed_rgb.dds", | ||
| ] |
There was a problem hiding this comment.
An alternative to splitting up PATHS would be to change test_save_jpeg to test_save, and forgo the quality argument.
Is there any reason not to do that?
There was a problem hiding this comment.
Um... test_save_jpeg always saves a JPEG (into memory).
That would make it run the same thing for more than one image - is there a reason to do that?
There was a problem hiding this comment.
Oh, I meant saving both JPEG and DDS images - akx#31
There was a problem hiding this comment.
Then we could be instead testing saving with the default benchmark image, into different formats?
There was a problem hiding this comment.
At this stage, I'm just wondering why it isn't helpful to test saving DDS images, since it would seem simpler to do so.
Let me ask a different question though - why use flower2.jpg, rather than hopper.jpg?
There was a problem hiding this comment.
Of course it can be helpful to also benchmark writing DDS images, it just didn't feel in scope to add that in a PR that's speeding up DDS reading.
I can whip up another PR that does that more comprehensively (e.g. benchmark all common codecs, both reading and writing)?
As for why flower2.jpg, I can't recall I had a particular reason for choosing it over hopper.jpg in #9654.
There was a problem hiding this comment.
If you'd like to go forth with #10001, then sure, but don't do it for my sake. I was only trying to understand why there was a distinction being made between formats.
|
As far as uncompressed_rgb.dds is concerned, I think I can do you one better. #7589 added DdsRgbDecoder in order to generalise this decoding path, but before that, this file was read with the raw decoder. So if apply the following change to main to use the raw decoder for this specific scenario, you will find it faster than the current state of this PR. diff --git a/src/PIL/DdsImagePlugin.py b/src/PIL/DdsImagePlugin.py
index 40012bc27..a6cec0557 100644
--- a/src/PIL/DdsImagePlugin.py
+++ b/src/PIL/DdsImagePlugin.py
@@ -371,8 +371,11 @@ class DdsImageFile(ImageFile.ImageFile):
mask_count = 3
masks = struct.unpack(f"<{mask_count}I", header[84 : 84 + mask_count * 4])
- self.tile = [ImageFile._Tile("dds_rgb", extents, 0, (bitcount, masks))]
- return
+ if masks == (0xFF0000, 0x00FF00, 0x0000FF):
+ rawmode = "BGR"
+ else:
+ self.tile = [ImageFile._Tile("dds_rgb", extents, 0, (bitcount, masks))]
+ return
elif pfflags & DDPF.LUMINANCE:
if bitcount == 8:
self._mode = "L" |
|
@radarhere Good stuff! 🎉 Applied that on here too. I checked pytest-cov, and with that applied the speedups from dadbf4f are still tested and used, so this is a double win :) |
2 participants
Follows up on #7589. I noticed this looking at
pytest --durations; a fuzzer test that just loads a DDS was surprisingly slow, and turns out doing 1-byte reads is not very efficient...Locally:
or 255.17 times lower minimum, or in average OPS terms, +22351.7% faster. 😂