bugfix(texture): Prevent compressed texture reduction below 4x4 limit and fix binarytstream texture rendering

[Mr-Sheerlock]

• Closes Binarystream texture is not drawing in Generals #1990

In Begin_Compressed_Load, the hardware aspect-ratio validation loop calculates Reduction to find a hardware-compliant mip level for a non-compliant texture (or thin textures like binary stream). However, the value was immediately overwritten following the loop.
This causes Load_Compressed_Mipmap to open the DDS file at reduction=0 (full 16×256) while trying to copy into a 4×32 surface which caused a silent dimension mismatch where DDSFileClass::Copy_Level_To_Surface hits its unimplemented scaling branch and copies nothing, leaving the texture blank.

The binary stream texture EXBinaryStream.tga is of dimensions 16×256 which is a thin texture (aspect ratio exceeding 1:8) and became invisible in Generals while still appearing correctly in Zero Hour because it replaced by EXBinaryStream32.tga (32×256), which has a compliant 1:8 aspect ratio and never triggers the buggy code path.

This fix removes the line that overwrites the aspect-ratio reduction and performs minimum dimension clamping (4px) to prevent over-shifting in Begin_Compressed_Load and Load_Compressed_Mipmap

Note on original Generals behavior

The original pre-PR Generals loader also had a bug in its reduction loop: it passed the wrong variables to Validate_Texture_Size, causing the loop to always break at i=1, producing 8×128 instead of the more consistent 4×32. This fix produces 4×32, which differs slightly from original Generals retail behavior. If exact retail compatibility is required, a precompiler flag approach might be considered.

Testing

Verified that Lotus' and patriot's binary stream renders in Generals and ZH.
Played multiple skirmishes in generals and ZH and no apparent unwanted changes were noticed.

[greptile-apps]

Greptile Summary

This fix addresses two related issues in the compressed texture loading pipeline: the overwritten aspect-ratio Reduction (which left Load_Compressed_Mipmap opening the DDS at reduction=0 while writing into an already-reduced surface), and the missing 4 px minimum clamp that prevented over-shifting in both Begin_Compressed_Load and Load_Compressed_Mipmap.

• Removes Reduction = reduction; so the aspect-ratio compliance loop's result is correctly propagated to Load_Compressed_Mipmap via Get_Reduction(), eliminating the silent dimension mismatch for thin textures like EXBinaryStream.tga (16×256).
• Adds a break-before-4 guard inside the Load_Compressed_Mipmap reduction loop (correct incremental approach), and a conditional skip in Begin_Compressed_Load for the same boundary (uses exact != 4 equality rather than clamping, which can still produce sub-4 values for wider textures with a large Reduction).

Confidence Score: 4/5

Safe to merge for the targeted bug; the fix correctly unblocks 4×4-minimum-clamped textures, but the bulk right-shift in Begin_Compressed_Load can still under-shoot the 4 px boundary for textures that hit the hardware-size limit with a Reduction > 1, unlike the incremental approach used in Load_Compressed_Mipmap.

The core removal of the Reduction overwrite is clearly correct and the Load_Compressed_Mipmap break-before-4 guard is solid. The Begin_Compressed_Load guard uses exact equality rather than clamping, leaving a narrow residual case where the two functions could disagree on the final pixel dimensions — but that path requires old hardware with small texture limits and specific source dimensions, so it won't be exercised in normal testing.

Core/Libraries/Source/WWVegas/WW3D2/textureloader.cpp — specifically the if (!mip_level_count) block in Begin_Compressed_Load around the new guard condition.

Important Files Changed

Filename	Overview
Core/Libraries/Source/WWVegas/WW3D2/textureloader.cpp	Removes the line that clobbered the aspect-ratio Reduction with the raw hardware reduction, and adds a 4 px guard in both Begin_Compressed_Load and Load_Compressed_Mipmap to stop over-shifting. The guard in Begin_Compressed_Load uses exact equality (!=4) rather than clamping, which still allows sub-4 results when Width is above 4 and Reduction is large.

Sequence Diagram

sequenceDiagram
    participant App
    participant BCL as Begin_Compressed_Load
    participant DDS as DDSFileClass
    participant LCM as Load_Compressed_Mipmap

    App->>BCL: load texture (e.g. 16x256)
    BCL->>BCL: Validate_Texture_Size fails (1:16 aspect)
    BCL->>BCL: "loop i=1..n find mip where ratio le 1:8"
    note over BCL: i=3 w=4 h=32 clamped valid Reduction+=3 Width=4 Height=32
    note over BCL: (removed) Reduction=reduction was overwriting to 0
    BCL->>BCL: "guard reducedWidth==4 skip shift mip_level_count=1"
    BCL->>DDS: _Create_DX8_Texture(4,32,fmt,1 mip)
    BCL-->>App: D3DTexture allocated at 4x32
    App->>LCM: copy pixel data
    LCM->>DDS: "open DDS at Get_Reduction()=3 mip3=4x32 block"
    LCM->>LCM: "width=4 height=32"
    LCM->>LCM: "Reduction loop width==4 break immediately"
    LCM->>DDS: "Copy_Level_To_Surface(level=0,4,32,...)"
    DDS-->>LCM: 4x32 pixels copied
    LCM-->>App: texture ready

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
Core/Libraries/Source/WWVegas/WW3D2/textureloader.cpp:1481-1486
The guard skips the whole block only when a dimension is **exactly** 4, but it still allows the bulk right-shift to produce values below 4 for wider dimensions. For example, if `Width = 8` and `Reduction = 2` (texture too large for hardware, needing two mip levels of reduction), the condition passes because `8 != 4`, but `reducedWidth >>= 2` yields 2 — below the DXT block minimum. `Load_Compressed_Mipmap` avoids this by reducing incrementally and breaking before going under 4, so a texture whose `Width` came out above 4 from the hardware-limit loop could end up with a D3D surface two bytes narrower than the data being copied into it. Clamping with `std::max` makes the intent explicit and aligns the two functions.

```suggestion
		reducedWidth  = max(4u, reducedWidth  >> Reduction);
		reducedHeight = max(4u, reducedHeight >> Reduction);
		mip_level_count = orig_mip_count-Reduction;//dds_file.Get_Mip_Level_Count();
```

_{Reviews (1): Last reviewed commit: "bugfix(texture): Prevent compressed text..." | Re-trigger Greptile}

[greptile-apps]

The guard skips the whole block only when a dimension is exactly 4, but it still allows the bulk right-shift to produce values below 4 for wider dimensions. For example, if Width = 8 and Reduction = 2 (texture too large for hardware, needing two mip levels of reduction), the condition passes because 8 != 4, but reducedWidth >>= 2 yields 2 — below the DXT block minimum. Load_Compressed_Mipmap avoids this by reducing incrementally and breaking before going under 4, so a texture whose Width came out above 4 from the hardware-limit loop could end up with a D3D surface two bytes narrower than the data being copied into it. Clamping with std::max makes the intent explicit and aligns the two functions.

Suggested change

	if(reducedWidth !=4 && reducedHeight !=4)
	{
	reducedWidth >>= Reduction;
	reducedHeight >>= Reduction;
	mip_level_count = orig_mip_count-Reduction;//dds_file.Get_Mip_Level_Count();
	}
	reducedWidth = max(4u, reducedWidth >> Reduction);
	reducedHeight = max(4u, reducedHeight >> Reduction);
	mip_level_count = orig_mip_count-Reduction;//dds_file.Get_Mip_Level_Count();

Prompt To Fix With AI

This is a comment left during a code review.
Path: Core/Libraries/Source/WWVegas/WW3D2/textureloader.cpp
Line: 1481-1486

Comment:
The guard skips the whole block only when a dimension is **exactly** 4, but it still allows the bulk right-shift to produce values below 4 for wider dimensions. For example, if `Width = 8` and `Reduction = 2` (texture too large for hardware, needing two mip levels of reduction), the condition passes because `8 != 4`, but `reducedWidth >>= 2` yields 2 — below the DXT block minimum. `Load_Compressed_Mipmap` avoids this by reducing incrementally and breaking before going under 4, so a texture whose `Width` came out above 4 from the hardware-limit loop could end up with a D3D surface two bytes narrower than the data being copied into it. Clamping with `std::max` makes the intent explicit and aligns the two functions.

```suggestion
		reducedWidth  = max(4u, reducedWidth  >> Reduction);
		reducedHeight = max(4u, reducedHeight >> Reduction);
		mip_level_count = orig_mip_count-Reduction;//dds_file.Get_Mip_Level_Count();
```

How can I resolve this? If you propose a fix, please make it concise.

[Mr-Sheerlock]

This fix would leave the binarystream texture with dimensions 4x4 which doesn't render. I agree that the logic might over-reduce though.