-
Notifications
You must be signed in to change notification settings - Fork 6k
Test Framework for SKP based shader warmup #19260
Conversation
374cdfa
to
d93faa9
Compare
d93faa9
to
0aad8b6
Compare
shell/common/rasterizer.cc
Outdated
|
||
frame->Raster(*tree, true); | ||
|
||
#if defined(OS_FUCHSIA) | ||
SkSerialProcs procs = {0}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Format issue: space doesn't align.
WaitForIO(shell.get()); | ||
|
||
// Count the number of shaders this builder generated. We use this as a proxy | ||
// for whether new shaders were generated, since skia will dump an skp any |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Flutter only dumps one skp per frame no matter how many shaders are compiled in that frame. So this will count the number of frames, not the number of shaders.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted. I think thats ok for the purpose of this test, since the goal is to tell if new shaders were generated and we don't care how many, and in all cases we only pump one frame. I can definitely change the comment to reflect this though
|
||
SkDeserialProcs procs = {0}; | ||
procs.fImageProc = DeserializeImageWithoutData; | ||
sk_sp<SkPicture> picture = SkPicture::MakeFromStream(stream.get(), &procs); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little surprised that SkPicture::MakeFromStream
can be used here without changing Skia's build configuration. I think Flutter has disabled that Skia feature. Is there a Fuchsia config file that re-enables it?
BTW, I can't find SkpWarmupTest
in https://api.cirrus-ci.com/v1/task/6670703166488576/logs/test_host.log
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First question i dunno, seems to work for me.
Second question: the whole test is ifdef'd with OS_FUCHSIA right now because it was failing on windows and i figured it was best to isolate any problems with it to Fuchsia since thats the only platform where this is relevant right now. I assume that means its not included in host_debug_unopt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM besides nits and a bug for tracking purposes
shell/common/rasterizer.cc
Outdated
@@ -457,16 +454,30 @@ static sk_sp<SkData> ScreenshotLayerTreeAsPicture( | |||
SkMatrix root_surface_transformation; | |||
root_surface_transformation.reset(); | |||
|
|||
#if defined(OS_FUCHSIA) | |||
// Our ScopedFrame implementation doesnt do the right thing here so |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make a bug for this and block fxb/46971 on it
9c40e5a
to
82f9de7
Compare
31f4254
to
24a742d
Compare
This change adds custom image serialization functions for skp serialization which write out onlythe image metadata and not the contents of the images themselves. This allows SKP's to be used for shader warmup with a significantly reduced disk space footprint.
24a742d
to
160313e
Compare
Description
This pull request mainly sets up a test framework for warming up the shader cache by drawing skia pictures serialized to disk at runtime, and some custom serialization hooks to decrease the disk footprint from Image data, which is not needed to warm up shaders. Includes other ancillary changes to make skp serialization work properly on fuchsia.
Related Issues
fxb/47057
Tests
I added the following tests:
SkpWarmupTest.Basic
SkpWarmupTest.Image
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]
). This will ensure a smooth and quick review process.Breaking Change
Did any tests fail when you ran them? Please read handling breaking changes.