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

Jump to bottom

Resolve PCSS issues, remove "experimental" flag, and "release" the feature #16155

Open
JMS55 opened this issue Oct 29, 2024 · 1 comment
Open

Resolve PCSS issues, remove "experimental" flag, and "release" the feature #16155

JMS55 opened this issue Oct 29, 2024 · 1 comment
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!
Milestone
0.16

Comments

@JMS55
Copy link
Contributor

JMS55 commented Oct 29, 2024

Bevy version

0.15

Info

  • Calculate the search radius based on the light radius in world units and the distance to the light in world units (for directional lights this should be configurable and perhaps default to the distance to the sun)
  • Average depth in world units when finding the blocker
  • Use the correct calculation for the filter radius that leverages the penumbra size calculation
  • Use blue noise random rotations of the MSAA sampling pattern

From this conversation on discord https://discord.com/channels/691052431525675048/743663924229963868/1300819753480228895
@JMS55 JMS55 added A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior labels Oct 29, 2024
@JMS55 JMS55 moved this to Todo in Shadow Maps Oct 29, 2024
@BenjaminBrienen BenjaminBrienen added the S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! label Oct 29, 2024
@alice-i-cecile alice-i-cecile added this to the 0.15 milestone Oct 30, 2024
@alice-i-cecile alice-i-cecile mentioned this issue Oct 30, 2024
Closed
@cart
Copy link
Member

cart commented Nov 12, 2024

Given the issues with the implementation, I think we should make PCSS experimental for 0.15 and hold off on announcing it until we resolve these problems.

@pcwalton does that sound reasonable?

@cart cart mentioned this issue Nov 14, 2024
Merged
@cart cart modified the milestones: 0.15, 0.16 Nov 14, 2024
@cart cart changed the title PCSS issues Resolve PCSS issues, remove "experimental" flag, and "release" the feature Nov 14, 2024
github-merge-queue bot pushed a commit that referenced this issue Nov 14, 2024
@cart
Make PCSS experimental (#16382)
6beeaa8 
# Objective

PCSS still has some fundamental issues (#16155). We should resolve them
before "releasing" the feature.

## Solution

1. Rename the already-optional `pbr_pcss` cargo feature to
`experimental_pbr_pcss` to better communicate its state to developers.
2. Adjust the description of the `experimental_pbr_pcss` cargo feature
to better communicate its state to developers.
3. Gate PCSS-related light component fields behind that cargo feature,
to prevent surfacing them to developers by default.
mockersf pushed a commit that referenced this issue Nov 16, 2024
@cart @mockersf
Make PCSS experimental (#16382)
e6fbcb7 
# Objective

PCSS still has some fundamental issues (#16155). We should resolve them
before "releasing" the feature.

## Solution

1. Rename the already-optional `pbr_pcss` cargo feature to
`experimental_pbr_pcss` to better communicate its state to developers.
2. Adjust the description of the `experimental_pbr_pcss` cargo feature
to better communicate its state to developers.
3. Gate PCSS-related light component fields behind that cargo feature,
to prevent surfacing them to developers by default.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!
Projects
Shadow Maps
Status: Todo
Milestone
0.16
Development

No branches or pull requests
4 participants
@cart @alice-i-cecile @BenjaminBrienen @JMS55