-
Notifications
You must be signed in to change notification settings - Fork 868
Update Neutral Lighting environment (aesthetics) #2249
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
Conversation
update to latest master branch
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
Can you give a little more detail on why this change is important? It feels a little arbitrary. Also, why have you changed from one light on the left to two? |
If you look at the chair example that's the most obvious, but partially observable on the mixer as well. The current neutral lighting (left) produces a broad wash of hot highlights, which I identified was caused by the backside of the area light (big piece). It's not visually pleasing or behavior of how material reacts to lights, per our production team experts. My solution is to break the light into two, reduce the size, move them bit further apart, which results in breaking up the big area of hot specular into smaller highlights, so the highlights are softer now (picture right). I then adjust all the light values to make sure the coloration and RGB values remain consistent to the current neutral lighting setup. Let me know if that makes sense to you. |
Interesting, thanks. A couple things:
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
Thank you for the explanation and comparisons! I think I'm convinced now. I'm not sure why it's not picking up the CLA you signed last time; can you try to sign it again? |
I'm just wondering, any of the pictures you show are from the back face of the model? Edit: |
The strength from above for the reflective specularity is exactly what this fix is about. It's not coming from top per se, but from the back of the area light as I found out...so that's fixed in this PR. In addition, I double-checked the top light value to make sure the RBG value on color chart is consistent as the sides. The front view: with the back wall's area light splits into two lights, you can see the top refection/ specularity is softened.
The back view: with the back wall's area light remains as a single area light, you can see the top refection/ specularity are the same (i.e. not softend)
the top view, the RGB values are same/ similar to the side faces.
|
@thomastgt, thanks a lot for the clarification and for the time spent on it!! With the pictures you've shown me, the use cases where my team would need a different HDR can be solved with this new setting |
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
@googlebot I signed it!
From: googlebot ***@***.***>
Sent: Monday, April 19, 2021 1:12 PM
To: google/model-viewer ***@***.***>
Cc: Thomas.Huang ***@***.***>; Mention ***@***.***>
Subject: [EXTERNAL] Re: [google/model-viewer] Update Neutral Lighting environment (aesthetics) (#2249)
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
📝 Please visit https://cla.developers.google.com/ to sign.
Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.
…________________________________
What to do if you already signed the CLA
Individual signers
* It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data<https://cla.developers.google.com/clas> and verify that your email is set on your git commits<https://help.github.com/articles/setting-your-email-in-git/>.
Corporate signers
* Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot<http://go/cla#troubleshoot> (Public version<https://opensource.google/docs/cla/#troubleshoot>).
* The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data<https://cla.developers.google.com/clas> and verify that your email is set on your git commits<https://help.github.com/articles/setting-your-email-in-git/>.
* The email used to register you as an authorized contributor must also be attached to your GitHub account<https://github.com/settings/emails>.
ℹ️ Googlers: Go here<https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2Fgoogle%2Fmodel-viewer%2Fpull%2F2249> for more info.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#2249 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ASB5VZIP5ROOH6XMZLJWGT3TJRW7FANCNFSM42IE44IQ>.
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
add description on +x
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
At long last! Sorry for the delay; it was a bug in the CLA-bot. |
Great that it has been merged - been waiting for this! Thanks. |
Yes, and it's a modification to the neutral lighting. While technically breaking, since the visual change is fairly small and it's a fairly new feature, we decided to allow it. |
Thank's for claryfing :) |
Reference Issue
The update on the neutral lighting is to keep the lighting value on color chart closely consistent with the current neutral lighting, but reduce the big splash of specular highlights. See the comparison of the current neutral lighting versus new new adjustment.