-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Shade mapping fix #5612
Shade mapping fix #5612
Conversation
Ah, this might be right. Why do you say "partially"? How does it look with this change? |
Hmm, well, if this is correct, wouldn't software transform need the change too? Also, it seems like the software renderer does actually do + for both, but it still doesn't look right according to the bug. So if this is wrong, maybe the software renderer is wrong. -[Unknown] |
Yeah, software transform would need the change too: uv[1] = (1.0f + Dot(lightpos1, normal))/2.0f; instead of uv[1] = (1.0f - Dot(lightpos1, normal))/2.0f; |
Yep , i think it is partially correct but sometimes reflection is not right.At least it is better than 1.0f minus |
For SW transform , if do the changes as @thedax mentioned above , it seems to be no blue color issue and renders correct. |
Why close this? I plan to merge this later today, after I'm done with the release. |
I see. That's would be good. |
I believe this is right, but hard to be 100% sure without a good test. |
@hrydgard @unknownbrackets Before (only the bottom portion of the car had reflections):- After (the whole car is covered, even the windshleld):- This pull request is definitely the right way to go. Thanks to @zschigi2 for the initial discovery. |
Reference
https://code.google.com/p/jpcsp/source/browse/trunk/src/jpcsp/graphics/shader.vert#208
Partially fix #5506