Skip to content

fix(upscaling): revert depth copy condition#2309

Merged
alandtse merged 1 commit into
devfrom
fix-upscaling-depth-copy
May 10, 2026
Merged

fix(upscaling): revert depth copy condition#2309
alandtse merged 1 commit into
devfrom
fix-upscaling-depth-copy

Conversation

@jiayev
Copy link
Copy Markdown
Collaborator

@jiayev jiayev commented May 10, 2026

reverted the added condition in #2193
to fix frozen depth after upscaling

Summary by CodeRabbit

  • Bug Fixes
    • Improved depth texture consistency in upscaling operations to ensure uniform visual quality across all application states.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 10, 2026

📝 Walkthrough

Walkthrough

The depth-copy operation in UpscaleDepth is simplified from being conditional on menu context to unconditionally attempting the copy when resources are non-aliased. The surrounding comment is updated to note when the copy may be skipped.

Changes

Depth Upscaling Logic

Layer / File(s) Summary
Unconditional Depth Copy
src/Features/Upscaling.cpp
Depth texture copy from depthCopy.texture to depth.texture is now always attempted when resources are non-aliased, removing the prior check for menu context. Comment updated to reflect cases where copy may be absent, such as the map menu.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested reviewers

  • doodlum
  • alandtse

Poem

🐰 A depth so true, no menus needed—
Just copy straight through when resources heed it!
No context to check, just aliasing's care,
Upscaling flows simpler, lighter, more fair! 🎨✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(upscaling): revert depth copy condition' directly describes the main change: reverting a conditional check in the upscaling depth copy logic to fix frozen depth issues.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-upscaling-depth-copy

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 OpenGrep (1.20.0)

OpenGrep fatal error (exit code 2): [00.19][ERROR]: Error: exception Unix_error: No such file or directory stat src/Features/Upscaling.cpp
Raised by primitive operation at UTmp.replace_named_pipe_by_regular_file_if_needed in file "libs/commons/UTmp.ml", line 145, characters 8-27
Called from Scan_CLI.replace_target_roots_by_regular_files_where_needed.(fun) in file "src/osemgrep/cli_scan/Scan_CLI.ml", lines 1086-1087, characters 19-65
Called from List_.fast_map in file "libs/commons/List_.ml", line 81, characters 17-20
Called from S


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

@github-actions
Copy link
Copy Markdown

No actionable suggestions for changed features.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/Features/Upscaling.cpp (1)

1922-1924: ⚡ Quick win

Add an issue-closing reference in the PR description.

Since this change is a bug fix, consider adding Fixes #<issue> or Closes #<issue> for traceability.

As per coding guidelines, "When reviewing PRs, please provide suggestions for: ... Issue References (if PR fixes bugs or implements features): Suggest adding appropriate GitHub keywords: - 'Fixes #123' or 'Closes #123' for bug fixes".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Features/Upscaling.cpp` around lines 1922 - 1924, The PR lacks an
issue-closing reference: update the PR description to include a GitHub keyword
like "Fixes #<issue>" or "Closes #<issue>" that corresponds to this bug fix (the
change around copyIfNonAliased(depthCopy.texture, depth.texture)); mention the
related issue number so the PR will auto-close the bug when merged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/Features/Upscaling.cpp`:
- Around line 1922-1924: The PR lacks an issue-closing reference: update the PR
description to include a GitHub keyword like "Fixes #<issue>" or "Closes
#<issue>" that corresponds to this bug fix (the change around
copyIfNonAliased(depthCopy.texture, depth.texture)); mention the related issue
number so the PR will auto-close the bug when merged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7d1dbdb3-6ce5-42e4-9f2d-406a7ea77395

📥 Commits

Reviewing files that changed from the base of the PR and between 0a6ad65 and 8a6bf6e.

📒 Files selected for processing (1)
  • src/Features/Upscaling.cpp

@github-actions
Copy link
Copy Markdown

✅ A pre-release build is available for this PR:
Download

@alandtse alandtse merged commit 57127d3 into dev May 10, 2026
15 checks passed
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.

3 participants