Fix shadows for billboarded Sprite3D's

[ecmjohnson]

This resolves #69026 and resolves #41420 in the master branch based on my reading of @clayjohn's suggested approach: the "main" camera transform is piped into the shader and the billboard always faces that transform, even when rendering shadow passes. I tested with both forward and mobile rendering backends.

The main_cam_inv_view_matrix needed is added to struct SceneData since it seemed like "scene data" and there might be a future case for using it elsewhere in the shader. If it would make more sense for a conditional uniform to be used (like for other material features), I can change that.

[ecmjohnson]

Sorry, my mistake; I see the page on code style now and I'll set up my format checker

[clayjohn]

This looks great!

I am unsure about the naming of the new builtin so I would like to discuss this with other rendering contributors as soon as we are back to merging new features/ enhancements. Accordingly I am bumping this to the 4.1 milestone

[ecmjohnson]

@clayjohn I think this is more of a bug than an enhancement since the shadows will be incorrect for billboarded Sprite3D's without this change. Currently the only way to accomplish billboarded Sprite3D's with shadows is by employing a workaround

[AThousandShips]

@ZainWD Please don't bump PRs without contributing significant new information. Use the 👍 reaction button on the first post instead.

[BastiaanOlij]

I don't think this is a good solution. If you're making a 2 player game with side by side screen (yeah people still make those), or if you have additional viewports for other reasons, you'll have two different camera that both show the billboard in a different way, but only for one shadows will be correct.

Billboard is a technique that doesn't work well with shadows. If you're using it for LOD purposes, a billboard is only used when the object is far enough away that a shadow doesn't matter.

If you're using it for something like a name above a player or a healthbar, than it just can't cast a shadow. In this case the object should not be billboarded in the shader but you should orient it in code.

[ecmjohnson]

@BastiaanOlij You're totally correct that this wouldn't make sense in any scenario with multiple cameras in the scene. I'd add to your example stereo rendering, where the shadows would be generated based on billboarding to the e.g. left camera and be slightly incorrect for the other eye. I also agree with you that most uses of billboards should not use shadows and will not benefit from this change, but this use case can simple disable shadows.

However, I disagree that this makes this solution not worth having. As is, a beginner trying to create a single camera sprite game with 3D lighting (a la Octopath Traveler or Cult of the Lamb) will have to rotate the sprites in software themselves. I think this use case should work as intuitively out-of-the-box to avoid hurting a beginner who simply wants their Sprite3Ds to cast shadows. More advanced users making a two-player or stereo game should have the wherewithal to identify the problem when their shadows are weird from one perspective.

[Calinou]

I think this change makes sense, but the class reference must be modified to mention that this approach won't work in multi-viewport or XR scenarios. In this case, the only workaround I know is to render multiple Sprite3Ds with different layers (and changing each viewport's visible layers accordingly).

[ecmjohnson]

@Calinou That would be a separate PR in the docs repo eh? I'll look into updating the Sprite3D documentation to clarify this behaviour

[BastiaanOlij]

In VR we always use quads that are properly positioned and aren't view aligned so there is stability in orientation regardless of the player turning their heads.

[Calinou]

@Calinou That would be a separate PR in the docs repo eh? I'll look into updating the Sprite3D documentation to clarify this behaviour

Class reference changes should be done in doc/Classes/Sprite3D.xml in this PR, not in godot-docs.

That said, a separate PR for the 3D lights and shadows page on godot-docs is welcome.

[ecmjohnson]

Sorry for the delay in such a simple change: it's currently summer in Canada. I changed docs/classes/BaseMaterial3D.xml to include an additional note reading:

			[b]Note:[/b] When billboarding is enabled and the material also casts shadows, billboards will face [b]the[/b] camera in the scene when rendering shadows. In scenes with multiple cameras, the intended shadow cannot be determined and this will result in undefined behaviour. See [url=https://github.com/godotengine/godot/pull/72638]GitHub Pull Request #72638[/url] for details.

I linked this PR since that could help a developer if they wanted to determine for which camera the shadow would be rendered (e.g. always wanting the shadow to be from a specific camera even with multiple cameras in the scene), but in most cases I would expect the billboards enabled and shadows enabled combination of settings to be a mistake when multiple cameras are used (e.g. couch co-op).

[ecmjohnson]

I also added the same note under docs/classes/SpriteBase3D.xml since that seems to be the one that shows in the editor when modifying the Sprite3D instance

[ecmjohnson]

It might be worth refactoring RenderForwardClustered::_render_shadow_pass() and RenderForwardClustered::_render_shadow_append() to take a struct parameter since the number of individual parameters is getting pretty nasty and this is a common point of extensibility for new features or bug fixes. I know this PR makes it one worse 😳 but I've noticed because I've had to manually rebase a couple times now

[ecmjohnson]

I also missed making this fix to the compatibility rendering backend so I've added those now and verified the 3D billboarded sprites shadows on all rendering backends

[clayjohn]

Sorry for taking such a long time to review this. It took awhile to discuss with other contributors and then I kept poking at it and dropping it again.

Overall, I think the implementation looks great. I don't think there is a better way to implement this and I think you have done everything correctly.

[akien-mga]

While fixing the couple spelling issues in the docs, I would suggest rebasing on latest master to make sure it still builds fine after the last few weeks of active development on the rendering architecture.

Edit: Done myself.

[akien-mga]

Thanks!