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

Generalize Reflector geometry #12631

Merged
merged 2 commits into from
Nov 12, 2017
Merged

Conversation

WestLangley
Copy link
Collaborator

Reflectors do not have to be rectangular. Any planar geometry will work.

The change here is trivial, but this PR will break other examples that depend on Reflector.


Ugh. There is some weird end-of-line character thingy going on that makes this PR seem bigger than it is. :/

@Mugen87
Copy link
Collaborator

Mugen87 commented Nov 11, 2017

The same should be true for Refractor and Water, too.

We should keep these objects equal so i think it's better to change and test them together.

@mrdoob
Copy link
Owner

mrdoob commented Nov 11, 2017

This is great!

@mrdoob
Copy link
Owner

mrdoob commented Nov 11, 2017

The change here is trivial, but this PR will break other examples that depend on Reflector.

I think that's ok in this case. I'll try to release the next version soon.

The same should be true for Refractor and Water, too.

Agreed! Do you want to do it in this PR or leave it for a different PR?

@WestLangley
Copy link
Collaborator Author

@Mugen87 @mrdoob I added additional commits as requested. I did not touch Water.js or Water2.js because I feel they can be considered separately -- unless I am missing something...

@mrdoob
Copy link
Owner

mrdoob commented Nov 12, 2017

Looks good! Merging for now.

@mrdoob mrdoob merged commit 820c61a into mrdoob:dev Nov 12, 2017
@mrdoob
Copy link
Owner

mrdoob commented Nov 12, 2017

Thanks!

@WestLangley WestLangley deleted the dev-reflector_geometry branch November 12, 2017 01:56
Mugen87 added a commit to Mugen87/three.js that referenced this pull request Nov 12, 2017
mrdoob added a commit that referenced this pull request Nov 15, 2017
Examples: Clean up Refractor and Water (see #12631)
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