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.

Sign up for GitHub

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

Jump to bottom

[Impeller] various StC fixes for GPU -> CPU readback plus BufferBindingGLES error #50951

Merged
merged 15 commits into from
Feb 27, 2024
Merged

[Impeller] various StC fixes for GPU -> CPU readback plus BufferBindingGLES error #50951

merged 15 commits into from
Feb 27, 2024

Conversation

jonahwilliams
Copy link
Member

@jonahwilliams jonahwilliams commented Feb 24, 2024 •
edited
Loading

More debugging for #50856

Potentially related to flutter/flutter#141472

@jonahwilliams
Copy link
Member Author

Might potentially need a flush too: https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkInvalidateMappedMemoryRanges.html

@jonahwilliams
Copy link
Member Author

Okay, we seemed to be hitting a case where we'd try to bind a void location (PADDING) in a shader which would fail. I added some code to handle that.

@jonahwilliams jonahwilliams changed the title Do everything plus barrier [Impeller] various StC fixes for GPU -> CPU readback plus BufferBindingGLES error Feb 25, 2024
@bdero
Copy link
Member

bdero commented Feb 27, 2024

Confirmed this unblocks the Vulkan tests on #50951

bdero
bdero approved these changes Feb 27, 2024
View reviewed changes
Copy link
Member

@bdero bdero left a comment

Choose a reason for hiding this comment

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

LGTM

@bdero
Copy link
Member

bdero commented Feb 27, 2024

(Pulling this out of draft to let the goldens run)

@bdero bdero marked this pull request as ready for review February 27, 2024 19:12
@flutter-dashboard
Copy link

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #50951 at sha 229f326

@bdero
Copy link
Member

bdero commented Feb 27, 2024

The golden diffs are gone. Let's smash that merge button.

@jonahwilliams jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 27, 2024
@auto-submit auto-submit bot merged commit 17dff87 into flutter:main Feb 27, 2024
28 checks passed
@jonahwilliams jonahwilliams deleted the do_everything_plus_barrier branch February 27, 2024 20:37
@zanderso zanderso mentioned this pull request Feb 28, 2024
Merged
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Feb 28, 2024
@zanderso
Manual roll Flutter Engine 2461280c38b7 to c79117b706e9 (#144290)
96a390a 
c79117b706 [Windows] Remove TODONE (flutter/engine#51005)
24c7b03ef0 [Windows] Introduce an accessibility plugin (flutter/engine#50975)
c805e43957 Roll Skia from 1beae57ea0dd to c8703c91675e (1 revision) (flutter/engine#51014)
17dff87ef3 [Impeller] various StC fixes for GPU -> CPU readback plus BufferBindingGLES error (flutter/engine#50951)
8457d10473 Roll Dart SDK from be6be2b9ba6e to a0e3c16eea3e (3 revisions) (flutter/engine#51015)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bdero bdero bdero approved these changes

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App e: impeller
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jonahwilliams @bdero