unify(dx8wrapper): Merge zh dx8wrapper to generals dx8wrapper part 1 #1423

ryan-de-boer · 2025-08-05T09:50:06Z

Some of zh dx8wrapper has been merged to the generals dx8wrapper.

Redoing this pull request: #1401
Small amounts at a time for easier review.

xezon

Currently this change has a lot of incorrect white space changes which

Make the diff larger than necessary
Make the format different to Zero Hour

When merging from Zero Hour to Generals, please preserve the formatting of Zero Hour. This will make reviewing and diffing easier.

.gitignore

xezon · 2025-08-06T16:15:20Z

Generals/Code/Libraries/Source/WWVegas/WW3D2/texturefilter.h

@@ -115,7 +115,8 @@ class TextureFilterClass
 	void Set_V_Addr_Mode(TxtAddrMode mode) { VAddressMode=mode; }

 	// This needs to be called after device has been created
-	static void _Init_Filters(void);
+	static void _Init_Filters(TextureFilterMode texture_filter);
+	static void _Init_Filters();


Why are there 2 functions now?

Zero Hour just has 1.

Its because other general files are still calling the old function, and we wanted this commit to be small. I'll investigate and see how big the diff will be.

Ah ok. Then that makes sense.

xezon · 2025-08-06T16:17:29Z

Generals/Code/Libraries/Source/WWVegas/WW3D2/sortingrenderer.cpp

-	DX8Wrapper::Set_Transform(D3DTS_VIEW,old_view);
-	DX8Wrapper::Set_Transform(D3DTS_WORLD,old_world);
+	DX8Wrapper::Set_Transform(D3DTS_VIEW, old_view);
+	DX8Wrapper::Set_Transform(D3DTS_WORLD, old_world);


This formatting does not match Zero Hour. If you copy & paste code, disable the auto format feature in your IDE to preserve the original format on copy & paste. Otherwise use WinMerge or similar to merge the code correctly with all identical formatting.

Good idea to change the IDE. I always have trouble with that. I do ctrl paste, undo which usually undoes the formatting but I don't think it always works. Could also use the merge tool.

Generals/Code/Libraries/Source/WWVegas/WW3D2/shdlib.h

ryan-de-boer · 2025-08-08T09:53:48Z

@xezon could you review the code review changes?

Generals/Code/Libraries/Source/WWVegas/WW3D2/dx8wrapper.cpp

OmniBlade · 2025-08-08T15:58:55Z

Looks like the branch has merge conflicts, you need to rebase on latest main?

xezon · 2025-08-08T16:08:18Z

Looks like the branch has merge conflicts, you need to rebase on latest main?

Yes. Try apply white space changes with scripts/cpp/remove_trailing_whitespace.py before merging if there are to many merge conflicts otherwise.

xezon · 2025-08-12T21:53:25Z

This review now claims to have touched 3800 files.

ryan-de-boer · 2025-08-13T11:04:18Z

[fixed] force push removed the in between commits by others.

This review now claims to have touched 3800 files.

Maybe a rebase was the wrong strategy? It seems to have included all commits on main between my commits and the rebased commit? Should I redo the whole pull request with just my 5 commits?

ryan-de-boer · 2025-08-13T11:06:50Z

[fixed] nevermind I missed the force push and it ended up commits by others.

Looks like the branch has merge conflicts, you need to rebase on latest main?

Can you do a rebase without including all the in between commits in github pull request? The pull request seems to now have 18 more commits than expected.
Would a pull be better than rebase? But that might include lots of unwanted changes in a merge commit...

Stubbjax · 2025-08-13T11:34:01Z

Can you do a rebase without including all the in between commits in github pull request? The pull request seems to now have 18 more commits than expected. Would a pull be better than rebase? But that might include lots of unwanted changes in a merge commit...

All a rebase with main should do is effectively undo all your commits, fast-forward your branch to the latest commit on main, and then reapply your commits as if that's where you had originally branched from. You should not see any commits from other branches or main in the diff.

I'm not sure what you've done but a simple solution would be to checkout latest main, cherry-pick each of your commits from this branch chronologically and then force-push the changes to the same remote branch. (Basically a manual rebase.)

ryan-de-boer · 2025-08-13T11:53:39Z

I'm not sure what you've done but a simple solution would be to checkout latest main, cherry-pick each of your commits from this branch chronologically and then force-push the changes to the same remote branch. (Basically a manual rebase.)

Thanks for this. I was missing a force-push, that's why it kept the old commits. All I had to do was point my branch to "Fix Makefile merge" (the last commit I want in the pull request) and force-push and the pull request fixed itself. I'm glad I didn't need to create another one.

xezon

Please make a pass on the whitespace differences. You can run the scripts/cpp/remove_trailing_whitespace.py script to help with that.

Also please double check that code has been merged to the right places. I commented a few that are not placed correctly.

xezon · 2025-08-13T17:51:59Z

Generals/Code/Libraries/Source/WWVegas/WW3D2/sortingrenderer.cpp

@@ -732,6 +739,7 @@ void SortingRendererClass::Flush()

 }

+


Zero Hour does not have this blank line

xezon · 2025-08-13T17:52:49Z

Generals/Code/Libraries/Source/WWVegas/WW3D2/sortingrenderer.cpp

 	Matrix4x4 old_view;
 	Matrix4x4 old_world;
 	DX8Wrapper::Get_Transform(D3DTS_VIEW,old_view);
 	DX8Wrapper::Get_Transform(D3DTS_WORLD,old_world);

 	while (SortingNodeStruct* state=sorted_list.Head()) {
 		state->Remove();
-
+		


Zero Hour does not have these tabs

xezon · 2025-08-13T17:59:13Z

Generals/Code/Libraries/Source/WWVegas/WW3D2/dx8wrapper.h

@@ -459,6 +462,8 @@ class DX8Wrapper
 	static void Set_Vertex_Shader_Constant(int reg, const void* data, int count);
 	static void Set_Pixel_Shader_Constant(int reg, const void* data, int count);

+	static ZTextureClass* Shadow_Map[MAX_SHADOW_MAPS];


This is placed at the wrong location.

xezon · 2025-08-13T18:00:14Z

Generals/Code/Libraries/Source/WWVegas/WW3D2/dx8wrapper.h

@@ -100,6 +100,8 @@ class TextureClass;
 class LightClass;
 class SurfaceClass;

+class ZTextureClass;


This is supposed to be before class SurfaceClass

xezon · 2025-08-13T18:00:48Z

Generals/Code/Libraries/Source/WWVegas/WW3D2/dx8wrapper.cpp

@@ -2406,7 +2473,7 @@ IDirect3DSurface8 * DX8Wrapper::_Create_DX8_Surface(const char *filename_)
 			strncpy(compressed_name,filename_, ARRAY_SIZE(compressed_name));
 			compressed_name[ARRAY_SIZE(compressed_name)-1] = '\0';
 			char *ext = strstr(compressed_name, ".");
-			if ( ext && (strlen(ext)==4) &&
+			if ( ext && (strlen(ext)==4) && 


Zero Hour does not have this whitespace

xezon · 2025-08-13T18:01:48Z

Core/Libraries/Source/WWVegas/WW3D2/shdlib.h

+#define SHD_INIT_SHADERS		
+#define SHD_SHUTDOWN_SHADERS	
+#define SHD_FLUSH					
+#define SHD_REG_LOADER			


There are whitespace changes that are not supposed to be here.

xezon added Gen Relates to Generals Unify Unifies code between Generals and Zero Hour labels Aug 6, 2025

xezon reviewed Aug 6, 2025

View reviewed changes

OmniBlade requested changes Aug 8, 2025

View reviewed changes

Generals/Code/Libraries/Source/WWVegas/WW3D2/dx8wrapper.cpp Outdated Show resolved Hide resolved

ryan-de-boer mentioned this pull request Aug 12, 2025

unify(dx8wrapper): Migrate dx8wrapper.cpp and h to the core WW3D2 folder #1401

Closed

ryan-de-boer added 5 commits August 12, 2025 18:22

Unify dx8wrapper part 1

92a4d0e

Fix up whitespace and share shdlib.h

c44b416

Remove _Init_Filters()

d1cf98a

Fix trailing whitespace

fcf210f

Fix Makefile merge

8202536

ryan-de-boer requested a review from OmniBlade August 13, 2025 10:56

ryan-de-boer force-pushed the rdeboer_directx9_2 branch from 1c9fa69 to 8202536 Compare August 13, 2025 11:49

xezon reviewed Aug 13, 2025

View reviewed changes

unify(dx8wrapper): Merge zh dx8wrapper to generals dx8wrapper part 1 #1423

Are you sure you want to change the base?

unify(dx8wrapper): Merge zh dx8wrapper to generals dx8wrapper part 1 #1423

Uh oh!

Conversation

ryan-de-boer commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xezon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ryan-de-boer commented Aug 8, 2025

Uh oh!

Uh oh!

OmniBlade commented Aug 8, 2025

Uh oh!

xezon commented Aug 8, 2025

Uh oh!

xezon commented Aug 12, 2025

Uh oh!

ryan-de-boer commented Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryan-de-boer commented Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Stubbjax commented Aug 13, 2025

Uh oh!

ryan-de-boer commented Aug 13, 2025

Uh oh!

xezon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ryan-de-boer commented Aug 5, 2025 •

edited

Loading

ryan-de-boer commented Aug 13, 2025 •

edited

Loading

ryan-de-boer commented Aug 13, 2025 •

edited

Loading