Skip to content

refactor(water): replace hardcoded offset#1728

Closed
Copilot wants to merge 8 commits into
devfrom
copilot/investigate-bsimagespaceshader-offset
Closed

refactor(water): replace hardcoded offset#1728
Copilot wants to merge 8 commits into
devfrom
copilot/investigate-bsimagespaceshader-offset

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jan 17, 2026

The code used a hardcoded 0x38 offset to access enableWaterTAA in the BSImagespaceShaderISTemporalAA structure, which is fragile across game updates and versions.

Changes

Upscaling.cpp

  • Added local ExtendedTemporalAAStruct definition that extends CommonLib's partial UNK_BSImagespaceShaderISTemporalAA structure
  • Structure includes enableWaterTAA at offset 0x38 with full memory layout
  • Updated ConfigureTAA() and Main_PostProcessing::thunk() to use structured access via reinterpret_cast
  • Added static_assert(sizeof(...) == 0x40) to verify layout
  • Includes TODO comment to contribute structure extension to alandtse/CommonLibVR upstream
// Before
bool* enableWaterTAA = reinterpret_cast<bool*>(reinterpret_cast<uintptr_t>(BSImagespaceShaderISTemporalAA) + 0x38LL);
*enableWaterTAA = upscaleMethod == UpscaleMethod::kNONE || upscaleMethod == UpscaleMethod::kTAA;

// After  
auto* extendedStruct = reinterpret_cast<ExtendedTemporalAAStruct*>(BSImagespaceShaderISTemporalAA);
extendedStruct->enableWaterTAA = upscaleMethod == UpscaleMethod::kNONE || upscaleMethod == UpscaleMethod::kTAA;

Structure Layout

struct ExtendedTemporalAAStruct {
    BSImagespaceShaderISTemporalAA* shader;                     // 0x00
    BSImagespaceShader*             BSImagespaceShaderISTemporalAA_UI;     // 0x08
    BSImagespaceShader*             BSImagespaceShaderISTemporalAA_Water;  // 0x10
    bool                            taaEnabled;                 // 0x18
    std::uint8_t                    pad19[7];                   // 0x19
    std::uint64_t                   unk20;                      // 0x20 - needs RE
    std::uint64_t                   unk28;                      // 0x28 - needs RE
    std::uint64_t                   unk30;                      // 0x30 - needs RE
    bool                            enableWaterTAA;             // 0x38
    std::uint8_t                    pad39[7];                   // 0x39
};
static_assert(sizeof(ExtendedTemporalAAStruct) == 0x40);

Members at 0x20-0x30 remain as placeholders pending further reverse engineering work.

Implementation Notes

The structure extension is defined locally in this repository rather than in CommonLibSSE-NG. This avoids dependency on unpushed submodule commits while still providing the benefits of structured access. The structure should be contributed upstream to alandtse/CommonLibVR in a future PR, at which point the local definition can be removed.

Original prompt

This section details on the original issue you should resolve

<issue_title>Investigate hardcoded offset for enableWaterTAA in BSImagespaceShaderISTemporalAA</issue_title>
<issue_description>## Description
The code currently uses a hardcoded memory offset (0x38LL) to access the enableWaterTAA member in the BSImagespaceShaderISTemporalAA structure:

bool* enableWaterTAA = reinterpret_cast<bool*>(reinterpret_cast<uintptr_t>(BSImagespaceShaderISTemporalAA) + 0x38LL);

Location: src/Features/Upscaling.cpp:705

Tasks

  • Investigate if CommonLibSSE/CommonLibVR has already reverse engineered the BSImagespaceShaderISTemporalAA structure. We use the ng branch of commonlibvr.
  • If available in CommonLib, refactor to use the proper structure member instead of hardcoded offset
  • If not available, perform RE work to properly define the structure
  • Document the structure definition for maintainability

Context

Hardcoded offsets are fragile and can break with game updates or across different game versions. Using proper structure definitions from CommonLib (if available) or creating our own RE definitions would make this code more maintainable.

References

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 17, 2026

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


Comment @coderabbitai help to get the list of available commands and usage tips.

GitHub Copilot and others added 3 commits January 17, 2026 08:18
…leWaterTAA

- Updated CommonLibSSE-NG to include enableWaterTAA in UNK_BSImagespaceShaderISTemporalAA
- Replaced hardcoded offset (0x38LL) with proper member access
- Added structure size assertion for safety
- Improves maintainability and reduces fragility across game versions

Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com>
Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com>
Copilot AI changed the title [WIP] Investigate hardcoded offset for enableWaterTAA in BSImagespaceShaderISTemporalAA Replace hardcoded offset with structure member for BSImagespaceShaderISTemporalAA::enableWaterTAA Jan 17, 2026
Copilot AI requested a review from alandtse January 17, 2026 08:22
Copy link
Copy Markdown
Collaborator

@alandtse alandtse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a strange spare root file here. Also, are you saying Commonlib already has this structure or does it need to have it PRed?

…module

- Removed _codeql_detected_source_root symlink (build artifact)
- Reverted CommonLibSSE-NG submodule to original commit
- Defined ExtendedTemporalAAStruct locally in Upscaling.cpp
- Added TODO comment to contribute structure to CommonLibVR upstream
- Structure now lives in this repo until it can be PRed to alandtse/CommonLibVR

Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jan 17, 2026

There's a strange spare root file here. Also, are you saying Commonlib already has this structure or does it need to have it PRed?

Fixed in f865286:

  1. Removed the _codeql_detected_source_root symlink (build artifact)
  2. Reverted CommonLibSSE-NG submodule to original commit - the structure does NOT exist in CommonLib yet

The extended structure is now defined locally in Upscaling.cpp with a TODO to contribute it to alandtse/CommonLibVR upstream. This avoids depending on unpushed submodule commits while still replacing the hardcoded offset.

Copilot AI and others added 3 commits January 17, 2026 08:35
- Updated Main_PostProcessing hook to use ExtendedTemporalAAStruct
- All accesses to taaEnabled now use the same structured approach
- Improves code consistency and maintainability

Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com>
- Added pad39[7] to ensure structure is exactly 0x40 bytes
- Fixes static_assert validation
- Structure size now matches memory layout requirements

Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com>
…ISTemporalAA::enableWaterTAA

Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com>
Copilot AI requested a review from alandtse January 17, 2026 08:38
@alandtse alandtse changed the title Replace hardcoded offset with structure member for BSImagespaceShaderISTemporalAA::enableWaterTAA refactor(water): replace hardcoded offset Jan 17, 2026
@github-actions
Copy link
Copy Markdown

Using provided base ref: 43b7727
Using base ref: 43b7727
Base commit date: 2026-01-17T01:57:42Z (Saturday, January 17, 2026 01:57 AM)
No actionable suggestions for changed features.

@alandtse alandtse closed this Jan 25, 2026
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.

Investigate hardcoded offset for enableWaterTAA in BSImagespaceShaderISTemporalAA

2 participants