Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make png to jpg converter (flatten to specified background color) #6298

Closed
ericselin opened this issue Sep 2, 2019 · 12 comments · Fixed by #6439
Labels
Milestone

Comments

@ericselin
Copy link
Contributor

@ericselin ericselin commented Sep 2, 2019

I think this would be a killer feature. Serving .png images is less than ideal compared to .jpg. But keeping the master images as large .png files is a must if you want transparency. Hence something like this would be preferred:

{{ $png := .Resources.GetMatch "image.png" }}
{{ $jpg := $png.toJPG "#FFFFFF" }}

or using filters as in #6255:

{{ $jpg := $png.Filter (images.Flatten "#FFFFFF") }}

I hope you get my point. We have an eCom site and our product images are large .png files with a transparent background. Right now we flatten these in a separate step into .jpg files with two different backgrounds depending on what the background on the underlying element is. But I think this kind of processing could (and should!) be included by default in hugo.

@bep bep added the Enhancement label Sep 3, 2019
@bep bep added this to the v0.59 milestone Sep 3, 2019
@roz3x

This comment has been minimized.

Copy link

@roz3x roz3x commented Sep 5, 2019

hey ! the code base is (very ) huge , can you point the specific file, where this feature could be added directly . like template parser or similar file .

@bep

This comment has been minimized.

Copy link
Member

@bep bep commented Sep 5, 2019

@roz3x we need to agree on the API first. I don't mind adding this "somehow", but we need to consider where/how.

@roz3x

This comment has been minimized.

Copy link

@roz3x roz3x commented Sep 5, 2019

is this something you wanna fix right now , or you want to add in future updates !?

@bep

This comment has been minimized.

Copy link
Member

@bep bep commented Sep 5, 2019

I don't mind having it fixed now, but we need to know how before doing it. $img.ToJPG does not look "right" to me, but this is not something I plan to spend time on right now.

@bep bep modified the milestones: v0.59, v0.60 Sep 6, 2019
@jansorg

This comment has been minimized.

Copy link
Contributor

@jansorg jansorg commented Sep 12, 2019

Speaking for my needs, something like this would feel "okay" to me:

{{ $png := ... }}
{{ $jpg := $png.Convert("jpeg bg=#ffffff") }}
// or: {{  $jpg := $png.Convert("jpeg #ffffff") }}
// or: {{  $jpg := $png.Convert("jpeg", (dict "bg" "#ffffff")) }}
{{ $webp := $png.Convert("webp") }}

My thinking is that the required parameters depend on the target image format. Required parameters have to be passed to Convert. If missing, Convert will raise an error.
For example, creating a JPG from an image with transparent background needs the background color. A solution with a flatten filter might be harder to get because converting a png to jpeg without flattening before will have to assume a default background color.

A generic Convert could be used for all sorts of image formats, i.e. we wouldn't need functions for each format, e.g. $img.ToPng, $img.ToJpeg, $img.ToWebP, $img.ToJpeg2000, etc.

@bep

This comment has been minimized.

Copy link
Member

@bep bep commented Sep 12, 2019

@jansorg thanks for the input. My thoughts on this are:

  • It doesn't fit as a filter (added recently) for several reasons.

That leaves two options:

  1. We may assume that you always want to combine this with some kind of resize ops, e.g:
{{ $jpg := $png.Resize "200x200 jpeg #ffff" }}

The above benefit would also have the benefit in that you could control the background on rotate operations (it's now hardcoded white).

Or we could add a Convert (where I would prefer a dict as a less clever and easier to reason about options map). But I'm not sure about the name, a little bit too similar from the others. If it's only job is to "recode" the image, maybe image.Encode sounds better? So, $png.Encode "jpeg".

Given that these image operations aren't exactly free (we do cache the result, but...), we should maybe consider starting with adding some "target format" options to Resize etc. -- which would avoid som encoding work.

Then, if we really need some special conversion handling we could consider adding a Convert/Encode method later.

@jansorg

This comment has been minimized.

Copy link
Contributor

@jansorg jansorg commented Sep 12, 2019

@bep Thanks for your further input on this.

In my use-case I'm certainly don't want to publish the original image source. I'm always operating on resized data.
Therefore, adding target output format and background to .Resize() sounds like the best alternative to me.
And if I really wanted to change the background without a resize, then I still can do something like $white := $img.Resize (printf "%dx%d jpeg #ffffff" $img.Width $img.Height) (untested).
Additional functions like .Convert could still be added later if all of the above isn't sufficient.

Not sure if this makes sense: the background color may use an alpha channel for some image formats (i.e. those which support transparency). Therefore, additionally supporting rgba values like '#ffffff42' might be useful to cover all supported formats.

(At some point the options string will be hard to handle. A dict might be easier to use as an alternative to the existing string arg.)

@gnalck

This comment has been minimized.

Copy link

@gnalck gnalck commented Sep 18, 2019

This would be useful for our use-case too. Is there any reason not to have the specification of image quality (e.g., "q75") be sufficient for automatic conversion of a png to jpg? That is, as an alternative to specifying jpeg explicitly in the Resize command. Right now the quality indicator is ignored for png, but it seems that once this functionality exists it can now be respected for the png case by automatic conversion to jpg.

That said, the "jpeg" specification in the Resize option would work great!

@jansorg

This comment has been minimized.

Copy link
Contributor

@jansorg jansorg commented Sep 19, 2019

@gnalck Quality is most probably not enough. There's a default quality setting in the config and it's (probably) not intuitive to output a jpeg if it's defined on the call and a png if the default quality is used. And there might be other formats in the future which support a quality setting.

I spend some time and created a wip PR, changes are at #6354.

This allows to do $jpg := $png.Resize "400x jpeg q70" and $jpg2 := $jpg.Resize "200x200", for example.
It's in an early stage, but seems to work well for me locally. It's not yet supporting the flattening background color.

I couldn't find a webp encoder in pure go. https://github.com/chai2010/webp contains a set of C files and https://github.com/nickalie/go-webpbin seems to link to a C library. Not sure if one of them would be okay for Hugo as a dependeny.
There's not Jpeg2000 encoder I could find.

@bep

This comment has been minimized.

Copy link
Member

@bep bep commented Sep 19, 2019

C libraries is not a route we want to go. Or: We do it only if we really, really need the particular library (e.g. libsass). I'm a little surprised that no company have stepped up and created native encoders for stuff like WebP and progressive jpegs.

jansorg added a commit to jansorg/hugo that referenced this issue Sep 19, 2019
… is defined in the operation definition string, then the converted image will be stored in this format.

See gohugoio#6298 for details.
jansorg added a commit to jansorg/hugo that referenced this issue Sep 19, 2019
The image format is defined as the image extension of the known formats.
All of img.Resize "600x jpeg", img.Resize "600x jpg", img.Resize "600x png" are valid
format definitions.
If the target format is defined in the operation definition string, then the converted image will be stored in this format.

See gohugoio#6298
@jansorg

This comment has been minimized.

Copy link
Contributor

@jansorg jansorg commented Sep 19, 2019

I decided not to add support for new formats in my PR. That's for a separate set of changes in the future. https://github.com/chai2010/webp might be usable, as it contains C files, but doesn't add a new dependency to a shared lib.

jansorg added a commit to jansorg/hugo that referenced this issue Sep 20, 2019
The image format is defined as the image extension of the known formats,
excluding the dot.
All of 'img.Resize "600x jpeg"', 'img.Resize "600x jpg"',
and 'img.Resize "600x png"' are valid format definitions.
If the target format is defined in the operation definition string,
then the converted image will be stored in this format. Permalinks and
media type are updated correspondingly.
Unknown image extensions in the operation definition have not effect.

See gohugoio#6298
jansorg added a commit to jansorg/hugo that referenced this issue Sep 21, 2019
The image format is defined as the image extension of the known formats,
excluding the dot.
All of 'img.Resize "600x jpeg"', 'img.Resize "600x jpg"',
and 'img.Resize "600x png"' are valid format definitions.
If the target format is defined in the operation definition string,
then the converted image will be stored in this format. Permalinks and
media type are updated correspondingly.
Unknown image extensions in the operation definition have not effect.

See gohugoio#6298
bep added a commit that referenced this issue Sep 21, 2019
The image format is defined as the image extension of the known formats,
excluding the dot.
All of 'img.Resize "600x jpeg"', 'img.Resize "600x jpg"',
and 'img.Resize "600x png"' are valid format definitions.
If the target format is defined in the operation definition string,
then the converted image will be stored in this format. Permalinks and
media type are updated correspondingly.
Unknown image extensions in the operation definition have not effect.

See #6298
jansorg added a commit to jansorg/hugo that referenced this issue Sep 21, 2019
This adds support to read and write webp images to hugo.
It uses https://github.com/chai2010/webp (BSD-3-clause license)
which isn't adding external dependencies.
webp iamges are decoded using Golang's x/image/webp package to simplify
the implementation, https://godoc.org/golang.org/x/image/webp .
Encoded webp images are stored lossy and with the defined quality
setting (i.e. identical to JPEG). The default quality set for Hugo
is used as a fallback.

See gohugoio#6298
@bep

This comment has been minimized.

Copy link
Member

@bep bep commented Sep 21, 2019

@jansorg we have the converter in place. What we need to close this particular issue is a way to set the background color. Webp is not in the scope of this particular issue. And I would suggest you take any discussions re. webp in another issue.

bep added a commit to bep/hugo that referenced this issue Oct 20, 2019
Closes gohugoio#6298
@bep bep modified the milestones: v0.60, v0.59 Oct 20, 2019
bep added a commit to bep/hugo that referenced this issue Oct 20, 2019
Closes gohugoio#6298
bep added a commit to bep/hugo that referenced this issue Oct 20, 2019
Closes gohugoio#6298
bep added a commit to bep/hugo that referenced this issue Oct 20, 2019
Closes gohugoio#6298
bep added a commit to bep/hugo that referenced this issue Oct 20, 2019
Closes gohugoio#6298
bep added a commit to bep/hugo that referenced this issue Oct 20, 2019
Closes gohugoio#6298
bep added a commit to bep/hugo that referenced this issue Oct 20, 2019
Closes gohugoio#6298
@bep bep closed this in #6439 Oct 20, 2019
bep added a commit that referenced this issue Oct 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.