Skip to content

Commit

Permalink
Temporarily increase IGL_VERTEX_BUFFER_MAX
Browse files Browse the repository at this point in the history
Summary:
This fixes a crash when a shader has more than 24 uniforms. To easily repro the crash add a shader in studio with the following shader, P795703802, or alternatively download the following effect(bunnylol fx 278731336647151)

Effect with id 278731336647151 (sillyface) began crashing between studio version 166 (cut 09-06-2023) and 167 (cut 23-06-2023), see attached task. In that period IGL_VERTEX_BUFFER_MAX was decreased to 24 (D46710998).

The problem is that the vertex buffer index, coming from AR Engine, doesn't start at zero, but rather at `uniformCnt + attrIndex`, see https://www.internalfb.com/code/fbsource/[7368b136dbf9]/xplat/arfx/engine/renderer/material/Material.cpp?lines=1093

When we don't use uniform blocks, this number ends up being quite high as it'll be `numberOfUniforms + numberOfAttributes`. When the index exceeds IGL_VERTEX_BUFFER_MAX we end up asserting here: https://www.internalfb.com/code/fbsource/[05362e263b6ced1eabc1d8959392c6d8d6a29fcd]/xplat/graphics/igl/public/src/igl/opengl/RenderPipelineState.cpp?lines=133

and here: https://www.internalfb.com/code/fbsource/[05362e263b6ced1eabc1d8959392c6d8d6a29fcd]/xplat/graphics/igl/public/src/igl/opengl/RenderCommandAdapter.cpp?lines=136

And apparently we crash in production (again see task).

The core problem seems to be that graphics library expects the vertex buffer index to start from zero, while engine starts after the uniform count. The real fix is likely a bit more involved, but seeing as the issue is quite serious, as it's not very uncommon to use more than 24 uniforms, it seems better to deploy a temp fix first.

Reviewed By: justinvjoseph

Differential Revision: D47671530

fbshipit-source-id: 5a83e59a6954861c5ef2acb54935a3fe9391e3d5
  • Loading branch information
syeh1 authored and facebook-github-bot committed Jul 22, 2023
1 parent 4951df8 commit e898a8c
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/igl/Common.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ using Deleter = void (*)(void* IGL_NULLABLE);
/// Device Capabilities or Metal Features
constexpr size_t IGL_TEXTURE_SAMPLERS_MAX = 16;
constexpr size_t IGL_VERTEX_ATTRIBUTES_MAX = 24;
constexpr size_t IGL_VERTEX_BUFFER_MAX = 24;
constexpr size_t IGL_VERTEX_BUFFER_MAX = 128;
constexpr size_t IGL_VERTEX_BINDINGS_MAX = 24;
constexpr size_t IGL_UNIFORM_BLOCKS_BINDING_MAX = 16;

Expand Down

0 comments on commit e898a8c

Please sign in to comment.