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

Fix missing ImageTexture::create_from_image call in GPUParticles3DEditor::_generate_emission_points #46671

Conversation

W4RH4WK
Copy link
Contributor

@W4RH4WK W4RH4WK commented Mar 4, 2021

From my understand, the texture for emission particles is create in this function. Additionally, there is the option to also generate a normal map, enabling the use of direct emission points.

It is likely that the create_from_image call got lost during some refactoring as mentioned in the related issue. While I have not tested this feature, it does not look like dead code to me, and the difference might be hard to notice for very bright, small particles. The code looks more correct to me with this patch.


Images for emission point texture and normals were created, but not
transferred to textures.

fix #43643

…tor::_generate_emission_points

Images for emission point texture and normals were created, but not
transferred to textures.

fix godotengine#43643
@W4RH4WK W4RH4WK requested a review from a team as a code owner March 4, 2021 18:24
@clayjohn
Copy link
Member

clayjohn commented Mar 4, 2021

Can you please test it to make sure that you are getting correct results with this change?

It looks like in the issue you linked no one confirmed that the current behaviour was broken. So ideally you need to create a test case that shows that:

  1. generate_emission_points is currently broken and
  2. This PR fixes it.

@Calinou Calinou added this to the 4.0 milestone Mar 4, 2021
@W4RH4WK
Copy link
Contributor Author

W4RH4WK commented Mar 4, 2021

I don't know how this feature is meant to be used, but I put together a short video showing a difference. When you compare the two versions, you can also see in the inspector that the generated images remain empty when using current master.

https://www.youtube.com/watch?v=0Q9YM81kHMU

@Calinou
Copy link
Member

Calinou commented Jul 20, 2022

Superseded by #60739. Thanks for the contribution nonetheless!

@Calinou Calinou closed this Jul 20, 2022
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.

Unused variable in gpu_particles_3d_editor_plugin.cpp
3 participants