Skip to content

[CP] Fixes for Impeller DrawVertices issues involving snapshots with empty sizes #163261

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
Merged
auto-submit merged 5 commits into from
Feb 26, 2025
Merged

[CP] Fixes for Impeller DrawVertices issues involving snapshots with empty sizes #163261

auto-submit merged 5 commits into from
Feb 26, 2025

Conversation

@jason-simmons
Copy link
Member

@jason-simmons jason-simmons commented Feb 13, 2025

See #162969

Includes #163099 and #163177

@github-actions github-actions bot added engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels Feb 13, 2025
@jason-simmons
Copy link
Member Author

jason-simmons commented Feb 13, 2025

Impacted Users

Impeller users calling the Canvas DrawVertices API in some cases.

Impact Description

DrawVertices will crash when using some kinds of shaders.

Workaround

None

Risk

Low

Test Coverage

Includes unit tests

Validation Steps

Run the example app in #162969 and check that the crash does not occur.

jonahwilliams
jonahwilliams approved these changes Feb 13, 2025
View reviewed changes
Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

LGTM

cc @zanderso

@jonahwilliams jonahwilliams added cp: review Cherry-picks in the review queue cp: stable cherry pick this pull request to stable release candidate branch labels Feb 13, 2025
@zanderso zanderso added the cp: approved Approved cherry-pick request label Feb 13, 2025
@zanderso
Copy link
Member

zanderso commented Feb 13, 2025

CP lgtm

jonahwilliams
jonahwilliams reviewed Feb 14, 2025
View reviewed changes
Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

Sorry, I forgot that we also need to update changelog for stable channel cherry picks.

See https://github.com/flutter/flutter/pull/163058/files#diff-06572a96a58dc510037d5efa622f9bec8519bc1beab13c9f251e97e657a9d4ed for an example.

@jason-simmons
Copy link
Member Author

jason-simmons commented Feb 15, 2025

Added a change log entry.

@eyebrowsoffire eyebrowsoffire added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 20, 2025
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Feb 20, 2025
@auto-submit
Copy link
Contributor

auto-submit bot commented Feb 20, 2025

autosubmit label was removed for flutter/flutter/163261, because - The status or check suite Mac clangd has failed. Please fix the issues identified (or deflake) before re-applying this label.

@eyebrowsoffire eyebrowsoffire added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 20, 2025
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Feb 20, 2025
@auto-submit
Copy link
Contributor

auto-submit bot commented Feb 20, 2025

autosubmit label was removed for flutter/flutter/163261, because - The status or check suite Mac mac_unopt has failed. Please fix the issues identified (or deflake) before re-applying this label.

@justinmc
Copy link
Contributor

justinmc commented Feb 25, 2025

The presubmits were fixed in #164050. Pushing a merge commit to see if this PR will pass now.

@justinmc justinmc added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 26, 2025
@auto-submit auto-submit bot merged commit 836b6fd into flutter:flutter-3.29-candidate.0 Feb 26, 2025
156 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025
@engine-flutter-autoroll
836b6fdfe [CP] Fixes for Impeller DrawVertices issues involving snaps…
8511b8b 
…hots with empty sizes (flutter/flutter#163261)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2025
@engine-flutter-autoroll
836b6fdfe [CP] Fixes for Impeller DrawVertices issues involving snaps…
2c7765b 
…hots with empty sizes (flutter/flutter#163261)
@reidbaker reidbaker mentioned this pull request Apr 4, 2025
Merged
9 tasks
Fintasys pushed a commit to Fintasys/flutter that referenced this pull request May 14, 2025
@jason-simmons @Fintasys
[CP] Fixes for Impeller DrawVertices issues involving snapshots with …
de61a34 
…empty sizes (flutter#163261)

See flutter#162969

Includes flutter#163099 and flutter#163177
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@jonahwilliams jonahwilliams jonahwilliams approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App cp: approved Approved cherry-pick request cp: review Cherry-picks in the review queue cp: stable cherry pick this pull request to stable release candidate branch e: impeller Impeller rendering backend issues and features requests engine flutter/engine related. See also e: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jason-simmons @zanderso @justinmc @jonahwilliams @eyebrowsoffire