-
Notifications
You must be signed in to change notification settings - Fork 6k
[Impeller] dont call SPIRV_CROSS_THROW in SkSL backend #37273
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. |
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 invoke impellerc in a unit test? It would be nice to have a simple verification of a specific error/shader combo
You could add something to run_tests.py that calls it and verifies output/exit code. |
…114640) * 27972e7d5 [fuchsia] mouse-input test (flutter/engine#37221) * 491032cfc Update docs to mention felt build --host (flutter/engine#37224) * 51089422d [macOS, multiwindow] Compositor gets FlutterView lazily (flutter/engine#36392) * 2e89bce45 Enter a scope before calling Dart APIs in ThrowIfUIOperationsProhibited (flutter/engine#37226) * 7843ae80c [Impeller] Correct the ordering of filters in 'Paint::WithFilters' (flutter/engine#37239) * 2cbe38be0 Produce both ddc and dart2js platform files. (flutter/engine#37162) * 2e3bc8052 Apply internal cl for C++20 prep (flutter/engine#37266) * c183e7701 Roll Dart SDK from 883ab3f70e3d to 94ac8f6cc756 (1 revision) (flutter/engine#37267) * d21f34673 Roll Fuchsia Mac SDK from BPxzJkBzD8R9GFg1n... to 8OZH-l7aK1-73Hyrf... (flutter/engine#37270) * edbba9108 Roll Fuchsia Linux SDK from 9P-WnaDSnineZtFz0... to np4MU3wmDOuhlg6CR... (flutter/engine#37269) * c7b2230e9 [Impeller] Wire-up AndroidSurfaceImpellerVulkan (flutter/engine#37249) * 73c588119 [Impeller] dont call SPIRV_CROSS_THROW in SkSL backend (flutter/engine#37273) * 893f5cd30 Roll Skia from f41fa8bffd58 to 78927395bf5c (20 revisions) (flutter/engine#37275) * 2a7f3d0c1 Roll Skia from 78927395bf5c to fe751b616832 (1 revision) (flutter/engine#37276) * fbe98c079 Roll Dart SDK from 94ac8f6cc756 to 8e089c61be58 (2 revisions) (flutter/engine#37277) * 95b9b5d83 [Impeller] Add blit command to copy texture to buffer (flutter/engine#37198) * 50c0fbc39 Roll Dart SDK from 8e089c61be58 to 866f5cfad18a (1 revision) (flutter/engine#37278) * 138acebc6 Roll Skia from fe751b616832 to fdfa00287cff (1 revision) (flutter/engine#37279) * 224b4013f Roll Fuchsia Mac SDK from 8OZH-l7aK1-73Hyrf... to mOXbRSWGSdWRXIefR... (flutter/engine#37282) * 74f021922 Announce alerts through SemanticsService on Windows (flutter/engine#37173) * a828fbb4a Roll Dart SDK from 866f5cfad18a to 433f075a852b (1 revision) (flutter/engine#37284) * 49165f1b0 Roll Fuchsia Linux SDK from np4MU3wmDOuhlg6CR... to -0Xq1c-TncmWBWzqg... (flutter/engine#37285) * d06616ecf Roll Skia from fdfa00287cff to cf3fa752a958 (2 revisions) (flutter/engine#37288) * 0b79b5c3f Roll Skia from cf3fa752a958 to af0582c7b223 (5 revisions) (flutter/engine#37290) * 66b244d9f [Impeller] validate calls to texture in SkSL (flutter/engine#37289)
…lutter#114640) * 27972e7d5 [fuchsia] mouse-input test (flutter/engine#37221) * 491032cfc Update docs to mention felt build --host (flutter/engine#37224) * 51089422d [macOS, multiwindow] Compositor gets FlutterView lazily (flutter/engine#36392) * 2e89bce45 Enter a scope before calling Dart APIs in ThrowIfUIOperationsProhibited (flutter/engine#37226) * 7843ae80c [Impeller] Correct the ordering of filters in 'Paint::WithFilters' (flutter/engine#37239) * 2cbe38be0 Produce both ddc and dart2js platform files. (flutter/engine#37162) * 2e3bc8052 Apply internal cl for C++20 prep (flutter/engine#37266) * c183e7701 Roll Dart SDK from 883ab3f70e3d to 94ac8f6cc756 (1 revision) (flutter/engine#37267) * d21f34673 Roll Fuchsia Mac SDK from BPxzJkBzD8R9GFg1n... to 8OZH-l7aK1-73Hyrf... (flutter/engine#37270) * edbba9108 Roll Fuchsia Linux SDK from 9P-WnaDSnineZtFz0... to np4MU3wmDOuhlg6CR... (flutter/engine#37269) * c7b2230e9 [Impeller] Wire-up AndroidSurfaceImpellerVulkan (flutter/engine#37249) * 73c588119 [Impeller] dont call SPIRV_CROSS_THROW in SkSL backend (flutter/engine#37273) * 893f5cd30 Roll Skia from f41fa8bffd58 to 78927395bf5c (20 revisions) (flutter/engine#37275) * 2a7f3d0c1 Roll Skia from 78927395bf5c to fe751b616832 (1 revision) (flutter/engine#37276) * fbe98c079 Roll Dart SDK from 94ac8f6cc756 to 8e089c61be58 (2 revisions) (flutter/engine#37277) * 95b9b5d83 [Impeller] Add blit command to copy texture to buffer (flutter/engine#37198) * 50c0fbc39 Roll Dart SDK from 8e089c61be58 to 866f5cfad18a (1 revision) (flutter/engine#37278) * 138acebc6 Roll Skia from fe751b616832 to fdfa00287cff (1 revision) (flutter/engine#37279) * 224b4013f Roll Fuchsia Mac SDK from 8OZH-l7aK1-73Hyrf... to mOXbRSWGSdWRXIefR... (flutter/engine#37282) * 74f021922 Announce alerts through SemanticsService on Windows (flutter/engine#37173) * a828fbb4a Roll Dart SDK from 866f5cfad18a to 433f075a852b (1 revision) (flutter/engine#37284) * 49165f1b0 Roll Fuchsia Linux SDK from np4MU3wmDOuhlg6CR... to -0Xq1c-TncmWBWzqg... (flutter/engine#37285) * d06616ecf Roll Skia from fdfa00287cff to cf3fa752a958 (2 revisions) (flutter/engine#37288) * 0b79b5c3f Roll Skia from cf3fa752a958 to af0582c7b223 (5 revisions) (flutter/engine#37290) * 66b244d9f [Impeller] validate calls to texture in SkSL (flutter/engine#37289)
SPIRV_CROSS_THROW can be configured to either throw an exception or print a message and abort. We don't enable exceptions, so the latter is the only choice. For now, though this has two additional problems:
Long-term, we might consider sending a patch to change this behavior. In the short term, we can improve the messages we add by using a differen macro
Before:
After:
Fixes flutter/flutter#114550
Work towards flutter/flutter#114536