Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@zanderso
Copy link
Member

@zanderso zanderso commented Jun 4, 2022

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.

Copy link
Member

@chinmaygarde chinmaygarde left a 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.

case TargetPlatform::kFlutterSPIRV:
spirv_options.SetOptimizationLevel(
shaderc_optimization_level::shaderc_optimization_level_size);
shaderc_optimization_level::shaderc_optimization_level_zero);
Copy link
Member

Choose a reason for hiding this comment

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

Curious, why?

Copy link
Member Author

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:

options.SetOptimizationLevel(shaderc_optimization_level_zero);

]
}

executable("glsl_to_spirv") {
Copy link
Member

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?

Copy link
Member Author

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.

@chinmaygarde chinmaygarde changed the title Use impellerc for engine FragmentProgram unit tests [Impeller] Use impellerc for engine FragmentProgram unit tests Jun 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants