This approach feels really fragile. We should have the geometry when we construct the mask blur, right?

Yea, in our current implementation we do. There is also a bug in our current implementation where it seems to only work for ColorSourceContents, which isn't what we want.

It isn't as fragile as it seems since the compiler is supporting it. If another subclass of ColorSourceContents is introduced, the visitor will have a compile error were you just implement the standard Visitor pattern and it will work.

The way this could break is if we introduce a new Contents that has a geometry that isn't a ColorSourceContents. But this way is probably more robust to that change than relying on having a ColorSourceContents when we create the blur.

Also, passing in the geometry as part of the filterinput and as a separate parameter can create the opportunity for them to differ which would be a logical error.

This just seems like way too much code to support mask blurs, which are only supported in relatively limited configrations. For example, we don't need to worry about mask blurs on arbitrary filers, its always going to be on a paint for a draw call, which means we should always have the geometry.

we don't need to worry about mask blurs on arbitrary filers

What's enforcing that? Why doesn't our API enforce that? I worry that trying to ignore what the API allows creates oversights like with what happened with mask blur filters on textures. That's the downside to doing it the other way.

Tests enforce it, same as it ever was!

@bdero I chatted with jonah and what we decided to do is just reduce the scope of the Visitor so that it only goes as far as ColorSourceContents. That's all I need to implement what I want and cuts down a lot of the code. If we need an more expansive visitor in the future we can implement that similarly to what I've done here.

I wish I understood the wizardry that std::variant uses to implement their visitor since it would resolve this issue.

I think you might also be able to simplify the visitor further too, if you're limiting the scope to just the work needed to land this change. In that case, you don't even need to know what you are visiting, just whether or not it has a geometry you can use.

Okay @jonahwilliams, please take a look, I removed all the subclasses of ColorSourceContents and FilterContents. I don't need to know anything below that to implement what we need.

Do we need overrides on all of the filter subtypes still?

Done. Thanks, I missed some of them. I think I have them all now.

uber-nit: I might call this something like "mask_geometry", to clarify that it is the geometry for the mask and not the destination geometry/rect of the filter.

Also need to set the geometry for the FilterContents::MakeGaussianBlur a few lines below.

It seems unnecessary to make the tests pass. Could I hit that with a gradient?

Yeah, setting any ColorSource other than solid color should trigger it.

I'm adding it, but it doesn't render correctly. I'll have to do a followup to add that and address the rendering bug.

TEST_P(AiksTest, GaussianBlurStyleInnerGradient) {
  Canvas canvas;
  canvas.Scale(GetContentScale());

  canvas.DrawPaint({.color = Color(0.1, 0.1, 0.1, 1.0)});

  Paint paint;
  paint.color = Color::Green();
  paint.mask_blur_descriptor = Paint::MaskBlurDescriptor{
      .style = FilterContents::BlurStyle::kInner,
      .sigma = Sigma(30),
  };

  std::vector<Color> colors = {Color{0.0, 0.7, 0.8, 1.0},
                               Color{0.0, 0.8, 0.7, 1.0}};
  std::vector<Scalar> stops = {0.0, 1.0};

  paint.color_source =
      ColorSource::MakeLinearGradient({0, 0}, {200, 200}, std::move(colors),
                                      std::move(stops), Entity::TileMode::kRepeat, {});

  canvas.DrawPath(PathBuilder()
                      .MoveTo({200, 200})
                      .LineTo({300, 400})
                      .LineTo({100, 400})
                      .Close()
                      .TakePath(),
                  paint);

  // Draw another thing to make sure the clip area is reset.
  Paint red;
  red.color = Color::Red();
  canvas.DrawRect(Rect::MakeXYWH(0, 0, 200, 200), red);

  ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
}

issue filed: flutter/flutter#144449

optional, but you could re-arrange the params so that blur_style (mask_blur_style?) and mask_geometry happen last and default them to normal/nullptr - here and in the ctor.

done