Skip to content

Fix broken lightdata iterator in shaders#366

Merged
robertosfield merged 3 commits into
vsg-dev:masterfrom
ArepMM:fix-broken-lightdata-iterator-in-shaders
Jul 27, 2025
Merged

Fix broken lightdata iterator in shaders#366
robertosfield merged 3 commits into
vsg-dev:masterfrom
ArepMM:fix-broken-lightdata-iterator-in-shaders

Conversation

@ArepMM

@ArepMM ArepMM commented Jul 26, 2025

Copy link
Copy Markdown
Contributor

Default shaders now have two brightness check to near-zero value for Directional or Spot Lights. First check is before than all increments of lightdata iterator are done, so each Light with zero intensity brake all next lights in buffer. In this PR I simple remove this early check from shaders. May be it is better to rewrite vsg::ViewDependentState::traverse() to don't add zero-brightness lights to buffer

@robertosfield robertosfield merged commit 0b4f233 into vsg-dev:master Jul 27, 2025
@robertosfield

Copy link
Copy Markdown
Collaborator

Thanks for spotting the issue. I have merged and then updated the built-in ShaderSets within the VSG itself:

vsg-dev/VulkanSceneGraph@d3c67c1

I thought I had put in a check for intensity to avoid assigning lights that are off to the ViewDependentState's lightData but I have just done a code review and haven't spotted one. I think the right way to tackle that wold be to add a check to the RecordtTraversal::apply(*Light&) implementations so that the lights that are off aren't added at all.

I will have a think about the required mods to RecordTraversal.

@ArepMM

ArepMM commented Jul 27, 2025

Copy link
Copy Markdown
Contributor Author

Thanks.

@robertosfield

Copy link
Copy Markdown
Collaborator

Do you have a scene that tests out the lights with low intensity?

@ArepMM

ArepMM commented Jul 27, 2025

Copy link
Copy Markdown
Contributor Author

In our railway game we made GUI that change common ambient and directional lights. Now we are working to add headlight for our rolling stock, by add spotlights to its .gltf with KHR_lights_punctual. And we found that every off spotlight broke all next spots, as well turning to zero directional broke all spots. Now it works correct.
Not sure that we may suggest something for creating tests in vsgExamples.
изображение

@robertosfield

Copy link
Copy Markdown
Collaborator

I have added a RecordTraversal::intensityMinimum control for culling lights that are switched off via the intensity.

vsg-dev/VulkanSceneGraph@adb8770

Could you test VSG master?

@ArepMM

ArepMM commented Jul 27, 2025

Copy link
Copy Markdown
Contributor Author

Test with new master and old shader with early check. Yes, it works too. I think RecordTraversal::intensityMinimum by default should be the same 0.001f as default brightnessCutoff value in shaders, not 0.00001

@robertosfield

Copy link
Copy Markdown
Collaborator

As well as default values it make be worth changing the naming to make it more consistent.

May also be worth passing the value to the shader, though this involves extra complexity. For now I'll stick with simplicity.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants