maplibre / maplibre-native

MapLibre Native - Interactive vector tile maps for iOS, Android and other platforms.
https://maplibre.org
BSD 2-Clause "Simplified" License
1.04k stars 300 forks source link

Add back CMake build config for macOS #2729

Closed louwers closed 1 month ago

louwers commented 1 month ago

This builds on #2715 with some more fixes to the CMake config for macOS.

Reinstating these CMake build files is the easiest way to get Node.js working with Metal (and probably Qt too).

github-actions[bot] commented 1 month ago

Bloaty Results (iOS) 🐋

Compared to main

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  [ = ]       0  [ = ]       0    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results-ios/pr-2729-compared-to-main.txt

github-actions[bot] commented 1 month ago

Bloaty Results 🐋

Compared to main

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  [ = ]       0  [ = ]       0    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-2729-compared-to-main.txt

Compared to d38709084a9865fe0bb8300aec70ebf8243b3d43 (legacy)

    FILE SIZE        VM SIZE    
 --------------  -------------- 
   +27% +31.0Mi  +420% +25.1Mi    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-2729-compared-to-legacy.txt

github-actions[bot] commented 1 month ago

Benchmark Results ⚡

Benchmark                                                     Time             CPU      Time Old      Time New       CPU Old       CPU New
------------------------------------------------------------------------------------------------------------------------------------------
OVERALL_GEOMEAN                                            -0.0116         -0.0111             0             0             0             0

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/benchmark-results/pr-2729-compared-to-main.txt

acalcutt commented 1 month ago

Seems like it compiles on x86_64, but the tests get this error

-[MTLTextureDescriptorInternal validateWithDevice:]:1357: failed assertion `Texture Descriptor Validation
MTLTextureDescriptor: Depth, Stencil, DepthStencil textures cannot be allocated with MTLStorageModeShared or MTLStorageModeManaged on this device.

Do you think this will only work with arm architecture?

Edit: I see that error here but it is in java code. their fix was https://github.com/openjdk/jfx-sandbox/commit/349410432b6609f90321f1fe3bf706e138579fef . Not sure it really helps here but it seems they implemented DirectBuffer

louwers commented 1 month ago

@acalcutt Should work, but GitHub might run some kind of emulation and not have a Metal enabled GPU.

I see this in our source code:

#if TARGET_OS_SIMULATOR
        switch (format) {
            case MTL::PixelFormatDepth16Unorm:
            case MTL::PixelFormatDepth32Float:
            case MTL::PixelFormatStencil8:
            case MTL::PixelFormatDepth24Unorm_Stencil8:
            case MTL::PixelFormatDepth32Float_Stencil8:
            case MTL::PixelFormatX32_Stencil8:
            case MTL::PixelFormatX24_Stencil8:
                // On iOS simulator, the default shared mode is invalid for depth and stencil textures.
                //  'Texture Descriptor Validation MTLTextureDescriptor: Depth, Stencil, DepthStencil
                //   textures cannot be allocated with MTLStorageModeShared on this device.
                textureDescriptor->setStorageMode(MTL::StorageMode::StorageModePrivate);
                break;
            default:
                break;
        }
#endif

On non-Apple family GPUs, the shared storage mode isn’t available for MTLTexture instances.

louwers commented 1 month ago

pre-commit.ci run