Horizontal overscroll is being clipped on Android

[lukef]

The horizontal overscroll glow is being clipped or rendered incorrectly. The vertical overscroll glow is correctly rendered.

[abarth]

Can you say a bit more about how to reproduce this issue?

[lukef]

Sure. Sorry, will make sure I include more details in the future.

Two scenarios:

1. Using a regular TabBarView inside a Positioned.fill
2. I'm using a PageView.custom:

List<Widget> children = ...
SliverChildDelegate childrenDelegate = new SliverChildListDelegate(children);
PageView pageView = new PageView.custom(
  childrenDelegate: this.childrenDelegate,
);

Both cases seem to be exhibiting the issue consistently.

FYI - I have a ListView inside the TabBarView that scrolls vertically and the overscroll indicators are fine for the vertical scroll.

[abarth]

@Hixie I think we still have a coordinate flipped somewhere in the horizontal glow math.

[Hixie]

What's the issue exactly?

[lukef]

The GlowingOverscrollIndicator is being clipped to the rect rather than being displayed as circular on android devices for any horizontal scroll actions. I'm not sure if its an issue with a radius or a center point calculation or something else. I tried to debug that code last night (very briefly) but got caught up with other things sorry. Vertical is working fine.

[eseidelGoogle]

This is easy to repro with a horizontal scrollview on Android, btw. It looks like the overscroll glow is drawing from the bottom of the screen with the correct cliprect, just wrong origin.

[Hixie]

Wow yeah I wonder what I did.

[Hixie]

Turns out this is a Skia bug. Consider this app. It has a function that draws a rectangle with a circle in the middle. The first one has no particular transforms applied. It renders fine (whitish). The second is rotate by pi/2 and flipped along the y-axis; it renders fine too (green).

The next three should all look identical (they are the red ones). They are the same as the green one but with the y-axis (now effectively the x-axis since we're rotated by pi/2) squeezed further, by 0.5 plus or minor 0.001. It should look like a squeezed version of the green one, blurred out a bit. Instead, two of the circles end up squeezed in the wrong direction.

import 'package:flutter/material.dart';
import 'dart:math' as math;

void main() {
  runApp(new CustomPaint(painter: new TestPainter()));
}

class TestPainter extends CustomPainter {
  void drawFlag(Canvas canvas, Paint paint) {
    canvas.drawRect(new Rect.fromLTWH(50.0, 50.0, 300.0, 200.0), paint);
    canvas.drawCircle(new Point(200.0, 150.0), 50.0, paint);
  }

  @override
  void paint(Canvas canvas, Size size) {
    Paint paint = new Paint()
      ..style = PaintingStyle.stroke
      ..strokeWidth = 10.0;

    paint.color = const Color(0xFFEEEEEE);
    drawFlag(canvas, paint);

    canvas.rotate(math.PI / 2.0);
    canvas.scale(1.0, -1.0);
    paint.color = const Color(0xFF00FF00);
    drawFlag(canvas, paint);

    canvas.save();
    canvas.scale(1.0, 0.4999);
    paint.color = const Color(0xFFFF0000);
    drawFlag(canvas, paint);
    canvas.restore();

    canvas.save();
    canvas.scale(1.0, 0.5000);
    paint.color = const Color(0xFFFF0000);
    drawFlag(canvas, paint);
    canvas.restore();

    canvas.save();
    canvas.scale(1.0, 0.5001);
    paint.color = const Color(0xFFFF0000);
    drawFlag(canvas, paint);
    canvas.restore();
  }
  @override
  bool shouldRepaint(TestPainter oldDelegate) => true;
}

This is what it renders:

cc @chinmaygarde to take over the investigation. :-)

[chinmaygarde]

Hmm. I can't reproduce this on the desktop shell. It is probably platform specific. But how ?! :( I guess thats a data point.

[eseidelGoogle]

@chinmaygarde can you escalate to Skia? :) Thanks!

[chinmaygarde]

Will do. Trying all platform variants first.

[chinmaygarde]

iOS device and simulator builds are fine too. One of them uses the OpenGL backend and the other one a software backend.

[eseidelGoogle]

So this is an Android-GL only bug?

[chinmaygarde]

Android is fine too. Am I being trolled ?!

[chinmaygarde]

Ok, I tried both debug and release variants on Android to make sure there were no differences between AOT vs JIT modes. Both are fine.

[chinmaygarde]

Ok. I was testing on ToT with local engine. I can reproduce this with the currently released engine. I need to do a bisect of the changes since the last release and current ToT. But this is definitely not an issue anymore. Rather worrying how this regressed though. Bisecting.

[chinmaygarde]

The only two changes in the engine repository between ToT and the released version is a minor change where I got rid of an iOS specific flag and literally a commit with a whitespace change. I tried bisecting the framework and cannot find a revision using a released version of the engine that is correctly rendering the test case. All I can think of is to manually pull the released engine and do a binary diff against the locally built variant. This is bizarre.

[Hixie]

What's the status of this bug? Did we figure out if it was a Skia problem? Local issue? Only devices @chinmaygarde hasn't touched? :-)

[chinmaygarde]

I have not made any progress on this. Local builds on all platforms were fine for me. I was building on a Mac and did ensure to checkout to the version of the released engine. Will try building on Linux. Besides that, out of ideas.

[Hixie]

Ah, could be something that happens with the compiler on Linux maybe?

[eseidelGoogle]

I'm able to reproduce this on genymotion on Mac:

However looks fine in iOS Simulator.

[eseidelGoogle]

Same problem on Android Emulator:
• Android SDK built for x86 64 • emulator-5554 • android-x64 • Android 6.0 (API 23) (emulator)

[eseidelGoogle]

Here is the code I used to repro:
https://github.com/eseidelGoogle/bug5823

I'm using 06144a7

flutter doctor is clean
[✓] Flutter (on Mac OS X 10.12.3 16D32, channel master)

[chinmaygarde]

I converted the code in the sample to the equivalent Skia calls and added it to this fiddle. This does not seem to be a Skia issue. Maybe the raster cache. Checking now.

[chinmaygarde]

@eseidelGoogle Did you try using a locally built engine on Mac or was this from the released engine?

[eseidelGoogle]

@chinmaygarde i was using a released engine.

[eseidelGoogle]

@chinmaygarde are you actively working on this? (Just checking our in-progress M2 bugs to make sure they're actually owned.)

[chinmaygarde]

No. I am not actively working on this because I am not able to reproduce this.

[sethladd]

I don't know if it's the same issue, but you can see a rectangular (instead of ellipse) vertical glow in the gallery: #8747

[lukef]

Yes, this is the same issue as far as I know

[jason-simmons]

Reproduced it and looked at the Skia code. This apparently happens if we flow through EllipseOp::Make

EllipseOp::Make is calculating the ellipse's x and y radius based on scale and skew values extracted from the viewMatrix that I don't think are accurate if a negative scale transform has been applied.

This works as expected if I patch EllipseOp::Make to call viewMatrix.mapRect(ellipse) and use the resulting radii.

[chinmaygarde]

Can you prepare a reduced test case on fiddle.skia.org so we can share it with the Skia team?

…

On Tue, Apr 11, 2017 at 5:17 PM Jason Simmons ***@***.***> wrote: Reproduced it and looked at the Skia code. This apparently happens if we flow through EllipseOp::Make EllipseOp::Make is calculating the ellipse's x and y radius based on scale and skew values extracted from the viewMatrix that I don't think are accurate if a negative scale transform has been applied. This works as expected if I patch EllipseOp::Make to call viewMatrix.mapRect(ellipse) and use the resulting radii. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#8523 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACsNeL50Z8RYAsKvuKXnunQj3Ubr9mHks5rvBgHgaJpZM4MQmSv> .

[jason-simmons]

Skia bug filed: https://bugs.chromium.org/p/skia/issues/detail?id=6495

[jvanverth]

Should be fixed by https://skia-review.googlesource.com/c/13989/.

[jason-simmons]

Fix has been integrated into Flutter

[github-actions]

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.