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

M1 in macOS incorrectly reports supported compressed texture formats #2452

Closed
superdump opened this issue Feb 4, 2022 · 4 comments
Closed
Labels
area: correctness We're behaving incorrectly help required We need community help to make this happen. type: bug Something isn't working

Comments

@superdump
Copy link
Contributor

Description
Apple M1 GPUs support ASTC, BC, EAC/ETC2, and even PVRTC compressed texture formats but on BC is reported as being supported.

Repro steps
Inspect the Features returned from Adapter::features() for an M1 GPU in macOS.

Expected vs observed behavior

  • Expected
    • TEXTURE_COMPRESSION_BC, TEXTURE_COMPRESSION_ASTC_LDR, and TEXTURE_COMPRESSION_ETC2 are all reported as being supported.
  • Observed
    • Only TEXTURE_COMPRESSION_BC is reported as being supported.

Platform
Tested with wgpu 0.8 and master on an M1 Max running macOS Monterey 12.2

@superdump
Copy link
Contributor Author

I dug into this a bit and with some hacking to report all of ASTC, BC, EAC/ETC2 as being supported, I have been able to make use of all on M1. I did this because at first it was unclear from Apple documentation exactly what M1 should support.

It looks like the checks in metal-rs probably need to be updated, but also the way the features are constructed in wgpu look like they could be improved as they seem to always report BC:

| F::TEXTURE_COMPRESSION_BC

@superdump
Copy link
Contributor Author

I didn't understand the relation between MTLGPUFamily and MTLFeatureSet and was trying to understand where the Apple7 family fit into the feature sets, but the table at the beginning of: https://developer.apple.com/metal/Metal-Feature-Set-Tables.pdf clarifies that there is not yet any corresponding feature set for Apple7 (which includes A14 and M1 GPUs.)

However, I see elsewhere in the code that device.supports_family(MTLGPUFamily::Apple7) is used so I think I can just make use of that.

@superdump
Copy link
Contributor Author

I've made a PR that fixes both the issues detailed above: #2453

In metal-rs, it seems that it is not currently possible to correctly detect compressed texture format support using the following methods:

    pub fn supports_pvrtc_pixel_formats(&self) -> bool {
        self.os() != OS::macOS
    }

    pub fn supports_eac_etc_pixel_formats(&self) -> bool {
        self.os() != OS::macOS
    }

    pub fn supports_astc_pixel_formats(&self) -> bool {
        match self.os() {
            OS::iOS => self.gpu_family() >= 2,
            OS::tvOS => true,
            OS::macOS => false,
        }
    }

...

    pub fn pvrtc_pixel_formats_capabilities(&self) -> PixelFormatCapabilities {
        if self.supports_pvrtc_pixel_formats() {
            PixelFormatCapabilities::Filter
        } else {
            PixelFormatCapabilities::empty()
        }
    }

    pub fn eac_etc_pixel_formats_capabilities(&self) -> PixelFormatCapabilities {
        if self.supports_eac_etc_pixel_formats() {
            PixelFormatCapabilities::Filter
        } else {
            PixelFormatCapabilities::empty()
        }
    }

    pub fn astc_pixel_formats_capabilities(&self) -> PixelFormatCapabilities {
        if self.supports_astc_pixel_formats() {
            PixelFormatCapabilities::Filter
        } else {
            PixelFormatCapabilities::empty()
        }
    }

as they only depend on the OS and not on the MTLGPUFamily and so they have insufficient information. Arguably reporting a lack of support when the GPU supports the format is less harmful than reporting support where there is none. It still unnecessarily limits use to BC on macOS for M1 GPUs.

@kvark kvark added area: correctness We're behaving incorrectly help required We need community help to make this happen. type: bug Something isn't working labels Feb 4, 2022
@jinleili
Copy link
Contributor

Verified on the master branch of wgpu, this issue has been fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: correctness We're behaving incorrectly help required We need community help to make this happen. type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants