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

feat: Implement basic light abstraction [flame_3d] #3182

Merged

Conversation

luanpotter
Copy link
Member

@luanpotter luanpotter commented Jun 4, 2024

Description

Implement a basic light abstraction.

Still only supports one light for now, and no parameters other than its position.
Note: I will fix the 0-lights case when I add support for multiple lights.

A few complications arose that led me to revamp our component system a bit. Basically splitting the Component3D class into a more fundamental version of itself that only includes the 3D positioning stuff, and none of the rendering stuff, and what used to be Component3D becomes Object3D (I am not sold on the names, feel free to suggest replacements).

I also added a Group3D class to allow for people to create groups of components without carrying the rendering code. Of course you can still add children to Object3D as well. I am open to removing it and just making Component3D concrete instead.

The image below explains the new structure:

image


The difference between the 3 "light" classes:

  • LightComponent → interacts with FCS. is Component3D, thus carries the position
  • LightSource → describes the light in abstract terms (minus position). used both by LightComponent and Light
  • Light → base description of a Light as a Resource. FCS agnostic. includes properties (source) + position

Again, I am open to any renames.


FAQ

why separate component3d and object 3d?

not really needed but I didn't want light component to "carry" the rendering code that is not used. that also makes the distinction more clear as objects are completely different than lights. that way we don't have to have a definition of "non-light objects/components"

why do we need group3d?

i removed group3d!

again not really needed, you can add children to objects, but if you want a pure container (no rendering), this would make more semantic sense. other options would be to not have anything (tell people to group things using object3d) or make component3d concrete (tell people to use that). having group3d consolidates the separation of light as its own thing.

why some classes end in 3D and some classes end in Component

answer (please help)

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.

@luanpotter luanpotter marked this pull request as ready for review June 4, 2024 12:21
@luanpotter luanpotter changed the title feat: Implement light abstraction [WIP] feat: Implement light abstraction Jun 23, 2024
@luanpotter luanpotter changed the title feat: Implement light abstraction feat: Implement basic light abstraction Jun 26, 2024
@luanpotter luanpotter changed the title feat: Implement basic light abstraction feat: Implement basic light abstraction [flame_3d] Jul 1, 2024
@luanpotter luanpotter force-pushed the feat(flame_3d)-make-shader-api-more-useful branch from 085f037 to db1465f Compare July 9, 2024 01:39
@luanpotter luanpotter changed the base branch from feat(flame_3d)-make-shader-api-more-useful to flame_3d July 9, 2024 01:40
@luanpotter luanpotter changed the base branch from flame_3d to feat(flame_3d)-make-shader-api-more-useful July 9, 2024 01:40
@luanpotter luanpotter force-pushed the feat(flame_3d)-make-shader-api-more-useful branch from db1465f to a15f851 Compare July 9, 2024 23:14
@luanpotter
Copy link
Member Author

@wolfenrain I think I came up with a nifty solution to solve the problem of LightComponent vs Light.
however I got excited and also attempted to solve other problems :P

@luanpotter
Copy link
Member Author

@wolfenrain removed the group component, please lmk if this makes more sense to you

still keeping the separation of obj3d/comp3d; lmk if you want me to drop that as well.

@wolfenrain
Copy link
Contributor

@wolfenrain removed the group component, please lmk if this makes more sense to you

still keeping the separation of obj3d/comp3d; lmk if you want me to drop that as well.

Yea let's park that for now as is, we can always talk about it with the rest of the team later!

Left one comment on an open thread but apart from that LGTM!

@luanpotter
Copy link
Member Author

@wolfenrain addressed final comments!

@luanpotter luanpotter merged commit f40a1d0 into feat(flame_3d)-make-shader-api-more-useful Jul 22, 2024
4 of 9 checks passed
@luanpotter luanpotter deleted the luan.flame-3d-light branch July 22, 2024 17:23
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.

None yet

2 participants