search

rebtd7 / FF13Fix

Performance and bug fixes for the PC versions of FF13 and FF13-2
GNU General Public License v3.0
343 stars 24 forks source link

Properly manage D3D9 objects i.e. proper reference count #60

Closed Nucleoprotein closed 3 years ago

Nucleoprotein commented 3 years ago

DXVK is little off with this, reference count of IDirect3DDevice9 is perfectly fine, but we need to count IDirect3DDevice9Ex too because IDirect3DDevice9 is superset of it. All crashes related to this should be gone, this commit also include basic IDirect3DDevice9Ex interface implementation.

Nucleoprotein commented 3 years ago

@rebtd7 Also i think i know why my attempt to wrap IDirect3DVertexBuffer9 failed.

rebtd7 commented 3 years ago

I get the WARNING: Reference count for IDirect3D9 is wrong!!! Expect application crash :( message when the test window is quitting (without DXVK). With DXVK, it doesn't happen =/. This happen in both FF13 and FF13-2

FF13Fix.log

Could you explain why we need to keep track of the reference count internally? I haven't understood that.

@rebtd7 Also i think i know why my attempt to wrap IDirect3DVertexBuffer9 failed. Awesome, why is that?

Nucleoprotein commented 3 years ago

I noticed that, after some tweaking i got ref count perfect i think. Now Steam Overlay works for me with and without DXVK! Older one works only in D3D9... I will push soon ™️

Nucleoprotein commented 3 years ago

My logs: d3d9_FF13Fix.log dxvk_FF13Fix.log

I also make some small changes, like unwrapped macros in interfaces and made it build with /std:c++latest

Nucleoprotein commented 3 years ago

Please test, "WARNING: Reference count for XXXX is wrong" in log should never happen. Steam Overlay should work in both D3D9 and DXVK. Please also check ReShade.

Binary for tests: d3d9.zip

rebtd7 commented 3 years ago

We need a QA :smile:

I've tested:

Works fine for me (but it usually does haha)

Feel free to merge + bump version and release

rebtd7 commented 3 years ago

Also, do you have the GeForce Experience overlay? I don't, but this is yet another variable to test...

Nucleoprotein commented 3 years ago

I think i will amend this commit before merge and change commit message as this is now not just basic IDirect3DDevice9Ex/IDirect3D9Ex interface implementation, it's like full one 😄 Yes, i use GeForce Experience but not tested if works.