-
-
Notifications
You must be signed in to change notification settings - Fork 36k
Create Option to Have Spotlights Use Rectangular Projection #24589
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
base: dev
Are you sure you want to change the base?
Conversation
We'll have to refactor the code a bit... |
Btw... Make sure to not include the builds nor changes to |
Sorry about that! Still very new to the whole pull-requesting thing 😓 |
All right the files are no longer being overwritten! How would you recommend I refactor things? I would be against a |
Are we against calling this something like "ProjectorLight", then, if a SpotLight modification isn't acceptable? I agree that "RectLight" is a bit confusing. |
I think ProjectorLight would make the most sense. Another thing I was thinking is perhaps having an attenuation mode for spotlights that can toggle between this and the traditional circle attenuation - maybe As I said before, I'm not sure the additional code complexity warranted by a |
If an "attenuation" field like you've suggested is not acceptable for the public API then I would think it would still be okay to implement it that way internally while the public facing API separates the lights into "Spot" and "Projector" lights. Best of both worlds! |
You, my friend, are a genius. ProjectorLight is a subclass of SpotLight that internally is just a SpotLight with an attenuated field. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably worth waiting for approval from @mrdoob on the path forward for the PR but I've added a few comments on code style to the review.
// WebGL 2 | ||
// WebGL 2ÍÍ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like an inadvertent change
hash.pointLength !== pointLength || | ||
hash.spotLength !== spotLength || | ||
hash.rectAreaLength !== rectAreaLength || | ||
hash.hemiLength !== hemiLength || | ||
hash.numDirectionalShadows !== numDirectionalShadows || | ||
hash.numPointShadows !== numPointShadows || | ||
hash.numSpotShadows !== numSpotShadows || | ||
hash.numSpotMaps !== numSpotMaps ) { | ||
hash.pointLength !== pointLength || | ||
hash.spotLength !== spotLength || | ||
hash.rectAreaLength !== rectAreaLength || | ||
hash.hemiLength !== hemiLength || | ||
hash.numDirectionalShadows !== numDirectionalShadows || | ||
hash.numPointShadows !== numPointShadows || | ||
hash.numSpotShadows !== numSpotShadows || | ||
hash.numSpotMaps !== numSpotMaps ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a few places in the PR where indentation looks off.
if (spotLight.projector) { | ||
vec3 spotLightCoord = spotCoord.xyz / spotCoord.w; | ||
spotAttenuation = clamp((-2.0 * sdBox(spotLightCoord.xy - vec2(0.5), vec2(0.5, 0.5))) * (-1.0 / ((1.0 - acos(spotLight.penumbraCos)) - 1.0)), 0.0, 1.0); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we update this to use the three.js code style and remove the unnecessary parentheses.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@N8python See the guide here, which includes tools for automatically formatting the code:
https://github.com/mrdoob/three.js/wiki/Mr.doob's-Code-Style%E2%84%A2
float sdBox( in vec2 p, in vec2 b ) | ||
{ | ||
vec2 d = abs(p)-b; | ||
return length(max(d,0.0)) + min(max(d.x,d.y),0.0); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: spacing around the function and remove the newline before the first brace
@@ -1,4 +1,4 @@ | |||
export default /* glsl */` | |||
export default /* glsl */ ` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: unnecessary change
@@ -1,4 +1,4 @@ | |||
export default /* glsl */` | |||
export default /* glsl */ ` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: same unnecessary change
lightHelperProjector.update(); | ||
if (spotLight.projector) { | ||
lightHelper.visible = false; | ||
lightHelperProjector.visible = true; | ||
} else { | ||
lightHelper.visible = true; | ||
lightHelperProjector.visible = false; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: code style (spaces inside parentheses and newlines within braces)
@N8python @gkjohnson Hello, has the rectangular projection been completed and can it be used simply by merging the code? Do you have any cases? How can I extract it as a plugin and use it locally? |
@paulzhu0903 Do you support the latest version? |
Can we share code? @paulzhu0903 |
Sorry, I am a newbie. I just modified N8python's code to create a scene with a projector. I found the light leakage issue. The code is similar to N8python's demo. |
With the new mapping capabilities of spotlights, many people will want to use them as movie-projectors. However, their shape is purely circular - this pull request allows you to set the
projector
boolean on a spot light to have it attenuate based off its shadowcam's frustum. It also includes theprojectorAspect
attribute to allow varying the width of the projection.It also supports the use of the penumbra parameter for attenuation (see image 3). This is accomplished via a signed distance function.