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 cleanup #439

Merged
merged 4 commits into from Jun 21, 2014
Merged

Lighting cleanup #439

merged 4 commits into from Jun 21, 2014

Conversation

degasus
Copy link
Member

@degasus degasus commented May 30, 2014

This branch cleans up our lighting code:

  • ppl doesn't require to upload the consts twice
  • also for shaderManagers
  • use structs for lighting constants

#define I_TRANSFORMMATRICES "ctrmtx"
#define I_NORMALMATRICES "cnmtx"
#define I_POSTTRANSFORMMATRICES "cpostmtx"
#define I_DEPTHPARAMS "cDepth" // farZ, zRange

This comment was marked as off-topic.

@degasus
Copy link
Member Author

degasus commented Jun 5, 2014

Anything else to fix? I'd like to see this merged soon to port PR #394 to normalize on the CPU.

@degasus degasus mentioned this pull request Jun 5, 2014
@galop1n
Copy link
Contributor

galop1n commented Jun 5, 2014

It is not possible to normalise on cpu, it is done this way so an other
channel can use the light as a non directional diffuse too.
On Jun 5, 2014 8:02 AM, "Markus Wick" notifications@github.com wrote:

Anything else to fix? I'd like to see this merged soon to port PR #394
#394 to normalize on the CPU.


Reply to this email directly or view it on GitHub
#439 (comment).

@degasus
Copy link
Member Author

degasus commented Jun 5, 2014

You have normalized all accesses of this constant. So which other channel are you refering to?

@galop1n
Copy link
Contributor

galop1n commented Jun 5, 2014

Oh was thinking about the light pos params that can be used as a position
AND a direction in the same shader. Not fully awake sorry
On Jun 5, 2014 9:45 AM, "Markus Wick" notifications@github.com wrote:

You have normalized all accesses of this constant. So which other channel
are you refering to?


Reply to this email directly or view it on GitHub
#439 (comment).

@degasus
Copy link
Member Author

degasus commented Jun 5, 2014

That's why I want this PR to get merged before this to be done on the CPU. It will be less confusing as this parameters are splitted into a struct :)

};

#define I_COLORS "color"

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@neobrain
Copy link
Member

LGTM once the fixes are squashed into the previous commits.

@degasus
Copy link
Member Author

degasus commented Jun 19, 2014

@dolphin-emu-bot rebuild

1 similar comment
@neobrain
Copy link
Member

@dolphin-emu-bot rebuild

@neobrain
Copy link
Member

Looks like build went well, even though there's not definite "okay" from the buildbot ...

neobrain added a commit that referenced this pull request Jun 21, 2014
@neobrain neobrain merged commit fbca397 into dolphin-emu:master Jun 21, 2014
@degasus degasus deleted the lighting-fix branch July 12, 2014 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants