Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[macOS, iOS] Dynamically load MetalFX. #101590

Closed
wants to merge 1 commit into from

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Jan 15, 2025

Fixes #101585

Tested building, exporting and running with enabled MetalFX upscaling on macOS 15.2 and iOS 18.2.1.

A bit hacky, but dynamically loading stuff from Apple frameworks should be fine for AppStore (not sure if we have any other options apart from providing multiple builds, removing MetalFX upscaling entirely, or bumping min. iOS version all the way to 16.0).

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Looks good to me. I'll merge this in an hour or two before starting a new 4.4 beta 1 build, until then more reviews are welcome.

Copy link
Contributor

@stuartcarnie stuartcarnie left a comment

Choose a reason for hiding this comment

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

I think if we change the detect.py for iOS to use -weak_framework for MetalFX, it should resolve this, as the dynamic linker will handle that for us.

Comment on lines +54 to +55
if (@available(macOS 13.0, iOS 16.0, tvOS 16.0, *)) {
MDCommandBuffer *obj = (MDCommandBuffer *)(p_command_buffer.id);
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need to change this and perform availability checks every frame, as higher-level code already guarantees that we won't use the upscaler on older OSs

Comment on lines +99 to +100
if (@available(macOS 13.0, iOS 16.0, tvOS 16.0, *)) {
RenderingDeviceDriverMetal *rdd = (RenderingDeviceDriverMetal *)RD::get_singleton()->get_device_driver();
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, the higher-level code already guarantees that we won't use the upscaler on older OSs

Comment on lines +134 to +135
if (@available(macOS 13.0, iOS 16.0, tvOS 16.0, *)) {
RenderingDeviceDriverMetal *rdd = (RenderingDeviceDriverMetal *)RD::get_singleton()->get_device_driver();
Copy link
Contributor

Choose a reason for hiding this comment

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

And here, this change seems unnecessary, as we guarantee it won't be executed on older OSs

@stuartcarnie
Copy link
Contributor

See #101614 as I think this is all that is necessary – as the Xcode project is missing a weak-linking of MetalFX

@stuartcarnie
Copy link
Contributor

This PR is superseded by #101614

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to export iOS builds
5 participants