Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[CP] [skwasm] Fix platform view occlusion logic. #54871

Merged
merged 3 commits into from
Sep 10, 2024
Merged

[CP] [skwasm] Fix platform view occlusion logic. #54871

merged 3 commits into from
Sep 10, 2024

Conversation

Rexios80
Copy link
Member

@Rexios80 Rexios80 commented Aug 29, 2024 •
edited
Loading

Cherry-pick of #54061

@eyebrowsoffire @Rexios80
[skwasm] Fix platform view occlusion logic. (flutter#54061)
c9f5242 
The occlusion rectangle for platform views was going through this `inverseMapRect` code path, which actually was giving us the wrong results. The operations should just be doing the normal transformation on the rectangles to get the right result. It actually turns out we don't need the inverse mapping function, so I removed it, and I renamed the somewhat confusingly named `cullRect` function to `mapRect` which I think makes a bit more sense.

This should resolve flutter/flutter#152139
@github-actions github-actions bot added the platform-web Code specifically for the web engine label Aug 29, 2024
@Rexios80 Rexios80 mentioned this pull request Aug 29, 2024
Closed
eyebrowsoffire
eyebrowsoffire approved these changes Aug 29, 2024
View reviewed changes
Copy link
Contributor

@eyebrowsoffire eyebrowsoffire left a comment

Choose a reason for hiding this comment

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

Cherry pick looks good to me! Thanks!

eyebrowsoffire
eyebrowsoffire suggested changes Aug 29, 2024
View reviewed changes
Copy link
Contributor

@eyebrowsoffire eyebrowsoffire left a comment

Choose a reason for hiding this comment

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

Looks like CI is failing, I'm guessing some there are some test changes that didn't make it over in the CP for some reason?

Rexios80
Rexios80 commented Aug 29, 2024
View reviewed changes
@Rexios80
Copy link
Member Author

Also I did notice that there is a flutter-3.24-candidate.1 branch. Is flutter-3.24-candidate.0 the correct one? The https://github.com/flutter/flutter/blob/stable/bin/internal/release-candidate-branch.version file points to flutter-3.24-candidate.0.

eyebrowsoffire
eyebrowsoffire approved these changes Sep 5, 2024
View reviewed changes
Copy link
Contributor

@eyebrowsoffire eyebrowsoffire left a comment

Choose a reason for hiding this comment

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

Looks good! I'm not 100% sure about which branch should be targeted, I'll let the release team handle that particular question.

@Rexios80 Rexios80 added cp: stable cherry pick to the stable release candidate branch cp: review add the cp request to the review queue of release engineers labels Sep 5, 2024
@itsjustkevin itsjustkevin added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 10, 2024
@auto-submit auto-submit bot merged commit a4777ff into flutter:flutter-3.24-candidate.0 Sep 10, 2024
30 checks passed
@itsjustkevin itsjustkevin mentioned this pull request Sep 11, 2024
Merged
auto-submit bot pushed a commit that referenced this pull request Sep 11, 2024
@itsjustkevin
[flutter_releases] Flutter stable 3.24.3 Engine Cherrypicks (#55108)
3633501 
# Flutter stable 3.24.3 Engine

## Scheduled Cherrypicks

- Roll dart revision: dart-lang/sdk@179da3ba6
- #54842
- #54843
- #54871
- #54870
- #54942
- #55007
- #55056
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
1 more reviewer

@eyebrowsoffire eyebrowsoffire eyebrowsoffire 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: review add the cp request to the review queue of release engineers cp: stable cherry pick to the stable release candidate branch platform-web Code specifically for the web engine
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Rexios80 @eyebrowsoffire @itsjustkevin