Fix D3D Forced Anisotropy #3672

EmptyChaos · 2016-02-25T10:14:55Z

The D3D backend is known to force anisotropic filtering on all textures when enabled which results in glitches that don't occur in the OpenGL backend under the same anisotropy level. I tried to trace the difference between the 2 implementations to find what was causing this and it seems to actually be an implementation-defined interaction between the anisotropy level and the texture min/mag filter setting in OpenGL. The Anisotropy specification says that the min/mag filters have an implementation-defined effect on the anisotropic filter; only the combination of GL_LINEAR+GL_LINEAR_MIPMAP_LINEAR has a clearly defined behavior as being "best effort". I guessed that nVidia's OpenGL drivers were implicitly disabling anisotropy when using GL_NEAREST as the filter so I patched D3D with that assumption to see what would happen and got this:

(Left is master; right is with this patch applied. Both are 16x Anisotropic in D3D11. The right image matches OpenGL)

I don't know anything about Flipper so this is just a proof of concept.

JMC47 · 2016-02-25T10:16:38Z

Considering that this is a fix to an enhancement and brings it to parity with OpenGL, this should be considered for 5.0. As long as anisotropic filtering off stays the same, I don't see any issue with at least giving this a good, hard look.

degasus · 2016-02-25T10:48:25Z

Sounds fine, but please do the same for OGL, also if this change has no effect.
https://github.com/dolphin-emu/dolphin/blob/master/Source/Core/VideoBackends/OGL/SamplerCache.cpp#L137

Mullin · 2016-02-25T15:00:56Z

This should fix Broken Sunrays in ZTP when using anisotropic filter.

https://wiki.dolphin-emu.org/images/7/72/TwilightPrincessGC_Sunrays_Broken.jpg

BhaaLseN · 2016-02-25T16:43:13Z

Did we ask fifoci for its opinion yet?

@dolphin-emu-bot rebuild

delroth · 2016-02-25T18:59:20Z

FifoCI doesn't run with anisotropic filtering enabled.

EmptyChaos · 2016-02-25T22:39:14Z

@Mullin I've taken a look at those and I don't think I can do anything about it. The way the game implements them is just not compatible with Anisotropic filtering; you'd need an Action Replay code to modify its implementation. The game sets TexMode0 to 0x00019A (ie. min_filter=4,mag_filter=1 = LINEAR+LINEAR) on the rays so it wants maximum filtering, but the angles on the geometry causes Anisotropic filtering to "correct" the image in an undesirable way giving that misrender. Maybe it's possible to do something in the shaders but I'm not an expert.

[The idea of this PR is to bring D3D to parity with OpenGL. If OpenGL w/ Anisotropic filtering was drawing correctly then this fixes it so D3D will also draw it correctly now. If OpenGL was drawing it wrong then nothing has changed either way.]

@BhaaLseN The non-anisotropic code paths are unchanged from master, FifoCI results should be identical.

Mullin · 2016-02-25T22:49:10Z

@EmptyChaos Okay, so sad :(

Source/Core/VideoBackends/D3D/D3DState.cpp

+	// Only use anisotropic filtering if one or both of the filters are set to Linear.
+	// If both filters are set to Point then using anisotropy is equivalent
+	// to "forced filtering" which will cause visual glitches.
+	if (state.max_anisotropy > 1 && (state.min_filter & 4 || state.mag_filter))


EmptyChaos · 2016-02-26T07:52:08Z

Review status: 0 of 7 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

Source/Core/VideoBackends/D3D/D3DState.cpp, line 265 [r1] (raw file):
Done.

Comments from the review on Reviewable.io

jroberts2010 · 2016-02-27T18:41:31Z

Would this fix the text in Mario Party 4-7 with AF enabled?

EmptyChaos · 2016-02-28T00:49:49Z

@straightflushin Possibly? Does it display correctly in OpenGL w/ Anisotropy (on an nVidia GPU, I don't know about AMD/Intel)? I don't have any Mario Partys to test, but I can take a look at a FIFO Log if you want to upload one.

jroberts2010 · 2016-02-28T02:50:27Z

Sorry, it looks like that bug only happens now with Force Texture Filtering enabled (on both backends).
Anisotropy is fine on those titles.

Source/Core/VideoBackends/OGL/SamplerCache.cpp

+		// Letting the game set other combinations will have varying arbitrary results;
+		// possibly being interpreted as equal to bilinear/trilinear, implicitly
+		// disabling anisotropy, or changing the anisotropic algorithm employed.
+		min_filter = (tm0.min_filter & 3) == TexMode0::TEXF_NONE ? GL_LINEAR : GL_LINEAR_MIPMAP_LINEAR;


delroth · 2016-02-28T23:51:11Z

@dolphin-emu-bot rebuild

EmptyChaos · 2016-02-29T21:27:33Z

I don't think I have any further changes to make. This is ready to be reviewed for merging.

JMC47 · 2016-03-02T18:21:38Z

LGTM in terms of what it fixes.

RisingFog · 2016-03-02T18:30:10Z

Can this PR be rebased to latest master?

EmptyChaos · 2016-03-03T01:42:58Z

Rebased on master.

RisingFog · 2016-03-15T20:54:24Z

Anything left holding this PR up?

degasus · 2016-03-23T14:45:04Z

Reviewed 1 of 4 files at r1, 2 of 6 files at r2, 4 of 8 files at r4, 4 of 4 files at r5.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

Source/Core/VideoBackends/OGL/SamplerCache.cpp, line 63 [r5] (raw file):
Is this line still fine?

Comments from the review on Reviewable.io

The D3D backend was always forcing Anisotropic filtering when that is enabled regardless of how the game chose to configure the texture filtering registers; this causes the same issues as "Force Filtering" without Anisotropy, such as causing game UI elements to no longer line up adjacent correctly. Historically, OpenGL's Anisotropy support has always worked "better" than D3D's due to seeming to not have this problem; unfortunately, OpenGL's Anisotropy specification only gives GL_LINEAR based filtering modes defined behavior, with only the mipmap setting being required to be considered. Some OpenGL implementations were implicitly disabling Anisotropy when the min/mag filters were set to GL_NEAREST, but this behavior is not required by the spec so cannot be relied on.

…nction

EmptyChaos · 2016-03-24T02:48:37Z

Review status: 2 of 11 files reviewed at latest revision, 1 unresolved discussion.

Source/Core/VideoBackends/OGL/SamplerCache.cpp, line 63 [r5] (raw file):
Probably not. min_filter should be 6 or 4, but that can produce 5 as well. I'll fix it.

Comments from the review on Reviewable.io

delroth · 2016-03-26T00:44:02Z

@EmptyChaos any progress?

EmptyChaos · 2016-03-26T01:05:34Z

@delroth I've already made the change. I screwed up the Reviewable by not clicking Publish.

Nucleoprotein · 2016-03-26T22:57:01Z

So neobrain and/or MaJoRoesch wasn't correct about this, I reported this issue long time ago : https://bugs.dolphin-emu.org/issues/7724 Thanks EmptyChaos.

delroth · 2016-03-26T23:20:27Z

neobrain is correct, this is in some sense a hack to disable anisotropy in
some situations to avoid unwanted effects.

On Sat, Mar 26, 2016 at 11:57 PM Robert Krawczyk notifications@github.com
wrote:

So neobrain and/or MaJoRoesch wasn't correct about this, I reported this
issue long time ago : https://bugs.dolphin-emu.org/issues/7724 Thanks
EmptyChaos.

—
You are receiving this because you modified the open/close state.

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

EmptyChaos · 2016-03-27T02:53:47Z

@delroth Not really. The screenshot I posted at the top actually also occurs with Anisotropic Filtering set to 1x (off) and Forced Texture Filtering set to On. The problem is specifically caused by forcing texture filtering when it is meant to be disabled, it isn't directly related to anisotropic filtering itself. You can cause similar issues in some commercial PC games by forcing Anisotropic Filtering from the Nvidia/Catalyst control panel.

Also, I think "fundamental difference between the APIs" may be overstating things a little.

A texture's maximum degree of anisotropy is specified independent
from the texture's minification and magnification filter (as
opposed to being supported as an entirely new filtering mode).
...  Implementations are also
permitted to ignore the minification or magnification filter and
implement the highest quality of anisotropic filtering possible.

"Implementations are also permitted to ignore ..." means this has always been broken in OpenGL as well.

CrossVR added this to the Dolphin Release 5.0 milestone Feb 25, 2016

EmptyChaos force-pushed the d3d-anisotropy branch from ff9df8e to 23c855d Compare February 25, 2016 12:43

lioncash reviewed Feb 25, 2016
View reviewed changes

EmptyChaos force-pushed the d3d-anisotropy branch 4 times, most recently from 6e29742 to f22981f Compare February 26, 2016 03:14

EmptyChaos force-pushed the d3d-anisotropy branch from f22981f to 83d3055 Compare February 28, 2016 00:40

EmptyChaos reviewed Feb 28, 2016
View reviewed changes

EmptyChaos force-pushed the d3d-anisotropy branch from 83d3055 to 5fd7db1 Compare February 28, 2016 11:21

EmptyChaos changed the title ~~[RFC] D3D Forced Anisotropy~~ Fix D3D Forced Anisotropy Feb 29, 2016

EmptyChaos mentioned this pull request Mar 1, 2016

D3D12: Fix uninitialized variable in SetSamplerState #3692

Merged

EmptyChaos force-pushed the d3d-anisotropy branch from 5fd7db1 to cb07e30 Compare March 3, 2016 01:38

EmptyChaos force-pushed the d3d-anisotropy branch from cb07e30 to f474dac Compare March 3, 2016 02:15

EmptyChaos added 2 commits March 24, 2016 13:43

VideoCommon: Refactor TexMode0 mipmaps disabled test into a helper fu…

0b9a72a

…nction

EmptyChaos force-pushed the d3d-anisotropy branch from f474dac to 0b9a72a Compare March 24, 2016 02:48

delroth merged commit 2fd0884 into dolphin-emu:master Mar 26, 2016

EmptyChaos deleted the d3d-anisotropy branch March 27, 2016 02:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix D3D Forced Anisotropy #3672

Fix D3D Forced Anisotropy #3672

EmptyChaos commented Feb 25, 2016

JMC47 commented Feb 25, 2016

degasus commented Feb 25, 2016

Mullin commented Feb 25, 2016

BhaaLseN commented Feb 25, 2016

delroth commented Feb 25, 2016

EmptyChaos commented Feb 25, 2016

Mullin commented Feb 25, 2016

This comment was marked as off-topic.

EmptyChaos commented Feb 26, 2016

jroberts2010 commented Feb 27, 2016

EmptyChaos commented Feb 28, 2016

jroberts2010 commented Feb 28, 2016

This comment was marked as off-topic.

This comment was marked as off-topic.

delroth commented Feb 28, 2016

EmptyChaos commented Feb 29, 2016

JMC47 commented Mar 2, 2016

RisingFog commented Mar 2, 2016

EmptyChaos commented Mar 3, 2016

RisingFog commented Mar 15, 2016

degasus commented Mar 23, 2016

EmptyChaos commented Mar 24, 2016

delroth commented Mar 26, 2016

EmptyChaos commented Mar 26, 2016

Nucleoprotein commented Mar 26, 2016

delroth commented Mar 26, 2016

EmptyChaos commented Mar 27, 2016

Fix D3D Forced Anisotropy #3672

Fix D3D Forced Anisotropy #3672

Conversation

EmptyChaos commented Feb 25, 2016

JMC47 commented Feb 25, 2016

degasus commented Feb 25, 2016

Mullin commented Feb 25, 2016

BhaaLseN commented Feb 25, 2016

delroth commented Feb 25, 2016

EmptyChaos commented Feb 25, 2016

Mullin commented Feb 25, 2016

This comment was marked as off-topic.

EmptyChaos commented Feb 26, 2016

jroberts2010 commented Feb 27, 2016

EmptyChaos commented Feb 28, 2016

jroberts2010 commented Feb 28, 2016

This comment was marked as off-topic.

This comment was marked as off-topic.

delroth commented Feb 28, 2016

EmptyChaos commented Feb 29, 2016

JMC47 commented Mar 2, 2016

RisingFog commented Mar 2, 2016

EmptyChaos commented Mar 3, 2016

RisingFog commented Mar 15, 2016

degasus commented Mar 23, 2016

EmptyChaos commented Mar 24, 2016

delroth commented Mar 26, 2016

EmptyChaos commented Mar 26, 2016

Nucleoprotein commented Mar 26, 2016

delroth commented Mar 26, 2016

EmptyChaos commented Mar 27, 2016