Open mrdoob opened 5 years ago
Love at first sight! 😊
Perhaps ColorBackground
could include alpha, and this could completely replace renderer.setClearColor()
and renderer.setClearAlpha()
.
A lot of passes right now have this messy save/restore pattern for the clearing stuff, eg. https://github.com/mrdoob/three.js/blob/dev/examples/js/postprocessing/RenderPass.js#L59
The fog names you have chosen define the curve that is used -- except the first one.
Regardless of the implementation details, I would suggest calling the first one THREE.LinearFog
, as it is close to linear. Unity uses that term. Alternatively, you could retain the existing nomenclature and just call it THREE.Fog
.
About shadow, this could not be light.shadow
?
light.shadow = new THREE.PCSSShadow();
About scene.background
and scene.fog
the idea of use class new THREE.ExponentialFog()
is that these generate
the shader itself, like something extended of ShaderMaterial
for example, without depending of core modification? So we can implement NodeMaterial
in this sense.
scene.background = new THREE.NodeBackground();
scene.fog = new THREE.NodeFog();
When using THREE.CubeTextureBackground, all meshes in the scene will use it as material.envMap by default
You would be auto-assigning an environment map to a material when the user may not want it. Think of MeshBasic
or MeshLambert
.
I like @sunag idea of making shadow light dependant as apposed to scene dependent. light.shadow
is already in use for matrix calcs which are probably better off independent of filtering/softness as they can can be used in conjuncture with different matrix warps which would lead to a lot of different permutations if using a single shadow type class. Maybe use light.shadow
for type of shadow projection and light.shadow.type
to define filtering/softness type.
I like the proposed changes. 👍
@mrdoob I think it may be cleanest to finish/merge #17355 first, and then (when done discussing here) add the renaming and deprecation warnings, as otherwise the present PR will grow too big and also enter an area where I don't know the current practice well. Or, actually, since I introduce FogExp, I may just as well give it the new name that is decided here (which I guess will be the one you proposed above), instead of introducing a name and deprecate it the week after.
Perhaps ColorBackground could include alpha, and this could completely replace renderer.setClearColor() and renderer.setClearAlpha().
What if you have a transparent skybox? Then you need a background color.
The fog names you have chosen define the curve that is used -- except the first one. Regardless of the implementation details, I would suggest calling the first one
THREE.LinearFog
, as it is close to linear. Unity uses that term. Alternatively, you could retain the existing nomenclature and just call itTHREE.Fog
.
It (the smoothstep
function) is a cubic Hermite spline with clamped input, so that it maps to [0,1] with a monotone curve. The curve is steeper than linear near the middle (slope of 1.5 in the middle), and much flatter near the ends (slope of 0 at the ends). Unity is either wrong (wouldn't surprise me) or has a different implementation. Copying others' mistakes is a bad habit. RangeFog
is a clear name that will serve the purpose, and it does not inherently promise a particular implementation (and most of all does not promise another implementation than the actual one).
For the most part I like the direction of these changes! 🎉 One comment, though:
When using THREE.CubeTextureBackground, all meshes in the scene will use it as material.envMap by default
I'm generally not a fan of this approach in part because of the reasons that @WestLangley mentioned but also because I would like to see the renderer move towards a more customize-able architecture rather than one that hardcodes features with a specific use case in mind. It would be great if uniform defaulting were available to any end user need (render resolution, time, lights, etc) -- If this type of "default uniform" behavior is going to be implemented why not add the feature in such a way that applications can leverage, too (#16922)? Then the environment map wouldn't magically be set but the behavior can easily be opted in to.
In terms of an example of renderer customize-ability -- Unity is moving towards a Scriptable Render Pipeline which allows more control over how a scene it rendered and provides a few defaults for quick starts. It would be great to see the three js rendering primitives exposed more readily with WebGLRenderer
using these to implement and provide a default rendering approach. But that's maybe a larger discussion.
Regarding lights:
About shadow, this could not be light.shadow?
light.shadow = new THREE.PCSSShadow();
I prefer this model because at the moment there's no way to have multiple lights in the scene that use different shadowing approaches (and the light is where I'd expect to look for shadow information). This would likely be a much bigger change, though, because it would require restructuring shader code, I think?
@WestLangley
The fog names you have chosen define the curve that is used -- except the first one.
Based on @EliasHasle PRs... How about LinearFog
and DensityFog
?
You would be auto-assigning an environment map to a material when the user may not want it. Think of
MeshBasic
orMeshLambert
.
We could add a autoEnvMap
property in Background
? (Default true
)
@sunag
About shadow, this could not be
light.shadow
?light.shadow = new THREE.PCSSShadow();
@supereggbert
Maybe use
light.shadow
for type of shadow projection andlight.shadow.type
to define filtering/softness type.
That sounds good to me! I still think we need them to be functions though. Example:
light.shadow.type = new THREE.PCFShadow( { radius: 5 } );
@DanielSturk
Perhaps
ColorBackground
could include alpha, and this could completely replacerenderer.setClearColor()
andrenderer.setClearAlpha()
.
Sounds good to me!
@gkjohnson
It would be great to see the three js rendering primitives exposed more readily with
WebGLRenderer
using these to implement and provide a default rendering approach. But that's maybe a larger discussion.
Right now I'm trying to focus on helping users get better renders by default. Right now there is too much knowledge needed in order to do so.
I prefer this model because at the moment there's no way to have multiple lights in the scene that use different shadowing approaches (and the light is where I'd expect to look for shadow information). This would likely be a much bigger change, though, because it would require restructuring shader code, I think?
I think it's pretty straightforward actually!
How about
LinearFog
andDensityFog
?
Yes, I do like DensityFog
!
Now with that change, I think RangeFog
is best for the other case because the names are describing how the fog is parameterized.
As I argued elsewhere, I do not think we need two forms of density fog -- just one -- likely the form we were using previously, so there is no breakage.
We could add a autoEnvMap property in Background? (Default true)
I think is it just as easy to assign the background you want.
I think it's pretty straightforward actually!
Awesome! This would be a great addition.
Right now I'm trying to focus on helping users get better renders by default. Right now there is too much knowledge needed in order to do so.
Yeah I can appreciate that! It would just be nice to expose the "defaulted environment map" capability with a more generalized feature if possible but I understand your priorities.
I am OK with RangeFog
and DensityFog
(named according to parameterization), as long as there will be support for exponential fog. It could be supported by adding a mode
or boolean flag/param to DensityFog
, hooking onto the fogExp
/fogExp2
shader code that I wrote for #17355. That would be 👍 from me. 😃
Yes, I do like
DensityFog
!Now with that change, I think
RangeFog
is best for the other case because the names are describing how the fog is parameterized.
RangeFog
and DensityFog
it is!
We could add a autoEnvMap property in Background? (Default true)
I think is it just as easy to assign the background you want.
I don't think that's easy... The user now has to traverse the scenegraph and apply to the relevant meshes oir search the mesh by name in the scene.
I think it's more common that people just want it to work than not users that want custom reflections (or no reflections) per mesh.
Taking #17355 into account...
scene.fog = new THREE.RangeFog( color, near, far );
scene.fog = new THREE.DensityFog( color, density, squared );
I think I can modify #17355 to use the new API with minimal changes and gradual deprecation of older code. Maybe not in time for r109, though. Done. See #17592
Hi @mrdoob! It's been a long time and I'm wondering what's blocking the new design? We're hitting issues like https://github.com/mrdoob/three.js/issues/22764, https://github.com/mrdoob/three.js/issues/20819 and https://github.com/mrdoob/three.js/issues/15382, so are particularly interested in landing THREE.ColorBackground
and defining the color space of the background. If the design has been agreed upon, maybe I can get started with a PR?
I thought that we could also move WebGLRenderer.shadowMap to Scene as Scene.shadow so it can be serialised (and will allow us to do some API redesign too).
But does not it make more sense being in WebGLRenderer? If we are rendering different scenes in one application with one renderer, it would generally make sense to render them all with the same shadow parameters.
This cycle I was hoping to do some API design clean up in the
Scene
object.This is the current thinking:
Background
Taking #16900 and #17199 into account...
Note: When using
THREE.CubeTextureBackground
, all meshes in the scene will use it asmaterial.envMap
by default so it'll be easier for users to get good looking results.Fog
Taking #17355 into account...
Shadow
I thought that we could also move
WebGLRenderer.shadowMap
toScene
asScene.shadow
so it can be serialised (and will allow us to do some API redesign too).Suggestions? Improvements?
/cc @WestLangley @Mugen87 @bhouston @sunag @DanielSturk @supereggbert @EliasHasle