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

[skwasm] Fix platform view occlusion logic. #54061

Merged
merged 3 commits into from
Jul 24, 2024

Conversation

eyebrowsoffire
Copy link
Contributor

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 Jul 23, 2024
Copy link
Contributor

@yjbanov yjbanov left a comment

Choose a reason for hiding this comment

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

A couple of nitpicky optimization suggestions, but LGTM.


@override
ui.Rect inverseMapRect(ui.Rect rect) => rect;
ui.Rect mapRect(ui.Rect contentRect) => (filter as SceneImageFilter).filterBounds(contentRect);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we cast it in the constructor so we don't have to cast on every usage?

final Matrix4 matrix = getMatrix()..invert();
return matrix.transformRect(rect);
}
ui.Rect mapRect(ui.Rect contentRect) => getMatrix().transformRect(contentRect);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the matrix immutable? If so, can we create the matrix in the constructor and reuse it when rendering?

@eyebrowsoffire eyebrowsoffire added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 24, 2024
@auto-submit auto-submit bot merged commit 86f4938 into flutter:main Jul 24, 2024
29 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 24, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jul 24, 2024
…152260)

flutter/engine@490576d...c2f489d

2024-07-24 [email protected] Fix embedder comments about struct_size (flutter/engine#54077)
2024-07-24 [email protected] share platform view slicing logic across iOS and Android. (flutter/engine#54010)
2024-07-24 [email protected] [skwasm] Fix platform view occlusion logic. (flutter/engine#54061)
2024-07-24 [email protected] Roll Skia from 25f26f673502 to c11932925658 (4 revisions) (flutter/engine#54081)
2024-07-24 [email protected] Upgrade Engine Android SDK to 35 (flutter/engine#53574)
2024-07-24 [email protected] Roll Skia from a9b1043eb23e to 25f26f673502 (1 revision) (flutter/engine#54079)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
TytaniumDev pushed a commit to TytaniumDev/flutter that referenced this pull request Aug 7, 2024
…lutter#152260)

flutter/engine@490576d...c2f489d

2024-07-24 [email protected] Fix embedder comments about struct_size (flutter/engine#54077)
2024-07-24 [email protected] share platform view slicing logic across iOS and Android. (flutter/engine#54010)
2024-07-24 [email protected] [skwasm] Fix platform view occlusion logic. (flutter/engine#54061)
2024-07-24 [email protected] Roll Skia from 25f26f673502 to c11932925658 (4 revisions) (flutter/engine#54081)
2024-07-24 [email protected] Upgrade Engine Android SDK to 35 (flutter/engine#53574)
2024-07-24 [email protected] Roll Skia from a9b1043eb23e to 25f26f673502 (1 revision) (flutter/engine#54079)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Rexios80 pushed a commit to Rexios80/flutter_engine that referenced this pull request Aug 29, 2024
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
Buchimi pushed a commit to Buchimi/flutter that referenced this pull request Sep 2, 2024
…lutter#152260)

flutter/engine@490576d...c2f489d

2024-07-24 [email protected] Fix embedder comments about struct_size (flutter/engine#54077)
2024-07-24 [email protected] share platform view slicing logic across iOS and Android. (flutter/engine#54010)
2024-07-24 [email protected] [skwasm] Fix platform view occlusion logic. (flutter/engine#54061)
2024-07-24 [email protected] Roll Skia from 25f26f673502 to c11932925658 (4 revisions) (flutter/engine#54081)
2024-07-24 [email protected] Upgrade Engine Android SDK to 35 (flutter/engine#53574)
2024-07-24 [email protected] Roll Skia from a9b1043eb23e to 25f26f673502 (1 revision) (flutter/engine#54079)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
auto-submit bot pushed a commit that referenced this pull request Sep 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App platform-web Code specifically for the web engine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[web, wasm, platform view] Widgets do not always render on top of platform view when compiled to WASM
2 participants