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 Attenuation Fixes #1941

Merged
merged 2 commits into from Jan 22, 2015

Conversation

10 participants
@NanoByte011
Contributor

NanoByte011 commented Jan 21, 2015

Fixes a lot of Specular Highlight issues such as Issue 6398, 6169, 7693 and possibly more!

Would like to find a real game case for the Spot Light Attenuation and test if that's ok too.

@magumagu

This comment has been minimized.

Show comment
Hide comment
@magumagu

magumagu Jan 21, 2015

Contributor

@NanoByte011 are you seriously having trouble finding a game that uses LIGHTATTN_SPOT? I tried a few games, and every one of them used it somewhere...

Contributor

magumagu commented Jan 21, 2015

@NanoByte011 are you seriously having trouble finding a game that uses LIGHTATTN_SPOT? I tried a few games, and every one of them used it somewhere...

@Linktothepast

This comment has been minimized.

Show comment
Hide comment
@Linktothepast

Linktothepast Jan 22, 2015

Contributor

It seems to fix car reflections in the need for speed series too, issue 8072.

Contributor

Linktothepast commented Jan 22, 2015

It seems to fix car reflections in the need for speed series too, issue 8072.

@NanoByte011

This comment has been minimized.

Show comment
Hide comment
@NanoByte011

NanoByte011 Jan 22, 2015

Contributor

The FifoCI thing is cool... is there a threshold setting? Because some of these look like they don't have a diff, even though it thinks there is.

Contributor

NanoByte011 commented Jan 22, 2015

The FifoCI thing is cool... is there a threshold setting? Because some of these look like they don't have a diff, even though it thinks there is.

@Sonicadvance1

This comment has been minimized.

Show comment
Hide comment
@Sonicadvance1

Sonicadvance1 Jan 22, 2015

Contributor

Yea, the threshold is if there is a different pixel value anywhere in the image then it is different.

Contributor

Sonicadvance1 commented Jan 22, 2015

Yea, the threshold is if there is a different pixel value anywhere in the image then it is different.

@JMC47

This comment has been minimized.

Show comment
Hide comment
@JMC47

JMC47 Jan 22, 2015

Contributor

@NanoByte011 Did you notice some of the lighting stuff in Mario tennis isn't fixed? Like the scores and whatnot? Are those another lighting glitch?

Contributor

JMC47 commented Jan 22, 2015

@NanoByte011 Did you notice some of the lighting stuff in Mario tennis isn't fixed? Like the scores and whatnot? Are those another lighting glitch?

@NanoByte011

This comment has been minimized.

Show comment
Hide comment
@NanoByte011

NanoByte011 Jan 22, 2015

Contributor

@JMC47 Yeah, I did notice, those seem to be a different issue, the preview court before you start the match is dark too. :(
@magumagu yeah, that code is hit in different games for spot, but I'm looking for a good test case were you can tell it's a spot light and there is an issue or if it works ok... I just want to verify the results are correct! :)
@Sonicadvance1 well there may be slight pixel differences because the lighting calc was off, but on my laptop here I can't see which pixel is different on some, does the "Bright" feature work? Could be hard to see on my laptop... just want to make sure the diff is working correctly.

Contributor

NanoByte011 commented Jan 22, 2015

@JMC47 Yeah, I did notice, those seem to be a different issue, the preview court before you start the match is dark too. :(
@magumagu yeah, that code is hit in different games for spot, but I'm looking for a good test case were you can tell it's a spot light and there is an issue or if it works ok... I just want to verify the results are correct! :)
@Sonicadvance1 well there may be slight pixel differences because the lighting calc was off, but on my laptop here I can't see which pixel is different on some, does the "Bright" feature work? Could be hard to see on my laptop... just want to make sure the diff is working correctly.

@JMC47

This comment has been minimized.

Show comment
Hide comment
@JMC47

JMC47 Jan 22, 2015

Contributor

@NanoByte011: Figures on the Mario Tennis thing. Dolphin's Lighting tends to be bad at doing stuff.

Contributor

JMC47 commented Jan 22, 2015

@NanoByte011: Figures on the Mario Tennis thing. Dolphin's Lighting tends to be bad at doing stuff.

@magumagu

This comment has been minimized.

Show comment
Hide comment
@magumagu

magumagu Jan 22, 2015

Contributor

Hmm... the Wind Waker title screen seems like a decent example?

Contributor

magumagu commented Jan 22, 2015

Hmm... the Wind Waker title screen seems like a decent example?

@NanoByte011

This comment has been minimized.

Show comment
Hide comment
@NanoByte011

NanoByte011 Jan 22, 2015

Contributor

@magumagu hmmm... it's more like a game where it's a spotlight and there is incorrect specular highlights... ie a flash light and then there is no shine on a door knob for example... maybe Luigi's Mansion might have lighting like this. But yeah any ideas on a game with lighting like that, where we can check to determine if Dolphin is doing it right would be good.

Contributor

NanoByte011 commented Jan 22, 2015

@magumagu hmmm... it's more like a game where it's a spotlight and there is incorrect specular highlights... ie a flash light and then there is no shine on a door knob for example... maybe Luigi's Mansion might have lighting like this. But yeah any ideas on a game with lighting like that, where we can check to determine if Dolphin is doing it right would be good.

@JMC47

This comment has been minimized.

Show comment
Hide comment
@JMC47

JMC47 Jan 22, 2015

Contributor

This also seems to finish off the remaining lighting quirks in MegaMan Network Transmission.

Contributor

JMC47 commented Jan 22, 2015

This also seems to finish off the remaining lighting quirks in MegaMan Network Transmission.

@NanoByte011

This comment has been minimized.

Show comment
Hide comment
@NanoByte011

NanoByte011 Jan 22, 2015

Contributor

Good stuff, feeling pretty good about killing multiple bugs in one shot! lol
magumagu did show me that in SMG, the Mii faces on the file select screen do show up brighter with this PR, so not sure what's up with that... Mii rendering seems to be problematic at any rate... I wonder if the data coming in for them is correct or if they have some special stuff to them that we aren't doing right.

Contributor

NanoByte011 commented Jan 22, 2015

Good stuff, feeling pretty good about killing multiple bugs in one shot! lol
magumagu did show me that in SMG, the Mii faces on the file select screen do show up brighter with this PR, so not sure what's up with that... Mii rendering seems to be problematic at any rate... I wonder if the data coming in for them is correct or if they have some special stuff to them that we aren't doing right.

Cleaned up whitespace
Fixed Directional Attenuation (assumed, data was light dir vector already, but it was not!)
@NanoByte011

This comment has been minimized.

Show comment
Hide comment
@NanoByte011

NanoByte011 Jan 22, 2015

Contributor

All should be good now, let me know if anyone finds any lighting issues this PR causes

Contributor

NanoByte011 commented Jan 22, 2015

All should be good now, let me know if anyone finds any lighting issues this PR causes

@degasus

This comment has been minimized.

Show comment
Hide comment
@degasus

degasus Jan 22, 2015

Member

Wow, how could it be that our lighting code was just /so/ wrong...

Member

degasus commented Jan 22, 2015

Wow, how could it be that our lighting code was just /so/ wrong...

@FioraAeterna

This comment has been minimized.

Show comment
Hide comment
@FioraAeterna

FioraAeterna Jan 22, 2015

Contributor

Yeah, wow, that's /embarrassing/ levels of bad. Awesome work!

Contributor

FioraAeterna commented Jan 22, 2015

Yeah, wow, that's /embarrassing/ levels of bad. Awesome work!

delroth added a commit that referenced this pull request Jan 22, 2015

@delroth delroth merged commit 4984215 into dolphin-emu:master Jan 22, 2015

10 checks passed

default Very basic checks passed, handed off to Buildbot.
Details
lint Build succeeded on the Buildbot.
Details
pr-android Build succeeded on the Buildbot.
Details
pr-deb-dbg-x64 Build succeeded on the Buildbot.
Details
pr-deb-x64 Build succeeded on the Buildbot.
Details
pr-osx-x64 Build succeeded on the Buildbot.
Details
pr-ubu-nogui-x64 Build succeeded on the Buildbot.
Details
pr-ubu-x64 Build succeeded on the Buildbot.
Details
pr-win-dbg-x64 Build succeeded on the Buildbot.
Details
pr-win-x64 Build succeeded on the Buildbot.
Details
@neobrain

This comment has been minimized.

Show comment
Hide comment
@neobrain

neobrain Jan 22, 2015

Member

Did you happen to take a quick look into the software renderer and check, if that one has similar issues? At least the mario tennis fifolog looks suspiciously similar.

Member

neobrain commented Jan 22, 2015

Did you happen to take a quick look into the software renderer and check, if that one has similar issues? At least the mario tennis fifolog looks suspiciously similar.

@dolphin-emu-bot

This comment has been minimized.

Show comment
Hide comment
@dolphin-emu-bot

dolphin-emu-bot Jan 22, 2015

Contributor

FifoCI detected that this change impacts graphical rendering. Here are the behavior differences detected by the system:

  • chibi-robo-zfighting on ogl-lin-mesa: diff
  • fifa-street on ogl-lin-mesa: diff
  • mario-tennis-menu on ogl-lin-mesa: diff
  • melee-lighting on ogl-lin-mesa: diff
  • mtennis-zfreeze on ogl-lin-mesa: diff
  • rs2-zfreeze on ogl-lin-mesa: diff
  • sms-bubbles on ogl-lin-mesa: diff
  • chibi-robo-zfighting on ogl-lin-nv: diff
  • mario-tennis-menu on ogl-lin-nv: diff
  • melee-lighting on ogl-lin-nv: diff
  • mtennis-zfreeze on ogl-lin-nv: diff
  • rs2-zfreeze on ogl-lin-nv: diff
  • sms-bubbles on ogl-lin-nv: diff

automated-fifoci-reporter

Contributor

dolphin-emu-bot commented Jan 22, 2015

FifoCI detected that this change impacts graphical rendering. Here are the behavior differences detected by the system:

  • chibi-robo-zfighting on ogl-lin-mesa: diff
  • fifa-street on ogl-lin-mesa: diff
  • mario-tennis-menu on ogl-lin-mesa: diff
  • melee-lighting on ogl-lin-mesa: diff
  • mtennis-zfreeze on ogl-lin-mesa: diff
  • rs2-zfreeze on ogl-lin-mesa: diff
  • sms-bubbles on ogl-lin-mesa: diff
  • chibi-robo-zfighting on ogl-lin-nv: diff
  • mario-tennis-menu on ogl-lin-nv: diff
  • melee-lighting on ogl-lin-nv: diff
  • mtennis-zfreeze on ogl-lin-nv: diff
  • rs2-zfreeze on ogl-lin-nv: diff
  • sms-bubbles on ogl-lin-nv: diff

automated-fifoci-reporter

@NanoByte011

This comment has been minimized.

Show comment
Hide comment
@NanoByte011

NanoByte011 Jan 22, 2015

Contributor

Thanks all, there are still some lighting issues to solve (ie Mario Tennis preview court and the Score are still dark and confirmation that spot light specular is working in any games).

@neobrain yeah I did briefly see the same problems in the SW Renderer with Mario Tennis in a test... so I assume it's broken as well. I can have a look at it for more future fixes on lighting.

Contributor

NanoByte011 commented Jan 22, 2015

Thanks all, there are still some lighting issues to solve (ie Mario Tennis preview court and the Score are still dark and confirmation that spot light specular is working in any games).

@neobrain yeah I did briefly see the same problems in the SW Renderer with Mario Tennis in a test... so I assume it's broken as well. I can have a look at it for more future fixes on lighting.

@NanoByte011 NanoByte011 deleted the NanoByte011:lighting_attn branch Jan 26, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment