Delete the SkPath/SkRRect code duplication in for physical model layers. #4519
Conversation
|
When landing this on the framework please keep an eye on the performance numbers to make sure this doesn't cause any regressions. Thanks. |
| #endif // defined(OS_FUCHSIA) | ||
| private: | ||
| SkPath path_; | ||
| bool isRect_; |
There was a problem hiding this comment.
add a constructor initializer for isRect_
flow/layers/physical_model_layer.h
Outdated
| SkRRect rrect = SkRRect::MakeEmpty(); | ||
| if (path.isRRect(&rrect)) { | ||
| frameRRect_ = rrect; | ||
| isRect_ = rrect.isRect(); |
There was a problem hiding this comment.
I think we'll want the isRect check on all platforms so Paint() can avoid the canvas.saveLayer() call in the common case of a rectangular shape
flow/layers/physical_model_layer.h
Outdated
| } | ||
| #if defined(OS_FUCHSIA) | ||
| if (path.isRRect(&rrect)) { | ||
| frameRRect_ = rrect; |
There was a problem hiding this comment.
Now it looks like isRect_ will not be set to true in the Fuchsia case. I'd recommend moving this to physical_model_layer.cc and having one code path for all platforms that calls path.isRRect(&frameRRect_) and sets isRect_ accordingly
amirh
force-pushed
the
physical-layer-path-only
branch
from
ab1f977 to
a6194d9
Compare
|
PTAL, also renamed physical model to physical shape. |
flow/layers/physical_model_layer.cc
Outdated
|
|
||
| PhysicalModelLayer::PhysicalModelLayer() { | ||
| isRect_ = false; | ||
| frameRRect_ = SkRRect::MakeEmpty(); |
There was a problem hiding this comment.
No need to do this explicitly - SkRRect's default constructor will make it empty
This adjusts the framework to work with the engine change in flutter/engine#4519
…del layers. (flutter#4519)"" That commit was reverted due to a performance regression, which we've root caused since. And going to followup with a fix.
4 participants
Just always use SkPath.
This is a breaking change, framework PR: flutter/flutter#13942.
Will announce a breaking change before submitting.