Switch to QImage scaling when downscaling as QPainter is awful

[j-b-m]

Seems like a Qt6 regression, but QPainter scaling gives terrible results when downscaling. So in such cases, scale using QImage before painting. Kdenlive issue 2123

Comparison of results (updated QImage scaling on top)

[Copilot]

Pull request overview

Adjusts Qt blending downscaling to avoid poor-quality QPainter scaling (Qt6 regression), by pre-scaling via QImage when reducing image size.

Changes:

• Detects rescale quality preference via consumer.rescale and toggles HQ painting accordingly.
• Pre-scales the source QImage when downscaling (instead of applying a QPainter scale transform).
• Makes QPainter render hints conditional on the chosen interpolation mode and draws either the scaled or original image.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[bmatherly]

The artifacts kind of look like the operation is not working on pixels that have some transparency (around the border of the logos). I did not recreate the problem myself. Does it only happen on images with transparency? Or does it still occur if the image background is fully opaque? I'm just wondering if there is some parameter we need to pass to the painter to make sure it handles transparency (alpha) correctly.

[bmatherly]

Can you explain why you copy the string? I think you can use it directly.

[ddennedy]

Using strdup on this string property is a common thing in our source code that dates way back. The property getter returns a pointer, and after the call the string it points to can be changed including NULLed, leading to comparison over-reads or crashes. I think this is more of an issue for a string property on a service getting changed by an application, but these are checking a frame property set by mlt_consumer that is not going to be mutated by another thread.

This technique goes back to a bug fix in 9a19d7b. The bug report is not available, and there are other fixes mixed in with that commit. So, hard to say what was the actual fix. Maybe this was the ultimate fix, and something later actually did.

[bmatherly]

This technique goes back to a bug fix in 9a19d7b

Thanks for the reference. I suppose the author (or the AI they used) just followed other examples for interp.

The property getter returns a pointer, and after the call the string it points to can be changed including NULLed, leading to comparison over-reads or crashes.

While that is a true statement, this strdup() trick doesn't fix that because the pointer could still be changed after the get() call and before the strdup call. So this trick only reduces the probability. If data being changed is a concern, then the service should be locked.

I would prefer that we not propagate this unnecessary strdup and start setting a good example for future authors. But I can appreciate that it is following an established convention.

[j-b-m]

Yes, I just copied code used in the qtblend transition that was initially based on the affine transition where this is still used. But sure, I will clean that up

[ddennedy]

But you removed protection for the string making it worse. If you are going to remove strdup() you need to add mlt_service_lock() before getting it and mlt_service_unlock() after you are done with using it. This must be fixesd.

[ddennedy]

does it still occur if the image background is fully opaque?

Looking at the images attached here, even the lines inside the opaque areas have very bad aliasing suggesting no interpolation.

[ddennedy]

JB, are the images you provided scaled-up for emphasis?

In the docs, both QPainter and QImage say the hints provided both do bilinear interpolation:
https://doc.qt.io/qt-6/qpainter.html#RenderHint-enum
https://doc.qt.io/qt-6/qt.html#TransformationMode-enum

However, Claude suggested this difference:

The Qt docs description is misleading/simplified. Despite the "bilinear filtering" wording, Qt's internal implementation of QImage::scaled() with SmoothTransformation uses an area-averaging algorithm (qSmoothScaleImage → qImgScaleAARGBA in Qt's source), not just a bilinear 4-sample lookup.

The meaningful difference is:

Method	Downscaling algorithm
QPainter with SmoothPixmapTransform	Bilinear: maps each output pixel back to the source and samples only the 4 nearest source pixels
QImage::scaled(SmoothTransformation)	Area-weighted: averages all source pixels that contribute to each output pixel, weighted by their coverage

For a 4× downscale, bilinear reads 4 out of ~16 contributing source pixels; the area-average reads all ~16. The Qt docs use "bilinear" as a loose synonym for "smooth/filtered" vs "nearest-neighbor" — not as a precise description of the algorithm.

So the fix is correct: pre-scaling via QImage::scaled(Qt::SmoothTransformation) produces proper area-averaged results, while relying on QPainter's transform for downscaling only ever bicubic/bilinearly-samples from the original high-res source, causing aliasing.

[bmatherly]

does it still occur if the image background is fully opaque?

Looking at the images attached here, even the lines inside the opaque areas have very bad aliasing suggesting no interpolation.

Oh yeah. I see what you mean.

[j-b-m]

As Dan said, this is unrelated to alpha.. but yes the images attached above were scaled to better show the difference.
Here is an unscaled comparison, reducing an image (red S on a white background) with an original height of 4000 to a height of 270. Using QPainter on the left, QImage on the right.

The quality loss is proportional to the scaling applied in the effect. We probably need to also apply the same trick to the qtblend transition.
I will try to cleanup a bit this MR in the next days.

[ddennedy]

FYI I plan to make a release by April 24 if you want to get this in.

[ddennedy]

You need to address the comments including the ones from Copilot and provide an overall status (possibly by requesting me to review it). "address" means to give a brief reply and resolve it if resolved. If I see some resolved ones but others unresolved that means not everything is resolved, and this should not be merged.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

[j-b-m]

Yes, sorry I was hoping to give it a review just after pushing my changes, but had to deal with other stuff, should have marked it as draft. Should be ok now, sorry.

[j-b-m]

Thanks for noticing the interpolation error!