Skip to content

[Merged by Bors] - Spherical Area Lights - #1901

Closed
Josh015 wants to merge 17 commits into
bevyengine:mainfrom
Josh015:main
Closed

Josh015 wants to merge 17 commits into
bevyengine:mainfrom
Josh015:main

Conversation

@Josh015

@Josh015 Josh015 commented Apr 12, 2021 •

Copy link
Copy Markdown
Contributor

I still need to simplify and optimize the code, but here's a preliminary working version of Spherical Area Lights. See the example image below from a modified version of my cubism-demo-rs app, which you can also clone and run to see them in action.

Spherical Area Lights v1

@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-Rendering Drawing game state to the screen labels Apr 12, 2021
@Josh015
Josh015 marked this pull request as ready for review April 13, 2021 02:01
@Josh015 Josh015 changed the title Spherical Area Lights test Spherical Area Lights Apr 13, 2021
@Josh015

Josh015 commented Apr 13, 2021

Copy link
Copy Markdown
Contributor Author

Okay, it stopped working after merging in the PointLight changes from another branch. Need to figure out what I messed up.

@ghost

ghost commented Apr 13, 2021

Copy link
Copy Markdown

Does it make sense to put it into something else than PointLight?
I looked at the five first APIs google showed me when searching PointLight and I'm just wondering about the semantics of radius on a struct with Point in it. None of those APIs used such a field:

@Josh015

Josh015 commented Apr 13, 2021

Copy link
Copy Markdown
Contributor Author

Yes it makes sense for Point Lights as well as, eventually, Spot Lights. To the best of my knowledge, that's how Unreal Engine does it. Those light types just describe the cutoff behavior for the area lights, such as the range and/or the cone.

@Josh015

Josh015 commented Apr 13, 2021 •

Copy link
Copy Markdown
Contributor Author

So in my cubism-demo-rs project I removed the code that used my new Spherical Area Lights and tested it with both bevy 0.5.0 and the latest directly from bevy's github main branch. Here's what I observed:

0.5.0
0_5_0

latest
latest

As you can see the latest appears broken even without my Spherical Area Light code. There appears to have been a lighting related PR that broke it within the last 24 hours. I believe this one is the culprit:
5c4f355

Does anyone have any thoughts on how I should proceed?

@mockersf

Copy link
Copy Markdown
Member

There was an issue introduced in #1778 when having more than one light, it should be fixed by #1940

Comment thread crates/bevy_pbr/src/light.rs Outdated
@Josh015

Josh015 commented Apr 18, 2021

Copy link
Copy Markdown
Contributor Author

I've applied the uniforms packing fix to my code and moved the radius value into that new vector rather than hijacking the color alpha channel. The PR should now be ready to go.

@cart

cart commented Apr 22, 2021

Copy link
Copy Markdown
Member

This looks good to me. The algorithm appears to line up with the reference, and a side-by-side comparison to similar Blender setups looks good.

image

@cart

cart commented Apr 22, 2021

Copy link
Copy Markdown
Member

bors r+

bors Bot pushed a commit that referenced this pull request Apr 22, 2021
I still need to simplify and optimize the code, but here's a preliminary working version of Spherical Area Lights. See the example image below from a modified version of my [cubism-demo-rs](https://github.com/Josh015/cubism-demo-rs) app, which you can also clone and run to see them in action.

![Spherical Area Lights v1](https://user-images.githubusercontent.com/8846132/114491862-60df6000-9be5-11eb-8950-f039b74e1e96.jpg)
@bors

bors Bot commented Apr 22, 2021

Copy link
Copy Markdown

@bors bors Bot changed the title Spherical Area Lights [Merged by Bors] - Spherical Area Lights Apr 22, 2021
@bors bors Bot closed this Apr 22, 2021
ostwilkens pushed a commit to ostwilkens/bevy that referenced this pull request Jul 27, 2021
I still need to simplify and optimize the code, but here's a preliminary working version of Spherical Area Lights. See the example image below from a modified version of my [cubism-demo-rs](https://github.com/Josh015/cubism-demo-rs) app, which you can also clone and run to see them in action.

![Spherical Area Lights v1](https://user-images.githubusercontent.com/8846132/114491862-60df6000-9be5-11eb-8950-f039b74e1e96.jpg)
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants