Skip to content

Android PlatformViews page back to non-platformview page, still glitch for one frame after this fix: https://github.com/flutter/engine/pull/30724/ #102508

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

Closed
eggfly opened this issue Apr 25, 2022 · 5 comments
Labels
a: platform-views Embedding Android/iOS views in Flutter apps engine flutter/engine repository. See also e: labels. platform-android Android applications specifically

Comments

@eggfly
Copy link
Member

eggfly commented Apr 25, 2022

Steps to Reproduce

Friendly cc @blasten 😄

  1. Use the commits after this fix: Remove glitch when displaying platform views engine#30724
  2. Use the stable 2.10.3 engine version, or use the github.com/flutter/flutter master branch commit(a1a305c), it's exactly before "Roll Engine" commit before the new android platform view architecture merged into main engine branch(the new arch is ok without bug😄): flutter/engine@5f0271a:
  1. Use the demo Dart code below:
  2. Click any of the button in the list, navigate to webview and go back, you can see one frame glitch (about 30% probability), which position is around the clicked button's gray background.

Screenrecord
By camera:

IMG_0176.MOV

By adb screen record:

video.mp4

Screenshot:
image

Demo code:

CupertinoPageRoute Demo
import 'package:flutter/material.dart';
import 'package:flutter/cupertino.dart';

import 'package:webview_flutter/webview_flutter.dart';


void main() {
  WebView.platform = SurfaceAndroidWebView();
  runApp(MyApp());
}

class MyApp extends StatelessWidget {
  const MyApp({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return const MaterialApp(home: HomeScreen());
  }
}

class HomeScreen extends StatefulWidget {
  const HomeScreen({Key? key}) : super(key: key);

  @override
  _HomeScreenState createState() => _HomeScreenState();
}

class _HomeScreenState extends State<HomeScreen> {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: SafeArea(
        child: Column(
          children: [
            ListTile(
              title: Text('Red'),
              onTap: () {
                Navigator.push(context, CupertinoPageRoute(builder: (context) {
                  return const ProducDetailPage();
                }));
              },
            ),
            ListTile(
              title: Text('Green'),
              onTap: () {
                Navigator.push(context, CupertinoPageRoute(builder: (context) {
                  return const ProducDetailPage();
                }));
              },
            ),
            ListTile(
              title: Text('Green 1'),
              onTap: () {
                Navigator.push(context, CupertinoPageRoute(builder: (context) {
                  return const ProducDetailPage();
                }));
              },
            ),
            ListTile(
              title: Text('Green 2'),
              onTap: () {
                Navigator.push(context, CupertinoPageRoute(builder: (context) {
                  return const ProducDetailPage();
                }));
              },
            ),
            ListTile(
              title: Text('Green 3'),
              onTap: () {
                Navigator.push(context, CupertinoPageRoute(builder: (context) {
                  return const ProducDetailPage();
                }));
              },
            ),
            ListTile(
              title: Text('Green 2'),
              onTap: () {
                Navigator.push(context, CupertinoPageRoute(builder: (context) {
                  return const ProducDetailPage();
                }));
              },
            ),
            ListTile(
              title: Text('Green 2'),
              onTap: () {
                Navigator.push(context, CupertinoPageRoute(builder: (context) {
                  return const ProducDetailPage();
                }));
              },
            ),
            ListTile(
              title: Text('Green 2'),
              onTap: () {
                Navigator.push(context, CupertinoPageRoute(builder: (context) {
                  return const ProducDetailPage();
                }));
              },
            ),
            ListTile(
              title: Text('Green 2'),
              onTap: () {
                Navigator.push(context, CupertinoPageRoute(builder: (context) {
                  return const ProducDetailPage();
                }));
              },
            ),
            ListTile(
              title: Text('Green 2'),
              onTap: () {
                Navigator.push(context, CupertinoPageRoute(builder: (context) {
                  return const ProducDetailPage();
                }));
              },
            ),
            ListTile(
              title: Text('Green 2'),
              onTap: () {
                Navigator.push(context, CupertinoPageRoute(builder: (context) {
                  return const ProducDetailPage();
                }));
              },
            ),
            CircularProgressIndicator(
              semanticsLabel: 'Linear progress indicator',
            ),
            CircularProgressIndicator(
              semanticsLabel: 'Linear progress indicator',
            ),
            CircularProgressIndicator(
              semanticsLabel: 'Linear progress indicator',
            ),
            CircularProgressIndicator(
              semanticsLabel: 'Linear progress indicator',
            ),
            CircularProgressIndicator(
              semanticsLabel: 'Linear progress indicator',
            ),
            ListTile(
              title: Text('Green 2'),
              onTap: () {
                Navigator.push(context, MaterialPageRoute(builder: (context) {
                  return const ProducDetailPage();
                }));
              },
            ),
            ListTile(
              title: Text('Green 2'),
              onTap: () {
                Navigator.push(context, MaterialPageRoute(builder: (context) {
                  return const ProducDetailPage();
                }));
              },
            ),
            ListTile(
              title: Text('Green 2'),
              onTap: () {
                Navigator.push(context, MaterialPageRoute(builder: (context) {
                  return const ProducDetailPage();
                }));
              },
            ),
            ListTile(
              title: Text('Green 2'),
              onTap: () {
                Navigator.push(context, MaterialPageRoute(builder: (context) {
                  return const ProducDetailPage();
                }));
              },
            ),
            ListTile(
              title: Text('Green 2'),
              onTap: () {
                Navigator.push(context, MaterialPageRoute(builder: (context) {
                  return const ProducDetailPage();
                }));
              },
            ),
            ListTile(
              title: Text('Green 2'),
              onTap: () {
                Navigator.push(context, MaterialPageRoute(builder: (context) {
                  return const ProducDetailPage();
                }));
              },
            ),
            ListTile(
              title: Text('Green 2'),
              onTap: () {
                Navigator.push(context, MaterialPageRoute(builder: (context) {
                  return const ProducDetailPage();
                }));
              },
            ),
            // CircularProgressIndicator(
            //   semanticsLabel: 'Linear progress indicator',
            // ),
            // SizedBox(
            //   height: 150,
            //   child: WebView(
            //     initialUrl: 'https://flutter.dev',
            //   ),
            // ),
          ],
        ),
      ),
    );
  }
}

class ProducDetailPage extends StatelessWidget {
  const ProducDetailPage({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return Scaffold(
        appBar: AppBar(
          title: const Text('Produc Detail'),
        ),
        body: Column(children: const [
          Padding(
            padding: EdgeInsets.all(8.0),
            child: SizedBox(
              height: 150,
              child: Placeholder(
                color: Colors.red,
              ),
            ),
          ),
          SizedBox(
            height: 150,
            child: WebView(
              initialUrl: 'https://flutter.dev',
            ),
          ),
            CircularProgressIndicator(
              semanticsLabel: 'Linear progress indicator',
            ),
        ]));
  }
}

class ProductDetailPage2 extends StatelessWidget {
  const ProductDetailPage2({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return Scaffold(
        appBar: AppBar(
          title: const Text('Produc Detail'),
        ),
        body: Column(children: const [
          SizedBox(
            height: 150,
            child: Placeholder(
              color: Colors.green,
            ),
          ),
        ]));
  }
}

Here's my diagnosis:

Firstly, the commit https://github.com/flutter/engine/pull/30724/files#diff-87fa95e503a7a95a32aee115dded76fc6f8ec76fc29c2d2c071b413df0c3a096R265
introduced the isPaused member, when isPaused is set to true, then the stopRenderingToSurface() is not called any more:
https://github.com/flutter/engine/pull/30724/files#diff-d3ae30246a95a9e78606b57b860941c4d74d5dfba9ecc394f202c9a62002531bR242

image

Let's see there's a variable named isDisplayingFlutterUi in FlutterRenderer.java :

https://github.com/flutter/engine/blob/f6dd681d4ec88b86a80bd367e6e050ae4ef7ffc7/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java#L44

It controls the logic in addIsDisplayingFlutterUiListener()

image

https://github.com/flutter/engine/blob/f6dd681d4ec88b86a80bd367e6e050ae4ef7ffc7/shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java#L78

when isDisplayingFlutterUi is true, the addIsDisplayingFlutterUiListener will directly calls the listener on current java call stack.

But in this issue's scenario, the code in RevertImageView():

  • firstly calls renderSurface.attachToRenderer(renderer); but not set isDisplayingFlutterUi to false;
  • secondly calls addIsDisplayingFlutterUiListener and run onDone.run() directly on current stack.
    It will
    onDone.run() will call finishFrame() and will set overlayView.setVisibility(View.GONE);:

image

image

But the gone of FlutterImageView is too early! And I think the first new frame rendering on SurfaceView is not ready yet. The previous image of SurfaceView shows the click state (with a gray shadow) of the button..

I think the wrong stack is here:
image

After revert this commit, the original right stack is below:

image

@eggfly eggfly added passed first triage a: platform-views Embedding Android/iOS views in Flutter apps platform-android Android applications specifically engine flutter/engine repository. See also e: labels. labels Apr 25, 2022
@blasten
Copy link

blasten commented Apr 25, 2022

@eggfly have you tried to run this on the master channel?
The code that you are referencing isn't used anymore. We now render WebView as a texture unless you use a new API called initExpensiveAndroidView, so there's no "hybrid" composition by default.

@blasten blasten added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Apr 25, 2022
@eggfly
Copy link
Member Author

eggfly commented Apr 26, 2022

@eggfly have you tried to run this on the master channel?
The code that you are referencing isn't used anymore. We now render WebView as a texture unless you use a new API called initExpensiveAndroidView, so there's no "hybrid" composition by default.

Yes the master branch has no bug. But our apps' min.sdk level is below 23(most of them are 20 or 21).

@github-actions github-actions bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Apr 26, 2022
@blasten
Copy link

blasten commented Apr 27, 2022

But our apps' min.sdk level is below 23(most of them are 20 or 21).

That's a limitation unfortunately. Contributions are welcome though.
This issue is relatively low priority since there's a solution for API level >=23.

@chinmaygarde
Copy link
Member

This seems to be working as intended. Closing.

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: platform-views Embedding Android/iOS views in Flutter apps engine flutter/engine repository. See also e: labels. platform-android Android applications specifically
Projects
None yet
Development

No branches or pull requests

3 participants