Remove saveLayer after clip from dart #18616
Conversation
e91d2bd to
82b936a
Compare
This is a follow up on flutter/engine#5420 and flutter#18057 As you can see from the diff, we also mistakenly saveLayer before the clip at some places previously.
|
Ping @chinmaygarde ... |
| canvas.saveLayer(bounds, new Paint()); | ||
| } else { | ||
| canvas.save(); | ||
| } |
There was a problem hiding this comment.
this sounds like it breaks the rendering, is there a test that verifies that this is ok?
There was a problem hiding this comment.
ah, that's #18729
please add a golden test when fixing this. thanks.
|
Shouldn't this land at the same time as the |
|
I'll add a golden test once #18739 lands. I landed this earlier because we've already removed saveLayer from the C++ engine side and no one seems to be bothered by the difference. Moreover, this PR opens our way to explore performance improvements on raster caches. |
|
@flar would the optimizations in flutter/engine#29775 make this unnecessary? |
These are unrelated. |
This is a follow up on flutter/engine#5420
and #18057
Note that our raster cache will most likely cache the SkPicture so
this should NOT have a big performance impact by itself. We'll
only get a small speedup before raster cache kicks in.
Once the raster cache is disabled, we can see that complex_layout
scroll speedups from 11ms/frame to 9ms/frame. With raster cache,
it's about 8ms/frame. (All on my Nexus 5X).
For flutter_gallery transition test, we get a big speedup from 38.5ms/frame
to 17.5ms/frame without raster cache. With raster cache and this PR,
it's about 15.5ms/frame.
So with this PR, the raster cache becomes much less important
at least for our own benchmarks.