Skip to content

add Metal support for MULTISAMPLE_ARRAY - #9300

Merged
cwfitzgerald merged 4 commits into
gfx-rs:trunkfrom
39ali:metal-multisample-array
Apr 9, 2026
Merged

add Metal support for MULTISAMPLE_ARRAY#9300
cwfitzgerald merged 4 commits into
gfx-rs:trunkfrom
39ali:metal-multisample-array

Conversation

@39ali

@39ali 39ali commented Mar 25, 2026

Copy link
Copy Markdown
Contributor

Connections
#8593

Description
add Metal support for MULTISAMPLE_ARRAY

Testing
no tests where added

Squash or Rebase?

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy --tests. If applicable, add:
    • --target wasm32-unknown-unknown
  • Run cargo xtask test to run tests.
  • If this contains user-facing changes, add a CHANGELOG.md entry.

@inner-daemons inner-daemons left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

2 nits, generally looks good

Comment thread wgpu-hal/src/metal/adapter.rs Outdated
Comment thread wgpu-hal/src/metal/adapter.rs Outdated

@inner-daemons inner-daemons left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This would also need a changelog entry just FYI.

But I actually have bigger concerns, which is that I have absolutely zero clue where you got the requirements from. Ctrl+f looking for "multisample` only pops up 3 items in the feature set tables, and the only non-footnote item is "Layered rendering to multisample textures", which seems related but also the limits there don't match what you did at all.

The fact that you referenced "Mac Family 1" also does not inspire confidence because that has been scrubbed for a while now from the tables.

If you can share a link to the archive.org page you were referencing that would be ideal (assuming you were actually looking at such a page).

Comment thread wgpu-hal/src/metal/adapter.rs Outdated
// Multisample 2D Array textures are supported on:
// - Apple Family 3 and higher (A10 chips/iPhone 7 and newer)
// - Mac Family 1 and higher (All Intel/AMD/Apple Silicon Macs)
if available!(macos = 10.14, ios = 14.0, tvos = 16.0, visionos = 1.0) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This should also require family_check or supportsFamily will crash.

Comment thread wgpu-hal/src/metal/adapter.rs Outdated
Comment on lines +1088 to +1090
// Multisample 2D Array textures are supported on:
// - Apple Family 3 and higher (A10 chips/iPhone 7 and newer)
// - Mac Family 1 and higher (All Intel/AMD/Apple Silicon Macs)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not seeing anything in the metal feature set tables about multisample 2D array textures. What is the exact feature called?

Also, the metal feature set tables have scrubbed all references of "Mac Family 1" a while ago, so are you looking at some older version of this on the internet archive?

@inner-daemons

Copy link
Copy Markdown
Collaborator

Just to be more transparent, my big concern here is that you vibecoded this PR and never bothered to check the references your LLM provided. I hope that's not true though and you can provide the document used.

@39ali

39ali commented Mar 27, 2026

Copy link
Copy Markdown
Contributor Author

@inner-daemons i actually hit the same issue when i was looking at the minimum req for this since there's no specific place that connects multisample to gpu family , so i had to do a bit of connecting by myself, so i looked at https://developer.apple.com/metal/Metal-Shading-Language-Specification.pdf : Metal 2 and later support 2D multisampled texture array
then i looked at https://gist.github.com/schwa/3893c225133fd93ae869492ff8fa1610 to connect the metal version to the gpu family, i might be wrong and i'm happy to fix whatever is the issue if

@39ali

39ali commented Mar 27, 2026

Copy link
Copy Markdown
Contributor Author

@inner-daemons i looked at MoltenVK since they do also have to target it, it looks like they just support it across the board if it's not TVOS ,https://github.com/KhronosGroup/MoltenVK/blob/c0d41c221b3903519c83753e559452d9a81ee286/MoltenVK/MoltenVK/GPUObjects/MVKDevice.mm#L2463

we could do the same as well

@inner-daemons

Copy link
Copy Markdown
Collaborator

@39ali What was the reasoning for this comment?

                // According to Metal Feature Set Tables:

I don't think that the feature set tables are relevant here?

And yes i will check on the sources you linked.

@39ali

39ali commented Mar 28, 2026

Copy link
Copy Markdown
Contributor Author

@inner-daemons the gist references it, but i do agree it's a bit confusing, i'll fix it according to what we decide to do

@inner-daemons

inner-daemons commented Mar 30, 2026

Copy link
Copy Markdown
Collaborator

Ok so I think that for feature support we only need to check for the OS versions in https://developer.apple.com/documentation/metal/mtltexturetype/type2dmultisamplearray.

As for the MSL side, it looks like the specifics are mainly for reading multisampled textures, which is not an operation I'm familiar with? If that requires new MSL features then our MSL writer needs to be updated and those changes should require a certain language version. Otherwise should be left alone.

@inner-daemons

Copy link
Copy Markdown
Collaborator

Filed #9342

@39ali
39ali force-pushed the metal-multisample-array branch from 89d7ca4 to 5bd4491 Compare April 1, 2026 10:51
@39ali

39ali commented Apr 1, 2026

Copy link
Copy Markdown
Contributor Author

@inner-daemons okay so i modified the check for MTLTextureType.type2DMultisampleArray support
and we can handle the support for msl in a pr for #9342

@39ali
39ali force-pushed the metal-multisample-array branch 2 times, most recently from ea3f11c to 5360ac0 Compare April 2, 2026 11:00

@inner-daemons inner-daemons left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, take a look @cwfitzgerald

@inner-daemons

Copy link
Copy Markdown
Collaborator

@39ali Do you have a discord or matrix account? I'd like to ask you about something, my handle on discord is inner_daemons and on matrix it's @supamaggie70:matrix.org.

@39ali

39ali commented Apr 6, 2026

Copy link
Copy Markdown
Contributor Author

@inner-daemons discord sent

@39ali
39ali force-pushed the metal-multisample-array branch from 5360ac0 to 18b93e6 Compare April 8, 2026 08:50
@39ali

39ali commented Apr 8, 2026

Copy link
Copy Markdown
Contributor Author

@ErichDonGubler ready

@cwfitzgerald cwfitzgerald left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One nit, then g2g


features.set(F::EXPERIMENTAL_RAY_QUERY, self.supports_raytracing);

features.set(F::MULTISAMPLE_ARRAY, self.supports_multisample_array);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The docs of this feature need to be expanded to show metal support as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@cwfitzgerald should be good to go

@cwfitzgerald
cwfitzgerald merged commit bde7091 into gfx-rs:trunk Apr 9, 2026
58 checks passed
slyedoc pushed a commit to slyedoc/wgpu that referenced this pull request May 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants