Limit image scales to those smaller than the original

[davisagli]

This makes the serialization of scales for image fields more consistent with the set of scales that comes from image_scales in catalog results. And it avoids storing duplicate data for large scales.

Related to plone/volto#7655

---

📚 Documentation preview 📚: https://plonerestapi--1992.org.readthedocs.build/

[mister-roboto]

@davisagli thanks for creating this Pull Request and helping to improve Plone!

TL;DR: Finish pushing changes, pass all other checks, then paste a comment:

@jenkins-plone-org please run jobs

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically.

Happy hacking!

[davisagli]

@jenkins-plone-org please run jobs

[sneridagh]

LGTM!

[jensens]

You need an exception for SVG images.

[davisagli]

@jensens Why an exception for SVG images?

[mauritsvanrees]

Makes sense.

I suppose for SVG a larger scale does not bloat the database, as we scale this by editing the XML contents. Use here in plone.namedfile after checking the content type. So this could work:

if actual_width > width and getattr(field, "contentType", "") != "image/svg+xml"

[jensens]

Why an exception for SVG images?

a) SVG naturally scales up - its vector.
b) it does not blow up in size

[pnicolli]

Correct, it's vector graphics and does not need scales at all
I mean, if the "scaling" is something that changes the default attributes, sure, but it would look overengineered to me

[jensens]

Correct, it's vector graphics and does not need scales at all

Misconception, if you dont apply a size to display it the SVG will show somehow (if not constrained by CSS, what you never know). So setting the size for display as we do already is good, but sufficient, we dont need to change anything, just keep as is.

[pnicolli]

I disagree that we should not assume we have a theme on our sites, which is what I understand from your comment. If the theme does not constrain images properly, I don't think it's an issue at the lower level.

That being said, I did not mean that a proper svg image should not have it's own set of dimensions, I'm saying you don't need to generate more scales just to set different dimensions on it, imho. I would rather not have this kind of code to maintain if it was mine.

[jensens]

The code its simple enough and no burden.

[davisagli]

I'm not sure what to do with this one.

My original goal was to avoid advertising unnecessary scales, which scrapers end up fetching (which then creates them and bloats the db).

But I think there are legitimate questions about how to tell when they are unnecessary. There's the case Jens brought up with SVGs that have a height and width embedded in the markup. (I think that's kind of dumb and counteracts one of the main benefits of a scalable vector format, but that's a bigger topic than we can tackle in this PR.) It's also possible to have scales that are not only scaling up or down but cropping to a different aspect ratio.

[jensens]

Tons of image scales polluting the ZODB is a topic broader than limiting the scales.

[pnicolli]

My original goal was to avoid advertising unnecessary scales, which scrapers end up fetching (which then creates them and bloats the db).

I still think your goal is valid.

But I think there are legitimate questions about how to tell when they are unnecessary. There's the case Jens brought up with SVGs that have a height and width embedded in the markup. (I think that's kind of dumb and counteracts one of the main benefits of a scalable vector format, but that's a bigger topic than we can tackle in this PR.) It's also possible to have scales that are not only scaling up or down but cropping to a different aspect ratio.

Yes, the questions are legitimate, and I also think the svg topic is a bigger and important one. I see a potential loss of value if a bigger topic blocks a small but interesting improvement like this one, so can we have svgs work like they used to and push for this improvement for other raster images?

[jensens]

Yes, the questions are legitimate, and I also think the svg topic is a bigger and important one. I see a potential loss of value if a bigger topic blocks a small but interesting improvement like this one, so can we have svgs work like they used to and push for this improvement for other raster images?

Quite OT here, what about opening an new topic for discussion?

[jensens]

There's the case Jens brought up with SVGs that have a height and width embedded in the markup.

Just skip if its a SVG, done.

It's also possible to have scales that are not only scaling up or down but cropping to a different aspect ratio.

How does the code here modify the output of images crops?

[jensens]

Sorry, accidentally closed, reopened.

[read-the-docs-community]

Documentation build overview

📚 plone.restapi | 🛠️ Build #31893528 | 📁 Comparing 479f29b against latest (abebc6b)

🔍 Preview build

Show files changed (1 files in total): 📝 1 modified | ➕ 0 added | ➖ 0 deleted

File	Status
endpoints/content-types.html	📝 modified