Skip to content

Fix errors when disabling gpu clustering#23547

Merged
mockersf merged 5 commits intobevyengine:mainfrom
beicause:android-clustering
Mar 28, 2026
Merged

Fix errors when disabling gpu clustering#23547
mockersf merged 5 commits intobevyengine:mainfrom
beicause:android-clustering

Conversation

@beicause
Copy link
Copy Markdown
Contributor

@beicause beicause commented Mar 28, 2026

Objective

Fix errors when disabling gpu clustering. Fixes #23208.

Solution

Set cluster buffer bindings correctly. Allow writing cluster on cpu buffer if gpu clustering is disabled.

Testing

Tested mobile example on android.

@alice-i-cecile alice-i-cecile requested a review from atlv24 March 28, 2026 04:43
@alice-i-cecile alice-i-cecile added this to the 0.19 milestone Mar 28, 2026
@alice-i-cecile alice-i-cecile requested a review from pcwalton March 28, 2026 04:43
@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Mar 28, 2026
@github-project-automation github-project-automation bot moved this to Needs SME Triage in Rendering Mar 28, 2026
@alice-i-cecile alice-i-cecile added the O-Android Specific to the Android mobile operating system label Mar 28, 2026
@alice-i-cecile alice-i-cecile requested a review from mockersf March 28, 2026 04:44
//
// Some android devices report the capabilities and limits wrong, so we can't rely on them.
// See <https://github.com/bevyengine/bevy/issues/23208> for Android issues
!cfg!(target_os = "android")
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.

Suggested change
!cfg!(target_os = "android")
!(cfg!(target_os = "android") || cfg!(target_os = "ios"))

If this gets a target_os for ios as well, it fixes #23428 for the ios simulator as well

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm unable to test if #23428 is fixed. It may work without gating ios or ios simulator.

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.

sorry, I could've been more clear.

What I mean is that I tested this PR by running make in the examples/mobile directory, and observed the problem from #23428 in the simulator. Then I applied the fix suggested here, which removed the issue.

@mockersf might be able to confirm on an actual device, although I don't think the issue appears on their actual ios device.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Does it only apply to ios simulator? If so we can use target_abi = "sim". But I'm not sure why it can't work.

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.

gfx-rs/wgpu#7057
the iOS simulator has some weird behaviour about GPU validation, something like it's reporting limits available by the host machine by using validation from an actual devices, which are not the same, and this causes a few features to break. so yes for limiting by the target_abi, we already do in a few places

@beicause
Copy link
Copy Markdown
Contributor Author

beicause commented Mar 28, 2026

I realized this is may not be the correct approach. This PR forcing non-GPU clustering to use UBO, but before GPU clustering #23036 we can use SSBO.

Edit: Resolved

@beicause beicause marked this pull request as draft March 28, 2026 07:11
@github-actions
Copy link
Copy Markdown
Contributor

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-23547

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

@beicause beicause marked this pull request as ready for review March 28, 2026 09:14
@kfc35 kfc35 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 Mar 28, 2026
@mockersf mockersf added this pull request to the merge queue Mar 28, 2026
Merged via the queue into bevyengine:main with commit 862ba07 Mar 28, 2026
40 checks passed
@github-project-automation github-project-automation bot moved this from Needs SME Triage to Done in Rendering Mar 28, 2026
@beicause beicause deleted the android-clustering branch March 28, 2026 17:34
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 C-Bug An unexpected or incorrect behavior O-Android Specific to the Android mobile operating system S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Rendering broken on android since GPU clustering for lights

6 participants