bugfix(textureloader): Fix faulty texture reduction implementations

[xezon]

Merge with Rebase

• Relates to Binarystream texture is not drawing in Generals #1990

This change attempts to fix the faulty implementations related to texture reduction.

It has 3 commits:

The first commit removes some dead code, streamlines some variable names and simplifies some code related to texture reduction in the 3 texture load classes: TextureLoadTaskClass, CubeTextureLoadTaskClass, VolumeTextureLoadTaskClass

The second commit removes the texture reduction code from TextureLoadTaskClass::Begin_Uncompressed_Load because the reduction was always 0 as far as I could see and the game never reduced TGA texture size.

The third commit fixes and streamlines the texture reduction code in TextureLoadTaskClass::Begin_Compressed_Load, CubeTextureLoadTaskClass::Begin_Compressed_Load, VolumeTextureLoadTaskClass::Begin_Compressed_Load. The former logic was incoherent and nonsensical. The new logic does work in so far that the binary stream texture in Generals would render correctly with an auto reduction to the lowest mip to satisfy the original 1:8 texture aspect ratio limit. No new issues have been observed. Texture reduction from Options Menu also still works.

TODO

• Add pull ids to commit titles

[greptile-apps]

Greptile Summary

This PR refactors and fixes the texture reduction logic across TextureLoadTaskClass, CubeTextureLoadTaskClass, and VolumeTextureLoadTaskClass. The core bugs flagged in previous reviews — swapped Apply_Reduction arguments, signed/unsigned comparison errors in Validate_Reduction, and missing dimension shifts in CubeTextureLoadTaskClass::Load_Compressed_Mipmap — are all addressed by extracting four focused helper functions with correct signatures.

• New helpers Get_Requested_Reduction, Validate_Reduction, Apply_Dim_Reduction/Apply_Dim_Reduction_With_Depth, and Apply_Mip_Reduction replace duplicated inline logic in all three load-task classes, eliminating the original argument-order bugs.
• Dead code removed: the duplicated old Begin_Compressed_Load commented-out block is deleted; the Begin_Uncompressed_Load reduction path (which was always a no-op) is simplified to Reduction = 0.
• Mipmap loading fixed in CubeTextureLoadTaskClass::Load_Compressed_Mipmap: width >>= Reduction and height >>= Reduction were missing and are now added, matching the fix already present in the other two classes.

Confidence Score: 4/5

Safe to merge for the vast majority of texture loads; one dimension-mismatch in the mip-level upper-bound guard can allow a too-high mip count through when hardware-limit reduction fires, but normal settings-driven reduction is unaffected.

The three previously-identified argument-order and sign-mismatch bugs are all fixed by the new helper functions. The remaining issue is that Apply_Mip_Reduction receives the original Width/Height instead of the post-reduction dimensions when computing its max_mip_level_count clamp; this only matters when hardware texture-size limits force additional reduction beyond what the settings requested, so it will be invisible on most hardware.

The single changed file textureloader.cpp — specifically the three Begin_Compressed_Load call sites where Apply_Mip_Reduction is invoked with Width/Height rather than reduced_width/reduced_height.

Important Files Changed

Filename

Overview

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

Refactored texture reduction logic into extracted helpers; fixes previously identified argument-order and signed/unsigned bugs. One logic gap remains: Apply_Mip_Reduction receives Width/Height (original, non-reduced dimensions) instead of reduced_width/reduced_height, causing its max_mip_level_count guard to compute an upper bound from the wrong (larger) base, which can silently allow too many mip levels when hardware-limit reduction is applied.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Begin_Compressed_Load] --> B[Get_Texture_Information]
    B --> C[Set Width/Height to orig_width/height]
    C --> D[Validate_Texture_Size on Width/Height]
    D --> E[Validate_Reduction\nclamps Reduction to MipLevelCount-1\nand mip_count-1]
    E --> F[Apply_Dim_Reduction\nreduced_width/height = orig >> r\ntries r=Reduction r+1 ... until HW fits]
    F --> G[Apply_Mip_Reduction\nmip_level_count = mip_count - Reduction\nmax clamped by Width/Height NOT reduced_width/height]
    G --> H[_Create_DX8_Texture with reduced_width/height]
    H --> I[Load_Compressed_Mipmap\nwidth = Get_Width >> Reduction]
    style G fill:#ffe066,stroke:#cc9900

Loading

_{Reviews (6): Last reviewed commit: "fixup! bugfix(textureloader): Fix faulty..." | Re-trigger Greptile}

[xezon]

Bot says:

For most textures in Generals (power-of-two DDS within hardware limits), Validate_Texture_Size leaves Width unchanged and the mip-loading path works correctly. When a texture exceeds hardware dimension limits, Width is clamped in-place and Load_Compressed_Mipmap computes incorrect mip surface dimensions, corrupting the DDS-to-D3D surface copy for the exact scenario this PR intends to fix.

Core/Libraries/Source/WWVegas/WW3D2/textureloader.cpp - specifically the Begin_Compressed_Load methods at lines 1493-1495, 2087-2089, and 2427-2430.

This is what the original code did and I did not encounter any issues with it so I left this behavior. The original code even has a comment there which explicitly describes this behavior. I have restored the comment now.

[xezon]

The bot says:

The remaining issue is that Apply_Mip_Reduction receives the original Width/Height instead of the post-reduction dimensions when computing its max_mip_level_count clamp; this only matters when hardware texture-size limits force additional reduction beyond what the settings requested, so it will be invisible on most hardware.

This is what the original code did as well and it not clear to me why it does that and I did not observe a problem with it so I did not change it.

[Skyaero42]

There is some hilariously bad code in the original.
This is a nice improvement.