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.

Sign up for GitHub

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

Jump to bottom

Split generic parts of ForwardRenderer into a Renderer class #4886

Merged
merged 2 commits into from Nov 24, 2022
Merged

Split generic parts of ForwardRenderer into a Renderer class #4886

merged 2 commits into from Nov 24, 2022

Conversation

mvaligursky
Copy link
Contributor

@mvaligursky mvaligursky commented Nov 24, 2022 •
edited

  • moving parts of ForwardRenderer into its base Renderer class, to keep the size of the file reasonable, before further render pass related refactoring for shadows rendering
  • in the future, this might allow us to write additional renderers (deferred for example), using the same blocks (culling, updates, uniform submission, draw calls ..)
  • there were no other updates other then moving code and members, and calling those functions appropriately. Tested on all engine examples.

Martin Valigursky added 2 commits November 24, 2022 14:10
@mvaligursky mvaligursky self-assigned this Nov 24, 2022
@mvaligursky mvaligursky added the area: graphics Graphics related issue label Nov 24, 2022
@mvaligursky mvaligursky merged commit 91f1fc8 into main Nov 24, 2022
@mvaligursky mvaligursky deleted the mv-renderer branch November 24, 2022 16:27
willeastcott
willeastcott reviewed Nov 25, 2022
View reviewed changes
src/scene/renderer/forward-renderer.js

this.lightTextureAtlas.destroy();
this.lightTextureAtlas = null;
super.destroy();
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't the whole subclassed destroy function redundant here?

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

@willeastcott willeastcott willeastcott left review comments

@slimbuck slimbuck slimbuck approved these changes

@GSterbrant GSterbrant Awaiting requested review from GSterbrant

Assignees

@mvaligursky mvaligursky

Labels
area: graphics Graphics related issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mvaligursky @willeastcott @slimbuck