typo icon indicating copy to clipboard operation
typo copied to clipboard

Image optimization

Open OleMussmann opened this issue 9 months ago • 9 comments

This PR introduces the following new tags:

  • #eager
  • #lazy
  • #nowebp
  • #nocompress

[!NOTE]
I included a demo post for experimentation. We should remove this from the PR before merging, since the images are rather large and I don't want to pollute the repo with that.

By default, the first image of a post now gets a loading="eager" tag. This means it is loaded early in the process to prevent Cumulative Layout Shifts, where the content shifts while resources are being loaded. The following images get a loading="lazy" tag, to be loaded later in the process. This improves the perceived loading speed.

This can be overridden with #lazy and #eager tags, to force one of those modes. A good rule of thumb is to load all images that are visible on first-load eagerly, the rest lazily. If the image resource is external (starting with http:// or https://) or an SVG, then that's it. The image will be loaded unaltered, in full.

Local resources, however, get a special treatment.

The main content width is now set to 720px (first commit). This is very close to what it is now, but it's not calculated anymore. The body width is then padded with var(--main-padding) on both sides. These changes makes sure the content always has a known width that we can use for image optimization. By default the images are resized to a width of 720px using the Lanczos resampling filter, and converted to WEBP format (second commit).

{{ $imgResource = $imgResource.Resize "720x webp q75 Lanczos" }}

This dramatically reduces image sizes without needing to convert them beforehand.

The original first two images are 1.5Mb and 4.8MB, respectively. With resizing and image compression, the images are ~18.2kb each with little loss in quality at the rendered size.

This is a bit of a worst-case scenario, but I can imagine that it would happen rather often that the images used are rather large, and when testing locally they will load very quickly. Once deployed, the performance hit becomes apparent for visitors.

The converted images are stored in the resources/_gen folder, as well as copied into public/posts/image_demo/. It is not always desirable to compress images, of course. Sometimes you want visitors to download an image at original resolution or format, or not force small images to fill the screen. The compression can be disabled with the #nowebp tag, the resizing with #noresize.

Normally I would opt for explicit image conversions, e.g. with a #webp tag, or #resize. In this case, however, the gains are so large that I'm leaning towards compressing by default. As opt-in it might be under-used. Once an author sees that the quality or format is not as desired, they can always switch it off.

For more fine-grained control, I could imagine setting the compression quality with a tag #qX, with 1<X<100, e.g. q50 for higher compression, or q90 for better quality. The default is, for now, hard-coded to q75.

[!TIP]
These changes are not only important for loading speed, but also help with Search Engine Optimization (SEO). Wrongly sized images and content shifts are penalized.

This is a rather blatant alteration of how image loading works, so this should stay a draft until we have agreed on the details. After that, we should include a wiki update as well.

I'm curious of your opinion on this. Also, I'm pretty sure the hackery in render-image.html can be cleaned up a bit.

OleMussmann avatar Feb 20 '25 22:02 OleMussmann

Wanted to add my 2 cents to this since I think there's quite a lot of stuff going on here:

  1. I personally think it's better not to add special tags/class based logic since it complicates the usage for the end-user and also bloats the implementation. But it would be good if the theme itself handles things transparently for 95% of the use cases even if it means hardcoding a decision. If some of these are really needed, it's probably better to look into .Attributes for this.

  2. I don't think it's required to special-case loading="lazy" as I think browsers are better equipped to prioritize and load images even with that applied for above-the-fold images and the logic is quite honestly ever-changing and inconsistent across browser platforms. CLS should already be dealt with since we are setting width/height attributes with natural dimensions already (if still some CLS issue is there, do mention).

    Plus, even if let's say there's some benefit at this point in time for a browser platform, why the first image? What if there are 2-3 images above the fold and the first two are rather small, icon-size but the third is actually the main image? This logic would fall apart in these kind of scenarios. It would probably also not be correct on non-content index pages where specific images might be positioned decoratively. Ignoring even that, it might be more beneficial to add fetchpriority="high" for such images if we could somehow deduce them properly.

    This 2024 talk highlights some of the heuristics and wild differences in browser implementations when it comes to loading priorities and when I just checked it again, things were again quite different across the browsers. So, unless there's any measurable differences, I don't think this matters much.

  3. Re: compression, I'm biased since I already do compression on my end via Obsidian so I wouldn't want another compression process kicking in during build. Though, I get that this might be desirable and if yes, it's probably better to see if a solution that satisfies 95% of the use cases exists and just add that with a straightforward boolean config enableCompression. Images needing to skip compression can use the <img> tag. Otherwise, the implementation/config will get hairy. (Side note: I wonder if Hugo itself provides some global setting for this?)

  4. For SVG, it makes sense that this fails and I think it's probably better documented than addressed via this hook. Reason being there are different ways to embed SVGs and the width/height is entirely user-specified. I don't think an assumption can be made about what might work best for most users. Documenting to use <img> tag directly with width/height or embedding the SVG markup would be better IMO.

runofthemillgeek avatar Mar 05 '25 21:03 runofthemillgeek

So, initially, while reading through the big list of PRs that @OleMussmann proposed, this seemed like a cool idea. However, after seeing the code, it significantly complicates the render-image logic, making it honestly seem unmaintainable.

@runofthemillgeek made some great points here, and I agree.

I would propose that @OleMussmann skip the lazy, eager, and compression tags entirely and instead introduce an enableCompression setting in the hugo.toml. Again, this complex list of tags could easily live in your version of the theme if someone needs it.

tomfran avatar Mar 05 '25 22:03 tomfran

Thanks for your feedback and discussions. Indeed, this PR was not meant to merge as-is, but serve as a basis for discussion on how to implement things and improve upon the elements you do want to have merged.

The code is indeed pretty horrible. It's the first thing that worked, to have a basis for discussing. I'm happy to clean it up and comment it well to keep it maintainable once you know if/what/how to implement things.

The idea for the defaults was to find reasonable defaults that work "most of the time", while allowing the user to override any of the suggestions. Users that don't care much would not need to use any of the tags, so I don't think it necessarily complicates things.

  1. The tag-logic is already implemented and used for image sizes. In my opinion it would make sense to use one way of formatting images. Maybe .Attributes would be a better choice here, it looks like a more "hugo-nic" way of doing things. If we hard-code decisions (compression or not, resizing or not, etc.) I would strongly suggest to allow for ways to override that.
  2. I did experiment with it quite a bit with Firefox and Chromium dev tools, especially with disabling cache and throttling network speed. It looks and feels significantly better when the images on the first page are loaded eagerly. I'll look into fetchpriority="high" as well to see how it differs. My idea was to find defaults that work for most cases, and then give the author the tools to fine-tune that if necessary. Maybe that's too hard to do.
  3. I like the idea to have compression configurable in the hugo config, but I would like to have options to override that both per post and per image. I'm not a big fan of using <img> for non-compressed images, that's quite a stretch for the user experience. I would prefer too hook into the existing workflow of either tags or .Attributes.
  4. I don't understand yet what you mean. Is using the filetype svg as a trigger to omit width/height tags not robust? Again, I'd prefer to not need to use <img> as an alternative way to deal with SVGs.

This whole area is somewhat new to me, so please continue to point out not only where you disagree on style or approach, but also when I overlook technical aspects or have the wrong approach to it.

OleMussmann avatar Mar 08 '25 12:03 OleMussmann

@OleMussmann

The code is indeed pretty horrible. It's the first thing that worked, to have a basis for discussing. I'm happy to clean it up and comment it well to keep it maintainable once you know if/what/how to implement things.

That's alright and expected. Thanks for the PR and kicking off the discussions related to this, it definitely helps if some of these end up in typo in a good shape.

  1. Agree with sticking with a consistent format. I'm not aware of the existing tag stuff but I'm guessing it's better to integrate .Attributes as escape-hatch/customization here which lines up with how Hugo thinks about it. Re: hard-coding configs, I think we can't always expect to give a choice for each and every parameter we decide. That'll just make things complicated and hard to maintain. If someone is interested in their own compression workflow or how images are rendered, they can always copy and tweak the render-image.html file. We can document this if it helps.
  2. I did a few checks as well but didn't notice any reasonable/consistent difference under throttling across macOS Safari/FF/Chrome. Adding fetchpriority/rel=preload did help in certain specific cases. I'm sure images that get prioritized and load early for above the fold would definitely be a better experience, not denying that. My point was about deciding "how many" and the inconsistencies across browser implementations and the possibility for heuristics to change with each iteration. Finding defaults here is difficult and depends entirely on how each article is laid out + browser implementation. In case you have any live URLs where you spot such issues, lmk, curious to check. And if we're going ahead with .Attributes spreading, this can be configured by the author.
  3. I think that's a bit too much to expect from the theme IMHO when you need per-image compression customization. In such cases, it's probably better to go with own render-image.html and add customizations as necessary. I think it's more maintainable and beneficial to stick to a sane default and apply for all. Though, it should be okay to allow per-image compress=false as an attribute that disables it as a whole.
  4. SVGs can be embedded in various ways, not just using <img> tags. My reasoning was that it's better for the user to be explicit about how they want to add an SVG than let the image hook handle it. But, thinking about this again from the authoring POV, I guess you are right. It would be okay I guess to not try to read width/height if file is an SVG and render it via <img>. If we spread .Attributes, that should take care of width/height.

I think we're all in agreement w.r.t:

  1. Having a compression config option to enable/disable for all, with a reasonable default compression algo. It's probably worth creating an issue separately for this and align on the defaults.

In addition, I'm good with the following that can also be taken as separate issues for enhancement (CC @tomfran):

  1. Spreading .Attributes in render-image hook. Care should be taken to prioritize user-specified width/height superseding computed width/height.
  2. Not computing width/height if resource is SVG.

runofthemillgeek avatar Mar 08 '25 13:03 runofthemillgeek

@OleMussmann thanks again for initiating this discussion, of course the code should not be perfect from the get go :)

Yes I agree opening an issue for the compression configuration. Default should be false in my opinion. If either one of you can open it feel free, otherwise I'll open later.

About other points, honestly I don't have knowledge about how browsers load stuff, and how they optimize those loads. I am not sure wether it's better to define priorities or not, perhaps we can leave this out for now.

Regarding the use of .Attributes, what would this change? Sorry I don't know much about those.

tomfran avatar Mar 08 '25 18:03 tomfran

@tomfran Re: .Attributes, this is the feature we were discussing: https://gohugo.io/content-management/markdown-attributes/

So to customize images, we can write markdown as follows:

![](https://example.com/foo.png)
{width=200 height=200 compress=false}

These attributes are available as .Attributes in the render image hook: https://gohugo.io/render-hooks/images/#attributes

runofthemillgeek avatar Mar 08 '25 20:03 runofthemillgeek

Alright, I am highly in favor of adopting those instead of tags, I opened an issue about it. If one of you wants to add something to it, or even start implementation please do.

After we have this new attribute-based logic in place, we can discuss how to proceed from it, potentially adding optional loading priorities, etc.

Please let me know if I missed something.

tomfran avatar Mar 09 '25 10:03 tomfran

@OleMussmann I'm not sure if something changed with Chrome recently but I'm seeing the content shifts you referred to. At first I thought this was just isolated to incognito/guest profiles but can observe in normal profiles too now. Still, I couldn't observe these on FF/Safari.

I believe the reason for these is because of setting both height and width to auto which seems to avoid reserving a suitable box of matching aspect-ratio. If I only do one of height: auto or width: auto, things seem to be fine but then that distorts the image if image is big and either portrait/landscape. My intuition was that setting both to auto should still allow for computing the layout given there's also max-width and max-height but I dunno, something's off there and I can't put a pin on it yet.

For now, this could probably be resolved by keeping one of the auto values and using object-fit but I was hoping that wouldn't be required. Anyway, bit perplexed tbh regarding this.

runofthemillgeek avatar Mar 15 '25 19:03 runofthemillgeek

#102 should fix the layout shifts for sure. I guess along with this, can incorporate a change to remove loading="lazy" and fetchpriority="high" for the first 1-3 images as discussed. I've created an issue for this #103.

runofthemillgeek avatar Mar 16 '25 03:03 runofthemillgeek