jcupitt wrote:

Can I help in any way? I'd be happy to have a go at doing this.

Is there a test harness I could use, or should I set up a tiny mediawiki install?

jgerber wrote:

Would be great if you give it a go.
I tend to use a VM with a minimal install.
build with https://github.com/bit/mediawiki_vm
but install outside of a VM should also be ok.

(In reply to comment #0)

VipsScalar currently works by running a series of separate vips commands.
Looking at the source here:

https://gerrit.wikimedia.org/r/gitweb?p=mediawiki/extensions/VipsScaler.git;
a=blob;f=VipsScaler_body.php;h=69f31b108d99849c36fd92591f135bfbfca69d02;
hb=HEAD

makeCommands() will typically run something like:

unpack png file to a huge disc temp

vips im_png2vips huge.png t1.v

It only does this if the "preconvert" option is set, which it isn't.

block shrink by large integer factor

vips im_shrink t1.v t2.v xx xx

bilinear resize to final exact dimensions

vips im_resize_linear t2.v t3.v xxxx yyyy

I don't know where you're getting this from. It either does im_shrink by a precise floating point factor, giving the exact dimensions required, or it does im_resize_linear, it can't do both. In the case of PNGs, it is configured to use im_shrink.

sharpen slightly to counteract softening effect of bilinear

vips im_convf t3.v t4.v sharpen_matrix

No, the sharpening counteracts for the reduction in the acutance perception of the human eye at smaller angular sizes. It's necessary regardless of how you scale the image.

Have a look at the sample images I attached to bug 23258.

any 90-degree rotation required

vips im_rotxx t4.v final.jpg

Instead, it should simply run vipsthumbnail:

vipsthumbnail huge.png -o final.jpg --size required-output-size-in-pixels
--interpolator bicubic

I said on bug 25990 comment 9 that the reason we can't use vipsthumbnail is because it stores the uncompressed source image on disk. It may be that this is no longer the case, however, using vipsthumbnail would still mean giving up control over the sharpening parameters.

I've looked at the sharpening code in vipsthumbnail.c git master. It's much simpler than what MediaWiki does. I am sure that MediaWiki is doing a better job of this than vipsthumbnail.

The sharpening parameters that we use (radius and threshold) are carefully chosen, and changing them has caused complaints in the past (bug 24857). So I was keen to have a scaling system that reproduced the current results as nearly as possible. That's why I asked Bryan to make VipsScaler the way it is.

jcupitt wrote:

(In reply to comment #3)

I don't know where you're getting this from. It either does im_shrink by a
precise floating point factor, giving the exact dimensions required, or it
does

Sorry for the misinformation, I read VipsScaler too quickly.

I said on bug 25990 comment 9 that the reason we can't use vipsthumbnail is
because it stores the uncompressed source image on disk. It may be that this
is
no longer the case, however, using vipsthumbnail would still mean giving up
control over the sharpening parameters.

vipsthumbnail no longer decompresses to disc, it streams the image.

I agree that control over the sharpening would be useful, I'll add an option to let you specify the convolution matrix.

Related URL: https://gerrit.wikimedia.org/r/62233 (Gerrit Change Ie92da659c78421163ce2a6c2bf774a767564460a)

Bryan.TongMinh wrote:

Note that Ie92da659c78421163ce2a6c2bf774a767564460a does not yet support John's new sharpening code.

John, could you perhaps provide a Windows build of the new vipsthumbnail?

jcupitt wrote:

Hi Bryan, I made a Windows build of current vips master for you:

http://www.vips.ecs.soton.ac.uk/development/vips-dev-7.33.0.zip

There's a new param, --sharpen, you can use to specify the sharpening mask.

I'm removing the blocks: 48545 from this bug as there are other methods of fixing 48545 (namely, enabling VipsScaler for PNGs of that size, which will be fixed when we close bug 28135). From my understanding of bug 32721 (the bug that spawned this one), the use of vipsthumbnail is an improvement to VipsScaler to reduce the amount of disk writing on conversion. Something not needed but a good idea when we can get it merged.

Please correct me if any of that is wrong.

km wrote:

If someone has test images you'd like us to run both rescalers on to check the results - please let me know. I'm sure vipsthumbnail is the way to go in terms of I/O efficiency.

I gather we also need to ensure the newest vips library is in the debian build...

Change 62233 had a related patch set uploaded (by Qgil):
Add vipsthumbnail support

https://gerrit.wikimedia.org/r/62233