search

mrdoob / three.js

JavaScript 3D Library.
https://threejs.org/
MIT License
102.28k stars 35.35k forks source link

Minor Improvements to Lensflare Module #27658

Open sciencecompliance opened 8 months ago

sciencecompliance commented 8 months ago

Description

After using the Lensflare module a few times now and running into its limitations, I think there are a few small improvements--one I might even consider a bug fix--that may be prudent for future releases.

The "bug", so-to-speak, is in the vertex shader for the LensflareElement object. The gl_Position variable uses the z-coordinate of the screenPosition uniform. This means that sometimes the lens flare can be rendered behind objects in the scene, which doesn't really make sense since lens flares are optical effects that occur in the camera or eye.

The other issues are related to the modulation of the lens flare's size and opacity. As it stands now, there is currently no way to change the size of the flare depending on conditions within the scene (i.e. the distance of the camera from the light source). The case I ran into where this was a problem for me was rendering the Sun at different astronomical distances and wanting to decrease the size of the glare centered on the Sun in the animation loop depending on Sun-camera distance. This would also be relevant to glare effects on more terrestrial scales for objects like streetlights, etc...

Similar to modulating size, one might also want to modulate opacity of lens flare elements based on the distance to the camera in a manner independent of size modulation. A good example of this is a lens flare that is produced by the optics when light bounces around inside a camera lens. Its 'size' as it is defined in three.js would not change based on distance. The 'size' in screen space is based on the internals of the camera lens and the angle of the incident light. As you move further from the light source, however, you might want to modulate the intensity (via opacity) of such a lensflare element.

Solution

With the exception of the "bug fix", the changes I'd like to make would change nothing from a three.js developer's perspective by default. People who have used the API previously and are comfortable with the way it currently works would see no difference (aside from proper z-placement in clip space). The proposed changes are enumerated below:  

  1. Change the z-value of the gl_Position variable in the LensflareElement vertex shader from screenPosition.z to -1.0.

  2. Add property to Lensflare object that exposes scale controls to the developer after creating a new Lensflare object and change code on what is currently Line 251 of the module to multiply the current product by this new scale property. Also, set size/scale property to default value of 1.

  3. Related to item 2, each LensflareElement would have a boolean parameter added to its constructor that controls whether the size/scale is affected by the change of the scale/size property of the Lensflare object. This could either be true or false by default since it would only affect usage if the developer decides to mess with the scale property. I would lean towards true by default so that messing around with the Lensflare object's size parameter by default modulates the size of the elements. This seems like the preferable behavior.

  4. Add property to Lensflare object that allows the opacity to be modulated in a manner that can be individually toggled between LensflareElement objects added to the LensFlare object.

  5. Similar to item 3, add boolean parameter to constructor for LensflareElement object that determines whether changes to the new Lensflare opacity-modulation property affect the lens flare element. Default to true. This would include changes to the LensflareElement fragment shader that includes a new uniform and line to incorporate the opacity adjustment.

Alternatives

An alternative would be to keep current functionality aside from the aforementioned "bug fix" and leave extended functionality for other developers/libraries; however, I think the proposed functionalities are fairly basic improvements considering how deep the rabbit hole goes for this topic and for how many use cases these changes would be valuable. It still wouldn't have support for modulating size/opacity/color based on translucent occluding materials, which is something I've needed in the past but is understandably a more complex problem than probably needs to be addressed in the basic three.js lens flare module.

Additional context

No response

Mugen87 commented 8 months ago

Let's focus in this issue on the first point. Meaning:

The "bug", so-to-speak, is in the vertex shader for the LensflareElement object. The gl_Position variable uses the z-coordinate of the screenPosition uniform. This means that sometimes the lens flare can be rendered behind objects in the scene, which doesn't really make sense since lens flares are optical effects that occur in the camera or eye.

It's best if you would demonstrate the behavior that you consider as buggy with a live example (https://jsfiddle.net/g3atw6k5/) so it's easier for us to understand what you mean.

sciencecompliance commented 8 months ago

Edit: Sorry, didn't realize the link changed: https://jsfiddle.net/6d05xLrn/34/

The problem should be demonstrated at that jsfiddle link and should be apparent from the initial position values of all the objects. You can simply zoom out to see when the lens flare moves to the front of the scene. In the example I provided, this becomes an issue when using the logarithmic depth buffer for the renderer. Simply changing line 351 of the Lensflare module as I described should fix this. The change to the lens flare element's vertex shader also doesn't appear to cause a problem if NOT using a logarithmic depth buffer from my local testing. I would need to copy and paste all the LensflareElement stuff from the Lensflare module into the jsfiddle to demonstrate that there (which I thought was overkill at the moment).

As for the other issues, simply exposing a property to be able to change the size of the lens flare including ALL of its elements on the fly I think would be a marked improvement in functionality and would require adding only 1 line of code in the constructor (this.flareScale = 1;) and modifying one existing one (Line 251, size = this.flareScale * element.size / viewport.w;).

(this.scale and this.size run into name collision issues)

Mugen87 commented 8 months ago

I have changed the following line by replacing screenPosition.z with -1.0 https://github.com/mrdoob/three.js/blob/e8e4f62a45f7d4877b9ef2e47ad1fe1344afe9c1/examples/jsm/objects/Lensflare.js#L351 But I don't see a change in behavior. I guess I overlook something. Do you mind providing more context and explain how both versions differ? A video that compares both behaviors would probably help best.

Right now, I don't understand how using - 1.0 solves any issues. It does not seem right to use a fixed value instead of the real clip space z component.

sciencecompliance commented 8 months ago

That's strange. I tested it in the fiddle, too, and experienced the same problem you're facing, but in my personal project it does seem to make the difference. I will try to find out what the difference is between the two and report back.

As for the rationale for having the clip space z-component fixed, a lens flare is a phenomenon that occurs inside the camera/eye, so in real terms the z-component in clip space for any glare/lens flare would be fixed in the camera/eye. The only thing that would get 'rendered' on top of a lens flare is CMOS/film/retinal damage or aberration, any of which would be handled in a post-processing step anyway.

sciencecompliance commented 8 months ago

Okay, I seem to have figured out the discrepancy, though I'm still not sure why the discrepancy is happening. When I pull the entire contents of the Lensflare module into the jsfiddle, changing the gl_Position value made a difference for me. When importing from the CDN, it doesn't, though. Maybe it's a caching issue? In any case, you should now be able to switch between the two gl_Position values in the jsfiddle linked to below and see the difference for yourself. The line in question is 355 in the fiddle.

https://jsfiddle.net/0ehac19w/20/