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

Visual glitch introduced by RenderListWheelViewport needsCompositing #41654

Closed
LongCatIsLooong opened this issue Sep 30, 2019 · 3 comments
Closed
Labels
c: rendering UI glitches reported at the engine/skia rendering level f: cupertino flutter/packages/flutter/cupertino repository framework flutter/packages/flutter repository. See also f: labels.

Comments

@LongCatIsLooong
Copy link
Contributor

There seems to be a small visual glitch after applying https://github.com/flutter/flutter/pull/33070/files to the current master (e8517a4).

wiggle

This blocks #14224 and #39919 from (re)landing.

@LongCatIsLooong
Copy link
Contributor Author

Widget test to reproduce:

 class TestTransform extends LeafRenderObjectWidget {
  const TestTransform(this.needsCompositing, this.angle) : super();

  final bool needsCompositing;
  final double angle;

  @override
  RenderObject createRenderObject(BuildContext context) => RenderTestTransform(needsCompositing, angle);

  @override
  void updateRenderObject(BuildContext context, RenderTestTransform renderObject) {
    renderObject
      ..compositing = needsCompositing
      ..angle = angle;
  }
}

class RenderTestTransform extends RenderConstrainedBox {
  RenderTestTransform(this._compositing, this._angle)
    : super(additionalConstraints: const BoxConstraints.tightFor(width: 100, height: 200));

  bool get compositing => _compositing;
  bool _compositing;
  set compositing(bool newValue) {
    if (newValue == compositing)
      return;
    _compositing = newValue;
    markNeedsPaint();
    markNeedsCompositingBitsUpdate();
  }

  double get angle => _angle;
  double _angle;
  set angle(double newValue) {
    if (newValue == angle)
      return;
    _angle = newValue;
    markNeedsPaint();
  }

  @override
  void paint(PaintingContext context, Offset offset) {
    context.pushTransform(
      compositing,
      offset,
      MatrixUtils.createCylindricalProjectionTransform(
        radius: 100,
        angle: angle,
        perspective: 0.003,
      ),
      (PaintingContext context, Offset offset) {
        context.canvas.drawRect(
          (offset + const Offset(200, 200)) & const Size(10, 10),
          Paint()..color = const Color(0xFFFF0000),
        );
      },
    );
  }
}


  testWidgets('This should fail', (WidgetTester tester) async {
    for (double angle = 0; angle <= math.pi/4; angle += 0.01) {
      await tester.pumpWidget(RepaintBoundary(child: TestTransform(true, angle)));
      final RenderBox renderBox = tester.binding.renderView.child;
      final OffsetLayer layer = renderBox.debugLayer;
      final ui.Image trueImage = await layer.toImage(renderBox.paintBounds);

      await tester.pumpWidget(RepaintBoundary(child: TestTransform(false, angle)));

      try {
        await expectLater(find.byType(RepaintBoundary), matchesReferenceImage(trueImage));
      } catch (e) {
        print(angle);
      }
    }
  });

@LongCatIsLooong
Copy link
Contributor Author

@liyuqian I'll try to reland https://github.com/flutter/flutter/pull/39919/files 30 days from now.

@Hixie Hixie added the framework flutter/packages/flutter repository. See also f: labels. label Nov 26, 2019
liyuqian added a commit to flutter/engine that referenced this issue Apr 16, 2020
This fixes flutter/flutter#53288 and flutter/flutter#41654. It removes the problematic `GetIntegralTransCTM`, but preserves the rect round-out in `RasterCacheResult::draw` for performance considerations: the average frame raster time doesn't change much but the worst frame raster time significantly regressed if rect round-out is removed. That's probably because a new shader needs to be compiled to draw raster cache with fractional offsets.
@xster xster moved this from Awaiting triage to Engineer reviewed in iOS Platform - Cupertino widget review Apr 30, 2020
liyuqian added a commit to flutter/engine that referenced this issue May 1, 2020
This reverts commit b5aedb3 and relands #17712.

Fixes flutter/flutter#53288 and flutter/flutter#41654.

Together with #17791, this reland addresses some of Jim's concerns in the original PR #17712.

The major part of this PR is still the same as the original PR, and the performance / golden image impacts should be the same.
liyuqian added a commit to liyuqian/engine that referenced this issue May 6, 2020
It turns out that our ios32 (iPhone42) performacne can regress a lot
without snapping. My theory is that although the picture boudns has a
fractional top left corner, many drawing operations inside the picture
have integral coordinations. In older hardwares, keeping those
coordinates integeral seems to be performance critical.

To avoid flutter/flutter#41654, the snapping
will still be disabled if the matrix has non-scale-translation
transformations.
liyuqian added a commit to liyuqian/engine that referenced this issue May 6, 2020
It turns out that our ios32 (iPhone4s) performacne can regress a lot
without snapping. My theory is that although the picture has a
fractional top left corner, many drawing operations inside the picture
have integral coordinations. In older hardwares, keeping those
coordinates integral seems to be performance critical.

To avoid flutter/flutter#41654, the snapping
will still be disabled if the matrix has non-scale-translation
transformations.
liyuqian added a commit to liyuqian/engine that referenced this issue May 6, 2020
It turns out that our ios32 (iPhone4s) performacne can regress a lot
without snapping. My theory is that although the picture has a
fractional top left corner, many drawing operations inside the picture
have integral coordinations. In older hardwares, keeping those
coordinates integral seems to be performance critical.

To avoid flutter/flutter#41654, the snapping
will still be disabled if the matrix has non-scale-translation
transformations.
liyuqian added a commit to flutter/engine that referenced this issue May 8, 2020
This reverts commit a7a25d3 and relands our reland #17915.

Additionally, we fixed the cull rect logic in `OpacityLayer::Preroll` which is  the root cause of flutter/flutter#56298. We've always had that root problem before but it did not trigger performance issues because we were using the OpacityLayer's `paint_bounds`, instead of its child's `paint_bounds` for preparing the layer raster cache. A correct handling of the cull rect should allow us to cull at any level.

It also turns out that our ios32 (iPhone4s) performacne can regress a lot
without snapping. My theory is that although the picture has a
fractional top left corner, many drawing operations inside the picture
have integral coordinations. In older hardwares, keeping those
coordinates integral seems to be performance critical.

To avoid flutter/flutter#41654, the snapping
will still be disabled if the matrix has non-scale-translation
transformations.
liyuqian added a commit to liyuqian/flutter that referenced this issue Jul 23, 2020
The test was first written in
flutter#41654 (comment)

This will ensure that flutter#41654
won't have regressions.

This test would fail without flutter/engine#18160
@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 Aug 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
c: rendering UI glitches reported at the engine/skia rendering level f: cupertino flutter/packages/flutter/cupertino repository framework flutter/packages/flutter repository. See also f: labels.
Projects
Development

No branches or pull requests

3 participants