-
Notifications
You must be signed in to change notification settings - Fork 17
Adds --sksl target to impellerc #131
Conversation
dnfield
left a comment
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.
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?
bbcc0dc to
7ae1f09
Compare
|
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. |
compiler/compiler.cc
Outdated
| return spirv_cross::CompilerMSL::Options::Platform::iOS; | ||
| case Compiler::TargetPlatform::kMacOS: | ||
| // Unknown should not happen due to prior validation. | ||
| case Compiler::TargetPlatform::kSkSLTranspiler: |
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.
@dnfield @chinmaygarde did you have any suggestions for a better name and/or command-line flag for this?
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.
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.
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 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
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.
It's a subset of spirv that the Dart SPIRV to SkSL compiler can understand.
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 went with TargetPlatfrom::FlutterSPIRV and --flutter-spirv.
7ae1f09 to
bee9e6a
Compare
|
Landing this now to avoid further bikeshedding =) |
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
FragmentProgramAPI 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.