Add Dart SPIR-V transpiler

[chriscraws]

This is the first step in implementing flutter.dev/go/shaders.

Code changes originate from https://github.com/chriscraws/engine/tree/shader-cc,
which has a working end-to-end draft implementation.

This change includes a dart library intended to be included as an
internal dependency (dart:_spirv) for dart:ui.

Dart has been chosen over C++ for the following reasons:

• Simpler to include with the HTML renderer.
• Code will be tree-shaken for applications that don't use it.
• AOT Dart performance is comparable to performance of C++
  implementation.
• Easier to read.

The transpiler currently supports a minimal set of features - planned additions for later PRs are structured types, control flow, and 2d-sampling. This should be after the feature is fully integrated.

Golden shader files are tested manually, but will be tested fully
by Flutter engine unit tests as the library is included by dart:ui.

Most of the files included in this change are tests and test-data.

flutter/flutter#58361
flutter/flutter#30763

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
• I listed at least one issue that this PR fixes in the description above.
• I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
  writing and running engine tests.
• I updated/added relevant documentation (doc comments with ///).
• I signed the CLA.
• All existing and new tests are passing.
• The reviewer has submitted any presubmit flakes in this PR using the engine presubmit flakes form before re-triggering the failure.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[brianosman]

skls supports float3x2 and other floatNxM where N != M, should they be listed here?

Public SkSL (the flavor that's accepted by SkRuntimeEffect) only supports square matrices (because GLSL ES 1.00 only supports square matrices).

[chinmaygarde]

cc @zanderso

[chinmaygarde]

Initial notes on testing and benchmarking as I review the other bits in detail. These are mostly around process and integration:

• Authoring new tests currently requires spirv-opt and glslangValidator on the developers paths. There are no instructions on how the developer might fetch these and which version of these tools may be necessary. However, these tools or their equivalents are already present in the source tree via the SwiftShader dependency. After adding GN rules for the appropriate targets, you can integrate this with generation of the goldens within the build and test process. This will avoid having to check in and maintain some (but not all) SPIRV goldens. These tests running from the GTest harness will also run in JIT and AOT modes automatically and integrate with the rest of the engine testing infrastructure monitored by sheriffs.
• Instead of inventing a bespoke benchmarking harness, please use the engine Google Benchmarking harness. This harness is capable of running benchmarks automatically and submitting the results to dashboards that are already being monitored. Also, this harness will run in profile and release AOT modes whose results are more relevant to the end user (and also work around the “warm up” issues you mentioned in the current harness).
• The user experience for SPIRV that does not adhere to the rules specified in the README are not clear. Please add tests that assert failure modes.
• The exercise of depending on SPIRV as the portable container for shaders was partially derived from the fact that SKSL had no stability guarantees. Towards that end, having SKSL goldens in the repo makes us susceptible to breaking changes to the format causing manual out of band regeneration of SKSL goldens. Instead of checking in SKSL goldens, is it possible to generate SKSL from GLSL (via an intermediate SPIRV translation step) and adding a test that ensures that the SKSL is valid using the current Skia version with SKSL reflection reflection used for test assertions? @brianosman, is there a Skia API for SKSL reflection? If not, we should at least ensure that the SKSL is valid.

[chriscraws]

• The exercise of depending on SPIRV as the portable container for shaders was partially derived from the fact that SKSL had no stability guarantees. Towards that end, having SKSL goldens in the repo makes us susceptible to breaking changes to the format causing manual out of band regeneration of SKSL goldens. Instead of checking in SKSL goldens, is it possible to generate SKSL from GLSL (via an intermediate SPIRV translation step) and adding a test that ensures that the SKSL is valid using the current Skia version with SKSL reflection reflection used for test assertions? @brianosman, is there a Skia API for SKSL reflection? If not, we should at least ensure that the SKSL is valid.

I think generating the intermediate goldens is fine - My plan here is to ensure conformance via flutter engine tests that applies each shader to a Canvas and checks to make sure each .spv file generates the correct colors, and catching any build issue on the way - so we'd have the exact build/compilation errors for each platform. The downside is that the tests are 'farther' from the code they're testing. Is that a reasonable approach?

For your other comments I agree and I'll take a pass at making the changes

[chinmaygarde]

I think generating the intermediate goldens is fine...

I am not concerned about the SPIRV intermediates as that is stable anyway. I meant the sksl.golden files specifically. Changes to the format would need manual out-of-band regeneration of the goldens which makes rolls harder.

My plan here is to ensure conformance via flutter engine tests that applies each shader to a Canvas and checks to make sure each .spv file generates the correct colors, and catching any build issue on the way.

I see. The bit about using pixel tests to assert correctness was not clear to me. I was assuming that tests would reflect SKSL properties to ensure correctness of translation. Is reflection not possible at all with SKSL (perhaps @brianosman can answer this)? Pixel tests are harder to maintain but we can manage it. I am more concerned about tests that assert updates to SKSL that don't affect pixel output, or, affect them too subtly to make the error clear from just the pixel output. Also, stuff like optimizations like dead constant/block/function elimination, folding, etc.. I am not sure how much Skia optimizes the SKSL on its own as well. In any case, its fine to have this be an open question to be answered later.

For your other comments I agree and I'll take a pass at making the changes

Sounds good. Please reach out for references to existing harnesses you can repurpose.

[brianosman]

To address a few questions:

• SkSL doesn't offer full reflection of the program structure. Attempts to compile it will succeed or fail (with error messages). From there, you can reflect the kinds of metadata you need to use it (how many uniforms, their types, etc...), but not anything about the internals of each function.
• There are almost no breaking changes to SkSL these days, and that pace will have to drop to ~0, as more clients pick it up.
• Alternatively - and this may be what you want for other reasons - we have built a C++ API for constructing SkSL programmatically. It's structured as a C++ embedded DSL, so you wouldn't necessarily have reflection, but you'd be directly constructing the program elements (expressions, statements, etc...), so that might be moot. The DSL is used today for some of our internal cases, and Ethan is currently working on SkRuntimeEffect integration. You can see the API here.
  Happy to chat about this - there are definitely some open questions around how this should all fit together.

[chriscraws]

I've updated the PR with the following changes, based on offline discussions:

1. Removed golden testing
2. Removed benchmarks (can add benchmarks in the proper place in a follow-up PR)
3. Added "exception tests" that compile commented spir-v assembly into spir-v binary using SPIR-V tools from the SwiftShader dependency. This required creating an assembler C++ binary to build with Flutter's configuration.
4. Removed a few things from the transpiler that are not used yet (only relevant for control flow, which is a later PR)
5. Updated the readme in lib/spirv with relevant information.

The following are things expected in follow-up PRs (order after #1 is flexible)

1. Making the library available within dart:ui as dart:_spirv and creating fragment_shader.cc to expose fragment shaders to dart. This change will include comprehensive pixel testing for all supported SPIR-V features.
2. Add support for basic control flow.
3. Add support for child shaders / samplers.
4. Extend support to the web_ui library.

[clocksmith]

Does this need another review?

[chinmaygarde]

I am taking a look now but its slow going since its a rather large patch. Will have initial comments soon. If Brian is able to chime in, that would be helpful as well.

[brianosman]

I'm OoO this week, just did a quick skim, but I'd really need to sit down and spend some more time. I'm curious about a few of the ops (composite assemble/extract, etc.), but without walking through the SPIRV spec (or SkSL's SPIRV backend), I can't say there's any problem.

[clocksmith]

OK sounds good

[brianosman]

Looked over this a bit better. As far as I can tell, things all make sense. I did point out one spot where opAccessChain assumes array indexing, which isn't always the case (although if you're not supporting structs yet, then it won't matter).

[clocksmith]

Can the pixel tests, along with the FragmentShader class, be landed in a followup PR (since this one is already quite large)? It is in progress

[chriscraws]

@brianosman Thanks for the review- not supporting structs in this PR (for now) but I can add a comment to clarify

@clocksmith, yes, that is my hope for follow up PRs.

The following are things expected in follow-up PRs (order after #1 is flexible)

1. Making the library available within dart:ui as dart:_spirv and creating fragment_shader.cc to expose fragment shaders to dart. This change will include comprehensive pixel testing for all supported SPIR-V features.

2. Add support for basic control flow.

3. Add support for child shaders / samplers.

4. Extend support to the web_ui library.

[Hixie]

That's a lot of Dart code on the engine side. Is it possible to pull it out as a package and just have the engine/dart boundary in this repo? (I didn't look very closely, maybe the whole of this Dart code is that boundary?) The reason I ask is that the more code we take out of the engine, the smaller the binary size footprint for users who aren't using this code, and the easier it is to fix bugs in the code going forward (e.g. the package can roll pretty independently).

[zanderso]

Make sure the Dart code is covered by the analyzer on CI by adding to https://github.com/flutter/engine/blob/master/ci/analyze.sh.

[zanderso]

That's a lot of Dart code on the engine side. Is it possible to pull it out as a package and just have the engine/dart boundary in this repo? (I didn't look very closely, maybe the whole of this Dart code is that boundary?) The reason I ask is that the more code we take out of the engine, the smaller the binary size footprint for users who aren't using this code, and the easier it is to fix bugs in the code going forward (e.g. the package can roll pretty independently).

The transpiler is needed so that the public dart:ui API can accept SPIR-V even though the Skia API accepts SkSL. We will have to make sure that the inclusion in dart:ui is structured so that the transpiler will be tree-shaken away when the shader API is not used.

[Hixie]

Ah, yeah, if the internal API is SkSL that makes sense.

[google-cla]

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

[google-cla]

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

[zanderso]

@chriscraws It looks like something about your branch has confused GitHub. I believe there are some git commands you can run locally to recover from this state, but it might be simpler to create a new PR.

[zanderso]

@christopherfujino suggested to me over chat that you can also re-create the branch locally from scratch, and then force push it over your existing feature branch on your mirror, and it will update the PR.

[zanderso]

It might also work to do a git pull --rebase, and then git push origin <branch name> --force.

[christopherfujino]

I was able to fix the diff by merging the latest master to the feature branch. Note, that when you merged your local master branch, it was still 5 weeks old. You'll have to update your local master branch first, or explicitly merge the remote master branch.

[chriscraws]

Ok I think I fixed the git weirdness. I do think it was from rebasing on to an older branch, and a lot of weird stuff happened 😅

It should all tree shake correctly since the transpiler is pure dart (no adding to the shared lib), so it should only be included if someone makes use of the dart:ui FragmentShader class (not in this PR)

[chriscraws]

I've opened this PR to fix the remaining build failure:
flutter/buildroot#454

[zanderso]

lgtm w/ suggested uses of constant maps instead of giant switch statements.

[clocksmith]

@chinmaygarde @zanderso @brianosman Can I merge this?

[zanderso]

Still lgtm. Feel free to merge when ready.

[jwinarske]

@chriscraws I imagine this will work seamlessly with Vulkan?

[chriscraws]

@chriscraws I imagine this will work seamlessly with Vulkan?

Yep - the SkSL should work on all of the back-ends that Skia supports (vulkan, metal, opengl, etc). The GLSL-ES output is still untested until FragmentProgram is implemented in the HTML renderer.