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

disable gpu preprocessing on android with Adreno 6xx GPU #13323

Conversation

mockersf
Copy link
Member

Objective

Solution

  • Disable gpu preprocessing when feature SAMPLED_TEXTURE_AND_STORAGE_BUFFER_ARRAY_NON_UNIFORM_INDEXING is not available

Testing

  • Tested on android device that used to crash

@mockersf mockersf added A-Rendering Drawing game state to the screen P-Crash A sudden unexpected crash labels May 10, 2024
@mockersf mockersf added this to the 0.14 milestone May 10, 2024
@mockersf mockersf requested a review from pcwalton May 10, 2024 21:03
Copy link
Contributor

@JMS55 JMS55 left a comment

Choose a reason for hiding this comment

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

Where are binding arrays used? The PR this fixes didn't use any as far as I can tell.

@mockersf
Copy link
Member Author

Where are binding arrays used? The PR this fixes didn't use any as far as I can tell.

I printed limits and features on a working device and on a crashing device, this one looked the most closely related
crashing:

SystemInfo { os: "Android 11 2201117TI", kernel: "4.19.157-perf-gc70fe342b493", cpu: "not available", core_count: "8", memory: "3.6 GiB" }
AdapterInfo { name: "Adreno (TM) 610", vendor: 20803, device: 100728833, device_type: IntegratedGpu, driver: "Qualcomm Technologies Inc. Adreno Vulkan Driver", driver_info: "Driver Build: 5eaa426211, I07ee46fc66, 1633700387\nDate: 10/08/21\nCompiler Version: EV031.32.02.16\nDriver Branch: \n", backend: Vulkan }
Limits { max_texture_dimension_1d: 16384, max_texture_dimension_2d: 16384, max_texture_dimension_3d: 2048, max_texture_array_layers: 2048, max_bind_groups: 4, max_bindings_per_bind_group: 1000, max_dynamic_uniform_buffers_per_pipeline_layout: 32, max_dynamic_storage_buffers_per_pipeline_layout: 16, max_sampled_textures_per_shader_stage: 524288, max_samplers_per_shader_stage: 524288, max_storage_buffers_per_shader_stage: 524288, max_storage_textures_per_shader_stage: 524288, max_uniform_buffers_per_shader_stage: 524288, max_uniform_buffer_binding_size: 65536, max_storage_buffer_binding_size: 536870912, max_vertex_buffers: 16, max_buffer_size: 2147483647, max_vertex_attributes: 16, max_vertex_buffer_array_stride: 2048, min_uniform_buffer_offset_alignment: 64, min_storage_buffer_offset_alignment: 64, max_inter_stage_shader_components: 112, max_compute_workgroup_storage_size: 16384, max_compute_invocations_per_workgroup: 1024, max_compute_workgroup_size_x: 1024, max_compute_workgroup_size_y: 1024, max_compute_workgroup_size_z: 64, max_compute_workgroups_per_dimension: 65535, max_push_constant_size: 128, max_non_sampler_bindings: 4294967295 }
Features(DEPTH_CLIP_CONTROL | TIMESTAMP_QUERY | INDIRECT_FIRST_INSTANCE | RG11B10UFLOAT_RENDERABLE | DEPTH32FLOAT_STENCIL8 | TEXTURE_COMPRESSION_ETC2 | TEXTURE_COMPRESSION_ASTC | TEXTURE_FORMAT_16BIT_NORM | TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES | PIPELINE_STATISTICS_QUERY | TIMESTAMP_QUERY_INSIDE_PASSES | MAPPABLE_PRIMARY_BUFFERS | TEXTURE_BINDING_ARRAY | BUFFER_BINDING_ARRAY | STORAGE_RESOURCE_BINDING_ARRAY | MULTI_DRAW_INDIRECT | MULTI_DRAW_INDIRECT_COUNT | PUSH_CONSTANTS | ADDRESS_MODE_CLAMP_TO_ZERO | ADDRESS_MODE_CLAMP_TO_BORDER | POLYGON_MODE_LINE | POLYGON_MODE_POINT | VERTEX_WRITABLE_STORAGE | CLEAR_TEXTURE | SPIRV_SHADER_PASSTHROUGH | MULTIVIEW | SHADER_UNUSED_VERTEX_OUTPUT | TEXTURE_FORMAT_NV12 | SHADER_I16 | SHADER_PRIMITIVE_INDEX | DUAL_SOURCE_BLENDING)

working:

SystemInfo { os: "Android 13 SM-S911B", kernel: "5.15.74-android13-8-26621433-abS911BXXU2AWF1", cpu: "not available", core_count: "8", memory: "6.9 GiB" }
AdapterInfo { name: "Adreno (TM) 740", vendor: 20803, device: 1124403713, device_type: IntegratedGpu, driver: "Qualcomm Technologies Inc. Adreno Vulkan Driver", driver_info: "Driver Build: d44197479c, I2991b7e11e, 1685536936\nDate: 05/31/23\nCompiler Version: E031.41.03.36\nDriver Branch: \n", backend: Vulkan }
Limits { max_texture_dimension_1d: 16384, max_texture_dimension_2d: 16384, max_texture_dimension_3d: 2048, max_texture_array_layers: 2048, max_bind_groups: 7, max_bindings_per_bind_group: 1000, max_dynamic_uniform_buffers_per_pipeline_layout: 32, max_dynamic_storage_buffers_per_pipeline_layout: 16, max_sampled_textures_per_shader_stage: 16777216, max_samplers_per_shader_stage: 16777216, max_storage_buffers_per_shader_stage: 16777216, max_storage_textures_per_shader_stage: 16777216, max_uniform_buffers_per_shader_stage: 16777216, max_uniform_buffer_binding_size: 65536, max_storage_buffer_binding_size: 134217728, max_vertex_buffers: 16, max_buffer_size: 2147483647, max_vertex_attributes: 32, max_vertex_buffer_array_stride: 2048, min_uniform_buffer_offset_alignment: 64, min_storage_buffer_offset_alignment: 64, max_inter_stage_shader_components: 112, max_compute_workgroup_storage_size: 32768, max_compute_invocations_per_workgroup: 1024, max_compute_workgroup_size_x: 1024, max_compute_workgroup_size_y: 1024, max_compute_workgroup_size_z: 1024, max_compute_workgroups_per_dimension: 65535, max_push_constant_size: 256, max_non_sampler_bindings: 4294967295 }
Features(DEPTH_CLIP_CONTROL | TIMESTAMP_QUERY | INDIRECT_FIRST_INSTANCE | RG11B10UFLOAT_RENDERABLE | DEPTH32FLOAT_STENCIL8 | TEXTURE_COMPRESSION_BC | TEXTURE_COMPRESSION_ETC2 | TEXTURE_COMPRESSION_ASTC | TEXTURE_FORMAT_16BIT_NORM | TEXTURE_COMPRESSION_ASTC_HDR | TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES | PIPELINE_STATISTICS_QUERY | TIMESTAMP_QUERY_INSIDE_PASSES | MAPPABLE_PRIMARY_BUFFERS | TEXTURE_BINDING_ARRAY | BUFFER_BINDING_ARRAY | STORAGE_RESOURCE_BINDING_ARRAY | SAMPLED_TEXTURE_AND_STORAGE_BUFFER_ARRAY_NON_UNIFORM_INDEXING | UNIFORM_BUFFER_AND_STORAGE_TEXTURE_ARRAY_NON_UNIFORM_INDEXING | PARTIALLY_BOUND_BINDING_ARRAY | MULTI_DRAW_INDIRECT | MULTI_DRAW_INDIRECT_COUNT | PUSH_CONSTANTS | ADDRESS_MODE_CLAMP_TO_ZERO | ADDRESS_MODE_CLAMP_TO_BORDER | POLYGON_MODE_LINE | POLYGON_MODE_POINT | CONSERVATIVE_RASTERIZATION | VERTEX_WRITABLE_STORAGE | CLEAR_TEXTURE | SPIRV_SHADER_PASSTHROUGH | MULTIVIEW | SHADER_UNUSED_VERTEX_OUTPUT | TEXTURE_FORMAT_NV12 | SHADER_I16 | SHADER_PRIMITIVE_INDEX | DUAL_SOURCE_BLENDING)

@JMS55 do you have a suggestion on what else I could limit?

@JMS55
Copy link
Contributor

JMS55 commented May 11, 2024

Probably try checking if storage buffers are supported.

@mockersf
Copy link
Member Author

They should be, at least the limits says so

@JMS55
Copy link
Contributor

JMS55 commented May 14, 2024

Looking at the two logs you posted, I really can't see anything else that would cause it. But I can't find any usage of binding array's in bevy's source code, except for env map lights / irradiance volumes.

@mockersf
Copy link
Member Author

I think we could:

  1. add a comment that this is a way to avoid issues on lower-end devices
  2. add a check that target is android, and only limit on that feature there
  3. instead of checking for features, limit on gpu model

I would like to avoid 3 because starting to list specific GPUs is without end, I would be ok with 1 or 2 or a combination of them.

@IceSentry
Copy link
Contributor

I think I'd prefer a mix of 1 and 2 personally, but 1 would be good enough

@mockersf
Copy link
Member Author

I limited the check to Android, and added a comment 👍

@pcwalton
Copy link
Contributor

pcwalton commented May 16, 2024

I'm not sure that using an unrelated feature as a proxy for Adreno 6xx series is better than just checking for Adreno 6xx series to begin with. Other GPU manufacturers might not support binding arrays but might be just fine with GPU preprocessing, and likewise they might have binding arrays but also have bugs that prevent use of GPU preprocessing. Checking for Adreno 6xx directly would make things clearer, and doesn't seem any more fragile in practice.

(I suppose you could also, as an alternative, check for driver date and denylist everything prior to 2022, though that also seems like a crude proxy and I don't like it...)

@alice-i-cecile alice-i-cecile added S-Needs-Design This issue requires design work to think about how it would best be accomplished S-Needs-Review Needs reviewer attention (from anyone!) to move forward S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 16, 2024
@mockersf mockersf changed the title disable gpu preprocessing without SAMPLED_TEXTURE_AND_STORAGE_BUFFER_ARRAY_NON_UNIFORM_INDEXING disable gpu preprocessing on android with Adreno 6xx GPU May 18, 2024
@mockersf
Copy link
Member Author

@pcwalton switched to disabling for Adreno 6xx on Android 👍

@mockersf mockersf added S-Needs-Review Needs reviewer attention (from anyone!) to move forward and removed S-Needs-Design This issue requires design work to think about how it would best be accomplished S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels May 18, 2024
Copy link
Contributor

@JMS55 JMS55 left a comment

Choose a reason for hiding this comment

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

Bizarre that it happens, but oh well. Such is Android graphics drivers.

@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 30, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 30, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 30, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 30, 2024
Merged via the queue into bevyengine:main with commit e208fb7 May 30, 2024
29 checks passed
github-merge-queue bot pushed a commit that referenced this pull request Jul 8, 2024
…#14176)

# Objective

Fix #14146 

## Solution

Expansion of #13323 , excluded Adreno 730 and earlier.

## Testing

Tested on android device(Adreno 730) that used to crash
mockersf pushed a commit that referenced this pull request Aug 2, 2024
…#14176)

# Objective

Fix #14146 

## Solution

Expansion of #13323 , excluded Adreno 730 and earlier.

## Testing

Tested on android device(Adreno 730) that used to crash
github-merge-queue bot pushed a commit that referenced this pull request Aug 27, 2024
# Objective

The Android example on Adreno 642L currently crashes on startup.

Previous PRs #14176 and #13323 have adressed this specific crash
occurring on some Adreno GPUs, that fix works as it should but isn't
applied when to the GPU name contains a suffix like in the case of
`642L`.

## Solution

- Amending the logic to filter out any parts of the GPU name not
containing digits thus enabling the fix on `642L`.

## Testing

- Ran the Android example on a Nothing Phone 1. Before this change it
crashed, after it works as intended.

---------

Co-authored-by: Sam Pettersson <sam.pettersson@geoguessr.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen P-Crash A sudden unexpected crash S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Android segfaults since MeshUniforms generation on the GPU
5 participants