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

[Merged by Bors] - increase the maximum number of point lights with shadows to the max supported by the device #4435

Closed

Conversation

mockersf
Copy link
Member

@mockersf mockersf commented Apr 6, 2022

Objective

  • Being limited to 10 pointlights with shadow is very limiting

Solution

  • Raise the limit

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Apr 6, 2022
@mockersf mockersf added A-Rendering Drawing game state to the screen and removed S-Needs-Triage This issue needs to be labelled labels Apr 6, 2022
crates/bevy_pbr/src/render/light.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@superdump superdump left a comment

Choose a reason for hiding this comment

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

There are some other references to MAX_POINT_LIGHT_SHADOW_MAPS in the codebase, in comments. Please double-check those.

crates/bevy_pbr/src/render/light.rs Show resolved Hide resolved
crates/bevy_pbr/src/render/light.rs Outdated Show resolved Hide resolved
@superdump
Copy link
Contributor

Approved, but maybe update the PR title? :)

@mockersf mockersf changed the title raise MAX_POINT_LIGHT_SHADOW_MAPS increase the maximum number of point lights with shadows to the max supported by the device Apr 7, 2022
.iter()
.filter(|light| light.1.shadows_enabled)
.count()
.min((render_device.limits().max_texture_array_layers / 6) as usize);
Copy link
Contributor

@IceSentry IceSentry Apr 7, 2022

Choose a reason for hiding this comment

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

What's the 6 here and on line 662? I'm not sure what it's supposed to mean

Copy link
Contributor

Choose a reason for hiding this comment

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

True. It is a ‘magic number’ I guess though it’s already been used elsewhere so I’d fix it in a separate PR. Point lights use cubemaps for shadow mapping. Cubemaps are array textures with 6 layers per cubemap, one for each cube face. That’s where the 6 comes from.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, doesn't need to be fixed here, it was just for my own learning purposes, thanks!

@mockersf mockersf force-pushed the raise-pointlight-with-shadow-limit branch from 9accd9a to d513335 Compare April 7, 2022 20:52
@cart
Copy link
Member

cart commented Apr 7, 2022

bors r+

bors bot pushed a commit that referenced this pull request Apr 7, 2022
…upported by the device (#4435)

# Objective

- Being limited to 10 pointlights with shadow is very limiting

## Solution

- Raise the limit
@bors bors bot changed the title increase the maximum number of point lights with shadows to the max supported by the device [Merged by Bors] - increase the maximum number of point lights with shadows to the max supported by the device Apr 7, 2022
@bors bors bot closed this Apr 7, 2022
aevyrie pushed a commit to aevyrie/bevy that referenced this pull request Jun 7, 2022
…upported by the device (bevyengine#4435)

# Objective

- Being limited to 10 pointlights with shadow is very limiting

## Solution

- Raise the limit
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
…upported by the device (bevyengine#4435)

# Objective

- Being limited to 10 pointlights with shadow is very limiting

## Solution

- Raise the limit
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants