Skip to content

refactor(flame_3d)!: Allow LightComponents to be nested#3883

Merged
luanpotter merged 4 commits intomainfrom
refactor(flame_3d)!-allow-LightComponents-to-be-nested
Mar 26, 2026
Merged

refactor(flame_3d)!: Allow LightComponents to be nested#3883
luanpotter merged 4 commits intomainfrom
refactor(flame_3d)!-allow-LightComponents-to-be-nested

Conversation

@wolfenrain
Copy link
Copy Markdown
Contributor

@wolfenrain wolfenrain commented Mar 25, 2026

Description

LightComponents can now be nested inside any Component3D allowing for a more flexible light system. On top of that refactored the light system so that every Material decided how lights should be applied.

lights.mp4

Checklist

  • I have followed the Contributor Guide when preparing my PR.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • I have updated/added relevant documentation in docs and added dartdoc comments with ///.
  • I have updated/added relevant examples in examples or docs.

Breaking Change?

  • Yes, this PR is a breaking change.
  • No, this PR is not a breaking change.

Related Issues

…rials decide themselves how lights should be used
@wolfenrain wolfenrain requested a review from luanpotter March 25, 2026 14:22
void update(double dt) {
// NOTE: this ensures the matrix gets computed if need be, so that
// the light position moves with it's anecstors.
worldTransformMatrix;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this feels a bit hacky - why is the same not needed for other nested components? 🤔

Copy link
Copy Markdown
Contributor Author

@wolfenrain wolfenrain Mar 25, 2026

Choose a reason for hiding this comment

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

Because the Object3D is computional but the Light one isn't, worldTransformMatrix is a getter so if it doesn't get... get it doesn't compute and in turn the light inside a nested component would not move

We only compute it when needed, to prevent unnecessary computations and for light we just need a little get call ✨ it's cached if nothing changes so it isn't expensive most of them time

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I guess I was confused as to where this value is actually "read" every tick, but I see it is just sharing memory with the light getter.

..setVector3('Lights.positions[$index]', light.position)
..setColor('Lights.colors[$index]', light.source.color)
..setFloat('Lights.intensities[$index]', light.source.intensity);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

tbh I don't see why not keep these as methods inside each Light subtype, I thought that was more organized (but same behaviour either way)

Copy link
Copy Markdown
Contributor Author

@wolfenrain wolfenrain Mar 25, 2026

Choose a reason for hiding this comment

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

Because that would force users who want to write custom materials to follow the same pattern

Lights are an inherit part of materials so all the logic should be in its respective material and how they act on different lights (including custom ones)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

got it, so we might have different (non-spatial?) materials in the future with different light behaviour and different shader code

Copy link
Copy Markdown
Member

@luanpotter luanpotter left a comment

Choose a reason for hiding this comment

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

Awesome! glad you found a way that doesn't require navigating the comp tree every tick

Comment thread packages/flame_3d/lib/src/components/light_component.dart Outdated
Copy link
Copy Markdown
Member

@luanpotter luanpotter left a comment

Choose a reason for hiding this comment

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

LGTM

@luanpotter luanpotter merged commit fbd9b67 into main Mar 26, 2026
6 checks passed
@luanpotter luanpotter deleted the refactor(flame_3d)!-allow-LightComponents-to-be-nested branch March 26, 2026 00:10
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