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

etcpak: Improve image padding and clean up the code #89567

Merged
merged 1 commit into from
Aug 16, 2024

Conversation

BlueCube3310
Copy link
Contributor

@BlueCube3310 BlueCube3310 commented Mar 16, 2024

Changes the etcpak implementation's code to avoid resizing images whose resolutions aren't multiples of 4, and to only pad them instead. This improves import performance since the image only has to be adjusted once, and not resized.

Also cleans up the code.

@AThousandShips AThousandShips added this to the 4.x milestone Mar 16, 2024
@AThousandShips AThousandShips requested a review from a team March 16, 2024 13:14
@BlueCube3310 BlueCube3310 force-pushed the etcpak-cleanup branch 2 times, most recently from 3e80426 to 835d9a0 Compare May 3, 2024 08:45
@BlueCube3310 BlueCube3310 changed the title etcpak: Avoid resizing images and clean up the code etcpak: Improve image padding and clean up the code May 21, 2024
@fire fire requested a review from lyuma June 18, 2024 14:52
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally, it works as expected.

This also makes textures sharper as they no longer need to be resized by a very slight amount (which affected sharpness negatively).

Testing project: test_pr_89567.zip

Before After
Before After

Benchmark

Using an optimized editor build (optimize=speed lto=full). Import times are measured with --verbose CLI output.

When importing a 4K texture set such as this one, this is the time taken to import 6 textures in 4095×4095 size with VRAM compression:

Before After (this PR)
5.31s 2.53s (2.1× faster)

With this PR, the import speed of non-multiple of 4 textures is now pretty much identical to the import time of the 4096×4096 textures (which have a size that's a multiple of 4).

PC specifications
  • CPU: Intel Core i9-13900K
  • GPU: NVIDIA GeForce RTX 4090
  • RAM: 64 GB (2×32 GB DDR5-5800 C30)
  • SSD: Solidigm P44 Pro 2 TB
  • OS: Linux (Fedora 39)

@akien-mga akien-mga modified the milestones: 4.x, 4.4 Jun 19, 2024
@fire fire requested a review from a team July 26, 2024 16:24
@akien-mga akien-mga merged commit 86df981 into godotengine:master Aug 16, 2024
18 checks passed
@akien-mga
Copy link
Member

Thanks!

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