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

[macOS] Eliminate Vulkan hack for macOS < 10.14 #37498

Merged
merged 1 commit into from
Nov 17, 2022

Conversation

cbracken
Copy link
Member

@cbracken cbracken commented Nov 10, 2022

Eliminates an undef of VK_USE_PLATFORM_METAL_EXT that works around some unguarded @available checks for macOS 10.13. Our minimum macOS SDK is now macOS 10.14 so we can safely assume Metal support since it's a requirement for macOS 10.14.

No test changes since this should only affect the graphics backend used in tests; the tests themselves should be the same regardless of which backend is in use.

Issue: flutter/flutter#114445

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@cbracken cbracken requested a review from dnfield November 10, 2022 21:52
@dnfield dnfield added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 10, 2022
@auto-submit
Copy link
Contributor

auto-submit bot commented Nov 10, 2022

auto label is removed for flutter/engine, pr: 37498, due to - The status or check suite Mac Host Engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 10, 2022
@cbracken
Copy link
Member Author

cbracken commented Nov 10, 2022

Seeing a segfault in the impeller tests Metal shader compile, it looks like:

[----------] 6 tests from Play/RuntimeStageTest
[ RUN      ] Play/RuntimeStageTest.CanRegisterStage/Vulkan
../../flutter/impeller/playground/playground_test.cc:17: Skipped
Playground doesn't support this backend type.
[  SKIPPED ] Play/RuntimeStageTest.CanRegisterStage/Vulkan (0 ms)
[ RUN      ] Play/RuntimeStageTest.CanCreatePipelineFromRuntimeStage/Metal
[ERROR:flutter/fml/backtrace.cc(108)] Caught signal SIGSEGV during program execution.
Frame 0: 0x7ff8110cd177 newRenderPipeline()
Frame 1: 0x7ff811159b7c __233-[MTLCompiler createVertexStageAndLinkPipelineWithFragment:fragmentVariant:vertexFunction:serializedVertexDescriptor:descriptor:destinationArchive:options:reflection:compileStatistics:fragmentCompileTimeData:error:completionHandler:]_block_invoke.1436
Frame 2: 0x7ff80854f0cc _dispatch_call_block_and_release
Frame 3: 0x7ff808550317 _dispatch_client_callout
Frame 4: 0x7ff808556317 _dispatch_lane_serial_drain
Frame 5: 0x7ff808556dfd _dispatch_lane_invoke
Frame 6: 0x7ff808560eee _dispatch_workloop_worker_thread
Frame 7: 0x7ff808703fd0 _pthread_wqthread
Frame 8: 0x7ff808702f57 start_wqthread

...

Failed Command:

/Volumes/Work/s/w/ir/cache/builder/src/out/host_profile/impeller_unittests --gtest_repeat=2 --gtest_shuffle

Exit Code: -11

The failure appears to not occur during the host_debug run but rather on the host_profile build. We do 2 shuffled runs. Random seed in case this is ordering specific:

Note: Randomizing tests' orders with a seed of 32951 .

@chinmaygarde
Copy link
Member

@cbracken Can you rebase and try again please? I am actively looking into flutter/flutter#114872 ATM. The failure you are running into is not related to your patch.

Eliminates an undef of VK_USE_PLATFORM_METAL_EXT that works around some
unguarded `@available` checks for macOS 10.13. Our minimum macOS SDK is
now macOS 10.14 so we can safely assume Metal support since it's a
requirement for macOS 10.14.

Issue: flutter/flutter#114445
@cbracken cbracken force-pushed the remove-vulkan-version-check branch from dc58376 to b765621 Compare November 17, 2022 21:47
@cbracken
Copy link
Member Author

cbracken commented Nov 17, 2022

Rebased and re-pushed.

@cbracken cbracken merged commit 80b25a3 into flutter:main Nov 17, 2022
@cbracken cbracken deleted the remove-vulkan-version-check branch November 17, 2022 22:54
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 18, 2022
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Nov 18, 2022
…115608)

* fe6f22ce6 Revert "[Impeller] reuse texture if size and type matches (#37527)" (flutter/engine#37727)

* 80b25a302 [macOS] Eliminate Vulkan hack for macOS < 10.14 (flutter/engine#37498)
shogohida pushed a commit to shogohida/flutter that referenced this pull request Dec 7, 2022
…lutter#115608)

* fe6f22ce6 Revert "[Impeller] reuse texture if size and type matches (flutter#37527)" (flutter/engine#37727)

* 80b25a302 [macOS] Eliminate Vulkan hack for macOS < 10.14 (flutter/engine#37498)
gspencergoog pushed a commit to gspencergoog/flutter that referenced this pull request Jan 19, 2023
…lutter#115608)

* fe6f22ce6 Revert "[Impeller] reuse texture if size and type matches (flutter#37527)" (flutter/engine#37727)

* 80b25a302 [macOS] Eliminate Vulkan hack for macOS < 10.14 (flutter/engine#37498)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants