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

Lossy WebP: Enable sharp RGB to YUV conversion #79257

Merged
merged 1 commit into from Jul 25, 2023

Conversation

DeeJayLSP
Copy link
Contributor

@DeeJayLSP DeeJayLSP commented Jul 9, 2023

Drastically increases lossy WebP quality by performing a sharp RGB to YUV conversion. This has minimal impact on size and some additional time to encode.

This article explains well how this works.

Here's a preview linked from it:
Sharp example

I wanted to make a new check option on the import tab to enable/disable. There is a High Quality import option that could be used, but it's designed with changing VRAM compressed formats in mind and can't be passed to WebPCommon. Both alternatives can't be done without making major changes to the ResourceSaver that will only benefit lossy WebP.

In the end, I decided that leaving it enabled was better, as the size increase is minimal.

@DeeJayLSP DeeJayLSP requested a review from a team as a code owner July 9, 2023 21:21
@clayjohn
Copy link
Member

clayjohn commented Jul 9, 2023

Remark from the linked blog:

In my tests, encoding with the Sharp YUV option added took on average 68 % more wall-clock time per image.

Perhaps we should do an analysis of current import times compared to import times with this change?

@DeeJayLSP
Copy link
Contributor Author

DeeJayLSP commented Jul 15, 2023

I did a test by importing 64 sample images from Kenney Game Assets 1 at once on two builds (because I don't trust cwebp), one of them with sharp_yuv enabled, and the other one disabled. To make import longer, I set WebP compression method to max (6).

If there was a difference between import times, it was a matter of miliseconds (both did around 3 seconds). The similar times are likely due to compression method, not conversion.

With this in mind, I would say the time increase is actually negligible.

@DeeJayLSP
Copy link
Contributor Author

DeeJayLSP commented Jul 17, 2023

@clayjohn

Remark from the linked blog:

In my tests, encoding with the Sharp YUV option added took on average 68 % more wall-clock time per image.

Perhaps we should do an analysis of current import times compared to import times with this change?

On the very same paragraph:

While this may sound like a lot, keep in mind that WebP image-encoding is already incredibly fast. In my opinion, the quality gains are well worth the one-time extra time expenditure.

Lossy WebP encoding is already extremely fast compared to lossless (on the same test, lossless took 36 seconds to import).

My above comment has some test details if anyone is interested.

@Calinou
Copy link
Member

Calinou commented Jul 21, 2023

I tested this PR locally, rebased against master f8dbed4:

Benchmark

OS: Fedora 38
CPU: Intel Core i9-13900K
Corpus size: 3,900 images (1.1 GB of source files)
Lossy quality: 0.7 (default)

.godot/ was removed from the project folder between each run. Import times were measured using a stopwatch by starting at the moment the progress bar appears, then stopped when it disappears.

Build type: optimize=none

Before After (this PR)
6 seconds 13 seconds

Build type: optimize=speed lto=full

This is what official builds use.

Before After (this PR)
4 seconds 7 seconds

3,900 images with lossy compression is a lot more than what most projects will use, so I think it's fine.

Copy link
Contributor

@lyuma lyuma left a comment

Choose a reason for hiding this comment

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

I agree with the assessment that this improvement in quality is worthwhile given Calinou's times. Seems like a win to me.

The only possible downside I can think is someone perhaps compressing lossy webp at runtime, might want to have an option for fast low-quality compression, but I would rather wait for the usecase to present itself, since I can't think of a case where fast-but-low quality would be desirable

Copy link
Member

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

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

I think this is a great improvement.

In the future if performance is a problem we could try to make this multi-threaded.

@YuriSizov YuriSizov merged commit 8dc1931 into godotengine:master Jul 25, 2023
13 checks passed
@YuriSizov
Copy link
Contributor

Thanks!

@DeeJayLSP DeeJayLSP deleted the sharpp branch July 25, 2023 22:20
@phantomdesvin
Copy link

Thank you!
A user commented this on reddit and I would also like to understand:

I tried importing PNG's as texture2D's on lossy mode at 0.8 but I'm not noticing a difference.

Does this improvement apply automatically or do I have to activate any setting?

Does this apply when we import PNG's (because those are compressed to WebP, right?) or does it only apply when we import WebP directly?

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.

None yet

8 participants