-
Notifications
You must be signed in to change notification settings - Fork 6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Impeller] Use impellerc for engine FragmentProgram unit tests #33824
Conversation
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.
Some nits but otherwise LGTM.
@@ -139,7 +139,7 @@ Compiler::Compiler(const fml::Mapping& source_mapping, | |||
break; | |||
case TargetPlatform::kFlutterSPIRV: | |||
spirv_options.SetOptimizationLevel( | |||
shaderc_optimization_level::shaderc_optimization_level_size); | |||
shaderc_optimization_level::shaderc_optimization_level_zero); |
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.
Curious, why?
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.
The transpiler is missing some ops: flutter/flutter#105396. Added a comment.
Also it's the level being used by the current test harness:
engine/lib/spirv/test/glsl_to_spirv.cc
Line 39 in d0ec220
options.SetOptimizationLevel(shaderc_optimization_level_zero); |
@@ -15,12 +15,4 @@ if (enable_unittests) { | |||
"//third_party/swiftshader_flutter:spvtools_val", | |||
] | |||
} | |||
|
|||
executable("glsl_to_spirv") { |
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.
Why can't this entire GN file be removed?
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.
The program above is still used. It looks like it is used for assembling invalid bytecode to make sure that the transpiler throws errors correctly.
570462a
to
7fb0612
Compare
Since we're shipping
impellerc
to the framework and using it to compile shaders there, that's what we should be using to run the unit tests in the engine repo.