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

VideoCommon: Workaround Intel OS X bug again #3796

Merged
merged 2 commits into from
Apr 29, 2016

Conversation

endrift
Copy link
Contributor

@endrift endrift commented Apr 24, 2016

This time without the behavior change of the last patch. Reverts the last patch. Thanks, Apple. Thapple.


This change is Reviewable

@delroth delroth added this to the Dolphin Release 5.0 milestone Apr 24, 2016
@degasus
Copy link
Member

degasus commented Apr 24, 2016

Restores the old behavior + simplifys the code.

LGTM

@mimimi085181
Copy link
Contributor

So this would reintroduce the bugs that pr #3570 fixed? I take it, you tried to find a solution that works for all drivers? What about combing this revert with: pr #3784 ? (just a random guess)

If this reintroduces bugs that were fixed, would it be justified to add specific driver workarounds for 5.0?

@RisingFog
Copy link
Member

RisingFog commented Apr 24, 2016

@mimimi085181 it fixes the bug in a different way if I read it correctly

@endrift
Copy link
Contributor Author

endrift commented Apr 24, 2016

@mimimi085181 This does not reintroduce the old bug. It looks like the original bug had something to do with sequence points or order of operations in the code getting done wrong (?????), which would imply 100% that it's a driver bug in Intel. Regardless, this reverts a behavior change that wasn't shown to be 100% necessary while fixing the bug, so it's much nicer in that regard, and may fix the AMD bug as well (yet to be shown).

@RisingFog
Copy link
Member

@dolphin-emu-bot rebuild

@endrift
Copy link
Contributor Author

endrift commented Apr 26, 2016

I've just confirmed that this fixes the dff in https://bugs.dolphin-emu.org/issues/9468

@mimimi085181
Copy link
Contributor

Nice, now maybe some other systems should be tested as well. Like the ones that were broken before the orignal pr.

@endrift
Copy link
Contributor Author

endrift commented Apr 26, 2016

It doesn't affect the ea-vp6.dff on any platform I tested it on, I don't think.

@endrift
Copy link
Contributor Author

endrift commented Apr 27, 2016

Anything holding this up now?

@degasus
Copy link
Member

degasus commented Apr 27, 2016

Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@degasus
Copy link
Member

degasus commented Apr 28, 2016

Nope. @delroth ready to merge.

@phire
Copy link
Member

phire commented Apr 28, 2016

We still need to do hardware tests in the future, but LGTM

@RisingFog
Copy link
Member

@dolphin-emu-bot rebuild

@dolphin-emu-bot
Copy link
Contributor

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

  • rs3-bumpmapping on ogl-lin-intel: diff
  • sadx-ui on ogl-lin-intel: diff
  • smg2-fog on ogl-lin-intel: diff
  • sms-bubbles on ogl-lin-intel: diff
  • sonic-riders-blur on ogl-lin-intel: diff
  • rs3-bumpmapping on ogl-lin-mesa: diff
  • sadx-ui on ogl-lin-mesa: diff
  • smg2-fog on ogl-lin-mesa: diff
  • sms-bubbles on ogl-lin-mesa: diff
  • sonic-riders-blur on ogl-lin-mesa: diff

automated-fifoci-reporter

@lioncash lioncash merged commit 2d7dfa0 into dolphin-emu:master Apr 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
8 participants