Skip to content

SKETCH: eliminate the scene class #1133

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

Closed
wants to merge 1 commit into from
Closed

SKETCH: eliminate the scene class #1133

wants to merge 1 commit into from

Conversation

hollasch
Copy link
Collaborator

@hollasch hollasch commented Jun 3, 2023

The long and winding road.

@hollasch hollasch added this to the v4.0.0 milestone Jun 3, 2023
@hollasch hollasch requested a review from trevordblack June 3, 2023 17:43
@hollasch hollasch self-assigned this Jun 3, 2023
@trevordblack
Copy link
Collaborator

Do we want to encapsulate all of the stuff in a render?

I think there's value in having a scene class, as it reinforces the idea that all of the geo+lighting+camera(?) Is packaged together and neatly reinforces the idea that a render should be treated like an artificial thing. Like a scene on Broadway or in Hollywood.

The previous encapsulation is kind of thin though. It does sort of act like an immutable struct after construction (and some initial parameter setting). That you have to reach into struct to change the camera is kind of not great

We've wittled down the mental model and structure of every other part of the code to a place that we're both pretty happy with. This part of the code is just a fountain for slightly misaligned mental models

Which, I mean, makes sense as this is part of most rendering engines that is the most poorly defined. There is a lot of play in terms of how you want a camera and scene represented. Whereas there is shockingly few in terms of the actual assets

@hollasch hollasch closed this Jun 4, 2023
@hollasch hollasch deleted the kill-scene branch June 21, 2023 21:18
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