perf(radar): Reduce cost of radar pixel draw in PartitionManager::refreshShroudForLocalPlayer by 96% #2072

xezon · 2026-01-06T19:35:13Z

Merge with Rebase

This optimization is needed to make HeightMapRenderObjClass::oversizeTerrain faster, which is relevant later for terrain size extension on low camera pitch.

The first commit consolidates some duplicated code.

The second commit implements the performance optimization.

Cost was measured and reduced by 96%.

TODO

Replicate in Generals

xezon · 2026-01-07T10:38:55Z

Generals fails to compile until replicated.

Core/GameEngineDevice/Source/W3DDevice/Common/System/W3DRadar.cpp

Stubbjax · 2026-01-07T07:24:12Z

Core/Libraries/Source/WWVegas/WW3D2/surfaceclass.cpp

+	RECT rect;
+	rect.left = min.I;
+	rect.top = min.J;
+	rect.right = max.I;
+	rect.bottom = max.J;


Suggested change

RECT rect;

rect.left = min.I;

rect.top = min.J;

rect.right = max.I;

rect.bottom = max.J;

RECT rect = { min.I, min.J, max.I, max.J };

I doubt this works in C++98

Did you try it? VC6 seems to compile for me.

Okay. Tweaked to your suggestion.

After thinking about this a bit more I reverted that to the previous. The initializer list is totally not intuitive to read, because the list has no names. Wheras assigning to the struct names is explicit and clear.

Core/GameEngine/Include/Common/Radar.h

Core/GameEngineDevice/Source/W3DDevice/Common/System/W3DRadar.cpp

GeneralsMD/Code/GameEngine/Source/GameLogic/Object/PartitionManager.cpp

Stubbjax · 2026-01-07T10:57:23Z

GeneralsMD/Code/GameEngine/Source/GameLogic/Object/PartitionManager.cpp


-	const Int playerIndex = rts::getObservedOrLocalPlayer()->getPlayerIndex();
-	for (int i = 0; i < m_totalCellCount; ++i)
+	if (m_totalCellCount != 0)


Maybe invert to reduce nesting?

if (m_totalCellCount == 0) return;

Prefer not, because it is not at the top of the function.

Why? A few lines down is still at the top of the function.

I am not so much a fan of returns unless they are at the start of the function or the function has a single purpose and will never change (e.g. findSomethingInContainer)

Stubbjax · 2026-01-07T10:59:24Z

GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DAssetManager.cpp


 	Int psize;
-	psize=PixelSize(desc);
+	psize=Get_Bytes_Per_Pixel(desc.Format);


Combine declaration and assignment?

const Int psize=Get_Bytes_Per_Pixel(desc.Format);

Could do, but for simplicity sake the refactor is enough as is.

Okay. I had assumed minor refactors were on the table due to the changes to Core/Libraries/Source/WWVegas/WW3D2/ww3dformat.cpp.

Yes sure but can be separate refactor then. Keeping it extra simple :)

Core/Libraries/Source/WWVegas/WW3D2/surfaceclass.cpp

stephanmeesters · 2026-01-09T11:34:13Z

Core/GameEngineDevice/Source/W3DDevice/Common/System/W3DRadar.cpp

+	m_shroudSurface = m_shroudTexture->Get_Surface_Level();
+	DEBUG_ASSERTCRASH( m_shroudSurface != NULL, ("W3DRadar::beginSetShroudLevel: Can't get surface for Shroud texture") );
+
+	if (surfaceRegion != NULL)


In what situation is surfaceRegion not NULL? The only invocation I found was TheRadar->beginSetShroudLevel()

I provided the option to lock a subregion only. But it is unused, because refreshShroudForLocalPlayer needs the full region locked to touch every pixel.

After rethinking this I removed the surfaceRegion argument from this function because it would not have worked correctly when locking a subregion.

stephanmeesters · 2026-01-09T12:03:32Z

Core/GameEngineDevice/Source/W3DDevice/Common/System/W3DRadar.cpp

+				switch (m_shroudSurfacePixelSize)
+				{
+				case 1: *column = (UnsignedByte)color; break;
+				case 2: *reinterpret_cast<UnsignedShort*>(column) = (UnsignedShort)color; break;


Is it valid to cast Color like this? Color is AARRGGBB and casting to 2 bytes would give you GGBB and casting to 1 byte gives BB which are both zero in this case

Indeed it looks strange. I copied this style from SurfaceClass::DrawPixel, so if this is wrong then it is original game bug (out the scope of this change).

I also prepared a follow up change for this, but unclear to me if this solves anything in practice. I was unable to force surface format to non A8R8G8B8, even if texture was created with different format.

I think this was to support videocards that were DX8 capable but did not support 32 bit textures for some reason, it's a very niche situation

I was able to force m_shroudSurfacePixelSize to 2 by doing this but I don't know exactly what to look for in regards to what would happen to the graphics

diff --git a/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dx8caps.h b/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dx8caps.h index 88a1dba2..d7e0a5fb 100644 --- a/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dx8caps.h +++ b/GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/dx8caps.h @@ -241,7 +241,12 @@ public: int Get_Pixel_Shader_Minor_Version() const { return 0xff&(PixelShaderVersion); } int Get_Max_Simultaneous_Textures() const { return MaxSimultaneousTextures;} - bool Support_Texture_Format(WW3DFormat format) const { return SupportTextureFormat[format]; } + bool Support_Texture_Format(WW3DFormat format) const { + if (format == WW3D_FORMAT_A8R8G8B8) { + return false; + } + return SupportTextureFormat[format]; + } bool Support_Render_To_Texture_Format(WW3DFormat format) const { return SupportRenderToTextureFormat[format]; } bool Support_Depth_Stencil_Format(WW3DZFormat format) const { return SupportDepthStencilFormat[format]; }

xezon · 2026-01-09T21:17:19Z

Perhaps we can combine this fix with jmarshall2323@9b1c399, which makes a lot of sense too.

xezon · 2026-01-10T19:10:10Z

Perhaps we can combine this fix with jmarshall2323@9b1c399, which makes a lot of sense too.

I prepared a follow up change that optimizes SurfaceClass::DrawPixel and SurfaceClass::DrawHLine for the other W3DRadar functions. But that change is quite big too so I will not append it here to keep it simple for reviews.

…el (#2072)

…reshShroudForLocalPlayer by 96% (#2072)

xezon · 2026-01-10T20:36:19Z

Merged down the changes into the respective commit.

…er::refreshShroudForLocalPlayer by 96% (#2072)

xezon added this to the Runtime performance fixes & optimizations milestone Jan 6, 2026

xezon added Minor Severity: Minor < Major < Critical < Blocker Performance Is a performance concern Gen Relates to Generals ZH Relates to Zero Hour Rendering Is Rendering related labels Jan 6, 2026

xezon commented Jan 7, 2026

View reviewed changes

Core/GameEngineDevice/Source/W3DDevice/Common/System/W3DRadar.cpp Outdated Show resolved Hide resolved

Stubbjax reviewed Jan 7, 2026

View reviewed changes

stephanmeesters reviewed Jan 9, 2026

View reviewed changes

xezon requested review from Stubbjax and stephanmeesters January 10, 2026 19:12

xezon added 2 commits January 10, 2026 21:35

refactor(ww3dformat): Merge function PixelSize into Get_Bytes_Per_Pix…

cd7137d

…el (#2072)

perf(radar): Reduce cost of radar pixel draw in PartitionManager::ref…

0be0f45

…reshShroudForLocalPlayer by 96% (#2072)

xezon force-pushed the xezon/optimize-radar-refresh-2 branch from 4843fd2 to 0be0f45 Compare January 10, 2026 20:35

fixup! perf(radar): Reduce cost of radar pixel draw in PartitionManag…

40da576

…er::refreshShroudForLocalPlayer by 96% (#2072)

perf(radar): Reduce cost of radar pixel draw in PartitionManager::refreshShroudForLocalPlayer by 96% #2072

Are you sure you want to change the base?

perf(radar): Reduce cost of radar pixel draw in PartitionManager::refreshShroudForLocalPlayer by 96% #2072

Conversation

xezon commented Jan 6, 2026

TODO

Uh oh!

xezon commented Jan 7, 2026

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!

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!

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!

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!

xezon commented Jan 9, 2026

Uh oh!

xezon commented Jan 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xezon commented Jan 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

xezon commented Jan 10, 2026 •

edited

Loading