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.

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

[web] Render in custom target #37738

Merged
merged 58 commits into from Dec 22, 2022
Merged

Conversation

ditman
Copy link
Member

@ditman ditman commented Nov 18, 2022

This PR is the first iteration of the embedding of a Flutter web app in a custom host element.

It touches the following bits of the engine:

  • view embedding (to render the flutter app in the correct location)
  • pointer binding (so pointer events are relative to the app's glassPane, and not the browser viewport)
  • window sizing (to detect resizing also in DIVs)

This PR is backwards compatible, and by default, Flutter web apps render in "full-screen" mode (the same as what flutter has been doing until this change). If an app is configured correctly, it can run in "custom-element" mode, and then, it'll be restricted to that single element in the DOM.

This introduces some bits of the new design for Flutter views:

  • DimensionsProvider provides an api to observe/retrieve measurements from flutter's host element.
  • EmbeddingStrategy provides an api to render flutter in its host element.

Issues

Known issues

In custom-element mode:

In full-screen mode:

  • n/a

In general:

  • A bunch of technical debt was uncovered while working on this PR. See individual issues mentioning this PR in the message threads below.

Testing

  • New files are unit-tested.
  • Old tests pass with minimal modifications.
  • Test app deployed to Firebase hosting.

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.

@flutter-dashboard flutter-dashboard bot added the platform-web Code specifically for the web engine label Nov 18, 2022
@ditman

This comment was marked as resolved.

@ditman
Copy link
Member Author

ditman commented Nov 23, 2022

(Force-pushed after rebasing with some conflicts)

@ditman

This comment was marked as resolved.

@ditman ditman force-pushed the render-in-custom-target branch 3 times, most recently from 4f2708b to f43d28e Compare December 6, 2022 20:31
@ditman
Copy link
Member Author

ditman commented Dec 8, 2022

I think I've resolved all the issues that were discussed (here and on a quick meeting), and added unit tests for the new code. PTAL @yjbanov!

@ditman ditman requested a review from yjbanov December 8, 2022 08:12
@ditman ditman force-pushed the render-in-custom-target branch 2 times, most recently from 03e603b to 79e9a16 Compare December 8, 2022 11:04
@ditman
Copy link
Member Author

ditman commented Dec 9, 2022

This currently has a bug while computing coordinates on top of a platform view, they get reset to 0,0 over the platform view, rather than continuing on the app.

This is easy to solve in a "flat" app, but not trivial when the app has had a transform applied to it. Working on a fix.

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.

Maybe not right away, but eventually it would be nice to have golden test for positioning, sizing, and resizing of the custom element mode.

lgtm

lib/web_ui/lib/src/engine/dom.dart Outdated Show resolved Hide resolved
lib/web_ui/lib/src/engine/embedder.dart Outdated Show resolved Hide resolved
lib/web_ui/lib/src/engine/embedder.dart Show resolved Hide resolved
lib/web_ui/lib/src/engine/embedder.dart Outdated Show resolved Hide resolved
lib/web_ui/lib/src/engine/embedder.dart Outdated Show resolved Hide resolved
lib/web_ui/lib/src/engine/window.dart Show resolved Hide resolved
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.

lgtm

@@ -0,0 +1,113 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do logics in this file work if the host page has a non-zero scroll offset? I think scroll offset doesn't affect clientX/Y, but it will affect the position of the cursor relative to the elements.

Copy link
Member Author

@ditman ditman Dec 22, 2022

Choose a reason for hiding this comment

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

Yes, methods that only use offsetX/Y are relative to the glassPane/platformView, so the scroll position does not matter there.

The one that requires scroll handling is the pageX/Y one, because that gives us positions from the top left position of the page (with all its scroll position added).

See the pageY docs: https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/pageY

@ditman ditman added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 22, 2022
@auto-submit auto-submit bot merged commit 2358215 into flutter:main Dec 22, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 22, 2022
ditman added a commit to ditman/flutter-engine that referenced this pull request Dec 22, 2022
auto-submit bot pushed a commit that referenced this pull request Dec 22, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 22, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 22, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 22, 2022
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Dec 23, 2022
…117563)

* 23582159a [web] Render in custom target (flutter/engine#37738)

* 766f8fd91 Roll Fuchsia Mac SDK from UsYNZnnfR_s0OGQoX... to Xu_G6EQQ2UG48e5qI... (flutter/engine#38457)

* 6f829b7e3 Roll Dart SDK from fc0a3217b39a to cb6245d8f8d3 (1 revision) (flutter/engine#38458)

* 74d9ba455 Roll Skia from f1610a251e3a to 67904a365fdc (1 revision) (flutter/engine#38459)

* 19de6262b Roll Skia from 67904a365fdc to c93fa176c9ca (6 revisions) (flutter/engine#38460)

* 147ac7d02 Display list R-Tree culling (flutter/engine#38429)

* 3549fb1ed Roll Skia from c93fa176c9ca to 33807a735c32 (3 revisions) (flutter/engine#38464)

* d76ccb8f4 Roll Dart SDK from cb6245d8f8d3 to 47b0d07e6be9 (3 revisions) (flutter/engine#38465)

* 2c9e1d98d Roll Skia from 33807a735c32 to 89742d768c97 (3 revisions) (flutter/engine#38467)

* 3b115f033 Revert "[web] Render in custom target (#37738)" (flutter/engine#38469)

* ca0c843bf Roll Fuchsia Mac SDK from Xu_G6EQQ2UG48e5qI... to W0GUdjHi4gI48optN... (flutter/engine#38468)
ditman added a commit to ditman/flutter-engine that referenced this pull request Dec 23, 2022
auto-submit bot pushed a commit that referenced this pull request Dec 23, 2022
* Reland "[web] Render in custom target (#37738)"

This reverts commit 3b115f0.

* Use physicalSize instead of logical in localClipBounds of the html scene.
loic-sharma pushed a commit to loic-sharma/flutter-engine that referenced this pull request Jan 3, 2023
* Introduce FullScreenApplicationDom, and wire it to meta viewport, event handlers and hot restart.

* Move internal stylesheet to HostNode from ViewEmbedder.

* Add setHostStyles and Attribute to ApplicationDom. Use it in the embedder.

* Move HotRestartCacheHandler to its own file.

* Remove Safari hack for visualViewport.

* No need to keep a ref to the viewport meta in full-screen.

* Add applicationDom.attachGlassPane and use it in the Embedder.

* Remove empty method bodies.

* Add attachResourcesHost and use it from the embedder.

* Removed some unused code.

* Some more cleanup.

* Add ResizeObserver JS interop API.

* Add the CustomElementApplicationDom and wire it to the ViewEmbedder.

* Add the DimensionsProvider classes.

* Reimplement engine.window using the DimensionsProvider.

* Delegate window metrics to engine window in html scene object.

* Wire DimensionsProvider into engine.window.

* Moved ApplicationDom into its own subdir.

* Make DimensionsProvider also an Observer. Expose onResize Stream.

* Delegate onResize and dpr from window to DimensionsObserver object.

* Remove or make most ApplicationDom methods private. Expose single initializeHost.

* Hook the new API.

* dart format

* ApplicationDom -> EmbeddingStrategy.

* Attach pointer move events to glassPaneElement

* Use offset positions for mouse events (relative to host element) rather than client (relative to viewport)

* Update TouchAdapter to understand scrolling (simulate offsetX/Y)

* Remove locale change handling from the embedding strategy.

Also, remove DomSubscription handling from the
hot_restart_cache_handler, now that it is not needed.

* Move locale handling from the embedder to the platform dispatcher

* Move some styles from host to glassPane so we are more friendly with external CSS.

* Make analyzer fixes

* Ensure DimensionsProvider is available in tests.

* Initialize the view DimensionsProvider next to where the EmbeddingStrategy is decided (more logical)

* Bring back the logic to support Firefox 83.

* Fix pointer_binding test for new anchor point in the DOM.

* Fix pointer_binding_test in Firefox.

* Add an iterable way of accessing 'rules'

From a CSSStyleSheet object.

Also add the cssText getter for a CSSRule so we can parse it later.

* Merge latest changes to host_node stylesheet.

* Add an id to the StyleSheet element that we add, so it can be selected
  later (in tests).
* Use the methods coming from browser_detection.dart to determine the
  browser runtime, instead of re-implementing them within the method.
* Merge the Edge stylesheet into the general one.
* Update tests so they can look at the CSS Rules that were added.

* Format test

* Try to use insertRule for -ms-reveal, and fallback in tests.

* Test hot_restart_cache_handler

Simplify API a little bit, make clear method private.

* Test dimensions_provider.

* Test full_page_dimensions_provider

* Test custom_element_dimensions_provider

* Test embedding_strategy. Make getDomCache util public.

* Fixes and tests for *_embedding_strategy.

* Move default text colors to our innermost style inside host_node (apply only to flt-scene-host). Remove code from the embedding strategies, and adjust tests.

* Safari expands shorthand properties in CSSOM.

Check individually for both font-family and font-size in Safari, rather
than font in the host_node_test.

* Add computeEventOffsetToTarget function, and use it.

* Address PR comments.

* Update licenses_flutter.

* Remove DomCSSRuleList class and instead use Iterable of DomCSSRule

* Make the embeddingStrategy final instead of late

* Attach mouse/pointermove events to domWindow.

* Rename DimensionsProvider.onHotRestart to .close, and slightly improve docs.

* Fix compute physicalX/Y for TalkBack events.

Extracted compute function to a helper file.

* Clarify what does (and does not) support 3D transforms in the event_position_helper file.

* Update licenses file
loic-sharma pushed a commit to loic-sharma/flutter-engine that referenced this pull request Jan 3, 2023
loic-sharma pushed a commit to loic-sharma/flutter-engine that referenced this pull request Jan 3, 2023
* Reland "[web] Render in custom target (flutter#37738)"

This reverts commit 3b115f0.

* Use physicalSize instead of logical in localClipBounds of the html scene.
loic-sharma pushed a commit to fluttergithubbot/flutter that referenced this pull request Jan 6, 2023
…lutter#117563)

* 23582159a [web] Render in custom target (flutter/engine#37738)

* 766f8fd91 Roll Fuchsia Mac SDK from UsYNZnnfR_s0OGQoX... to Xu_G6EQQ2UG48e5qI... (flutter/engine#38457)

* 6f829b7e3 Roll Dart SDK from fc0a3217b39a to cb6245d8f8d3 (1 revision) (flutter/engine#38458)

* 74d9ba455 Roll Skia from f1610a251e3a to 67904a365fdc (1 revision) (flutter/engine#38459)

* 19de6262b Roll Skia from 67904a365fdc to c93fa176c9ca (6 revisions) (flutter/engine#38460)

* 147ac7d02 Display list R-Tree culling (flutter/engine#38429)

* 3549fb1ed Roll Skia from c93fa176c9ca to 33807a735c32 (3 revisions) (flutter/engine#38464)

* d76ccb8f4 Roll Dart SDK from cb6245d8f8d3 to 47b0d07e6be9 (3 revisions) (flutter/engine#38465)

* 2c9e1d98d Roll Skia from 33807a735c32 to 89742d768c97 (3 revisions) (flutter/engine#38467)

* 3b115f033 Revert "[web] Render in custom target (flutter#37738)" (flutter/engine#38469)

* ca0c843bf Roll Fuchsia Mac SDK from Xu_G6EQQ2UG48e5qI... to W0GUdjHi4gI48optN... (flutter/engine#38468)
gspencergoog pushed a commit to gspencergoog/flutter that referenced this pull request Jan 19, 2023
…lutter#117563)

* 23582159a [web] Render in custom target (flutter/engine#37738)

* 766f8fd91 Roll Fuchsia Mac SDK from UsYNZnnfR_s0OGQoX... to Xu_G6EQQ2UG48e5qI... (flutter/engine#38457)

* 6f829b7e3 Roll Dart SDK from fc0a3217b39a to cb6245d8f8d3 (1 revision) (flutter/engine#38458)

* 74d9ba455 Roll Skia from f1610a251e3a to 67904a365fdc (1 revision) (flutter/engine#38459)

* 19de6262b Roll Skia from 67904a365fdc to c93fa176c9ca (6 revisions) (flutter/engine#38460)

* 147ac7d02 Display list R-Tree culling (flutter/engine#38429)

* 3549fb1ed Roll Skia from c93fa176c9ca to 33807a735c32 (3 revisions) (flutter/engine#38464)

* d76ccb8f4 Roll Dart SDK from cb6245d8f8d3 to 47b0d07e6be9 (3 revisions) (flutter/engine#38465)

* 2c9e1d98d Roll Skia from 33807a735c32 to 89742d768c97 (3 revisions) (flutter/engine#38467)

* 3b115f033 Revert "[web] Render in custom target (flutter#37738)" (flutter/engine#38469)

* ca0c843bf Roll Fuchsia Mac SDK from Xu_G6EQQ2UG48e5qI... to W0GUdjHi4gI48optN... (flutter/engine#38468)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App platform-web Code specifically for the web engine
Projects
None yet
4 participants