Layer::Preroll methods should implement pixel snapping consistent with their Paint methods

[flar]

Recently we've been doing work on the state tracking in the Preroll, Paint, and hopefully soon Diff methods and we noticed that Paint will sometimes pixel snap the transform, but Preroll does not (and Diff has complicated work arounds for that distinction).

We should make Preroll and Paint consistent and then Diff can remove some of its workarounds.

[flar]

This will help with #116198

Some of the issues on why this is needed and the technical hurdles involved are discussed in the comments on that other issue.

[knopp]

I was wrong in the original issue. It's not that Preroll() and Paint() are inconsistent. That was the case before. Now the problem is just in Paint:

void ContainerLayer::PaintChildren(PaintContext& context) const {
  for (auto& layer : layers_) {
    // <<< Here, the `needs_painting` check works with non-integer CTM >>>
   if (layer->needs_painting(context)) {
      layer->Paint(context);
    }
  }
}

void DisplayListLayer::Paint(PaintContext& context) const {
 
  // <<< We got here by passing check with non integer CTM >>>
  FML_DCHECK(needs_painting(context));

  if (context.raster_cache) {
    // <<< But now we paint children on integer CTM >>>
    mutator.integralTransform();
    ...
    }
  }

Not sure what we can do about this. The parent layer that does culling doesn't know child layer will paint to integer CTM. So maybe for the time being we may need to keep replicating this behavior in diff context, just maybe in nicer way.

[flar]

In that PaintChildren code, the bounds should reflect that the content was pixel snapped. In that case the check(s) should work just fine.

[flar]

So the problem is that Preroll doesn't adjust the paint bounds to account for pixel snapping...

[flar]

Pixel snapping adjusts the translation components by a fraction of a pixel.

Preroll is dealing with local coordinates.

So, Preroll would need to deal with offsetting the paint bounds by the inverse delta-transform of the bias that was applied by the pixel snapping.

[knopp]

This seems like it might be a bit too much work just for the sake of DiffContext. I mean this does complicate things a bit in DiffContext, but probably not enough to warrant transforming each rect to device coordinates and back.

[flar]

In looking through it more, I'm becoming more and more convinced that Preroll should be working more and more in device bounds rather than local bounds. This will eventually lead to potentially collapsing Diff and Preroll together and making the pixel snapping more accessible to the operation of both. Some uses of layer paint_bounds include:

1. needs_painting() which does a content_culling on the bounds which transforms them into device bounds
2. saveLayer is defined in terms of local bounds
3. TextureLayer uses paint_bounds to render, but it seems a silly use since the bounds are literally x,y,size-of-texture so the offset would be just as good of a rendering parameter as the bounds.

For 1, we transform the bounds to device space anyway so we might as well start with device bounds. SkCanvas doesn't offer a quickRejectDeviceBounds() method, but we could replace that method with use of getDeviceClipBounds.

For 2, there is no saveDeviceLayer. We could add one in DL/Impeller and we could work around it in SkCanvas somehow. Perhaps this needs to wait until we are fully using Impeller internally, but logically it could be done. Also, the few flutter:Layers that do this could conditionally save their local bounds for use in the saveLayer while the other use for paint tree culling would use device bounds.

For 3, it isn't clear why it uses paint_bounds() to position the texture at offset.xy, that could just be reconfigured in the flutter::Texture API.

[flar]

In looking at the culling code in ContainerLayer::Paint it normally uses SkCanvas::quickReject which uses the quick reject bounds. This bounds is computed with an (optional, on by default) outset of 1 pixel to account for AA. That same outset will also help with the sub-pixel normalization of the pixel snapping operations.

We should probably institute a similar policy in DisplayListBuilder::quickReject() until we get a handle on how to make all of this code consistent in a more intentional fashion.