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

Optimize ImageLoaderSVG::create_image_from_utf8_buffer #98329

Conversation

beru
Copy link
Contributor

@beru beru commented Oct 19, 2024

This PR optimizes ImageLoaderSVG::create_image_from_utf8_buffer method.

I built the engine with the following command.

python -m SCons platform=windows target=editor debug_symbols=yes

And profiled CPU usage with Microsoft Visual Studio's Perfromance Profiler by launching godot.windows.editor.x86_64.exe file. A little bif of performance improvement can be confirmed.

(Tested with AMD Ryzen 9 7900X processor)

Checked version ImageLoaderSVG::create_image_from_utf8_buffer CPU usage (%)
Before (44fa552) 2.59
After 1.43

@beru beru requested a review from a team as a code owner October 19, 2024 04:32
@beru beru force-pushed the opt_ImageLoaderSVG__create_image_from_utf8_buffer branch from 19db14b to 9522661 Compare October 19, 2024 04:53

Verified

This commit was created on github.com and signed with GitHub’s verified signature.
@beru beru force-pushed the opt_ImageLoaderSVG__create_image_from_utf8_buffer branch from 9522661 to 4dad9c6 Compare October 19, 2024 05:09
Copy link
Member

@fire fire left a comment

Choose a reason for hiding this comment

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

That's neat and the ABGR8888S from ARGB8888S trick is pretty cool so it can directly render from lottie thorvg.

Only did an online review. Did not test.

Tested: The code is clean. Did not profile.

image

The colours are not scrambled which means it passes.

@fire fire added the topic:2d label Oct 19, 2024
@fire fire requested a review from a team October 19, 2024 06:40
@AThousandShips AThousandShips added this to the 4.4 milestone Oct 19, 2024
Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

Looks good to me. I looked at this pretty carefully as I was surprised that such a simple change could be so beneficial.

Testing with a debug build and the icon.svg at 6x scale I was able to consistently reproduce a 30% overall improvement in re-import time(100 ms -> 70ms)

@Repiteo Repiteo merged commit 0953d48 into godotengine:master Oct 24, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Oct 24, 2024

Thanks, and congratulations on your first contribution! 🎉

@beru beru deleted the opt_ImageLoaderSVG__create_image_from_utf8_buffer branch October 24, 2024 20:29
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

5 participants