Skip to content
This repository was archived by the owner on Apr 29, 2022. It is now read-only.

Conversation

@zanderso
Copy link
Member

The goal of this PR is to add support to impellerc for compiling glsl fragment shaders to SPIR-V such that they'll be accepted by the SPIR-V to SkSL transpiler in the Engine.

This will help migrate the hardcoded SPIR-V bytecode out of the framework at:

https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/material/ink_sparkle.dart#L690

and into a file asset: flutter/flutter#99783.

Using impellerc for this rather than another tool will make it easier to generate the platform specific shaders to feed to the FragmentProgram API when Impeller is enabled because the Engine will only need to vend one additional binary. The next step after this PR is getting impellerc building on Linux and then Windows.

Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a test that actually compiles to spirv?

Also is SkSL the right now if we're really just outputing a particular subset of SPIR-V?

@zanderso
Copy link
Member Author

I added an impellerc unit test. After impellerc is building for Linux, we can use it for the spir-v transpiler unit tests, as well.

I'm open to suggestions on naming. I was trying to avoid giving the impression that exposing more options for the spir-v and glsl versions was an okay thing to do, which led me to add a flag with a name that's describing what the intended use is.

return spirv_cross::CompilerMSL::Options::Platform::iOS;
case Compiler::TargetPlatform::kMacOS:
// Unknown should not happen due to prior validation.
case Compiler::TargetPlatform::kSkSLTranspiler:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dnfield @chinmaygarde did you have any suggestions for a better name and/or command-line flag for this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps something like DartSPIRV or FlutterSPIRV?

We didn't really come up with any special name for what flutter.dev/go/shaders uses.

Alternatively, since this just outputs spirv, we could just call it spirv and document that it's intentionally limited to a subset of spirv and will be deprecated in the near future.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think sksl is a fine name. This doesn't really have anything to do with Dart, and this is specifically a spirv subset that targets sksl

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a subset of spirv that the Dart SPIRV to SkSL compiler can understand.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went with TargetPlatfrom::FlutterSPIRV and --flutter-spirv.

@zanderso
Copy link
Member Author

Landing this now to avoid further bikeshedding =)

@zanderso zanderso merged commit deab73b into flutter:main Apr 14, 2022
@zanderso zanderso deleted the impellerc-sksl branch April 14, 2022 19:14
zanderso added a commit to flutter/engine that referenced this pull request Apr 14, 2022
zanderso added a commit to flutter/engine that referenced this pull request Apr 14, 2022
justinmc pushed a commit to justinmc/engine that referenced this pull request Apr 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants