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

Conversation

cbracken
Copy link
Member

@cbracken cbracken commented Jun 2, 2023

For consistency with Flutter (and all other platforms), Flutter views in the macOS embedder set isFlipped to ensure a co-ordinate system with the origin in the top-left, with y co-ordinates increasing towards the bottom edge of the view.

Previously, we were using a stock NSView as the container, which uses a bottom-left origin by default. Instead we extract the PlatformView container view as its own class with isFlipped true.

This doesn't correct the issue of the view anchorpoint/position but does correct rotation direction.

This also applies the transform back to origin prior to other transforms when adjusting the platformview position rather than after.

Issue: flutter/flutter#124490

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Copy link
Contributor

@gspencergoog gspencergoog left a comment

Choose a reason for hiding this comment

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

32384589-a60f0e74-c078-11e7-9bc1-e5b5287aea9d


@implementation FlutterPlatformViewContainer

- (BOOL)isFlipped {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe isYFlipped or isVerticallyFlipped? Someday there might be a system where the origin is in the lower right...

Copy link
Member

Choose a reason for hiding this comment

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

This is Cocoa method that needs to be overriden to switch into a sane coordinate system...

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh, OK, so no choice there then.

Copy link
Member

Choose a reason for hiding this comment

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

Someday there might be a system where the origin is in the lower right...

Please don't give them any suggestions...


@implementation FlutterPlatformViewContainer

- (BOOL)isFlipped {
Copy link
Member

Choose a reason for hiding this comment

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

This is Cocoa method that needs to be overriden to switch into a sane coordinate system...

cbracken added 2 commits June 2, 2023 15:38
For consistency with Flutter (and all other platforms), Flutter views in
the macOS embedder set `isFlipped` to ensure a co-ordinate system with
the origin in the top-left, with y co-ordinates increasing towards the
bottom edge of the view.

Previously, we were using a stock NSView as the container, which uses a
bottom-left origin by default. Instead we extract the
PlatformView container view as its own class with `isFlipped` true.

This doesn't correct the issue of the view anchorpoint/position but does
correct rotation direction.

This also applies the transform back to origin prior to other transforms
when adjusting the platformview position rather than after.

Issue: flutter/flutter#124490
@cbracken cbracken added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 2, 2023
@auto-submit auto-submit bot merged commit 5883356 into flutter:main Jun 2, 2023
@cbracken cbracken deleted the rotation branch June 2, 2023 23:37
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 3, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 3, 2023
goderbauer pushed a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 3, 2023
goderbauer pushed a commit to flutter/flutter that referenced this pull request Jun 3, 2023
…128158)

flutter/engine@8769e9c...5429372

2023-06-03 jonahwilliams@google.com [Impeller] Fix 1-d grid computation
for compute (flutter/engine#42516)
2023-06-02 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from
PuYA-6NVHeHPlkCdk... to VtLnfLmVda1_h1AtM... (flutter/engine#42529)
2023-06-02 chris@bracken.jp [macOS] Top-left origin for PlatformView
container (flutter/engine#42523)
2023-06-02 skia-flutter-autoroll@skia.org Manual roll Dart SDK from
9d8df2a5210b to d198f84f5e4e (1 revision) (flutter/engine#42527)
2023-06-02 flar@google.com Revert "Reland "add non-rendering operation
culling to DisplayListBuilder" (#41463)" (flutter/engine#42525)
2023-06-02 godofredoc@google.com Move benchmarks no upload to staging.
(flutter/engine#42524)
2023-06-02 mdebbar@google.com [web] Support platform view creation
params (flutter/engine#42255)
2023-06-02 goderbauer@google.com MultiView changes for dart:ui
(flutter/engine#42493)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from PuYA-6NVHeHP to VtLnfLmVda1_

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 jonahwilliams@google.com,rmistry@google.com,zra@google.com 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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
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-macos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants