Closed wyatt8740 closed 3 years ago
GdkPixbuf.InterpType.HYPER is deprecated and should be replaced by GdkPixbuf.InterpType.BILINEAR. Remove it and check user preference before do anything (recommended to do it in check_old_preferences
in preferences.py).
If you do want to add interpolation filter provided by PIL anyway:
1, The 'scaling quality' prefs key is also directly used by GdkPixbuf in lens.py and library/book_area.py. Create a new prefs key (e.g. 'pil scaling filter') and default to '-1' to use same filter set by 'scaling quality'.
2, Only add 'BICUBIC' and 'LANCZOS' from PIL (or, in my opinion, only 'LANCZOS'). 'BOX' is not better than 'BILINEAR', and 'HAMMING' quality does not worth. It is nonsence to make little better or even worse quality with such a high cost.
3, Then, clean up the changes in fit_in_rectangle
, especially when the size is not changed.
(But personally I am not very positive to do scaling in such a aggressive way. mcomix is only a comic book viewer. Usually a comic, no matter scanning or digital source, is large enough for human reading, while downscaling always leads low quality if target size is too small.)
Ah, I was reading an old version of the GDK document from when HYPER
was not yet deprecated. Thank you for noticing that. However, HYPER
was already being used in the code; I did not introduce that, and it is outside the scope of adding new filters to remove old ones. So I don't know if I will do that part (at least, not in this pull request).
I had not noticed this usage in lens.py
and book_area.py
(rather, I think I might have but then forgot about it). Thank you for drawing that to my attention. Making a new preference like that does sound like a much better idea, and I'm embarrassed I hadn't thought of it.
BOX
is handy for pixel art, which is the only reason I considered it worth including. All the filters inside mcomix do blurring when enlarging an image, and BOX
is therefore nice if using mcomix as a general purpose image viewer (which I do).
I will make changes so it only offers Lanczos, but I still think in the future it might be good to add an option for using nearest-neighbor when over 100% size.
If image size has not changed, then scaling_quality is set to NEAREST before the part I altered. The original code path is used if scaling_quality is not one of the PIL types, e.g., I did handle this properly in my PR. But it is something to watch out for when I redo it.
I often read raw scans, since I've done scanlation projects in the past. It's just a bit less blurry/easier on the eyes this way. I will likely re-do this (hopefully to your satisfaction) later this week.
Do you think I should just add a checkbox for something like "Prefer Lanczos filter for scaling images in viewing area"? A boolean might be the easiest way if there's only one PIL filter to worry about.
OK, I just pushed to the branch. Any better? I used a combobox for the PIL filter selection and made up a name and preferences description.
I did not touch the hyperbolic stuff yet, and I did not do anything in check_old_preferences as a result.
I would do changes of image_tools.py in such way:
--- a/mcomix/mcomix/image_tools.py
+++ b/mcomix/mcomix/image_tools.py
@@ -170,6 +170,13 @@ def fit_in_rectangle(src, width, height, keep_ratio=True, scale_up=False,
keep_ratio=keep_ratio,
scale_up=scale_up)
+ if (width != src_width or height != src_height) and pil_filter > -1:
+ # scale by PIL interpolation filter
+ src = pil_to_pixbuf(pixbuf_to_pil(src).resize(
+ [width,height], resample=pil_filter))
+ src_width = src.get_width()
+ src_height = src.get_height()
+
if src.get_has_alpha():
if prefs['checkered bg for transparent images']:
check_size, color1, color2 = 8, 0x777777, 0x999999
And, I suggest to move the preference to the 'Advanced' tab, since it does not only impact performance, but also take more peak RAM (a enlarged image in both gdkpixbuf and pil will be stored just before assignment).
Thanks. That sounds good to me, and this patch looks much cleaner. I did my work at around 2 in the morning and I remember feeling a little frustrated (like I knew I was thinking about it wrong).
I'll try to update that tonight.
I feel like it makes more sense to leave the filter options near the other ones. Is this acceptable (putting them under an "advanced" heading)?
If you think it needs to be under the 'advanced' tab, I can move it. The tooltip warns of performance reductions as-is.
PIL contains several resampling algorithms that are not a part of GDK PixBuf (the only options currently provided in MComix are for the PixBuf resampling types). Since MComix is already using PIL, these resampling options can be added at minimal extra cost; performance is likely slightly worse, on account of having to convert the PixBuf to a PIL
Image
and then back again when the resampling is finished, but the quality of a couple of the resamplers (particularlyLANCZOS
andHAMMING
) is subjectively superior (definitely sharper) when compared to that ofGdk.InterpType.HYPER
/BILINEAR
/TILE
.Images with alpha channels are resampled in RGBA and then composited using same PixBuf methods as before, and the conversions to and from PIL are skipped if the scaling type is set to one of the options already handled by Gdk itself.
I think the logic to handle this is sound, but it could probably be made more future-proof if the
scaling type
preference were changed from an int value to a string or some other type. Since this could break existing config files, I have left it as an integer and used a little extra logic to add the PIL types after the PixBuf ones.Let me know if there's something you want changed; I'd be happy to do it.