Skip to content

Commit

Permalink
[Impeller] relax conditions for SkRRect.isSimple conversion to impell…
Browse files Browse the repository at this point in the history
…er::RRect. (flutter#53083)

The flickering in flutter/flutter#148412 is caused by us switching between the RRect fast path and a gaussian blur. The reason is that the SkRect.isSimple check doesn't handle fp precision very well. On one of the frames the difference was :

```
D/skia (18362): SkRect::MakeLTRB(74, 179.666672f, 374, 479.666656f);
D/skia (18362): const SkPoint corners[] = {
D/skia (18362): { 150, 149.999969f },
D/skia (18362): { 150, 150 },
D/skia (18362): { 150, 149.999969f },
D/skia (18362): { 150, 150 },
D/skia (18362): };
```

So lets used a relaxed check for RRect.isSimple instead.

Fixes flutter/flutter#148412
  • Loading branch information
jonahwilliams committed Jun 5, 2024
1 parent 1a948ed commit e372d65
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 2 deletions.
4 changes: 2 additions & 2 deletions impeller/display_list/dl_dispatcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -814,8 +814,8 @@ void DlDispatcher::drawCircle(const SkPoint& center, SkScalar radius) {
}

// |flutter::DlOpReceiver|
void DlDispatcher::drawRRect(const SkRRect& rrect) {
if (rrect.isSimple()) {
void DlDispatcherBase::drawRRect(const SkRRect& rrect) {
if (skia_conversions::IsNearlySimpleRRect(rrect)) {
canvas_.DrawRRect(skia_conversions::ToRect(rrect.rect()),
skia_conversions::ToSize(rrect.getSimpleRadii()), paint_);
} else {
Expand Down
14 changes: 14 additions & 0 deletions impeller/display_list/skia_conversions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,20 @@
namespace impeller {
namespace skia_conversions {

bool IsNearlySimpleRRect(const SkRRect& rr) {
auto [a, b] = rr.radii(SkRRect::kUpperLeft_Corner);
auto [c, d] = rr.radii(SkRRect::kLowerLeft_Corner);
auto [e, f] = rr.radii(SkRRect::kUpperRight_Corner);
auto [g, h] = rr.radii(SkRRect::kLowerRight_Corner);
return SkScalarNearlyEqual(a, b, kEhCloseEnough) &&
SkScalarNearlyEqual(a, c, kEhCloseEnough) &&
SkScalarNearlyEqual(a, d, kEhCloseEnough) &&
SkScalarNearlyEqual(a, e, kEhCloseEnough) &&
SkScalarNearlyEqual(a, f, kEhCloseEnough) &&
SkScalarNearlyEqual(a, g, kEhCloseEnough) &&
SkScalarNearlyEqual(a, h, kEhCloseEnough);
}

Rect ToRect(const SkRect& rect) {
return Rect::MakeLTRB(rect.fLeft, rect.fTop, rect.fRight, rect.fBottom);
}
Expand Down
7 changes: 7 additions & 0 deletions impeller/display_list/skia_conversions.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@
namespace impeller {
namespace skia_conversions {

/// @brief Like SkRRect.isSimple, but allows the corners to differ by
/// kEhCloseEnough.
///
/// An RRect is simple if all corner radii are approximately
/// equal.
bool IsNearlySimpleRRect(const SkRRect& rr);

Rect ToRect(const SkRect& rect);

std::optional<Rect> ToRect(const SkRect* rect);
Expand Down
10 changes: 10 additions & 0 deletions impeller/display_list/skia_conversions_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "flutter/testing/testing.h"
#include "impeller/display_list/skia_conversions.h"
#include "impeller/geometry/scalar.h"
#include "include/core/SkRRect.h"

namespace impeller {
namespace testing {
Expand Down Expand Up @@ -174,5 +175,14 @@ TEST(SkiaConversionsTest, GradientConversionNonMonotonic) {
ASSERT_TRUE(ScalarNearlyEqual(converted_stops[3], 1.0f));
}

TEST(SkiaConversionsTest, IsNearlySimpleRRect) {
EXPECT_TRUE(skia_conversions::IsNearlySimpleRRect(
SkRRect::MakeRectXY(SkRect::MakeLTRB(0, 0, 10, 10), 10, 10)));
EXPECT_TRUE(skia_conversions::IsNearlySimpleRRect(
SkRRect::MakeRectXY(SkRect::MakeLTRB(0, 0, 10, 10), 10, 9.999)));
EXPECT_FALSE(skia_conversions::IsNearlySimpleRRect(
SkRRect::MakeRectXY(SkRect::MakeLTRB(0, 0, 10, 10), 10, 9)));
}

} // namespace testing
} // namespace impeller

0 comments on commit e372d65

Please sign in to comment.