Rendering errors with Rect.largest when used with toImage (e.g. gold image tests) using impeller

If you run the following using flutter test --enable-impeller, you get a test.png containing a hollow square with the top left corner damaged, but it should just be a hollow square:

// Copyright 2014 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:io';
import 'dart:ui';

const double z = 1000000000; // aka _giantScalar, used in Rect.largest

void main() async {
  final PictureRecorder recorder = PictureRecorder();
  final Canvas canvas = Canvas(recorder);
  final Path clip = Path()
    ..fillType = PathFillType.evenOdd
    ..addRect(const Rect.fromLTRB(-z, -z, z, z))
    ..addRect(const Offset(50.0, 50.0) & const Size(100.0, 100.0));
  canvas.clipPath(clip);
  canvas.drawPaint(Paint()..color = const Color(0xFF33CC66));
  final Picture picture = recorder.endRecording();
  await File('test.png').writeAsBytes((await (await picture.toImage(200, 200)).toByteData(format: ImageByteFormat.png))!.buffer.asUint8List());
}

We're going to very quickly learn if this is a bug in entities or a bug in libtess...

#144647 is the PR that switches the technique used to render evenOdd paths

I can reproduce this before: #144659 but not after. I suspect switching to Stencil then Cover and removing the usage of libtess2 has fixed this issue.

We should still probably try to clean this up, since libtess2 will be a fallback path for some cases right?

Disregard, the plan is to drop libtess2 and related codepaths once StC sticks.

right, libtess2 will be deleted!

On trunk I get this:

...which is still wrong, it's missing 20 pixels on the left side.

seems less wrong though :)

I dunno man, the old version had more correct pixels. ;-)
(But yeah looks like maybe we had two bugs, and now we're down to one? The original rendering was pretty strange.)

@flar

This repros the problem without clips:

// Copyright 2014 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:io';
import 'dart:ui';

const double z = 1000000000; // aka _giantScalar, used in Rect.largest

void main() async {
  final PictureRecorder recorder = PictureRecorder();
  final Canvas canvas = Canvas(recorder);
  canvas.drawCircle(const Offset(12.0, 12.0), 10.0, Paint()..color = const Color(0xFFFF0000)); // should be obscured                                                               
  canvas.drawRect(const Rect.fromLTRB(-z, -z, z, z), Paint()..color = const Color(0xFF00FF00)); // should cover everything               
  final Picture picture = recorder.endRecording();
  await File('test.png').writeAsBytes((await (await picture.toImage(200, 200)).toByteData(format: ImageByteFormat.png))!.buffer.asUint8List());
}

You would expect a 200x200 green square, but instead you get:

The red circle should get obscured but the rectangle doesn't paint to the left of x=18.

cc @flar since @chinmaygarde mentioned you've been working in this area

Ok after further investigation, I am indeed finding that I can't reproduce this with Impeller when rendering to a real device (I tried Android and Linux with @jason-simmons' patch).

I was playing with this some more to try to get inspiration about what the bug could be (I traced this through a bunch of Impeller, logging out coordinates and so on, without finding anything wrong) and found a rectangle that renders wildly wrong again, similar to the original failure. So removing libtess2 didn't improve matters, it just made them different.

In the following snippets, "..." should be replaced with a path to the directory containing the flutter and engine checkouts.

// Copyright 2014 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:async';
import 'dart:io';
import 'dart:typed_data';
import 'dart:ui';

const double z = 1000000000; // 1e9

void main() async {
  final Timer timer = Timer.periodic(const Duration(seconds: 1), (Timer timer) { });
  final PictureRecorder recorder = PictureRecorder();
  final Canvas canvas = Canvas(recorder);
  canvas.drawCircle(const Offset(12.0, 12.0), 10.0, Paint()..color = const Color(0xFFFF0000)); // should be obscured
  canvas.drawRect(const Rect.fromLTRB(-z, -z, z/7, z/3), Paint()..color = const Color(0xFF009900)); // should cover everything
  final Picture picture = recorder.endRecording();
  final Image image = picture.toImageSync(100, 100);
  final ByteData? bytes = await image.toByteData(format: ImageByteFormat.png);
  await File('/.../test.png').writeAsBytes(bytes!.buffer.asUint8List());
  timer.cancel();
}

I compile this using:

echo -e 'native-assets file:///.../flutter/packages/flutter/build/native_assets/linux/native_assets.yaml\ncompile file:///.../flutter/packages/flutter/test/widgets/magnifier_test.dart' | /.../engine/src/out/host_debug_unopt/dart-sdk/bin/dartaotruntime --disable-dart-dev /.../engine/src/out/host_debug_unopt/dart-sdk/bin/snapshots/frontend_server_aot.dart.snapshot --sdk-root /.../engine/src/out/host_debug_unopt/flutter_patched_sdk/ --incremental --no-print-incremental-dependencies --target=flutter --experimental-emit-debug-metadata --output-dill /.../test.dill --packages /.../flutter/packages/flutter/.dart_tool/package_config.json -Ddart.vm.profile=false -Ddart.vm.product=false --enable-asserts --track-widget-creation --initialize-from-dill /.../flutter/packages/flutter/build/test_cache/build/cache.dill.track.dill --native-assets file:///.../flutter/packages/flutter/build/native_assets/linux/native_assets.yaml --verbosity=error

...and run it using:

/.../engine/src/out/host_debug_unopt/flutter_tester --disable-vm-service --enable-checked-mode --verify-entry-points --enable-impeller --enable-dart-profiling --non-interactive --use-test-fonts --disable-asset-fonts --packages=/.../flutter/packages/flutter/.dart_tool/package_config.json --flutter-assets-dir=/.../flutter/packages/flutter/build/unit_test_assets /.../test.dill

The result should be a 100x100 green PNG, but is instead:

Ok I have a pure impeller repro:

diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc
index 7dcebc01f8..c587488901 100644
--- a/impeller/entity/entity_unittests.cc
+++ b/impeller/entity/entity_unittests.cc
@@ -2805,14 +2805,27 @@ TEST_P(EntityTest, FailOnValidationError) {
   EXPECT_DEATH(
       // The easiest way to trigger a validation error is to try to compile
       // a shader with an unsupported pixel format.
       GetContentContext()->GetBlendColorBurnPipeline({
           .color_attachment_pixel_format = PixelFormat::kUnknown,
           .has_depth_stencil_attachments = false,
       }),
       "");
 }
 
+TEST_P(EntityTest, Repro144643) {
+  SetWindowSize({731, 168}); // or manually shrink your window
+  auto contents = std::make_shared<SolidColorContents>();
+  contents->SetGeometry(Geometry::MakeRect(Rect::MakeXYWH(-1e9, -1e9, 2e9, 2e9)));
+  contents->SetColor(Color::Red());
+
+  Entity entity;
+  // entity.SetTransform(Matrix::MakeScale(GetContentScale()));
+  entity.SetContents(contents);
+
+  ASSERT_TRUE(OpenPlaygroundHere(std::move(entity)));
+}
+
 }  // namespace testing
 }  // namespace impeller
 
 // NOLINTEND(bugprone-unchecked-optional-access)

Compile and run with:

gclient sync -D
flutter/tools/gn --unopt
ninja -C out/host_debug_unopt
out/host_debug_unopt/impeller_unittests --gtest_filter="*Repro144643/Vulkan" --enable_playground --use_swiftshader

Output looks like this:

Notice that it should cover the whole window but is missing a bit on the left. Different sizes change the intensity of the effect, e.g.:

You get particularly weird results with:
contents->SetGeometry(Geometry::MakeRect(Rect::MakeLTRB(-1e9, -1e9, 1e9/7.0, 1e9/3.0)));

The rendering errors reproduce in renderdoc when using the llvmpipe ICD if you capture the above rendering, and notably change as you change the zoom level. I think this means this is probably an upstream bug in the mesa drivers. I wonder if we should consider reducing Rect.largest to use 1e8 instead of 1e9 for now.

Navigation Menu

Repository navigation

Rendering errors with Rect.largest when used with toImage (e.g. gold image tests) using impeller #144643

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Rendering errors with Rect.largest when used with toImage (e.g. gold image tests) using impeller #144643

Description

Activity

jonahwilliams commented on Mar 5, 2024

jonahwilliams commented on Mar 6, 2024

dnfield commented on Mar 6, 2024

dnfield commented on Mar 6, 2024

jonahwilliams commented on Mar 6, 2024

Hixie commented on Mar 6, 2024

jonahwilliams commented on Mar 6, 2024

Hixie commented on Mar 6, 2024

Hixie commented on Mar 8, 2024

Hixie commented on Mar 18, 2024

Hixie commented on Mar 19, 2024

Hixie commented on Mar 22, 2024

Hixie commented on Mar 26, 2024

Hixie commented on Mar 26, 2024

Hixie commented on Mar 26, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions