Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: flutter-team-archive/engine
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 30b7e9ded7a0
Choose a base ref
...
head repository: flutter-team-archive/engine
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: cdcbdcc7ccba
Choose a head ref
  • 5 commits
  • 44 files changed
  • 3 contributors

Commits on Sep 15, 2023

  1. Do not convert an open path to a closed rect. (#45903)

    Fixes flutter/flutter#134816.
    
    ---
    
    Confirmed this only happens when utilizing `DisplayListBuilder`.
    Consider:
    
    ```cc
    TEST_P(DisplayListTest, CanDrawAnOpenPath) {
      flutter::DisplayListBuilder builder;
      flutter::DlPaint paint;
    
      paint.setColor(flutter::DlColor::kRed());
      paint.setDrawStyle(flutter::DlDrawStyle::kStroke);
      paint.setStrokeWidth(10);
    
      builder.Translate(300, 300);
    
      // Move to (50, 50) and draw lines from:
      // 1. (50, height)
      // 2. (width, height)
      // 3. (width, 50)
      SkPath path;
      path.moveTo(50, 50);
      path.lineTo(50, 100);
      path.lineTo(100, 100);
      path.lineTo(100, 50);
      builder.DrawPath(path, paint);
    
      ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
    }
    ```
    
    ![Screenshot 2023-09-15 at 2 06 53
    PM](https://github.com/flutter/flutter/assets/168174/3a3868e1-f1d1-4e29-affa-4bd32d26ccdc)
    
    ---
    
    The bug is in `dl_dispatcher.cc`:
    
    ```cc
    // |flutter::DlOpReceiver|
    void DlDispatcher::drawPath(const SkPath& path) {
      SkRect rect;
      SkRRect rrect;
      SkRect oval;
    
      if (path.isRect(&rect)) {
        canvas_.DrawRect(skia_conversions::ToRect(rect), paint_);
      } else if (path.isRRect(&rrect) && rrect.isSimple()) {
        canvas_.DrawRRect(skia_conversions::ToRect(rrect.rect()),
                          rrect.getSimpleRadii().fX, paint_);
      } else if (path.isOval(&oval) && oval.width() == oval.height()) {
        canvas_.DrawCircle(skia_conversions::ToPoint(oval.center()),
                           oval.width() * 0.5, paint_);
      } else {
        canvas_.DrawPath(skia_conversions::ToPath(path), paint_);
      }
    }
    ```
    
    Note the documentation for `isRect`:
    
    ```cc
    /** Returns true if SkPath is equivalent to SkRect when filled.
        If false: rect, isClosed, and direction are unchanged.
        If true: rect, isClosed, and direction are written to if not nullptr.
    
        rect may be smaller than the SkPath bounds. SkPath bounds may include kMove_Verb points
        that do not alter the area drawn by the returned rect.
    
        @param rect       storage for bounds of SkRect; may be nullptr
        @param isClosed   storage set to true if SkPath is closed; may be nullptr
        @param direction  storage set to SkRect direction; may be nullptr
        @return           true if SkPath contains SkRect
    
        example: https://fiddle.skia.org/c/@Path_isRect
    */
    bool isRect(SkRect* rect, bool* isClosed = nullptr, SkPathDirection* direction = nullptr) const;
    ```
    
    ... the `isClosed` is critical, without checking that we're doing an
    incorrect optimization.
    
    I'll send a fix and test.
    matanlurey authored Sep 15, 2023
    Configuration menu
    Copy the full SHA
    97705b8 View commit details
    Browse the repository at this point in the history
  2. Roll Skia from 0057898979a1 to 917fd16e6f26 (1 revision) (#45906)

    https://skia.googlesource.com/skia.git/+log/0057898979a1..917fd16e6f26
    
    2023-09-15 johnstiles@google.com Add device-name to Caps when test-utils is active.
    
    If this roll has caused a breakage, revert this CL and stop the roller
    using the controls here:
    https://autoroll.skia.org/r/skia-flutter-autoroll
    Please CC bdero@google.com,brianosman@google.com,jmbetancourt@google.com,rmistry@google.com on the revert to ensure that a human
    is aware of the problem.
    
    To file a bug in Skia: https://bugs.chromium.org/p/skia/issues/entry
    To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose
    
    To report a problem with the AutoRoller itself, please file a bug:
    https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug
    
    Documentation for the AutoRoller is here:
    https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
    skia-flutter-autoroll authored Sep 15, 2023
    Configuration menu
    Copy the full SHA
    6f6daca View commit details
    Browse the repository at this point in the history
  3. Roll Skia from 917fd16e6f26 to 7c179932cc06 (1 revision) (#45907)

    https://skia.googlesource.com/skia.git/+log/917fd16e6f26..7c179932cc06
    
    2023-09-15 skia-autoroll@skia-public.iam.gserviceaccount.com Roll SK Tool from 18fd0925a396 to 76d21d4985f8
    
    If this roll has caused a breakage, revert this CL and stop the roller
    using the controls here:
    https://autoroll.skia.org/r/skia-flutter-autoroll
    Please CC bdero@google.com,brianosman@google.com,jmbetancourt@google.com,rmistry@google.com on the revert to ensure that a human
    is aware of the problem.
    
    To file a bug in Skia: https://bugs.chromium.org/p/skia/issues/entry
    To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose
    
    To report a problem with the AutoRoller itself, please file a bug:
    https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug
    
    Documentation for the AutoRoller is here:
    https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
    skia-flutter-autoroll authored Sep 15, 2023
    Configuration menu
    Copy the full SHA
    a33235e View commit details
    Browse the repository at this point in the history

Commits on Sep 16, 2023

  1. Roll Skia from 7c179932cc06 to c19cc483c619 (1 revision) (#45911)

    https://skia.googlesource.com/skia.git/+log/7c179932cc06..c19cc483c619
    
    2023-09-16 michaelludwig@google.com Update SetTraceValue() to use a template, remove bitpun union
    
    If this roll has caused a breakage, revert this CL and stop the roller
    using the controls here:
    https://autoroll.skia.org/r/skia-flutter-autoroll
    Please CC bdero@google.com,brianosman@google.com,jmbetancourt@google.com,rmistry@google.com on the revert to ensure that a human
    is aware of the problem.
    
    To file a bug in Skia: https://bugs.chromium.org/p/skia/issues/entry
    To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose
    
    To report a problem with the AutoRoller itself, please file a bug:
    https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug
    
    Documentation for the AutoRoller is here:
    https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
    skia-flutter-autoroll authored Sep 16, 2023
    Configuration menu
    Copy the full SHA
    3983b50 View commit details
    Browse the repository at this point in the history
  2. Revert "[Impeller] construct text frames on UI thread." (#45910)

    Reverts #45418
    
    Some google3 tests are hitting the CHECK I added in the DlSkCanvasDispatcher::drawTextFrame, which indicates that the SkParagraph code likely thinks impeller is enabled, whereas other code might be running with Skia.
    
    Perhaps this could happen if its software rendering? It should be a fatal error on startup so we can track this down.
    Jonah Williams authored Sep 16, 2023
    Configuration menu
    Copy the full SHA
    cdcbdcc View commit details
    Browse the repository at this point in the history
Loading