[Merged by Bors] - Spotlights #4715

robtfm · 2022-05-10T13:00:36Z

Objective

add spotlight support

Solution / Changelog

add spotlight angles (inner, outer) to PointLight struct. emitted light is linearly attenuated from 100% to 0% as angle tends from inner to outer. Direction is taken from the existing transform rotation.
add spotlight direction (vec3) and angles (f32,f32) to GpuPointLight struct (60 bytes -> 80 bytes) in pbr/render/lights.rs and mesh_view_bind_group.wgsl
reduce no-buffer-support max point light count to 204 due to above
use spotlight data to attenuate light in pbr.wgsl
do additional cluster culling on spotlights to minimise cost in assign_lights_to_clusters
changed one of the lights in the lighting demo to a spotlight
also added a spotlight demo - probably not justified but so reviewers can see it more easily

notes

increasing the size of the GpuPointLight struct on my machine reduces the FPS of many_lights -- sphere from ~150fps to 140fps.

i thought this was a reasonable tradeoff, and felt better than handling spotlights separately which is possible but would mean introducing a new bind group, refactoring light-assignment code and adding new spotlight-specific code in pbr.wgsl. the FPS impact for smaller numbers of lights should be very small.

the cluster culling strategy reintroduces the cluster aabb code which was recently removed... sorry. the aabb is used to get a cluster bounding sphere, which can then be tested fairly efficiently using the strategy described at the end of https://bartwronski.com/2017/04/13/cull-that-cone/. this works well with roughly cubic clusters (where the cluster z size is close to the same as x/y size), less well for other cases like single Z slice / tiled forward rendering. In the worst case we will end up just keeping the culling of the equivalent point light.

…engine#4602) - noticed a few Vec3 and Vec2 that could be const - Declared them as const - It seems to make a tiny improvement in example `many_light`, but given that the change is not complex at all it could still be worth it

superdump · 2022-05-10T13:23:12Z

Yey! Adding support for spotlights is long overdue. 😄

Two initial comments before I look into this in detail:

Increasing the size of GpuPointLight will mean that we can fit fewer point lights in one uniform buffer binding, which will reduce the number we can support on WebGL2. With storage buffers, which should be available basically everywhere else, it shouldn't be a big deal. We could move to using 'data' textures instead though if the reduction is significant with the alignment requirements. Looks at the diff to see the struct changes - ah, right, you calculated it already. 256 lights down to 204. I'm going to say that that feels reasonable, and if the difference of 52 lights is important, then we kind of need to implement data textures anyway.
The assignment of spot lights to clusters can be done using the planes method with slide 45+ from: http://newq.net/dl/pub/s2015_practical.pdf

superdump · 2022-05-10T13:40:18Z

It seems that perhaps the bartwronski culling method could be faster. Cool. I need to read more about it. Shame we have to dig up all the aabb code though. :)

robtfm · 2022-05-14T09:52:27Z

updated shadows to use a dedicated path for spotlights, rather than reusing the point light shadow path (which used 6 passes for each face).

add point vs spotlight to light sort order
queue a single shadow pass per spot light
- oriented to the light angle
- using a basis/view matrix constructed from the direction
- writing to the directional light shadow map array instead of the point light cube array
in the shader, reconstruct the view matrix from the spotlight direction and query the directional texture to get spotlight shadows

~~todo: overload the point light struct projection_lr field which is only used for point shadows - this will get us back to 64 bytes per light and 256 lights max for uniform buffers~~

app.2022-05-14.10-53-26.mp4

.

superdump

This is pretty much LGTM. A few comments.

Cargo.toml

crates/bevy_pbr/src/light.rs

crates/bevy_pbr/src/render/clustered_forward.wgsl

crates/bevy_pbr/src/render/light.rs

superdump · 2022-06-25T23:00:39Z

crates/bevy_pbr/src/render/light.rs

+            // 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.
+            intensity: point_light.intensity / (4.0 * std::f32::consts::PI),


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.

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..?

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.

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

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.

crates/bevy_pbr/src/render/light.rs

crates/bevy_pbr/src/render/pbr_functions.wgsl

examples/README.md

Co-authored-by: Robert Swain <[email protected]>

superdump · 2022-06-26T14:58:29Z

I'd like us to add support for loading spot lights from gltf, which should be a pretty small change, but it can be in a separate PR.

Also, while fiddling with code, I got caught out by SpotlightBundle (note the lower-case L) vs PointLightBundle. This is due to me saying earlier that spotlight is idiomatic in English. But, I think the inconsistency is more likely to catch people out, and the filament docs all use "spot light" and "point light" which is more logically consistent. I think we should go with spot light and point light and similar for struct/trait types.

superdump · 2022-06-27T14:03:55Z

I have a gltf file with a point and spot light if anyone wants to try that out. I've tested it and it works.

superdump · 2022-07-05T08:52:44Z

crates/bevy_pbr/src/light.rs

@@ -589,27 +612,122 @@ fn cluster_space_light_aabb(

 const NDC_MIN: Vec2 = const_vec2!([-1.0, -1.0]);
 const NDC_MAX: Vec2 = const_vec2!([1.0, 1.0]);
+fn screen_to_view(screen_size: Vec2, inverse_projection: Mat4, screen: Vec2, ndc_z: f32) -> Vec4 {


As a note for @cart - all the code for calculating cluster aabbs below is just resurrected from before. In my opinion no need to check it so closely.

cart

Just wrapped up a first pass. Generally I'm happy with this!

Side-by-side Bevy and Blender looks good / reasonably in-sync:

crates/bevy_pbr/src/bundle.rs

cart · 2022-07-08T19:57:27Z

bors r+

# Objective add spotlight support ## Solution / Changelog - add spotlight angles (inner, outer) to ``PointLight`` struct. emitted light is linearly attenuated from 100% to 0% as angle tends from inner to outer. Direction is taken from the existing transform rotation. - add spotlight direction (vec3) and angles (f32,f32) to ``GpuPointLight`` struct (60 bytes -> 80 bytes) in ``pbr/render/lights.rs`` and ``mesh_view_bind_group.wgsl`` - reduce no-buffer-support max point light count to 204 due to above - use spotlight data to attenuate light in ``pbr.wgsl`` - do additional cluster culling on spotlights to minimise cost in ``assign_lights_to_clusters`` - changed one of the lights in the lighting demo to a spotlight - also added a ``spotlight`` demo - probably not justified but so reviewers can see it more easily ## notes increasing the size of the GpuPointLight struct on my machine reduces the FPS of ``many_lights -- sphere`` from ~150fps to 140fps. i thought this was a reasonable tradeoff, and felt better than handling spotlights separately which is possible but would mean introducing a new bind group, refactoring light-assignment code and adding new spotlight-specific code in pbr.wgsl. the FPS impact for smaller numbers of lights should be very small. the cluster culling strategy reintroduces the cluster aabb code which was recently removed... sorry. the aabb is used to get a cluster bounding sphere, which can then be tested fairly efficiently using the strategy described at the end of https://bartwronski.com/2017/04/13/cull-that-cone/. this works well with roughly cubic clusters (where the cluster z size is close to the same as x/y size), less well for other cases like single Z slice / tiled forward rendering. In the worst case we will end up just keeping the culling of the equivalent point light. Co-authored-by: François <[email protected]>

bors · 2022-07-08T20:18:44Z

Pull request successfully merged into main.

Build succeeded:

# Objective add spotlight support ## Solution / Changelog - add spotlight angles (inner, outer) to ``PointLight`` struct. emitted light is linearly attenuated from 100% to 0% as angle tends from inner to outer. Direction is taken from the existing transform rotation. - add spotlight direction (vec3) and angles (f32,f32) to ``GpuPointLight`` struct (60 bytes -> 80 bytes) in ``pbr/render/lights.rs`` and ``mesh_view_bind_group.wgsl`` - reduce no-buffer-support max point light count to 204 due to above - use spotlight data to attenuate light in ``pbr.wgsl`` - do additional cluster culling on spotlights to minimise cost in ``assign_lights_to_clusters`` - changed one of the lights in the lighting demo to a spotlight - also added a ``spotlight`` demo - probably not justified but so reviewers can see it more easily ## notes increasing the size of the GpuPointLight struct on my machine reduces the FPS of ``many_lights -- sphere`` from ~150fps to 140fps. i thought this was a reasonable tradeoff, and felt better than handling spotlights separately which is possible but would mean introducing a new bind group, refactoring light-assignment code and adding new spotlight-specific code in pbr.wgsl. the FPS impact for smaller numbers of lights should be very small. the cluster culling strategy reintroduces the cluster aabb code which was recently removed... sorry. the aabb is used to get a cluster bounding sphere, which can then be tested fairly efficiently using the strategy described at the end of https://bartwronski.com/2017/04/13/cull-that-cone/. this works well with roughly cubic clusters (where the cluster z size is close to the same as x/y size), less well for other cases like single Z slice / tiled forward rendering. In the worst case we will end up just keeping the culling of the equivalent point light. Co-authored-by: François <[email protected]>

robtfm and others added 12 commits May 10, 2022 13:29

spotlights

63b91a8

update demos

fc0a685

spotlight cluster culling

d85cfa2

vary angle in example

19aa631

optimise

61120b5

format

f300809

spotlight cluster culling

2756a8c

optimise

585d101

format

c0c5437

format

0cb0544

remove debug output

cadcad0

fix rebase

55fb290

IceSentry added A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible labels May 10, 2022

robtfm added 5 commits May 12, 2022 12:08

add front_cull test

91ec7ff

shadow maps

47fe2ea

fix frustum for culling light entities

8464d7b

fix spot map offset

87ffc6f

format

6c39cd1

robtfm added 5 commits May 16, 2022 11:27

reduce pointlight struct size

5f97f1e

ci

2d8a8ed

ci?

6f8c9f9

clean up maths

3de7c9a

fmt

f562869

robtfm force-pushed the spotlight branch from b603af8 to f562869 Compare May 17, 2022 12:33

Merge branch 'main' into spotlight

9357a5e

superdump reviewed Jun 25, 2022

View reviewed changes

robtfm and others added 4 commits June 26, 2022 00:41

fix point light sort order comment

612813a

add comment on spotlight intensity divisor

d8c362c

Merge branch 'main' into spotlight

42cb994

Update Cargo.toml

68b8ebb

Co-authored-by: Robert Swain <[email protected]>

robtfm and others added 4 commits June 26, 2022 22:29

build-example-pages

6c8ec1c

spotlight / Spotlight -> spot_light / SpotLight

723bfeb

bevy_gltf: Support loading spotlights

a3809f7

spotlight / Spotlight -> spot_light / SpotLight

ea909d0

remove unused SPOT_LIGHT flag

974fd52

cart added this to the Bevy 0.8 milestone Jun 30, 2022

superdump approved these changes Jul 5, 2022

View reviewed changes

cart reviewed Jul 7, 2022

View reviewed changes

crates/bevy_pbr/src/bundle.rs Outdated Show resolved Hide resolved

robtfm added 5 commits July 8, 2022 09:19

separate using-facing SpotLight struct

32a72f2

Merge branch 'main' into spotlight

8a81014

doc comment

1617119

ci

45f68ad

ci ci

ab4d91c

bors bot changed the title ~~Spotlights~~ [Merged by Bors] - Spotlights Jul 8, 2022

bors bot closed this Jul 8, 2022

robtfm deleted the spotlight branch July 21, 2022 11:31

james7132 mentioned this pull request Nov 12, 2022

Import Lights from GLTF #1331

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Spotlights #4715

[Merged by Bors] - Spotlights #4715

robtfm commented May 10, 2022

superdump commented May 10, 2022

superdump commented May 10, 2022

robtfm commented May 14, 2022 •

edited

Loading

superdump left a comment

superdump Jun 25, 2022

robtfm Jun 25, 2022 •

edited

Loading

superdump Jun 26, 2022

robtfm Jun 26, 2022

superdump Jun 26, 2022

superdump commented Jun 26, 2022

superdump commented Jun 27, 2022

superdump Jul 5, 2022

cart left a comment

cart commented Jul 8, 2022

bors bot commented Jul 8, 2022

[Merged by Bors] - Spotlights #4715

[Merged by Bors] - Spotlights #4715

Conversation

robtfm commented May 10, 2022

Objective

Solution / Changelog

notes

superdump commented May 10, 2022

superdump commented May 10, 2022

robtfm commented May 14, 2022 • edited Loading

superdump left a comment

Choose a reason for hiding this comment

superdump Jun 25, 2022

Choose a reason for hiding this comment

robtfm Jun 25, 2022 • edited Loading

Choose a reason for hiding this comment

superdump Jun 26, 2022

Choose a reason for hiding this comment

robtfm Jun 26, 2022

Choose a reason for hiding this comment

superdump Jun 26, 2022

Choose a reason for hiding this comment

superdump commented Jun 26, 2022

superdump commented Jun 27, 2022

superdump Jul 5, 2022

Choose a reason for hiding this comment

cart left a comment

Choose a reason for hiding this comment

cart commented Jul 8, 2022

bors bot commented Jul 8, 2022

robtfm commented May 14, 2022 •

edited

Loading

robtfm Jun 25, 2022 •

edited

Loading