Page MenuHomePhabricator

Tweak common ThumbnailSteps handling in PHP and JS
Closed, ResolvedPublic2 Estimated Story Points

Description

File::adjustThumbWidthForSteps manipulates thumbnail widths to fit the ThumbnailSteps config var when generating thumbnails and thumbnail links from PHP, and I've recently added a JS implementation that's meant to give the same results in mw.util.adjustThumbWidthForSteps for extensions such as Popups to use (T411013).

There's some question about the details of the logic though, which we should decide on and fix on both ends for all core & extensions code to use consistently. :)

Current logic:

  • for each step:
    1. if the step is larger than the original size, return the requested size
    2. if the step is exactly the requested size, return the requested size
    3. if the step is larger than the requested size, return the step size
  • if the requested thumb is larger than the step, but smaller than the original, return the requested size

I think the main open question is whether to change case 1) to return the original file instead of generating the requested thumbnail size -- in the case of Pops for instance we might have it ask for a 640px thumbnail of a 900px image: it's past the 500px step but before the 960px step and ends up returning a 640px thumbnail instead of rounding up to the full-size image.

(Note JS callers such as Popups will need to be able to distinguish between original-size and thumbnails when generating URLs, to avoid generating calls to exact-size thumbnails. This logic, also should be encapsulated some time.)

CC'ing @Ladsgroup as this is relevant to ongoing work with thumbnail size normalization.

Event Timeline

Change #1211762 had a related patch set uploaded (by Matthias Mullie; author: Matthias Mullie):

[mediawiki/core@master] Round to original file width if there is no steep between that & requested

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

Change #1211762 merged by jenkins-bot:

[mediawiki/core@master] Round to original file width if there is no steep between that & requested

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

ehm. does this account for upsizing of vectorized images, where the 'registered' / 'inherent' size can be smaller than a desired size ?

So a legitimate step size should be able to be larger than the original size in that case.

Wouldn't browsers been able to do it on the fly? I'd be surprised if FF can't upscale a svg. I can try in beta cluster but it's down.

Wouldn't browsers been able to do it on the fly? I'd be surprised if FF can't upscale a svg. I can try in beta cluster but it's down.

we don't support clientside SVG rendering. All our SVG are converted to PNG when used as thumbnails (and even if we fix that, we still have the translations and the too complex [15MB+ vector maps of the entire world] SVGs that would get rendered to PNG)

See also T208578: SVG client side rendering for specific SVGs

Ah fair. Thanks for spotting it. Let me see if I can fix it easily.

Change #1212166 had a related patch set uploaded (by Ladsgroup; author: Amir Sarabadani):

[mediawiki/core@master] File: Allow scaling up vectorized images to larger sizes

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

Change #1212166 merged by jenkins-bot:

[mediawiki/core@master] File: Allow scaling up vectorized images to larger sizes

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

Change #1213584 had a related patch set uploaded (by Bvibber; author: Bvibber):

[mediawiki/core@master] Unit tests for vector overrides to step original size checks

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

I added unit test coverage in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1213584

If someone will merge that I'm content to close this out. :D

Change #1213584 merged by jenkins-bot:

[mediawiki/core@master] Unit tests for vector overrides to step original size checks

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

Tweak and tests merged -- _closed resolved_. :D