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

Lighting #46

Merged
merged 5 commits into from
Aug 31, 2015
Merged

Lighting #46

merged 5 commits into from
Aug 31, 2015

Conversation

JayFoxRox
Copy link
Collaborator

I'm aware that this is quite a bit of code and it could possibly break tons of stuff [spotlights assert + lighting could turn a lot of otherwise working things black].
I'm tired of rebasing this all the time - I was going to add spotlights before merging this and was worried it would break a ton of games.
However, @JohnGodgames tested 100+ games with this and the majority still seems to work and look way better.

Some formulas and used variables might not be perfect now, but it's better than nothing? Mhkay?

@espes
Copy link
Owner

espes commented Aug 31, 2015

Looks good, thanks!

espes added a commit that referenced this pull request Aug 31, 2015
@espes espes merged commit 7e2a5dd into espes:xbox Aug 31, 2015
@JayFoxRox JayFoxRox deleted the lighting branch August 31, 2015 19:32
@JayFoxRox
Copy link
Collaborator Author

This caused a regression in Smashing Drive.
Intro logos are missing and the sky in the first scene is also broken.

/* lightLocalRange will be 1e+30 here */

qstring_append_fmt(h,
"uniform vec3 lightInfiniteHalfVector%d;\n"

Choose a reason for hiding this comment

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

Are these two infinite vectors initialized anywhere?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They are uniforms?

@@ -4496,6 +4674,11 @@ static void pgraph_method(NV2AState *d,
pg->constants[59].dirty = true;
break;

case NV097_SET_EYE_POSITION ...
NV097_SET_EYE_POSITION + 12:

Choose a reason for hiding this comment

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

+12 implies DWORD offsets 0, 4, 8 and 12 are handled here. But, is eye a vec4 or a vec3? In case of vec3, the fourth write could corrupt another register...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is also declared as a four component vector. I don't remember why right now, but I'd assume this might go through some matrix or is normalized somehow (or w is simply ignored).

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