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

Cosine-based specular env map LOD selection #7414

Merged
merged 11 commits into from
Oct 30, 2015

Conversation

bhouston
Copy link
Contributor

Built upon the simplified lighting PR: #7324

I've implemented the cosine env map lod queries in this PR based on the ideas here: #7411

@bhouston bhouston changed the title Cosine-based specular env map LOD selection without dependencies on LOD extension [not yet done] Cosine-based specular env map LOD selection without dependencies on LOD extension Oct 23, 2015
@mrdoob
Copy link
Owner

mrdoob commented Oct 26, 2015

@WestLangley thoughts?

@WestLangley
Copy link
Collaborator

After things settle down, please.

@bhouston
Copy link
Contributor Author

I've updated this to work with the latest DEV branch and the other changes.

(Although I am very confused as to why a number of changes by @mrdoob in the dev branch appear in this PR -- is this github screwing up? I've never seen this before? Or is there a revert in the history?)

@bhouston
Copy link
Contributor Author

The weirdest thing is that if I do a straight compare between these two branches, it doesn't show these @mrdoob commits on dev branch -- it is proper, so I am fairly confused here:

dev...bhouston:cosine_envMap_lod

@mrdoob
Copy link
Owner

mrdoob commented Oct 29, 2015

I think it's github screwing up. It's been doing it for a while. I only look at the "Files Changed" section.

@mrdoob
Copy link
Owner

mrdoob commented Oct 29, 2015

Which, actually, shows that you're including the builds somehow.

@bhouston
Copy link
Contributor Author

I submitted some extra stuff by accident as well. I'll remove it now.

@mrdoob
Copy link
Owner

mrdoob commented Oct 29, 2015

Yeah. Try rebasing.

@bhouston
Copy link
Contributor Author

I removed two changes that probably are not necessary in this PR (although I didn't remove any builds, because there doesn't seem to be any) and it seems to be focused now: dev...bhouston:cosine_envMap_lod

@mrdoob
Copy link
Owner

mrdoob commented Oct 29, 2015

There is still a lot of noise in "Files changed". Hard to review until that it's clean...
https://github.com/mrdoob/three.js/pull/7414/files

@bhouston
Copy link
Contributor Author

Rebase done.

@bhouston bhouston changed the title Cosine-based specular env map LOD selection without dependencies on LOD extension Cosine-based specular env map LOD selection Oct 29, 2015
@mrdoob
Copy link
Owner

mrdoob commented Oct 29, 2015

/ping @WestLangley

@WestLangley
Copy link
Collaborator

This PR looks OK to me.

mrdoob added a commit that referenced this pull request Oct 30, 2015
Cosine-based specular env map LOD selection
@mrdoob mrdoob merged commit 7939105 into mrdoob:dev Oct 30, 2015
@mrdoob
Copy link
Owner

mrdoob commented Oct 30, 2015

Thanks guys!

@bhouston bhouston deleted the cosine_envMap_lod branch March 14, 2016 13:45
@@ -1,4 +1,4 @@
float calcLightAttenuation( float lightDistance, float cutoffDistance, float decayExponent ) {
float calcLightAttenuation( const in float lightDistance, const in float cutoffDistance, const in float decayExponent ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to have "const in" here really, it just leads to more verbose code and I've never seen it make a performance difference.

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