Project

General

Profile

Actions

Emulator Issues #5606

closed

Okami menu differences between OpenGL and Directx

Added by eclipse606 over 11 years ago.

Status:
Fixed
Priority:
Normal
Assignee:
% Done:

0%

Operating system:
N/A
Issue type:
Bug
Milestone:
Regression:
No
Relates to usability:
No
Relates to performance:
No
Easy:
No
Relates to maintainability:
No
Regression start:
Fixed in:

Description

  1. Game Name and ID (as it appears in right click > properties:
    ROWE08

2) What is the expected output? What do you see instead?
OpenGl displays menus more correctly than directx. likely and optimization that was not port over to directx.

3) Did the game ever work correctly (i.e. not have this problem) on an
earlier version of dolphin? Please specify the exact revision when the
problem began.
Menus used to be worse.

4) What steps will reproduce the problem?

  1. Load Game
  2. Open Menu

5) What version of dolphin are you using (32bit/64bit along with the
version as it appears in the title bar, etc)?
3.0-735
3.0-765

6) Please provide any additional information below.
Windows 7 64bit
AMD Radeon HD 6850 Crossfire x2

7) Attachments. IMPORTANT! We have a limited storage quota on
GoogleCode, so please use a 3rd party host for screenshots or any other
OpenGL:http://imageshack.us/photo/my-images/850/okamimenuopengl.jpg/
Directx:http://imageshack.us/photo/my-images/338/okamimenudirectx.jpg/

Actions #1

Updated by NeoBrainX over 11 years ago

What do you mean with Directx? we have a D3D9 and a D3D11 backend. Be more precise with your information, please.

Actions #2

Updated by eclipse606 over 11 years ago

srry, it happens in both directx 9 and 11. also i forgot to say im using 64-bit versions of Dolphin.

Actions #3

Updated by Autoran1 over 11 years ago

This issue looks interesting, i will investigate.

Actions #4

Updated by NeoBrainX over 11 years ago

Could anyone provide a 2 frame fifo log showing the issue?

Actions #5

Updated by Autoran1 over 11 years ago

Well let's start with fact that DX11 was never good, since there was no Zcomploc code in VertexManager.cpp
DX11 always was like this and it's still remain like this
http://i.minus.com/i4umn8bJ81C76.png
http://i.minus.com/ibrMUnH9VbJggR.png
http://i.minus.com/ibmhdfjnKVYRab.png
the icons were centered in all backends as it shown here
http://i.minus.com/idp5tRyfV8Sj9.png
until the when their position was corrected in rbfde41895f52
For OGL and moved for DX9 and DX11
Until r0efd4e5c2976 Okami menu actually looked PERFECTLY on OGL and DX9except icons
after it and until r08a9c6603712 there are the same issues as in DX11 but could be fixed by patch which is here
Right now all backends are in the same state as DX11 except one thing they're all reacting on Enabling PerPixelDepth Was never before the reverting commit r08a9c6603712 as it shown here
http://i.minus.com/iZTBCd2gk4PCs.png
http://i.minus.com/ibzpwcJctj9unL.png
http://i.minus.com/ibhZQIReNTUOcT.png
http://i.minus.com/icRJd1FUakZOf.png
I think some sort of alphatest is required here
I actually miss rodolfo's Zcomploc

Actions #6

Updated by Autoran1 over 11 years ago

I've made more tests
Basing on commit message from rbfde41895f52 i've reverted gx-optimizations branch and it actually makes Icons in menu look perfect on DX9 and DX11 as well
There were 5 commits in the branch i'll try to minimize the info and find the one

Actions #7

Updated by NeoBrainX over 11 years ago

You might want to experiment with this "if": http://code.google.com/p/dolphin-emu/source/browse/Source/Core/VideoCommon/Src/BPStructs.cpp#135

If adding one or two specific BP commands to that condition fixes the issue it should be really easy to fix this properly.

Actions #8

Updated by Autoran1 over 11 years ago

Sorry i'm not a coder, and i'm not sure how exactly i must experiment with this, but i've reverted just BPStructs.cpp nad issue still here, than just XFStructs and issue gone

Actions #9

Updated by NeoBrainX over 11 years ago

Ok, nevermind the BPStructs thing then, I guess. Can you check which of the changes to XFStructs broke stuff? It's either in one of the "case" blocks or one of the two code blocks around an XFMemWritten call.

Actions #11

Updated by Autoran1 over 11 years ago

Ok, r5a77cae2e343
reverting first "case" block should do it

Actions #12

Updated by delroth over 11 years ago

  • Status changed from New to Accepted

Autoran1: do you mean the XFMEM_SETVIEWPORT in XFStructs.cpp?

Does reverting only this part of the change fix the issue?

Actions #13

Updated by NeoBrainX over 11 years ago

What happens if you replace
"u8 size = std::min(transferSize, 6 * 4);
if (memcmp((u32*)&xfregs + (address - 0x1000), pData + dataIndex, size))
{
VertexManager::Flush();
VertexShaderManager::SetViewportChanged();
PixelShaderManager::SetViewportChanged();
}
nextAddress = XFMEM_SETVIEWPORT + 6;"

with
a) "u8 size = std::min(transferSize, 6 * 4);
VertexManager::Flush();
if (memcmp((u32*)&xfregs + (address - 0x1000), pData + dataIndex, size))
{
VertexShaderManager::SetViewportChanged();
PixelShaderManager::SetViewportChanged();
}
nextAddress = XFMEM_SETVIEWPORT + 6;"

b) "u8 size = std::min(transferSize, 6 * 4);
VertexManager::Flush();
VertexShaderManager::SetViewportChanged();
if (memcmp((u32*)&xfregs + (address - 0x1000), pData + dataIndex, size))
{
PixelShaderManager::SetViewportChanged();
}
nextAddress = XFMEM_SETVIEWPORT + 6;"

c) "u8 size = std::min(transferSize, 6 * 4);
VertexManager::Flush();
PixelShaderManager::SetViewportChanged();
if (memcmp((u32*)&xfregs + (address - 0x1000), pData + dataIndex, size))
{
VertexShaderManager::SetViewportChanged();
}
nextAddress = XFMEM_SETVIEWPORT + 6;"

d) "u8 size = std::min(transferSize, 6 * 4);
PixelShaderManager::SetViewportChanged();
VertexShaderManager::SetViewportChanged();
if (memcmp((u32*)&xfregs + (address - 0x1000), pData + dataIndex, size))
{
VertexManager::Flush();
}
nextAddress = XFMEM_SETVIEWPORT + 6;"

?

Actions #14

Updated by NeoBrainX over 11 years ago

Sorry, d) should've actually been

d) "u8 size = std::min(transferSize, 6 * 4);
if (memcmp((u32*)&xfregs + (address - 0x1000), pData + dataIndex, size))
{
VertexManager::Flush();
}
PixelShaderManager::SetViewportChanged();
VertexShaderManager::SetViewportChanged();
nextAddress = XFMEM_SETVIEWPORT + 6;"

Actions #15

Updated by NeoBrainX over 11 years ago

Meh, sorry for spamming, but before you test anything, can you test THIS:

Move the whole if block (~10 lines of code) below the next if block, i.e. between lines 454 and 455.

Actions #16

Updated by Autoran1 over 11 years ago

Moved setviewport block after setprojection
didn't work
a) doesn't work
b) works
c) doesn't work
d1) works
d2) works
And basing on all 3 working
here's
u8 size = std::min(transferSize * 4, 6 * 4);
VertexShaderManager::SetViewportChanged();
if (memcmp((u32*)&xfregs + (address - 0x1000), pData + dataIndex, size))
{
VertexManager::Flush();
PixelShaderManager::SetViewportChanged();
}

nextAddress = XFMEM_SETVIEWPORT + 6;
Also workable

Actions #17

Updated by NeoBrainX over 11 years ago

Uh, sorry no idea how I missed the link, but what I actually meant to write before:

"

Meh, sorry for spamming, but before you test anything, can you test THIS:

http://code.google.com/p/dolphin-emu/source/browse/Source/Core/VideoCommon/Src/VertexShaderManager.cpp#311

Move the whole if block (~10 lines of code) at line 311 below the next if block, i.e. between lines 454 and 455."

Could you test that one ? :)

Actions #18

Updated by Autoran1 over 11 years ago

Moved "if viewport" block after "if projection" one, and nothing issue still here

Actions #19

Updated by NeoBrainX over 11 years ago

Got one more...
Insert "UpdateViewport(s_viewportCorrection);"
between line 453 and 454 (i.e. between the inner two closing brackets). Nothing else should've been changed.

Actions #20

Updated by Autoran1 over 11 years ago

Nothing again

Actions #21

Updated by NeoBrainX over 11 years ago

Uh, did you insert it in VertexShaderManager.cpp or in XFStructs.cpp? :D

Actions #22

Updated by Autoran1 over 11 years ago

in VertexShaderManager.cpp, there are no lines 453 and 454 in XFStructs.cpp only 314

Actions #23

Updated by NeoBrainX over 11 years ago

Yeah, just wanted to make sure.. xD

Well, thanks for testing so far, I kinda ran out of ideas now.. :p Maybe we'll be able to resolve this issue at some point tho..

Actions #24

Updated by Autoran1 over 11 years ago

Ok, will use hackish solution for now:)

Actions #25

Updated by NeoBrainX over 11 years ago

Okay one more maybe:
Try exchanging the two if blocks in VertexShaderManager.cpp like suggested in comment #19, but additionally change "if (bProjectionChanged)" to "if (bProjectionChanged || bViewportChanged)"

Actions #26

Updated by Autoran1 over 11 years ago

one more test in XFStructs
JUST moving the line VertexShaderManager::SetProjectionChanged(); in "setprojection" block
like previously we did in viewport block but DOING NOTHING with it this time

u8 size = std::min(transferSize * 4, 7 * 4);
VertexShaderManager::SetProjectionChanged();
if (memcmp((u32*)&xfregs + (address - 0x1000), pData + dataIndex, size))
{
VertexManager::Flush();
}

nextAddress = XFMEM_SETPROJECTION + 7;

Also fixes the issue

Actions #27

Updated by Autoran1 over 11 years ago

and nothing again

Actions #28

Updated by NeoBrainX over 11 years ago

Ugh, I'm an idiot, sorry. That couldn't have worked.

Try replacing
"if (bViewportChanged)
{
bViewportChanged = false;
SetVSConstant4f(C_DEPTHPARAMS,
xfregs.viewport.farZ / 16777216.0f,
xfregs.viewport.zRange / 16777216.0f,
-1.f / (float)g_renderer->EFBToScaledX((int)ceil(2.0f * xfregs.viewport.wd)),
1.f / (float)g_renderer->EFBToScaledY((int)ceil(-2.0f * xfregs.viewport.ht)));
// This is so implementation-dependent that we can't have it here.
UpdateViewport(s_viewportCorrection);
bProjectionChanged = true;
}

    if (bProjectionChanged)"

with

"
bool my_temp_var = bViewportChanged;
if (bViewportChanged)
{
bViewportChanged = false;
SetVSConstant4f(C_DEPTHPARAMS,
xfregs.viewport.farZ / 16777216.0f,
xfregs.viewport.zRange / 16777216.0f,
-1.f / (float)g_renderer->EFBToScaledX((int)ceil(2.0f * xfregs.viewport.wd)),
1.f / (float)g_renderer->EFBToScaledY((int)ceil(-2.0f * xfregs.viewport.ht)));
// This is so implementation-dependent that we can't have it here.
UpdateViewport(s_viewportCorrection);
bProjectionChanged = true;
}

    if (bProjectionChanged || my_temp_var)

"

i.e. first and last line changed. That's the last one from my side for today :p

Actions #29

Updated by Autoran1 over 11 years ago

Unfortunately still nothing....

Actions #30

Updated by NeoBrainX over 11 years ago

Meh okay, thanks anyway :)

Actions #31

Updated by Autoran1 over 11 years ago

At least we've found the problem

Actions #32

Updated by NeoBrainX over 11 years ago

http://code.google.com/p/dolphin-emu/source/browse/Source/Core/VideoCommon/Src/VertexManagerBase.cpp#162

Try inserting a g_renderer->RestoreAPIState(); there? :D (before vFlush is called)

Actions #33

Updated by NeoBrainX over 11 years ago

Sorry, actually you should insert this:
Matrix44 somematrix;
g_renderer->UpdateViewport(somematrix);
before the vFlush call.

Actions #34

Updated by Autoran1 over 11 years ago

Holy Jesus!!!!
UNFUCKINGBelieveable!!!!
You did it! it works finally!
http://i.minus.com/iO5Drc6ypMXrH.jpg
And look at the screen it almost 4 AM at my place :P

Actions #35

Updated by NeoBrainX over 11 years ago

Yeah, not so fast. I got a clue what's wrong now, but it's not the final fix :)

@ delroth: One of {EFB copies, EFB access, any other stuff messing with the viewport} is setting a viewport and not restoring it. So when flushing our pipeline, we end up using some random previous viewport but not one which was actually set by GX.

Actions #36

Updated by Autoran1 over 11 years ago

Anyway it's still 4 AM for me i'm offline :)

Actions #37

Updated by Autoran1 over 11 years ago

Neo, you told yesterday, that making some manipulations in BPStructs can fix this issue properly, well can you give me an example, so i could make my further tests)

Actions #38

Updated by NeoBrainX over 11 years ago

Nevermind that one, it's not necessary anymore with the info you provided last night :)

Actions #39

Updated by NeoBrainX over 11 years ago

FYI, if you want to tackle this yourself, the issue is about SOME place in the source code setting a viewport without restoring it. Thus, when rendering the game's vertices in VertexManager.cpp, we don't actually use the viewport that was requested by the game previously. So the challenge in this issue is to find out what place changed the viewport without restoring it... :D

Actions #40

Updated by Autoran1 over 11 years ago

Not sure i actually can find smth like this myself, but i tested more: the solutions like a) b) c) d) can cause graphic corruption in various games, and g_renderer->RestoreAPIState(); seems safe but can cause some slowdows

Actions #41

Updated by NeoBrainX over 11 years ago

  • Status changed from Accepted to Fixed

This issue was closed by revision 4f652c40861b.

Actions

Also available in: Atom PDF