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

[JPG] Add ResourceSaverJPG. #60660

Closed
wants to merge 2 commits into from
Closed

Conversation

Faless
Copy link
Collaborator

@Faless Faless commented Apr 30, 2022

This PR adds the possibility to save an ImageTexture as jpg using the library default parameters:

extends Control

func _ready():
	var tex = ImageTexture.new()
	var img = load("res://icon.png").get_image()
	tex.create_from_image(img)
	printt(tex, img, ResourceSaver.save("res://icon.jpg", tex))

Or saving an Image via the exposed ResourceSaverJPG optionally setting custom encoding parameters:

extends Control

func _ready():
	var img = load("res://icon.png").get_image()
	var saver = ResourceSaverJPG.new()
	saver.set_encode_options({
		"quality": 90,
		"two_pass_flag": true,
		"subsampling": ResourceSaverJPG.SUBSAPLING_H2V1,
		"use_std_tables": true,
	})
	printt(img, saver.save_image("res://icon.jpeg", img))

I'm opening this PR since it has been discussed in the meetings, and I'd like to leave a trace of this work, but I think we should change our load_{format}_from_buffer/save_{format}[_to_buffer] to something like:

  • load_from_buffer(const StringName &p_format, p_buffer).
  • save_image(const StringName &p_format, const String &p_path, Dictionary p_options) (where p_options is format specific).
  • save_image_buffer(const StringName &p_format, Dictionary p_options).

And add an ImageSaver similar to ImageLoader (i.e. ImageSaver::add_image_format_saver/remove/etc)

Note that format is a StringName and not an enum, because I think that way GDExtensions can interact better with it, and I believe this could be beneficial for other similar cases (godotengine/godot-proposals#2461, godotengine/godot-proposals#3726, godotengine/godot-proposals#676)

This work has been originally sponsored by Maffle LLC, and there is interest in contributing similar work as GDExtension for AVIF encoding/decoding (i.e. godotengine/godot-proposals#2461).

Uses the default values of jpeg-compressor (same library we use for
decompressing):

Quality: 85
Chroma subsampling: H2V2
Single pass.
Uses mozjpeg's default quantization tables instead of JPEG Annex K.
Comment on lines +40 to +44
BIND_ENUM_CONSTANT(SUBSAPLING_Y_ONLY);
BIND_ENUM_CONSTANT(SUBSAPLING_H1V1);
BIND_ENUM_CONSTANT(SUBSAPLING_H2V1);
BIND_ENUM_CONSTANT(SUBSAPLING_H2V2);
BIND_ENUM_CONSTANT(SUBSAPLING_MAX);
Copy link
Member

@Calinou Calinou Apr 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: it should be "SUBSAMPLING" 🙂

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

subsapling.jpg

Y (grayscale) only.
</constant>
<constant name="SUBSAPLING_H1V1" value="1" enum="SubSamplingFactor">
YCbCr, no subsampling (H1V1, YCbCr 1x1x1, 3 blocks per MCU).
Copy link
Member

@Calinou Calinou Apr 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the terms 4:4:4/4:2:2/4:2:0 often used in the audiovisual community, so I suggest referring them here:

Suggested change
YCbCr, no subsampling (H1V1, YCbCr 1x1x1, 3 blocks per MCU).
YCbCr, no subsampling (H1V1, YCbCr 1x1x1, 3 blocks per MCU). Also called 4:4:4 chroma subsampling.

YCbCr, no subsampling (H1V1, YCbCr 1x1x1, 3 blocks per MCU).
</constant>
<constant name="SUBSAPLING_H2V1" value="2" enum="SubSamplingFactor">
YCbCr, H2V1 subsampling (YCbCr 2x1x1, 4 blocks per MCU).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
YCbCr, H2V1 subsampling (YCbCr 2x1x1, 4 blocks per MCU).
YCbCr, H2V1 subsampling (YCbCr 2x1x1, 4 blocks per MCU). Also called 4:2:2 chroma subsampling.

YCbCr, H2V1 subsampling (YCbCr 2x1x1, 4 blocks per MCU).
</constant>
<constant name="SUBSAPLING_H2V2" value="3" enum="SubSamplingFactor">
YCbCr, H2V2 subsampling (YCbCr 4x1x1, 6 blocks per MCU. Very common.
Copy link
Member

@Calinou Calinou Apr 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
YCbCr, H2V2 subsampling (YCbCr 4x1x1, 6 blocks per MCU. Very common.
YCbCr, H2V2 subsampling (YCbCr 4x1x1, 6 blocks per MCU. Also called 4:2:0 chroma subsampling. This is a common option, as it provides significant size reduction with little quality loss for photos. However, this option is not recommended for drawings as it negatively impacts quality in a noticeable way.

@Calinou
Copy link
Member

Calinou commented Apr 30, 2022

It might be worth opening a proposal to detail some use cases for saving JPEGs from Godot.

One use case that might come to mind is screenshots, but using PNG for screenshots is heavily favored nowadays thanks to its lossless nature. The downside is that saving PNG screenshots can take a while, making the game freeze temporarily while the screenshot is taken. This can be mostly resolved by moving PNG saving to a thread though. In contrast, saving a JPEG is much faster, but it will never be a lossless format (even at quality 100).

@fire
Copy link
Member

fire commented May 1, 2022

I also wish to have openexr saving, but openexr was removed from the game templates by Calinou. It was used by the material maker and others.

@Calinou
Copy link
Member

Calinou commented May 1, 2022

but openexr was removed from the game templates by Calinou

I didn't remove OpenEXR from export templates, as it was never present there in the first place. In fact, I was considering adding it in #42947, but I decided against it due to the added binary size.

@fire
Copy link
Member

fire commented Jun 1, 2022

Do we have a proposal for the generic image i/o apis?

@Faless
Copy link
Collaborator Author

Faless commented Jun 2, 2022

Do we have a proposal for the generic image i/o apis?

AFAIK no, it needs to be written.

@akien-mga
Copy link
Member

Superseded by #62122.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants