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

Added support for transforming homogenous W geometry into world geomtry #199

Open
wants to merge 32 commits into
base: master
Choose a base branch
from

Conversation

dkollmann
Copy link
Contributor

This is an update on the previous pull request #188.

ddraw/IDirect3DDeviceX.cpp Outdated Show resolved Hide resolved
@elishacloud
Copy link
Owner

Thanks @dkollmann! I will take a look at this soon.

ddraw/IDirect3DDeviceX.cpp Outdated Show resolved Hide resolved
ddraw/IDirect3DDeviceX.cpp Outdated Show resolved Hide resolved
@dkollmann
Copy link
Contributor Author

Also FYI if you try this code with Black & White, things will have a blue tint. This is because the water plane which should be rendered before all the other geometry, is rendered on top of it instead of behind the other geometry.

… disable lighting. Commented out unneeded XYZW code.
@dkollmann dkollmann force-pushed the reverse_xyzrhw_new branch from 7c87321 to e8ff88d Compare May 16, 2023 22:17
@dkollmann
Copy link
Contributor Author

I made some fixes. Of course lpD3DMatrix has to be replaced, after getting the game camera properties from it.
I added an option to disable the lighting which is needed when DdrawConvertHomogeneousW is true but DdrawConvertHomogeneousToWorld is false. At least for Black & White.

The force push was just an update on the commit comment.

ddraw/DebugOverlay.cpp Outdated Show resolved Hide resolved
@elishacloud
Copy link
Owner

For some reason when I add this DdrawConvertHomogeneousW = 1 line to the ini file I just get the following:

image

@elishacloud
Copy link
Owner

For some reason when I add this DdrawConvertHomogeneousW = 1 line to the ini file I just get the following:

Ok, I figured this out. You have to set DdrawDisableLighting = 1 for Black & White.

@elishacloud
Copy link
Owner

elishacloud commented May 17, 2023

My only last comment is that I prefer to create a struct and add it to the IDirect3DDeviceX.h file rather than a whole new RednerData class for the DdrawConvertHomogeneous settings/matrices. Since the code is in IDirect3DDeviceX.cpp the settings should be in IDirect3DDeviceX.h.

Other than that, I think this is about ready to merge.

@dkollmann
Copy link
Contributor Author

Ah okay, I created the RenderData.h, because it was not possbile to include the device header in the DebugOverlay.cpp file.

@elishacloud
Copy link
Owner

I created the RenderData.h, because it was not possbile to include the device header in the DebugOverlay.cpp file.

DebugOverlay.h already has #include "ddraw.h", which includes IDirect3DDeviceX.h. So it is already covered.

@elishacloud
Copy link
Owner

It seems like there may be more than just something wrong with DdrawConvertHomogeneousToWorldUseGameCamera. I am seeing an issue in Dark Reign 2 when just using DdrawConvertHomogeneousToWorld. The sky seems stretched

With DdrawConvertHomogeneousToWorld enable:

image

How it is supposed to look:

image

I can also see it even when just DdrawConvertHomogeneousW is enabled, but it is not quite as pronounced.

@dkollmann
Copy link
Contributor Author

dkollmann commented May 20, 2023

Okay, here are the things are are wrong here and why "position" is zero.

  • "lpD3DMatrix = &view;" is set after the whole "if (Config.DdrawConvertHomogeneousToWorld)" block.
  • toViewSpace should be "DirectX::XMMATRIX toViewSpace = DirectX::XMLoadFloat4x4((DirectX::XMFLOAT4X4*)&view);"

The result will be the same, but the code should be a bit clearer.

So what happens is...

  • With the inverse matrix, "view" is applied. So the geometry is centered in front of the camera.
  • Then the inverse view projection is added, so the geometry is transformed into world space.
  • From there, the view matrix does not need to contain "view", since the error was already corrected by the inverse matrix.

I hope this all makes sense and is correct. Please take a look.

@dkollmann
Copy link
Contributor Author

dkollmann commented May 20, 2023

Not sure if it is related, but I noticed something curious with the reconstructed world space in Black & White.

image

The whole world is rotated by 24.4° degreees. But the thing is, if you look at the trees at the bottom left, they are straight, which implies that this is correct. I wonder why this is. Is this due to the angle of the reflection? In game, the camera is completely horizontal. I checked the original view matrix.

But I also noticed that the Y direction seems to be flipped. It is negative when looking up and positive when looking down.

@dkollmann
Copy link
Contributor Author

Okay this seems to be a Black & White thing. No matter how I look att he view matrix, the angle only seems to be half of the actual angle. MAybe this is related to faking bent normals.

@dkollmann
Copy link
Contributor Author

dkollmann commented May 23, 2023

Hey Elisha, so I finally got around to take a closer look at this for Black & White. So for me, the practical difference between DdrawConvertHomogeneousToWorldUseGameCamera is that I use the original view matrix to remove the camera's tilt, so the world geometry comes out as horizontal. This way I can place a new water plane and it will always correctly align with the terrain. Of course this also means that the code became quite specialised for Black & White now and it might be a better option to remove DdrawConvertHomogeneousToWorldUseGameCamera, unless you see general value in this special case.

And just as a note, in the following code, yaw and pitch look like they should be the other way around, but this is indeed correct. Yaw is the rotation of X and pitch is the rotation of a vector around X.

Here is what I do now:

DirectX::XMVECTOR position, direction;
if (Config.DdrawConvertHomogeneousToWorldUseGameCamera)
{
	// To reconstruct the 3D world, we need to know where the camera is and where it is looking
	position = DirectX::XMVectorSet(0.0f, 0.0f, 0.0f, 0.0f);

	const float x = lpD3DMatrix->_11;
	const float y = lpD3DMatrix->_12;
	const float z = lpD3DMatrix->_13;

	float pitch = std::atan2(y, z);
	if(pitch < 0.0f && y * z > 0.0f)  // check if y and z have the same sign
	{
		// handle flipping of the pitch. This is not because the camera is looking up.
		pitch += DirectX::XM_PI;
	}

	float yaw = std::asin(x);
	if(yaw < 0.0f)
	{
		yaw += DirectX::XM_2PI;
	}

	// mirror the transform
	float pitchneg = -pitch;

	float pitch_cos = std::cos(pitchneg);
	float x2 = 0.0f;  //std::cos(yaw) * pitch_cos;
	float y2 = std::sin(pitchneg);
	float z2 = /*std::sin(yaw) **/ pitch_cos;

	direction = DirectX::XMVectorSet(x2, y2, z2, 0.0f);

	ConvertHomogeneous.ToWorld_GameCameraYaw = yaw;
	ConvertHomogeneous.ToWorld_GameCameraPitch = pitch;
}
else
{
	position = DirectX::XMVectorSet(0.0f, 0.0f, 0.0f, 0.0f);
	direction = DirectX::XMVectorSet(0.0f, 0.0f, 1.0f, 0.0f);
}

@elishacloud
Copy link
Owner

@dkollmann, sorry for the delay in responding. I updated the code to match your branch. But I am wondering if there is an issue with DdrawConvertHomogeneousW which is getting exacerbated in DdrawConvertHomogeneousToWorld.

Could there be an issue because we are losing data in the DrawIndexedPrimitive() when we overwrite data in lpVertices?

The reason I ask is because I am still seeing distortion in the sky in Dark Reign 2 whenever DdrawConvertHomogeneousToWorld is enabled, even if DdrawConvertHomogeneousToWorldUseGameCamera is disabled. I also see distortion even if just DdrawConvertHomogeneousW is enabled, though the distortion is much smaller in this last case.

@dkollmann
Copy link
Contributor Author

Hey, I just wanted to say that I am still on this, but I changed my job and am moving to another country, so lately I am super busy.

@elishacloud
Copy link
Owner

@dkollmann, no problem. Take your time. I understand. Real life has to come first. Thanks for letting me know.

@elishacloud
Copy link
Owner

Because there are a lot of things added since this PR was created, I updated it to keep it in-sync with the master branch.

@elishacloud
Copy link
Owner

Keeping PR in-sync with master branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants