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

Decals using ViewportTexture are not updated #73400

Open
maiself opened this issue Feb 15, 2023 · 18 comments
Open

Decals using ViewportTexture are not updated #73400

maiself opened this issue Feb 15, 2023 · 18 comments

Comments

@maiself
Copy link
Contributor

maiself commented Feb 15, 2023

Godot version

aa6ec76

System information

Arch Linux, Mesa Intel® UHD Graphics (CML GT2), Vulkan Forward+

Issue description

Decals using ViewportTexture are not updated when the viewport texture is updated.

Workarounds are possible, clearing Decal.texture_* properties and resetting causes an update. Some updates must be preformed twice a frame apart to have effect.

The workaround code can be found in character_ring_decal/character_ring_decal.gd file of the MRP.

The following error is also reported, but its unclear why:

Node not found: "SubViewport" (relative to "CharacterRingDecal").
ViewportTexture: Path to node is invalid.

Steps to reproduce

  • run the MRP, observe decal is not visible / updating
  • on the CharacterRingDecal node, enable the Enable Update Workaround property
  • rerun the project, and see that the decal updates

Minimal reproduction project

viewport-texture-decal-issue.zip

@clayjohn
Copy link
Member

I'm not sure if this is something that we should support. Decals are added to a decal atlas so that you can have multiple decals per surface. That means when you create a decal you have to copy the texture over onto the texture atlas (potentially resizing the atlas in the process).

Since ViewportTextures can update every frame, you would potentially have to do that copy every single frame. I think it should be possible, but I am not sure it is something that we should support.

We have the same issue when using ViewportTextures as projector texture or with PointLight2Ds

@maiself
Copy link
Contributor Author

maiself commented Feb 17, 2023

I'm aware of the decal atlas and the potential pitfalls of updating it frequently. In my project I'm intentionally avoiding updating the decals whenever possible by using SubViewport.UPDATE_ONCE, caching the textures for reuse, and quantizing the shader parameters to reduce cache misses. Perhaps this is a more advanced usage.

The need for workarounds to force updates leaves things feeling very unrefined.

Maybe this could be properly supported, with clear warnings about performance pitfalls?

Or document as undefined behavior?

Or disallow ViewportTexture and Decal use, and maybe document some path for creating decals from a one off dynamic source.

@maiself maiself closed this as completed Feb 17, 2023
@maiself maiself reopened this Feb 17, 2023
@maiself
Copy link
Contributor Author

maiself commented Feb 17, 2023

On mobile, accidently hit close with comment while writing...

@betalars
Copy link
Contributor

betalars commented Mar 4, 2023

I'm not sure if this is something that we should support.

This is not Apple, this is open Source. Please: let people break stuff.

You can already disable updates on view-ports. I can even see a configuration warning being used, when a seemingly static viewport seems to be set to update when assigned to a ViewportTexture.

But 90% of the use cases I can imagine for assigning a viewport texture to a projection require the projection to be dynamic.

But right now it is clearly a bug. And I've just wasted 2 hours on figuring this out.

@QbieShay
Copy link
Contributor

+1 to this. I will eventually need a way to animate decals in VFX. Maybe we can have mesh decals for these usecases.

@Calinou
Copy link
Member

Calinou commented Apr 22, 2023

+1 to this. I will eventually need a way to animate decals in VFX. Maybe we can have mesh decals for these usecases.

There's a proposal for mesh decals, but it's unlikely to be implemented given the complexity (and given how it duplicates an existing system). Nothing prevents you from using quads (or procedurally generated meshes) closely attached to a surface, still.

@CarpenterBlue
Copy link

CarpenterBlue commented Jun 10, 2023

I am not sure #77585 should have been closed.
It wasn't about making the decals work but making the un-supported textures unavailable to the decals as they are not planned feature. Maybe I could edit the original Issue to make that more clear as it still persists.

@Calinou
Copy link
Member

Calinou commented Jun 10, 2023

We could change the property hint type to list all the underlying Texture2D classes that are supported, but I don't remember if it's possible to list multiple object types to choose from (so we can avoid the unsupported types).

@AThousandShips
Copy link
Member

It should be possible, GPUParticles2D makes use of this

@Calinou Calinou changed the title Decals using ViewportTexture not updated Decals or light projectors using ViewportTexture not updated Sep 1, 2023
@Bimbam360
Copy link

Bimbam360 commented Sep 2, 2023

Chipping in, is there any reason we cannot have a toggle in editor (at least temporarily) for the light node of 'Static'/'Dynamic' with subsequent performance warning in popup text? I think the assumed behaviour of most users would be that SubViewport.UPDATE_ALWAYS should do this anyway surely.

This would at least expose the option to users while a better solution re. performance be found?

@Calinou
Copy link
Member

Calinou commented Sep 2, 2023

Chipping in, is there any reason we cannot have a toggle in editor (at least temporarily) for the light node of 'Static'/'Dynamic' with subsequent performance warning in popup text? I think the assumed behaviour of most users would be that SubViewport.UPDATE_ALWAYS should do this anyway surely.

If we allow using ViewportTexture, I don't think this needs to be an option within Light3D or Decal. We should just document the performance caveats of using a ViewportTexture that uses the Always update mode with decals or light projectors.

@clayjohn
Copy link
Member

clayjohn commented Sep 2, 2023

Chipping in, is there any reason we cannot have a toggle in editor (at least temporarily) for the light node of 'Static'/'Dynamic' with subsequent performance warning in popup text? I think the assumed behaviour of most users would be that SubViewport.UPDATE_ALWAYS should do this anyway surely.

If we allow using ViewportTexture, I don't think this needs to be an option within Light3D or Decal. We should just document the performance caveats of using a ViewportTexture that uses the Always update mode with decals or light projectors.

I think a setting is still needed as the renderer needs to know to update that texture every frame. Right now the current workaround method may cause the decal atlas to shuffle everything every frame. But what we want is for the decal atlas to have a list of textures that it blits from each frame.

@QbieShay
Copy link
Contributor

QbieShay commented Sep 4, 2023

I'm thinking that rather than supporting viewport textures, we should perhaps think of more ad-hoc solutions for this. For example there can be in the rendering a special decal pass that outputs to the relevant part of the screen for the decal for all the buffers that decal support, and the shader is assigned directly to the decal node. This way Godot keeps control on how the atlases are done under the hood (which most likely will create a more optimized sytem). Thoughts?

@betalars
Copy link
Contributor

betalars commented Sep 5, 2023

If we allow using ViewportTexture, I don't think this needs to be an option within Light3D or Decal. We should just document the performance caveats of using a ViewportTexture that uses the Always update mode with decals or light projectors.

But then explain to me ... if I am not allowed to use a dynamic texture, how am I supposed to do a projector using the projector property?

I mean projectors do exist in some games and I would assume that a projector property would allow me to implement a projector.

@clayjohn
Copy link
Member

clayjohn commented Sep 5, 2023

@betalars I think you misunderstand what Calinou is saying. Calinou is saying that there shouldn't be a setting on the Light/Decal to enable dynamic updates, it should just work automatically.

@betalars
Copy link
Contributor

betalars commented Sep 5, 2023

@betalars I think you misunderstand what Calinou is saying.

hm okay I see your point, sorry about that ...

@golddotasksquestions
Copy link

golddotasksquestions commented Oct 4, 2023

I just tried to add a ViewportTexture as SpotLight3D light_projector in Godot 4.2 dev6 and everything seemed to work well in the editor. When running the scene it looked like there was no light_projector texture assigned at all.

This very much feels like a bug to me. I think if it's possible in the Editor viewport, it should be possible when running the scene as well. If there are limitations (like light_projector can't be updated), then they need to be documented. But having this divergence between editor and running scene seems like the worst of all options to me.

For the record: I very much would like to use ViewportTextures as textures for both Decals and SpotLights.

@so1975
Copy link

so1975 commented Oct 27, 2024

Minimal reproduction project

viewport-texture-decal-issue.zip

I tested this(4.3), and there was no need to await (in this case). If textures are set null and refreshed %SubViewport.get_texture(). But I see the point, texture is not updated if not resetted.

func _update_inner():
	max_health = max(max_health, 0.0)
	health = clamp(health, 0.0, max_health)
	if not is_inside_tree():
		return
	%Decal.texture_emission = null
	%Decal.texture_albedo = null
	%Decal.size = Vector3(radius * 1.75, 0.6, radius * 1.75) * 2.0

	%RingMesh.material_override.set_shader_parameter(&'max_health', max_health)
	%RingMesh.material_override.set_shader_parameter(&'health', health)
	%RingMesh.material_override.set_shader_parameter(&'radius', radius)
	
	%Decal.texture_albedo = %SubViewport.get_texture()
	%Decal.texture_emission = %SubViewport.get_texture()
	%SubViewport.render_target_update_mode = SubViewport.UPDATE_ONCE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants