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

RectAreaLight: cleanup and refactoring #11232

Merged
merged 1 commit into from
Apr 25, 2017
Merged

Conversation

WestLangley
Copy link
Collaborator

Cleanup, removal of unused code, and some refactoring. There is no change to the algorithm or rendered output.

Some of the refactoring is subjective, obviously, but this is a first step of several in making sure our units are correct.


Regarding our implementation of area lights: Although the LTC algorithm we are using can be applied to other BRDFs, the corresponding ltcMag and ltcMat data is based on the GGX distribution specifically, and is not appropriate for Phong.

Just as Lambert does not support area lights, I would recommend we remove the Phong area light support, too. This will further simplify the codebase.

Users can use MeshStandardMaterial and MeshPhysicalMaterial, instead.

We hope to support other area light types -- and near and distant light probes -- too, I expect, and having to support two similar specular models doesn't make a lot of sense.

There have been suggestions to remove Phong completely, since it really is redundant at this point, but keeping it around as a simple specular illumination model, with Lambert as a simple non-specular model, is reasonable, I guess.

@bhouston
Copy link
Contributor

The rect area light code-base is so huge, I would even split it into brdfs_area.glsl as it is taking over that brdf file and making it hard to find the other brdfs.

@bhouston
Copy link
Contributor

Is the rect area light code even a BRDF? It is a little confusing as it seems to be mostly about light integration rather than surface evaluation. I would maybe try to split out the light evaluation (which is light eval code, and should go in lights.glsl) from the surface evaluation (which should go into brdf.glsl) if possible.

I think with the LTC area lights it is a little mixed up though, so maybe cleaning that up isn't possible.

@mrdoob
Copy link
Owner

mrdoob commented Apr 25, 2017

Just as Lambert does not support area lights, I would recommend we remove the Phong area light support, too. This will further simplify the codebase.

Sounds good to me 👍

@mrdoob mrdoob merged commit 1558bf0 into mrdoob:dev Apr 25, 2017
@mrdoob
Copy link
Owner

mrdoob commented Apr 25, 2017

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants