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

Split apply_pbr_lighting into many many small functions #17541

Open
ethereumdegen opened this issue Jan 26, 2025 · 0 comments
Open

Split apply_pbr_lighting into many many small functions #17541

ethereumdegen opened this issue Jan 26, 2025 · 0 comments
Labels
A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Design This issue requires design work to think about how it would best be accomplished

Comments

@ethereumdegen
Copy link
Contributor

What problem does this solve or what need does it fill?

Please split apply_pbr_lighting into many many small functions. You can keep the function name the same and what it does the same. But please make it perform a bunch of small functions which can be called individually for when we are making Extension Material Shaders.

This is because I want to change the way shadows work (cel shading) but it seems VERY VERY horribly hard to do with the way that apply_pbr_lighting is so monolithic right now.

What solution would you like?

for example I want each input of this line to have its own small function

   // Total light
    output_color = vec4<f32>(
        (view_bindings::view.exposure * (transmitted_light + direct_light + indirect_light)) + emissive_light,
        output_color.a
    );

And i want to be able to effectively easily modify that specific line of code in my extension material shader . Right now, I have to completely re-write (re-implement) the entirety of apply_pbr_lighting just to change the way direct light affects my output color because this is too monolithic.

What alternative(s) have you considered?

Offer a few preset different apply_pbr_lighting functions that work differently? Make some of them have different input props or a bit flag to be able to change the way inner parts of it work?

But overall i think the best soln here is to just take chunks of the innards of apply_pbr_lighting and put them into small functions which apply_pbr_lighting calls. That way, I can implement those small functions into my shader exactly how i wish and only have to rewrite perhaps a small part of apply_pbr_lighting to change the way my color reacts to point and dynamic lights. !

Additional context

@ethereumdegen ethereumdegen added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Jan 26, 2025
@BenjaminBrienen BenjaminBrienen added A-Rendering Drawing game state to the screen S-Needs-Design This issue requires design work to think about how it would best be accomplished D-Straightforward Simple bug fixes and API improvements, docs, test and examples and removed S-Needs-Triage This issue needs to be labelled labels Jan 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Design This issue requires design work to think about how it would best be accomplished
Projects
None yet
Development

No branches or pull requests

2 participants