Skip to content

GH-1345 Simplify Vulkan check logic - #1371

Merged
Arctis-Fireblight merged 1 commit into
Redot-Engine:masterfrom
JoltedJon:GH-1345
Sep 3, 2026
Merged

Arctis-Fireblight merged 1 commit into
Redot-Engine:masterfrom
JoltedJon:GH-1345

Conversation

@JoltedJon

@JoltedJon JoltedJon commented Sep 1, 2026 •

Copy link
Copy Markdown
Contributor

Resolves #1345

The checking logic I originally implemented did not take into account OpenXR which needs to handle it's own vulkan devices. This keeps some of the logic without initializing any vulkan devices.

Summary by CodeRabbit

  • Bug Fixes
    • Improved Vulkan support detection for faster, more reliable startup.
    • Corrected the OpenXR initialization error message to reference Redot when falling back to normal mode.

@coderabbitai

coderabbitai Bot commented Sep 1, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 445f4e74-68f4-48c6-9cc2-050982a7f0b1

📥 Commits

Reviewing files that changed from the base of the PR and between 335d159 and 6552290.

📒 Files selected for processing (2)
  • drivers/vulkan/rendering_context_driver_vulkan.cpp
  • modules/openxr/register_types.cpp

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


Walkthrough

The Vulkan support check no longer creates a probe instance or enumerates physical devices. With USE_VOLK, it checks volk initialization and vkGetInstanceProcAddr. The OpenXR initialization failure message now refers to Redot.

Changes

Vulkan support detection

Layer / File(s) Summary
Simplify Vulkan availability checks
drivers/vulkan/rendering_context_driver_vulkan.cpp
_vulkan_is_supported() removes VulkanHooks and probe-instance checks. With USE_VOLK, it initializes volk and verifies vkGetInstanceProcAddr.

OpenXR initialization message

Layer / File(s) Summary
Update initialization failure text
modules/openxr/register_types.cpp
The fallback message now says “Redot will start in normal mode”.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: 🔵 Low · up to 65522

The change simplifies Vulkan/OpenXR startup checks without bypassing final device validation. If device acquisition fails after instance creation, a failed rendering context may retain partial Vulkan state until it is destroyed; this appears bounded but warrants owner awareness or follow-up cleanup.

Suggested reviewers: mcdubhghlas, generalprotectionfault

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The OpenXR message change from "Godot will start in normal mode" to "Redot will start in normal mode" is unrelated to the Vulkan logic required by issue #1345. Remove the unrelated OpenXR message change, or link an issue that requires the branding correction.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: simplifying Vulkan support-check logic.
Linked Issues check ✅ Passed The Vulkan change removes temporary Vulkan instance and physical-device initialization while retaining the volk availability check. This directly supports OpenXR device management and addresses issue …
Full details: Linked Issues check

Explanation

The Vulkan change removes temporary Vulkan instance and physical-device initialization while retaining the volk availability check. This directly supports OpenXR device management and addresses issue #1345, which reports null rendering devices when OpenXR is enabled.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@DaveTheEggman DaveTheEggman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM from the rendering side

@Arctis-Fireblight Arctis-Fireblight left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@Arctis-Fireblight
Arctis-Fireblight merged commit 8a1e359 into Redot-Engine:master Sep 3, 2026
17 checks passed
@github-project-automation github-project-automation Bot moved this from Open to Done in Engine Overview Sep 3, 2026
@JoltedJon
JoltedJon deleted the GH-1345 branch September 3, 2026 06:56
@JoltedJon JoltedJon linked an issue Sep 5, 2026 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Redot crashes in play mode when MangoHud is active OpenXR breaks get_rendering_device and create_local_rendering_device

3 participants