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

[Merged by Bors] - Spotlights #4715

Closed
wants to merge 53 commits into from
Closed

[Merged by Bors] - Spotlights #4715

Changes from 1 commit
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
63b91a8
spotlights
robtfm May 4, 2022
fc0a685
update demos
robtfm May 6, 2022
d85cfa2
spotlight cluster culling
robtfm May 10, 2022
19aa631
vary angle in example
robtfm May 10, 2022
61120b5
optimise
robtfm May 10, 2022
f300809
format
robtfm May 10, 2022
c2db39b
use const Vec2 in lights cluster and bounding box when possible (#4602)
mockersf May 6, 2022
2756a8c
spotlight cluster culling
robtfm May 10, 2022
585d101
optimise
robtfm May 10, 2022
c0c5437
format
robtfm May 10, 2022
0cb0544
format
robtfm May 10, 2022
cadcad0
remove debug output
robtfm May 10, 2022
55fb290
fix rebase
robtfm May 10, 2022
91ec7ff
add front_cull test
robtfm May 12, 2022
47fe2ea
shadow maps
robtfm May 13, 2022
8464d7b
fix frustum for culling light entities
robtfm May 13, 2022
87ffc6f
fix spot map offset
robtfm May 14, 2022
6c39cd1
format
robtfm May 14, 2022
5f97f1e
reduce pointlight struct size
robtfm May 16, 2022
2d8a8ed
ci
robtfm May 16, 2022
6f8c9f9
ci?
robtfm May 16, 2022
3de7c9a
clean up maths
robtfm May 17, 2022
f562869
fmt
robtfm May 17, 2022
9357a5e
Merge branch 'main' into spotlight
robtfm May 17, 2022
5f66f57
remove near/far
robtfm May 17, 2022
695bdc5
compress light direction
robtfm Jun 6, 2022
00d3cc2
filament
robtfm Jun 6, 2022
eccfa1b
apply suggestions from code review
robtfm Jun 6, 2022
fb28014
add SpotlightBundle
robtfm Jun 6, 2022
d9b20f4
use xz for direction, revert sign() change
robtfm Jun 7, 2022
e241daa
format
robtfm Jun 7, 2022
109077e
lift condition
robtfm Jun 17, 2022
d3ccb12
fix webgl mipmap sample
robtfm Jun 17, 2022
5968759
remove println
robtfm Jun 21, 2022
b0f651f
Squashed commit of the following:
robtfm Jun 21, 2022
e8e6f1b
reapply to main
robtfm Jun 21, 2022
b1a82d3
Merge branch 'main' into spotlight
robtfm Jun 22, 2022
970a3cc
Merge branch 'main' into spotlight
robtfm Jun 22, 2022
8fd40b2
fmt
robtfm Jun 22, 2022
612813a
fix point light sort order comment
robtfm Jun 25, 2022
d8c362c
add comment on spotlight intensity divisor
robtfm Jun 26, 2022
42cb994
Merge branch 'main' into spotlight
robtfm Jun 26, 2022
68b8ebb
Update Cargo.toml
robtfm Jun 26, 2022
6c8ec1c
build-example-pages
robtfm Jun 26, 2022
723bfeb
spotlight / Spotlight -> spot_light / SpotLight
robtfm Jun 26, 2022
a3809f7
bevy_gltf: Support loading spotlights
superdump Jun 26, 2022
ea909d0
spotlight / Spotlight -> spot_light / SpotLight
robtfm Jun 26, 2022
974fd52
remove unused SPOT_LIGHT flag
robtfm Jun 27, 2022
32a72f2
separate using-facing SpotLight struct
robtfm Jul 8, 2022
8a81014
Merge branch 'main' into spotlight
robtfm Jul 8, 2022
1617119
doc comment
robtfm Jul 8, 2022
45f68ad
ci
robtfm Jul 8, 2022
ab4d91c
ci ci
robtfm Jul 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
add comment on spotlight intensity divisor
  • Loading branch information
@robtfm
robtfm committed Jun 26, 2022
commit d8c362ce71ce9237c7b45fba1cc4db5bcd543dc5
3 changes: 3 additions & 0 deletions crates/bevy_pbr/src/render/light.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,9 @@ pub fn extract_lights(
// NOTE: Map from luminous power in lumens to luminous intensity in lumens per steradian
// for a point light. See https://google.github.io/filament/Filament.html#mjx-eqn-pointLightLuminousPower
// for details.
// Note: Filament uses a divisor of PI for spotlights. We choose to use the same 4*PI divisor
// in both cases so that toggling between point light and spotlight keeps lit areas lit equally,
// which seems least surprising for users
intensity: point_light.intensity / (4.0 * std::f32::consts::PI),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think filament suggests a different intensity unit for spotlights to enable specifying a consistent brightness even if you change the radius. The above constant factor was for point lights.

Copy link
Contributor Author

@robtfm robtfm Jun 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

yes, they use I = Phi / pi for spotlights, as opposed to I = Phi / (4*pi) for point lights (assuming we don't want intensity to vary with radius edit:angle, which i don't think we do).

i'm happy to change that, but i don't see why they make spotlights 4x brighter than the equivalent intensity point lights by default. i also am pretty sure that This new formulation can also be considered physically based if the spot's reflector is replaced with a matte, diffuse mask that absorbs light perfectly is flat-out wrong if the factor is not the same as for point lights..?

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, phi / pi is what we should use for spotlights, if it does indeed make the apparent intensity of the light be consistent across inner/outer radius changes.

I think the 4x brighter is them saying that a spotlight will cover roughly 1/4 a sphere worth of solid angle and that all the luminous power is directed in the cone of the spotlight so it is indeed brighter for the same luminous power. And yes, in that case I agree with you that it would rather mean that the spotlight's reflector is perfect such that all energy is reflected out in the direction of the spotlight's cone. If it were matte and perfectly absorbent, then I agree it should have the same factor, intuitively. I haven't done the maths though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

as discussed, kept as 4pi for PoLS when toggling, added comment

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, we think it is more user-friendly if the mapping from luminous power (rate of energy radiation from the light source) to luminous intensity (rate of energy radiation per unit angle) is the same for point and spot lights as then you can change the type and the brightness will be the same. The counter argument could be if people have calibrated spot light power values or something and this doesn't behave properly. I'm fine to try this and then iterate if it turns out people don't like it.

range: point_light.range,
radius: point_light.radius,
Expand Down