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

Verbose display list comparisons #32737

Merged
merged 3 commits into from
Apr 20, 2022

Conversation

flar
Copy link
Contributor

@flar flar commented Apr 18, 2022

As we move towards having our layers speak to DisplayList objects directly, we might need to shift the way that we write layer unittests from the "vector of drawing records" that is driven by implementing SkCanvas to a new mechanism that compares DisplayList objects.

This PR will add a (pair of) mechanisms to do Verbose DisplayList comparisons to the MockLayer system. To use it, you hand the display_list version of the paint_context to a Layer's Paint() method and then manually construct a DisplayList that represents what should have been drawn (the expected_display_list). Then handing the MockLayer's display list and the expected version to DisplayListsEQ_Verbose (or the NE variant) and the two rendering paths will be compared operation by operation and if they don't match, a hierarchically pretty-printed version of the DisplayList will be written out alongside the error report so you can see what was different between the expectation and the layer's rendering.

Along with the new mechanism a layer unittest was modified to demonstrate the new pattern. You can see the changes in opacity_layer_unittests.cc. Here is an example of the error output if I omit one of the setColor() commands to show how debugging will work under the new system:

Open to see sample test failure output
[ RUN      ] OpacityLayerTest.HalfTransparent
[ERROR:flutter/testing/display_list_testing.cc(19)] 

DisplayList {
  save();
  {
    translate(0.5, 1.5);
    tranformReset();
    transform2DAffine(
      [1, 0, 1], 
      [0, 1, 2], 
    );
    setColor(SkColor(7f000000));
    saveLayer(SkRect(left: 0, top: 0, right: 5, bottom: 5), SaveLayerOptions(can_distribute_opacity: true, renders_with_attributes: true));
    {
      setColor(SkColor(ff00ff00));
      drawPath(SkPath(bounds: SkRect(left: 0, top: 0, right: 5, bottom: 5)));
      restore();
    }
    restore();
  }
}

not identical to ...

DisplayList {
  save();
  {
    translate(0.5, 1.5);
    tranformReset();
    transform2DAffine(
      [1, 0, 1], 
      [0, 1, 2], 
    );
    saveLayer(SkRect(left: 0, top: 0, right: 5, bottom: 5), SaveLayerOptions(can_distribute_opacity: true, renders_with_attributes: true));
    {
      setColor(SkColor(ff00ff00));
      drawPath(SkPath(bounds: SkRect(left: 0, top: 0, right: 5, bottom: 5)));
      restore();
    }
    restore();
  }
}

../../flutter/flow/layers/opacity_layer_unittests.cc:323: Failure
Value of: DisplayListsEQ_Verbose(display_list(), expected_display_list)
  Actual: false
Expected: true
[  FAILED  ] OpacityLayerTest.HalfTransparent (0 ms)

@zanderso zanderso requested review from bdero and dnfield April 20, 2022 03:25
Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants