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
cbracken merged 1 commit into
flutter-team-archive:mainfrom
cbracken:remove-vulkan-version-check
Nov 17, 2022
Merged

[macOS] Eliminate Vulkan hack for macOS < 10.14#37498
cbracken merged 1 commit into
flutter-team-archive:mainfrom
cbracken:remove-vulkan-version-check

Conversation

@cbracken

@cbracken cbracken commented Nov 10, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

auto-submit Bot commented Nov 10, 2022

Copy link
Copy Markdown
Contributor

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

cbracken commented Nov 10, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

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 .

@zanderso zanderso mentioned this pull request Nov 14, 2022
Closed
@chinmaygarde

chinmaygarde commented Nov 17, 2022

Copy link
Copy Markdown
Contributor

@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.

@cbracken
[macOS] Eliminate Vulkan hack for macOS < 10.14
b765621 
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 commented Nov 17, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Rebased and re-pushed.

@cbracken cbracken merged commit 80b25a3 into flutter-team-archive:main Nov 17, 2022
@cbracken cbracken deleted the remove-vulkan-version-check branch November 17, 2022 22:54
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Nov 18, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 18, 2022
@engine-flutter-autoroll
80b25a3 [macOS] Eliminate Vulkan hack for macOS < 10.14 (flutter-team…
27d33b0 
…-archive/engine#37498)
auto-submit Bot pushed a commit to flutter/flutter that referenced this pull request Nov 18, 2022
@engine-flutter-autoroll
Roll Flutter Engine from 49b52db to 80b25a3 (2 revisions) (#115608)
915c3de 
* fe6f22c Revert "[Impeller] reuse texture if size and type matches (#37527)" (flutter-team-archive/engine#37727)

* 80b25a3 [macOS] Eliminate Vulkan hack for macOS < 10.14 (flutter-team-archive/engine#37498)
shogohida pushed a commit to shogohida/flutter that referenced this pull request Dec 7, 2022
@engine-flutter-autoroll @shogohida
Roll Flutter Engine from 49b52db to 80b25a3 (2 revisions) (flutter#11…
676810b 
…5608)

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

* 80b25a3 [macOS] Eliminate Vulkan hack for macOS < 10.14 (flutter-team-archive/engine#37498)
gspencergoog pushed a commit to gspencergoog/flutter that referenced this pull request Jan 19, 2023
@engine-flutter-autoroll @gspencergoog
Roll Flutter Engine from 49b52db to 80b25a3 (2 revisions) (flutter#11…
1d5a6ca 
…5608)

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

* 80b25a3 [macOS] Eliminate Vulkan hack for macOS < 10.14 (flutter-team-archive/engine#37498)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
@dnfield dnfield dnfield approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cbracken @chinmaygarde @dnfield