Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Vulkan: Use of hard-coded MSAA value (e.g. in mesh preview) causes GPU hangs #53940

Closed
follower opened this issue Oct 17, 2021 · 4 comments · Fixed by #54606
Closed

[WIP] Vulkan: Use of hard-coded MSAA value (e.g. in mesh preview) causes GPU hangs #53940

follower opened this issue Oct 17, 2021 · 4 comments · Fixed by #54606

Comments

@follower
Copy link
Contributor

follower commented Oct 17, 2021

Godot version

Godot Engine v4.0.dev.20211015.official.f113dc986 (and earlier)

System information

Linux, Elementary OS 5.1, Intel(R) HD Graphics 4400 (HSW GT2), Vulkan API 1.2.182

Issue description

Summary:

Viewing a Mesh resource (seemingly of any subtype but most recently tested with BoxMesh) in the inspector results in a GPU hang.

Details:

  • Initially the GPU hang issue was observed to occur when opening/editing a MeshInstance3D that contained a mesh (or when creating one) but the specific cause was narrowed down to the rendering of the preview.

    (This means that if the "Node" tab is top-most--rather than the "Inspector" tab--it is possible to avoid the variant of the hang that prevents a newly created mesh from being saved when first created.)

  • It also turns out the underlying issue was the reason why selecting a MSAA setting of 2X (Viewport::MSAA_2X) for the project settings also led to a GPU hang.

My impression is that this specific issue may be GPU/driver specific however I have not isolated the lowest level of bug detail yet to be able to confirm this.

Specific cause of hang

The hang occurs as a result of this code:

viewport->set_msaa(Viewport::MSAA_2X);

When running under gdb, if the line is skipped (resulting in a default MSAA value of Viewport::MSAA_DISABLED) or the value is patched (to be the value of Viewport::MSAA_DISABLED) then the GPU hang does not occur.

(see also:

godot/scene/main/viewport.h

Lines 103 to 110 in 58aa020

enum MSAA {
MSAA_DISABLED,
MSAA_2X,
MSAA_4X,
MSAA_8X,
// 16x MSAA is not supported due to its high cost and driver bugs.
MSAA_MAX
};
)

Note: The issue does not occur for the Material Editor, presumably because it uses a value of Viewport::MSAA_4X (which for whatever reason doesn't appear to be affected):

viewport->set_msaa(Viewport::MSAA_4X);

Vulkan Validation output

When run with Vulkan validation layers enabled the output (in part) includes the message:

ERROR: VALIDATION - Message Id Number: 1607255571 | Message Id Name: VUID-VkImageCreateInfo-samples-02258
        Validation Error: [ VUID-VkImageCreateInfo-samples-02258 ] Object 0: handle = 0xabcee50, type = VK_OBJECT_TYPE_DEVICE; | MessageID = 0x5fccc613 |vkCreateImage(): samples VK_SAMPLE_COUNT_2_BIT is not supported by format 0x0000000D. The Vulkan spec states: samples must be a bit value that is set in imageCreateSampleCounts (as defined in Image Creation Limits) (https://vulkan.lunarg.com/doc/view/1.2.182.0/linux/1.2-extensions/vkspec.html#VUID-VkImageCreateInfo-samples-02258)
        Objects - 1
                Object[0] - VK_OBJECT_TYPE_DEVICE, Handle 180153936
   at: _debug_messenger_callback (drivers/vulkan/vulkan_context.cpp:157)

Note particularly the portion which reads samples VK_SAMPLE_COUNT_2_BIT is not supported by format 0x0000000D.

That said, the format 0x0000000D does look a bit suspicious to me because AFAICT it doesn't actually match with a valid format value--which, based on some debugging I've performed, makes me wonder if something unexpected is happening at some point as the parameters are being processed.

Additional code context

The initial call to Viewport::set_msaa() ends up here:

godot/scene/main/viewport.cpp

Lines 2777 to 2784 in 56078cc

void Viewport::set_msaa(MSAA p_msaa) {
ERR_FAIL_INDEX(p_msaa, MSAA_MAX);
if (msaa == p_msaa) {
return;
}
msaa = p_msaa;
RS::get_singleton()->viewport_set_msaa(viewport, RS::ViewportMSAA(p_msaa));
}

Which then eventually seems to find its way to RendererViewport::viewport_set_msaa() here:

void RendererViewport::viewport_set_msaa(RID p_viewport, RS::ViewportMSAA p_msaa) {
Viewport *viewport = viewport_owner.get_or_null(p_viewport);
ERR_FAIL_COND(!viewport);
if (viewport->msaa == p_msaa) {
return;
}
viewport->msaa = p_msaa;
_configure_3d_render_buffers(viewport);
}

(See also:

Which then heads to RendererViewport::_configure_3d_render_buffers() here:

void RendererViewport::_configure_3d_render_buffers(Viewport *p_viewport) {
if (p_viewport->render_buffers.is_valid()) {
if (p_viewport->size.width == 0 || p_viewport->size.height == 0) {
RSG::scene->free(p_viewport->render_buffers);
p_viewport->render_buffers = RID();
} else {
float scale_3d = p_viewport->scale_3d;
if (Engine::get_singleton()->is_editor_hint()) {
// Ignore the 3D viewport render scaling inside of the editor.
// The Half Resolution 3D editor viewport option should be used instead.
scale_3d = 1.0;
}
// Clamp 3D rendering resolution to reasonable values supported on most hardware.
// This prevents freezing the engine or outright crashing on lower-end GPUs.
const int width = CLAMP(p_viewport->size.width * scale_3d, 1, 16384);
const int height = CLAMP(p_viewport->size.height * scale_3d, 1, 16384);
RSG::scene->render_buffers_configure(p_viewport->render_buffers, p_viewport->render_target, width, height, p_viewport->msaa, p_viewport->screen_space_aa, p_viewport->use_debanding, p_viewport->get_view_count());
}
}
}

[TODO: Add additional details.]

Steps to reproduce

[TODO]

Minimal reproduction project

[TODO]

@Calinou
Copy link
Member

Calinou commented Oct 18, 2021

Given your hardware (Intel Haswell IGP) only has partial Vulkan support, this kind of issue will likely crop up throughout the engine.

That said, we should be able to use 4x MSAA in the 3D import preview just fine.

@Calinou Calinou added this to the 4.0 milestone Oct 18, 2021
@Calinou Calinou changed the title [WIP] Use of hard-coded MSAA value (e.g. in mesh preview) causes GPU hangs [WIP] Vulkan: Use of hard-coded MSAA value (e.g. in mesh preview) causes GPU hangs Oct 18, 2021
@follower
Copy link
Contributor Author

Thanks, I appreciate you considering that option & not writing off my current primary computing/Godot device entirely. :)

Vulkanisation...

While I will (now, after digging further & finding some semi-concrete factual backup :D ) concede that Haswell is indeed not a conformant Vulkan implementation, my impression is that...there is far less able to be attributed to that fact than what may appear to be the case. :)

Irregardless, while I'll admit that I'm probably heading toward the edgiest of edge cases, I still think that these edge cases are useful in terms of what steps we might want to take in order to increase the robustness of poor, sweet, Godot 4.0 as it journeys into the horrifying world of GPU drivers, hardware & documentation I've just been digging through. :D

(And, of course, attempting to enable Godot to run on as many older but serviceable Vulkan-capable devices to enable more people to reuse & recycle other cast off machines. :) Plus better to be me running into these issues with my...inquisitive...mind than an eager first time game designer/developer. :) )

Anyway, this doesn't immediately seem to be a Vulkan transgression, and, indeed the spec seems to suggest it's legit to only return a bare minimum of capability in this case. [TODO: Link to something justifying this bold claim. :) ]

Robustifying

Given the variability & unknowns around GPU drivers/hardware/docs I do think it's probably not a great idea to have hardcoded values like these MSAA values in Godot if we can avoid it.

While changing both values to 4X would reduce the risk, I do wonder if now might be a good time to evaluate whether it might be good to ensure that any such/similar values can support "potato" mode (i.e. ideally null mode) in addition to whatever default values make sense.

Anyway, it turns out that the absence of the 2X option is indeed a hardware limitation (related, AIUI to memory alignment details) & not a bug: https://gitlab.freedesktop.org/mesa/mesa/-/blob/91ff83b6c871dcfc33629924a7ebfb0d8b17be98/src/intel/isl/isl.c#L310-313

However, what I'm not 100% sure of yet is whether the driver has a related bug in what formats it has available or if Godot's current implementation is just expecting something & not checking if it's actually available.

Next

Still got a few notes to write-up but assuming the 4X option is good & doesn't break anything else I'll look at putting together a related PR as at least a concrete move forward.

Thanks again for looking at the issue & commenting! :)

@follower
Copy link
Contributor Author

follower commented Nov 7, 2021

The result of running Godot Engine v4.0.dev.custom_build.181d9337c with test project
vulk-msaa-issue-repro--002.zip*:

msaa-pr-54606-test-screenshot

* The project actually has MSAA 4X selected in the project settings but I turned that off for this screenshot.

So, #54606 does appear to successfully work around the specific issue related to the mesh preview, as the GPU did not hang and the preview can be seen.

@Calinou
Copy link
Member

Calinou commented May 4, 2022

Note that the issue about MSAA support not being checked before being applied is now tracked in #56233.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants