Skip to content

Add missing vpv.push_back(pv); in render_target_get_sdf_texture. #107763

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Ivorforce
Copy link
Member

@Ivorforce Ivorforce commented Jun 20, 2025

From the context, this looks like it was intended.
The alternative would be not creating pv or vpv, instead defaulting to empty.

I'm not sure what the difference will be; at the very least it should now check for the size requirements.

Code came from #59984

@Ivorforce Ivorforce added this to the 4.x milestone Jun 20, 2025
@Ivorforce Ivorforce requested a review from BastiaanOlij June 20, 2025 12:47
@Ivorforce Ivorforce requested a review from a team as a code owner June 20, 2025 12:47
@@ -3889,7 +3889,7 @@ RID TextureStorage::render_target_get_sdf_texture(RID p_render_target) {
pv.resize(16 * 4);
memset(pv.ptrw(), 0, 16 * 4);
Vector<Vector<uint8_t>> vpv;

vpv.push_back(pv);
Copy link
Member

Choose a reason for hiding this comment

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

Can't this just be replaced with:

Vector<Vector<uint8_t>> vpv = { pv };

Copy link
Member Author

Choose a reason for hiding this comment

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

Also possible, I just copied the style of all the other vpv parts in this file to stay agnostic.

Copy link
Member Author

Choose a reason for hiding this comment

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

(Also, initializer_list makes an unnecessary Vector copy, see godotengine/godot-proposals#12247)

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.

2 participants