[Impeller] Allow metal shaders to compile through SPIR-V with openGL semantics. #40616
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
| @@ -666,12 +676,26 @@ template("_impeller_shaders_vk") { | |||
| # | |||
| # The SPIR-V version required by the shaders. | |||
| # | |||
| # @param[options] use_half_textures | |||
| # | |||
| # Whether the metal shader is using half-precision textures and requires | |||
There was a problem hiding this comment.
Can this be the default for Metal? Or is this because it interferes with the other features like framebuffer fetch? Perhaps we can detect that instead of tuning flags? Not sure. I bet this is fine too.
There was a problem hiding this comment.
It interferes with framebuffer fetch specifically. I think there is a way to make that work but I haven't found it yet. I was planning to handle this as a follow up.
There was a problem hiding this comment.
There's not really a great way to detect it automatically, since we only really get the chance to inspect the shader after its in a SPIR-V ast and at that point the validation that rejects the f16sampler2d has already run.
chinmaygarde
changed the title
|
Friendly ping:) @dnfield @chinmaygarde |
impeller/compiler/switches.h
Outdated
| @@ -36,6 +36,7 @@ struct Switches { | |||
| uint32_t gles_language_version; | |||
| std::string metal_version; | |||
| std::string entry_point; | |||
| bool use_half_textures; | |||
There was a problem hiding this comment.
Default initialize all members. This and (if you are in the mood for drive-by changes) the other members in the struct.
jonahwilliams
added
the
autosubmit
…th openGL semantics. (flutter/engine#40616)
…th openGL semantics. (flutter/engine#40616)
…th openGL semantics. (flutter/engine#40616)
…th openGL semantics. (flutter/engine#40616)
…123664) * 1d506dfd7 [Impeller] Allow metal shaders to compile through SPIR-V with openGL semantics. (flutter/engine#40616) * 3287e2b36 [Impeller] support half precision uniforms and half precision samplers (flutter/engine#40590) * 5c1c07fba Revert "Revert "Reland "Default the CanvasKit base URL to local artifacts. (#40293)" (#40470)" (#40700)" (flutter/engine#40717) * 9e4cedc81 Forward stdout and stderr from dart2wasm when verbose. (flutter/engine#40731) * 6941c1820 [Impeller] Allow toggling vulkan validation using a command line test flag. (flutter/engine#40728) * 6ef595829 Roll buildroot to build CanvasKit for speed instead of code size (flutter/engine#40737) * 5b8e02479 [Impeller] Gaussian blur: Add alpha mask specialization (flutter/engine#40707) * 6852bea71 Roll Skia from 7311e9220faf to e3eeabb14e9c (4 revisions) (flutter/engine#40745) * 235e42bea Add an option to malioc_diff.py to print a unified diff (flutter/engine#40732) * 38e6d772f Roll Dart SDK from 1acd1e649fb7 to 69867ba60bb7 (2 revisions) (flutter/engine#40738) * d81c4f2c7 [Impeller] migrate texture fill shaders to half precision. (flutter/engine#40735)
Fixes flutter/flutter#123409
Using
float16sampler2drequires compiling through openGL semantics. We shouldn't do this by default because it prevents us from using the framebuffer fetch extension.