[Impeller] use point/line primitve for hairlines.

[jonahwilliams]

For hariline (<= 1.0 physial pixel) strokes and points, we can use the line and point geometry primitives instead of tessellation. This gives substantially better performance.

https://github.com/flutter/flutter/issues/152702

Still need to make sure we only handle single contour lines this way, since we can't insert degenerate triangles to break a line strip. We may be able to do this via primitive restart, but we don't yet use that.

[flutter-dashboard[bot]]

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #55230 at sha 2d8f1ebfdd3af53559173dc9c4952e4a9ad85ff1

[jonahwilliams]

I also adjusted some of the alpha scaling params since this doesn't do MSAA specific scaling anymore.

[jonahwilliams]

I don't know how many times I've mashed rerun but the bot failure seems unrelated.

[flutter-dashboard[bot]]

Golden file changes are available for triage from new commit, Click here to view.

Changes reported for pull request #55230 at sha eb7f8411c6ff2e403b2be595c9d0421432d2a352

[flar]

Some of the golden failures don't make sense as they look like something that shouldn't have been affected.

I think some of the other failures could be due to pixel registration. What exactly is the equation for a line primitive? Does it extend half a pixel on both sides of the line?

[jonahwilliams]

I suspect some of the goldens are just from switching to MaxBasisXY in the fill code. I'm removing that for now.

[flutter-dashboard[bot]]

Golden file changes are available for triage from new commit, Click here to view.

Changes reported for pull request #55230 at sha 6ff4f7fef8be878352e9d97edd9ced2bbe66829c

[jonahwilliams]

I pulled the max basis change into https://github.com/flutter/engine/pull/55670 (thanks for the review), so whatever is left should just be from this change

[flar]

I pulled the max basis change into #55670 (thanks for the review), so whatever is left should just be from this change

I don't see how this change could cause the diffs in ImageFilteredSaveLayerWithUnboundedContents

[jonahwilliams]

There is definitely a bug to be found, which I will look for ... next week :)

[jonahwilliams]

oh wait, its the cross hatches drawn behind the save layers: https://github.com/flutter/engine/blob/92b5b318190b8256ca3e795bdcc2b8912dfa5bde/impeller/display_list/aiks_dl_unittests.cc#L662-L668

[flar]

oh wait, its the cross hatches drawn behind the save layers:

Ah, in that case this is an indication that the new code is not drawing the lines in the same location and we should look into that. We should have a test case that draws lines next to each other using a filled rect of the outline where we expect them to appear and right next to it a drawLine of the line - they should be on the same pixels, something like:

      (A)             (B)
--------------   --------------
--------------
      (C)

using cap = square and round A is drawLine({x, y}, {x+w, y}) B is fillRect(XYWH{x+hoffset-.5, y-.5, w+1, 1}) C is fillRect(XYWH{x-.5, y+voffset-0.5, w+1, 1})

using cap = butt B is fillRect(XYWH{x+hoffset, y-.5, w, 1}) C is fillRect(XYWH{x, y+voffset-0.5, w, 1})

and choosing hoffset and voffset so that they are close enough to visualize the alignment with a screen magnifier.

That sequence should be repeated with x and y being located at a variety of sub-pixel offsets to verify that the lines "move" at a similar sub-pixel offset. Maybe one column where the y subpixel offset goes from 0.0 to 1.0 in small increments and then another colum where the x subpixel offset varies similarly...?

Possibly the same pattern for some vertical lines as well in a second test case?

[flar]

I'm writing a test case and discovering that DrawPath(SkPath::Line(p0, p1)) draws nothing, so that explains the missing registration lines in the save layer tests.

• We should probably change the color of those lines to be visible
• We should probably not display playground output on a solid color background so when there is transparency it is more obvious (most viewers use a gray-gray checkerboard for the default background so transparency is obvious)
• We need to fix the DrawPath(hairline) issue.

[flar]

Here is the test case that I wrote. The DrawPath version of the lines does not appear and when I print out the DL it looks like it was never recorded. D'oh!? We should include a version of this that does both horizontal and vertical lines and includes all of the cap modes...

Hairline unit test

TEST_P(AiksTest, HorizontalHairlinesButtCapPixelRegistration) {
  DlStrokeCap cap = DlStrokeCap::kButt;
  const int cap_pad = 0.0f;

  DisplayListBuilder builder;
  DlPaint stroke_paint;
  stroke_paint.setColor(DlColor::kBlack());
  stroke_paint.setDrawStyle(DlDrawStyle::kStroke);
  stroke_paint.setStrokeWidth(0.0f);
  stroke_paint.setStrokeCap(cap);
  DlPaint fill_paint;
  stroke_paint.setColor(DlColor::kBlack());
  stroke_paint.setDrawStyle(DlDrawStyle::kFill);

  const int pad = 5;
  const int line_length = 20;
  const int x_pad = line_length + pad;
  const int y_pad = pad;
  const int x_test_offset = x_pad * 2 + pad;
  const int y_test_offset = y_pad + pad * 2;

  auto draw_one = [&stroke_paint, &fill_paint](DlCanvas& canvas,
                                               DlScalar x_base,
                                               DlScalar y_base) {
    DlScalar x0 = x_base + 0.5f;
    DlScalar x1 = x0 + line_length;
    DlScalar y0 = y_base + 0.5f;
    DlScalar y1 = y0;

    SkRect expected_rect =
        SkRect::MakeLTRB(x0 - cap_pad, y0 - 0.5f, x1 + cap_pad, y1 + 0.5f);
    SkPath expected_path = SkPath::Line({x0, y0}, {x1, y1});

    canvas.DrawLine({x0, y0}, {x1, y1}, stroke_paint);
    canvas.DrawRect(expected_rect.makeOffset(x_pad, 0), fill_paint);
    canvas.DrawRect(expected_rect.makeOffset(0, y_pad), fill_paint);
    canvas.DrawPath(expected_path.offset(x_pad, y_pad), stroke_paint);
  };

  builder.DrawColor(DlColor::kWhite(), DlBlendMode::kSrc);

  DlScalar y_base = pad;
  for (int i = 0; i <= 10; i++) {
    DlScalar subpixel_offset = (i / 10.0f);

    DlScalar x_base = pad;
    draw_one(builder, x_base + subpixel_offset, y_base);
    x_base += x_test_offset;
    draw_one(builder, x_base, y_base + subpixel_offset);

    y_base += y_test_offset;
  }

  auto dl = builder.Build();
  FML_LOG(ERROR) << *dl;
  ASSERT_TRUE(OpenPlaygroundHere(dl));
}

[flar]

I'm trying to create a PR with a test that makes sure that h/v Path.line() instances render and it is showing up just fine, but the test case I wrote within this PR shows those paths as missing - so now I have no idea what is preventing those lines from showing up - still investigating...

[jonahwilliams]

Its not white path on white background is it?

[flar]

Its not white path on white background is it?

It's white path on transparent background I believe, so you don't normally see it, but the golden test comparison notices it is missing. In that test case I included above I draw a black path on a white background and it doesn't show up.

[flar]

Its not white path on white background is it?

Running that test in playground the lines are visible but the background is black. So they haven't disappeared, though the choice of white on transparent is unfortunate for understanding the goldens...

[flutter-dashboard[bot]]

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

[jonahwilliams]

Okay, this is up to date and I've disabled the optimization for round/square caps. Filled https://github.com/flutter/flutter/issues/157618 . I need to do some refactoring of the start/end vector to handle the arbtirary path case.

[jonahwilliams]

I'm having a hard time tracking down the failures in this PR. Instead, I'm going to split this up into smaller changes so i can hopefully find where the error was introduced.