support additional source for webp for cover/content images

[provos]

Best practices for web images is to organize them inside a picture tag and provide web optimized image formats.

Since Hugo now natively supports webp, this change wraps an image into a picture tag and provides an additional set of webp sources. The gallery css is moved from the img tag to the picture tag which required a small change to the gallery typescript.

You can check an example page that uses this at: https://www.provos.org/p/finetuning-stable-diffusion-part2/

Ps: This does not check whether the original image is webp already

[CaiJimmy]

Thank you so much for submitting your pull request!

Sorry for the late reply. Unfortunately, I'm still tied up with academic tasks and won't be able to review pull requests for the time being.

Once I have more time, I'll take a look at your pull request and provide feedback.

Thank you again for your contributions!

[CaiJimmy]

Thanks for the pull request and sorry for the late response!

This is definitely a nice thing. But with this PR, each image is resized 4 times, resulting in 5 images (including the original).

I think we could just include srcset for the WebP format, since it's widely supported, and fall back to the original image in the worst case. (So only 2 resizes will be done, as it was)

What do you think?

---

And here's my render-image.html after making such change:

{{- $image := .Page.Resources.GetMatch (printf "%s" (.Destination | safeURL)) -}}
{{- $Permalink := .Destination | relURL | safeURL -}}
{{- $alt := .PlainText | safeHTML -}}
{{- $Width := 0 -}}
{{- $Height := 0 -}}
{{- $Srcset := "" -}}

{{/* SVG and external images won't work with gallery layout, because their width and height attributes are unknown */}}
{{- $galleryImage := false -}}

{{- if $image -}}
	{{- $notSVG := ne (path.Ext .Destination) ".svg" -}}
	{{- $Permalink = $image.RelPermalink -}}

	{{- if $notSVG -}}
		{{- $Width = $image.Width -}}
		{{- $Height = $image.Height -}}
		{{- $galleryImage = true -}}

		{{- if (default true .Page.Site.Params.imageProcessing.content.enabled) -}}
			{{- $small := $image.Resize `webp 480x` -}}
			{{- $big := $image.Resize `webp 1024x` -}}
			{{- $Srcset = printf `%s 480w, %s 1024w` $small.RelPermalink $big.RelPermalink -}}
		{{- end -}}
	{{- end -}}
{{- end -}}

<picture
	{{ if $galleryImage }}
		class="gallery-image" 
		data-flex-grow="{{ div (mul $image.Width 100) $image.Height }}"
		data-flex-basis="{{ div (mul $image.Width 240) $image.Height }}px"
	{{ end }}
>
	{{ with $Srcset }}
		<source type="image/webp"
			srcset="{{ $Srcset }}"
		>
	{{ end }}
	<img src="{{ $Permalink }}"
		{{ with $Width }}width="{{ . }}"{{ end }}
		{{ with $Height }}height="{{ . }}"{{ end }}
		loading="lazy"
		decoding="async"
		{{ with $alt }}
			alt="{{ . }}"
		{{ end }}
	>
</picture>

[CaiJimmy]

And the additional reason is that if a browser does not support WebP, it's highly probable that it does not support srcset either.

[provos]

I agree with you. When I originally implemented this I was worried about backwards compatibility. I believe almost every browser supports webp now and the additional complication is not needed. Niels.

…

On Sat, Sep 2, 2023 at 3:00 PM Jimmy Cai ***@***.***> wrote: And the additional reason is that if a browser does not support WebP, it's highly probable that it does not support srcset either. [image: image] <https://user-images.githubusercontent.com/5889006/265232919-2a4962db-3cd7-42b8-a6b3-e9455fe7a670.png> — Reply to this email directly, view it on GitHub <#766 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABHR7UCCRURNAZVPRIJLCLXYOUBPANCNFSM6AAAAAAT4GKFFM> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[ottok]

Can you @provos please rebase this on latest upstream git head? I could test it on my website to see how well it works with various browsers etc, and thus provide additional evidence that this should be merged.

PageSpeed nags on my site that webp would be better for all my images (now jpg/png) and also that I should use webm instead of animated gifs. Additionally PageSpeed does not like that all images have loading="lazy", even the ones above the fold, I this PR would also change that to decoding=async which may be better with modern browsers.

[ottok]

I tried testing this but mainline has diverged so much that the patch does not apply directly, and my attempts to apply it manually failed on parse failed: template: _default/_markup/render-image.html:71: unexpected EOF and I didn't figure out which of the conditionals has a mismatch..

[ottok]

@CaiJimmy May I ask why you merged this on master? The point of a PR is to propose that a new branch is merged into master. Merging that proposed branch on master inside the PR seems to go against that idea. Perhaps you intended to rebase?

The PR is based on code from commit 4272da7 from December 2022. If it was rebased, the history would be linear. Now the history is non-linear, and the gif below shows what non-linearity means:

[CaiJimmy]

@CaiJimmy May I ask why you merged this on master? The point of a PR is to propose that a new branch is merged into master. Merging that proposed branch on master inside the PR seems to go against that idea. Perhaps you intended to rebase?

The PR is based on code from commit 4272da7 from December 2022. If it was rebased, the history would be linear. Now the history is non-linear, and the gif below shows what non-linearity means:

I did this because GitHub says "This branch is out of date with the base branch", so I clicked the button to update the branch to the latest version of the base branch. Is this a bad practice?

[CaiJimmy]

also that I should use webm instead of animated gifs

@ottok I believe this is not currently supported by Hugo (gohugoio/hugo#10030). You'll have to do the transformation manually with another tool.

[provos]

Thanks for working on this. I have unfortunately been too busy to help here.

[ottok]

@CaiJimmy May I ask why you merged this on master? The point of a PR is to propose that a new branch is merged into master. Merging that proposed branch on master inside the PR seems to go against that idea. Perhaps you intended to rebase?

I did this because GitHub says "This branch is out of date with the base branch", so I clicked the button to update the branch to the latest version of the base branch. Is this a bad practice?

Does the button offer to do a rebase instead of a merge? How is your project configured? You know the concepts of linear history ("git fast-forward") and merge vs rebase, right?

[ottok]

Thanks @provos for responding. This idea seems cool, however I can't get it working neither using the original version or any successive version, and there are too many conflicting changes so I can't get it working even when manually trying to apply the patch.

Could you rebase this on latest upstream 'master' branch and then as for re-review and testing? Thanks