Closed elishacloud closed 6 years ago
Let me explain this change in more detail since this is a rather large change.
For the first part of this change, I created a new class called AddressLookupTable
to handle all the caching of addresses. Along with this I added two new files: _lookuptable.cpp and _lookuptable.h
This caching class has three functions that are exposed for each Direct3D8 class. The function names are SaveAddress
, FindAddress
and DeleteAddress
.
Here is an example of the Direct3DSurface8
class functions:
void SaveAddress(Direct3DSurface8*, IDirect3DSurface9*);
Direct3DSurface8* FindAddress(IDirect3DSurface9*);
void DeleteAddress(Direct3DSurface8*);
The AddressLookupTable
class creates an array of vectors, one vector for each Direct3D8 class. The index of each Direct3D8 vector is stored in a static const
so that it can be referred to by name (for code clarity):
static constexpr DWORD SurfaceVector = 0;
static constexpr DWORD TextureVector = 1;
static constexpr DWORD CubeTextureVector = 2;
static constexpr DWORD VolumeTextureVector = 3;
static constexpr DWORD VolumeVector = 4;
static constexpr DWORD VertexBufferVector = 5;
static constexpr DWORD IndexBufferVector = 6;
static constexpr DWORD SwapChainVector = 7;
I also created three private functions in AddressLookupTable
class to handle the actual storing, retrieving and deleting of items in the cache. Each of the class SaveAddress
, FindAddress
and DeleteAddress
functions will call these private functions to do the actual work. The private functions use void*
as their address type so each of the public classes need to cast the address as this type and give the index of their vector so that the private functions know which vector to use.
Here is an example of the Direct3DSurface8
class functions calling the private functions:
Direct3DSurface8* AddressLookupTable::FindAddress(IDirect3DSurface9* pSurface9)
{
return reinterpret_cast<Direct3DSurface8*>(FindAddress(reinterpret_cast<void*>(pSurface9), SurfaceVector));
}
void AddressLookupTable::SaveAddress(Direct3DSurface8* pSurface8, IDirect3DSurface9* pSurface9)
{
SaveAddress(reinterpret_cast<void*>(pSurface8), reinterpret_cast<void*>(pSurface9), SurfaceVector);
}
void AddressLookupTable::DeleteAddress(Direct3DSurface8* pSurface8)
{
DeleteAddress(reinterpret_cast<void*>(pSurface8), SurfaceVector);
}
This function will take two addresses, an address for a Direct3D8 class and an address for a Direct3D9 class and store these two addresses in the vector for later retrieval.
This function, given a Direct3D9 address (since that is what we get from ProxyInteface
), will retrieve the associated Direct3D8 address.
This function, given a Direct3D8 address will delete the cache for this entry. It does this by switching the location of the requested item with the item in the back of the vector and then removing the item in the back (which is now this item).
The AddressLookupTable
class is instantiated in the Direct3D8Device
class constructor and it is removed (deleted) in the Direct3D8Device
class deconstructor, so it is available to all Direct3D8 classes via their Device
variable.
Because the Direct3D9 addresses are cached there is no longer any need for the CurrentRenderTarget
and CurrentDepthStencilSurface
variables. These can be retrieved by calling the ProxyInterface
and then looking up their associated Direct3D8 cached values. This greatly simplifies the CreateDevice
and SetRenderTarget
functions.
In addition the AddRef
and the Release
functions were simplified and changed to point directly to the device ProxyInterface
so that the RefCount
variable was no longer needed for this class.
Whenever a Get
request was called (such as GetRenderTarget
) the code was changed to first get the Direct3D9 address by calling the associated ProxyInterface
function and then looking up the address in the cache, like this example:
const HRESULT hr = ProxyInterface->GetRenderTarget(0, &SurfaceInterface);
if (FAILED(hr))
{
return hr;
}
*ppRenderTarget = ProxyAddressLookupTable->FindAddress(SurfaceInterface);
if (*ppRenderTarget == nullptr)
{
*ppRenderTarget = new Direct3DSurface8(this, SurfaceInterface);
}
If the address was not found in the cache then a new Direct3D8 instance was created.
This change updates the following Direct3D8 classes by adding caching:
For each one of these classes there is a new line added to the constructor to record the Direct3D8 address and the Direct3D9 address using the SaveAddress
function.
In addition the AddRef
and the Release
functions where changed for these classes as well to point directly to the device ProxyInterface
so that the RefCount
variable was no longer needed for these classes. The Release
function was also changed so the class is no longer deleted once the reference count got down to 0. Since in Windows all of the addresses are always available even when the reference count reaches 0, as long as the device is still available, I replicated that function here as well. So these classes will remain until the main device has been removed.
There are also two new variables Active
and CleanUpFlag
created for each of these classes. The Active
flag remains true until all the reference counts are removed. This allows us to tell when all references have been removed from this class. Since the class is not deleted after all references are removed there needed to be a way to tell when all the references are gone. The reason for this is that a reference gets added to the Device
for each class instantiation and we need a way to remove the reference for this class but only removing it once. Therefore when the Device->Release()
function has been called it sets the Active
flag to false
to prevent from releasing the device again.
The CleanUpFlag
is used to determine if the class should be deleted from cache and its references removed from the device. This flag is used when the AddressLookupTable
deconstructor is called and the classes are deleted. Since the AddressLookupTable
deconstructor is called when the Direct3D8Device
is removed (in the deconstructor) they should not try to release the device or remove items from cache at this time.
There is also a new DeleteMe
function added to each of these classes. This function is only used in the AddressLookupTable
deconstructor. This function simply sets the CleanUpFlag
to false
and then deletes the class.
This change also fixes the palette issue with Need for Speed III. What is happening is that the game is calling SetCurrentTexturePalette
with NULL palette peFlags. A NULL palette was allowed in IDirect3D8, see here. But as far as I can tell SetCurrentTexturePalette
will crash in IDirect3D9 unless the peFlags are set to PC_NOCOLLAPSE
. This site also seems to indicate that you must set peFlags to PC_NOCOLLAPSE
.
This line is added in SetCurrentTexturePalette
and SetPaletteEntries
:
PaletteNumber = (PaletteNumber & 0xFFF) | (PC_NOCOLLAPSE << 12);
There are a number of advantages to these changes:
CurrentRenderTarget
and CurrentDepthStencilSurface
variables which are not thread safe.In addition these changes it completely resolves all issues in Need for Speed III.
For all of these changes I have tested them extensively with the following Direct3D8 games to ensure that there are no ill side-effects:
Well done! However, these function names are a bit confusing. Better use a term like "HostResource" or something along those lines
Good point! The names were confusing. I changed the names as follows:
Direct3DCache
changed to AddressLookupTable
MyDirect3DCache
changed to ProxyAddressLookupTable
SetDirect3D
changed to SaveAddress
GetDirect3D
changed to FindAddress
DeleteDirect3D
changed to DeleteAddress
I also changed the name of the files to lookup_table.cpp
and lookup_table.h
and moved these to the 'helpers' folder in the Visual Studio project.
My comments above have been modified to use the new names as well. These changes should make the code easier to read.
Note: I did not want to use the word 'Resource' since that has a very specific meaning in a Visual Studio project.
Magnificent!
About the bugfix - since DeleteDirect3D
previously called GetDirect3D
, the test-results need to be revisited too, right?
You are right I did not go back and retest all the games again after this last change. I just did a quick smoke test on Need for Speed III. However I am not too worried about the issue I fixed with DeleteDirect3D
causing any issues since that function is not actually used. It is only called if the class is deleted separately from the DeleteMe
function, which currently does not happen. If needed I can go back and retest all the games again. :)
I fixed an issue with VolumeTexture
and CubeTexture
I did not notice before and did a quick test of the 10 games again to ensure everything is working ok.
I started a review and made some comments, but am not sure if you saw them.
I am not seeing any of your comments for some reason. Where should I be looking?
Whoops. Forgot to click that big "Submit" button.
Great ideas! I made the requested changes.
FYI, I modified the code slightly so it uses templates instead of a bunch of function overloads to reduce code size/redundancy a bit.
Added caching for the following classes:
Simplified reference counting by removing
RefCount
,CurrentRenderTarget
andCurrentDepthStencilSurface
This fixes the problems reported in issue #21.