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

Fix bug with texture coordinate q value being 0 #3707

Merged
merged 7 commits into from Aug 31, 2016
Merged

Conversation

@OrN
Copy link
Member

OrN commented Mar 4, 2016

This exists only to get fifoci diffs and for JMC47 to play with for now.

Review on Reviewable

@OrN OrN changed the title [WIP] Fix bug with texture coordinate Z being 0 [WIP] Fix bug with texture coordinate Z being 0 with XF_TEXPROJ_STQ Mar 4, 2016
@JMC47

This comment has been minimized.

Copy link
Contributor

JMC47 commented Mar 5, 2016

The Lesson08 fifolog more or less shows what's going on with this.

@OrN OrN force-pushed the OrN:proj_stq-zz branch 4 times, most recently from bd6de40 to 2485e90 Mar 5, 2016
@JMC47

This comment has been minimized.

Copy link
Contributor

JMC47 commented Mar 7, 2016

This is the difference in the NSMBWii coins is a lie.

damn

@JMC47

This comment has been minimized.

Copy link
Contributor

JMC47 commented Mar 7, 2016

This is the most heavily tested change in the history of Dolphin. 50+ dols, multiple fifologs, screenshots vs hardware, and tons of crazy stuff.

The behavior doesn't affect any games, but fixes Lesson08 and a whole bevy of other texture matrix issues that games don't seem to care about.

@OrN OrN force-pushed the OrN:proj_stq-zz branch 3 times, most recently from fee161f to 254b6f0 Mar 11, 2016
@OrN OrN changed the title [WIP] Fix bug with texture coordinate Z being 0 with XF_TEXPROJ_STQ [WIP] Fix bug with texture coordinate Z value being 0 Mar 13, 2016
@OrN OrN force-pushed the OrN:proj_stq-zz branch 2 times, most recently from 077b906 to 7d6dba2 Mar 13, 2016
@OrN OrN force-pushed the OrN:proj_stq-zz branch 2 times, most recently from 3368b1b to ff37367 Mar 21, 2016
@OrN OrN closed this Mar 25, 2016
@AdmiralCurtiss

This comment has been minimized.

Copy link
Contributor

AdmiralCurtiss commented Mar 27, 2016

So what is actually going on with this PR?

@OrN OrN reopened this Mar 27, 2016
@OrN OrN force-pushed the OrN:proj_stq-zz branch from ff37367 to 2710b93 Mar 27, 2016
@OrN OrN changed the title [WIP] Fix bug with texture coordinate Z value being 0 [WIP] Fix bug with texture coordinate q value being 0 Mar 27, 2016
@JMC47

This comment has been minimized.

Copy link
Contributor

JMC47 commented Mar 27, 2016

Hardest part about this: finding a fucking game that used it. I told ya some developers would be inept enough to use it!

Fixes HB Arcade Disc Golf and HB Arcade Cards

Master
wl5ell-3

PR
wl5ell-2

@JMC47

This comment has been minimized.

Copy link
Contributor

JMC47 commented Mar 27, 2016

HB Arcade Cards
Master
wlgell-2
PR
wlgell-1

@OrN

This comment has been minimized.

Copy link
Member Author

OrN commented Mar 27, 2016

Lesson08 is the only true case.

@OrN OrN force-pushed the OrN:proj_stq-zz branch 3 times, most recently from 53a9eb2 to d7c172a Mar 27, 2016
@JMC47

This comment has been minimized.

Copy link
Contributor

JMC47 commented Mar 27, 2016

A previous version of this PR made Metroid Prime 3's power armor on Samus. But apparently it was bugged. I wonder if there's more to it.

@OrN OrN force-pushed the OrN:proj_stq-zz branch from d7c172a to 3637251 Mar 27, 2016
@OrN

This comment has been minimized.

Copy link
Member Author

OrN commented Mar 27, 2016

@JMC47 It wasn't being handled correctly

@BhaaLseN

This comment has been minimized.

Copy link
Member

BhaaLseN commented Mar 29, 2016

Sigh, switching to reviewable since keeping track is getting a little harder :P


Reviewed 1 of 2 files at r20, 2 of 3 files at r22.
Review status: all files reviewed at latest revision, 2 unresolved discussions.


Source/Core/VideoCommon/VertexShaderGen.cpp, line 309 [r22] (raw file):
Space after the if


Comments from the review on Reviewable.io

@OrN OrN force-pushed the OrN:proj_stq-zz branch from 5dde623 to 68ac65c Mar 29, 2016
@degasus

This comment has been minimized.

Copy link
Member

degasus commented Mar 30, 2016

Reviewed 1 of 3 files at r22, 1 of 1 files at r23.
Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.


Source/Core/VideoBackends/Software/TransformUnit.cpp, line 197 [r23] (raw file):
By the way, where is the else clause? I miss the division. This z==0 clause should be close to the division IMO


Comments from the review on Reviewable.io

@Parlane Parlane added the WIP label May 3, 2016
@JMC47

This comment has been minimized.

Copy link
Contributor

JMC47 commented Jul 10, 2016

Can this be rebased now that the feature freeze is over?

@JMC47

This comment has been minimized.

Copy link
Contributor

JMC47 commented Jul 22, 2016

Documenting this here because I remember us messing with Fishing Resort a bit -

" Speaking of the Fishing Resort Wii map is inverted thing
A while ago I looked in to that, and it looks like it is doing a perspective divide on the texture coordinates with the perspective divide value being 0.0 "

@OrN OrN force-pushed the OrN:proj_stq-zz branch 5 times, most recently from 8f4f01d to 763ea9a Jul 23, 2016
@OrN OrN force-pushed the OrN:proj_stq-zz branch from 763ea9a to f586140 Aug 30, 2016
@phire

This comment has been minimized.

Copy link
Member

phire commented Aug 30, 2016

I think everything looks good, you just need to replace those // TODO comment lines. It's fine to leave the // TODO check if it only affects XF_TEXGEN_REGULAR for another PR.

If you are worried about grammar in the comments, I can review that too (not spelling, someone else can review spelling).


Reviewed 1 of 3 files at r22, 1 of 2 files at r24, 1 of 1 files at r25.
Review status: all files reviewed at latest revision, 2 unresolved discussions.


Source/Core/VideoCommon/VertexShaderGen.cpp, line 385 [r25] (raw file):

check if it only affects XF_TEXGEN_REGULAR more


Comments from Reviewable

OrN added 6 commits Mar 4, 2016
This is causing the 0 divide case to run when source row is using tex0-7 and inputform is ABC1.
@OrN OrN force-pushed the OrN:proj_stq-zz branch from f586140 to e6ccd07 Aug 31, 2016
@OrN OrN changed the title [WIP] Fix bug with texture coordinate q value being 0 Fix bug with texture coordinate q value being 0 Aug 31, 2016
@OrN OrN force-pushed the OrN:proj_stq-zz branch from 9382076 to 94cbe0c Aug 31, 2016
@JMC47

This comment has been minimized.

Copy link
Contributor

JMC47 commented Aug 31, 2016

Can anyone review this. I've retested it and it fixes 4 games while breaking nothing I know about.

@phire

This comment has been minimized.

Copy link
Member

phire commented Aug 31, 2016

:lgtm:


Reviewed 2 of 2 files at r26.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

@JMC47

This comment has been minimized.

Copy link
Contributor

JMC47 commented Aug 31, 2016

I reviewed the code comments to make sure they were accurate and such. I also did the hw testing and have like 50 dols that prove this is accurate to hardware.

@CrossVR CrossVR merged commit 9a660fd into dolphin-emu:master Aug 31, 2016
11 checks passed
11 checks passed
code-review/reviewable 2 files reviewed
Details
default Very basic checks passed, handed off to Buildbot.
Details
lint Build succeeded on builder lint
Details
pr-android Build succeeded on builder pr-android
Details
pr-deb-dbg-x64 Build succeeded on builder pr-deb-dbg-x64
Details
pr-deb-x64 Build succeeded on builder pr-deb-x64
Details
pr-freebsd Build succeeded on builder pr-freebsd
Details
pr-osx-x64 Build succeeded on builder pr-osx-x64
Details
pr-ubu-x64 Build succeeded on builder pr-ubu-x64
Details
pr-win-dbg-x64 Build succeeded on builder pr-win-dbg-x64
Details
pr-win-x64 Build succeeded on builder pr-win-x64
Details
@dolphin-emu-bot

This comment has been minimized.

Copy link
Contributor

dolphin-emu-bot commented Sep 1, 2016

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

  • hb-discgolf on ogl-lin-intel: diff
  • inverted-depth-range on ogl-lin-intel: diff
  • last-story-shadows on ogl-lin-intel: diff
  • lesson08 on ogl-lin-intel: diff
  • rs3-bumpmapping on ogl-lin-intel: diff
  • hb-discgolf on ogl-lin-mesa: diff
  • inverted-depth-range on ogl-lin-mesa: diff
  • last-story-shadows on ogl-lin-mesa: diff
  • lesson08 on ogl-lin-mesa: diff
  • rs3-bumpmapping on ogl-lin-mesa: diff
  • hb-discgolf on ogl-lin-nv: diff
  • inverted-depth-range on ogl-lin-nv: diff
  • last-story-shadows on ogl-lin-nv: diff
  • lesson08 on ogl-lin-nv: diff
  • rs3-bumpmapping on ogl-lin-nv: diff
  • hb-discgolf on sw-lin-mesa: diff
  • inverted-depth-range on sw-lin-mesa: diff
  • last-story-shadows on sw-lin-mesa: diff
  • lesson08 on sw-lin-mesa: diff

automated-fifoci-reporter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
10 participants
You can’t perform that action at this time.