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

Metal backend ASTC HDR formats support #2477

Merged
merged 1 commit into from
Feb 15, 2022
Merged

Metal backend ASTC HDR formats support #2477

merged 1 commit into from
Feb 15, 2022

Conversation

jinleili
Copy link
Contributor

@jinleili jinleili commented Feb 13, 2022

Connections
#2371

Description

  • Add all metal.framework defined HDR ASTC formats;
  • Add a new TEXTURE_COMPRESSION_ASTC_HDR feature;
  • Properly change metal surface configure fn;

Testing

Tested on MacBook Pro(M1 Max), iPhone 12 Pro(A14), using wgpu-on-app.

Verified Intel cpu Mac and iOS simulator can not support HDR ASTC formats.

.astc test file generated by ARM astc encoder

Loading the .astc file to create textures does not require any external crates:

let astc_data = &include_bytes!("../../assets/6x6.astc")[..];
let size = wgpu::Extent3d {width: 4096,height: 2048,depth_or_array_layers: 1};
let texture = device.create_texture_with_data(queue, &wgpu::TextureDescriptor {
		size, mip_level_count: 1, sample_count: 1,
		dimension: wgpu::TextureDimension::D2,
		format: TextureFormat::Astc { block: AstcBlock::B6x6,  channel: AstcChannel::Hdr },
		usage: wgpu::TextureUsages::TEXTURE_BINDING | wgpu::TextureUsages::COPY_DST,
		label: None,
	},
	// need offset header size, witch is equal to one block size
	&astc_data[16..],
);
IMG_1027.MOV

Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

LGTM besides one line which I'm unsure about

wgpu-hal/src/metal/surface.rs Outdated Show resolved Hide resolved
Copy link
Member

@kvark kvark left a comment

Choose a reason for hiding this comment

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

Amazing work here! I have a few suggestions

wgpu-types/src/lib.rs Outdated Show resolved Hide resolved
wgpu-hal/src/metal/surface.rs Show resolved Hide resolved
wgpu-hal/src/metal/surface.rs Outdated Show resolved Hide resolved
jinleili added a commit to jinleili/wgpu-in-app that referenced this pull request Feb 15, 2022
Copy link
Member

@kvark kvark left a comment

Choose a reason for hiding this comment

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

Very nice, thank you!

@kvark kvark enabled auto-merge (squash) February 15, 2022 14:43
@kvark kvark merged commit 1c17d15 into gfx-rs:master Feb 15, 2022
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Apr 6, 2022
…r=jgilbert

New versions of several crates are introduced to third_party/rust, by
changing the versions requested in `gfx/wgpu_bindings/Cargo.toml` and
running `mach vendor rust`:

- `wgpu-core`, `wgpu-hal`, and `wgpu-types`, as used by `wgpu_bindings`
- `naga`, `ash`, and `metal`, as used by the above

These are all exact copies of the upstream sources, at the git
revisions listed in `.cargo/config.in`.

This brings in fixes for some upstream `wgpu` bugs that were fuzzblockers:

- Compute pipelines never freed at runtime, leaking memory #2564
  gfx-rs/wgpu#2564

- Device::drop doesn't actually free the device when using backend::direct::Context #2563
  gfx-rs/wgpu#2563

The Firefox sources also needed some adjustments to catch up with
upstream changes:

- The C type `mozilla::webgpu::ffi::WGPUTextureFormat` is now a struct
  containing a tag enum and a union, not just an enum. This is needed
  for [gfx-rs/wgpu#2477](gfx-rs/wgpu#2477).

  (Note that Firefox's `WebGPU.webidl` is behind the current spec,
  so even though the newest ASTC texture formats are supported in `wgpu`,
  they're not available in Firefox yet.)

- `wgpu` got a new feature, `id32`, which cbindgen needed to be told
  about so that it would generate preprocessor-protected code like
  this:

      #if defined(WGPU_FEATURE_ID32)
      typedef uint32_t WGPUNonZeroId;
      #endif

      #if !defined(WGPU_FEATURE_ID32)
      typedef uint64_t WGPUNonZeroId;
      #endif

  instead of just spitting out two conflicting definitions of
  `WGPUNonZeroId`.

- The `wgpu_core::hub::IdentityHandlerFactory` trait's `spawn` method
  no longer takes a `min_index` argument. (Our implementations of that
  trait never used that argument anyway, so this was easy to
  accommodate.)

Differential Revision: https://phabricator.services.mozilla.com/D142779
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Apr 6, 2022
…r=jgilbert

New versions of several crates are introduced to third_party/rust, by
changing the versions requested in `gfx/wgpu_bindings/Cargo.toml` and
running `mach vendor rust`:

- `wgpu-core`, `wgpu-hal`, and `wgpu-types`, as used by `wgpu_bindings`
- `naga`, `ash`, and `metal`, as used by the above

These are all exact copies of the upstream sources, at the git
revisions listed in `.cargo/config.in`.

This brings in fixes for some upstream `wgpu` bugs that were fuzzblockers:

- Compute pipelines never freed at runtime, leaking memory #2564
  gfx-rs/wgpu#2564

- Device::drop doesn't actually free the device when using backend::direct::Context #2563
  gfx-rs/wgpu#2563

The Firefox sources also needed some adjustments to catch up with
upstream changes:

- The C type `mozilla::webgpu::ffi::WGPUTextureFormat` is now a struct
  containing a tag enum and a union, not just an enum. This is needed
  for [gfx-rs/wgpu#2477](gfx-rs/wgpu#2477).

  (Note that Firefox's `WebGPU.webidl` is behind the current spec,
  so even though the newest ASTC texture formats are supported in `wgpu`,
  they're not available in Firefox yet.)

- `wgpu` got a new feature, `id32`, which cbindgen needed to be told
  about so that it would generate preprocessor-protected code like
  this:

      #if defined(WGPU_FEATURE_ID32)
      typedef uint32_t WGPUNonZeroId;
      #endif

      #if !defined(WGPU_FEATURE_ID32)
      typedef uint64_t WGPUNonZeroId;
      #endif

  instead of just spitting out two conflicting definitions of
  `WGPUNonZeroId`.

- The `wgpu_core::hub::IdentityHandlerFactory` trait's `spawn` method
  no longer takes a `min_index` argument. (Our implementations of that
  trait never used that argument anyway, so this was easy to
  accommodate.)

Differential Revision: https://phabricator.services.mozilla.com/D142779
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Apr 7, 2022
…r=jgilbert

New versions of several crates are introduced to third_party/rust, by
changing the versions requested in `gfx/wgpu_bindings/Cargo.toml` and
running `mach vendor rust`:

- `wgpu-core`, `wgpu-hal`, and `wgpu-types`, as used by `wgpu_bindings`
- `naga`, `ash`, and `metal`, as used by the above

These are all exact copies of the upstream sources, at the git
revisions listed in `.cargo/config.in`.

This brings in fixes for some upstream `wgpu` bugs that were fuzzblockers:

- Compute pipelines never freed at runtime, leaking memory #2564
  gfx-rs/wgpu#2564

- Device::drop doesn't actually free the device when using backend::direct::Context #2563
  gfx-rs/wgpu#2563

The Firefox sources also needed some adjustments to catch up with
upstream changes:

- The C type `mozilla::webgpu::ffi::WGPUTextureFormat` is now a struct
  containing a tag enum and a union, not just an enum. This is needed
  for [gfx-rs/wgpu#2477](gfx-rs/wgpu#2477).

  (Note that Firefox's `WebGPU.webidl` is behind the current spec,
  so even though the newest ASTC texture formats are supported in `wgpu`,
  they're not available in Firefox yet.)

- `wgpu` got a new feature, `id32`, which cbindgen needed to be told
  about so that it would generate preprocessor-protected code like
  this:

      #if defined(WGPU_FEATURE_ID32)
      typedef uint32_t WGPUNonZeroId;
      #endif

      #if !defined(WGPU_FEATURE_ID32)
      typedef uint64_t WGPUNonZeroId;
      #endif

  instead of just spitting out two conflicting definitions of
  `WGPUNonZeroId`.

- The `wgpu_core::hub::IdentityHandlerFactory` trait's `spawn` method
  no longer takes a `min_index` argument. (Our implementations of that
  trait never used that argument anyway, so this was easy to
  accommodate.)

Differential Revision: https://phabricator.services.mozilla.com/D142779
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Apr 7, 2022
…r=jgilbert

New versions of several crates are introduced to third_party/rust, by
changing the versions requested in `gfx/wgpu_bindings/Cargo.toml` and
running `mach vendor rust`:

- `wgpu-core`, `wgpu-hal`, and `wgpu-types`, as used by `wgpu_bindings`
- `naga`, `ash`, and `metal`, as used by the above

These are all exact copies of the upstream sources, at the git
revisions listed in `.cargo/config.in`.

This brings in fixes for some upstream `wgpu` bugs that were fuzzblockers:

- Compute pipelines never freed at runtime, leaking memory #2564
  gfx-rs/wgpu#2564

- Device::drop doesn't actually free the device when using backend::direct::Context #2563
  gfx-rs/wgpu#2563

The Firefox sources also needed some adjustments to catch up with
upstream changes:

- The C type `mozilla::webgpu::ffi::WGPUTextureFormat` is now a struct
  containing a tag enum and a union, not just an enum. This is needed
  for [gfx-rs/wgpu#2477](gfx-rs/wgpu#2477).

  (Note that Firefox's `WebGPU.webidl` is behind the current spec,
  so even though the newest ASTC texture formats are supported in `wgpu`,
  they're not available in Firefox yet.)

- `wgpu` got a new feature, `id32`, which cbindgen needed to be told
  about so that it would generate preprocessor-protected code like
  this:

      #if defined(WGPU_FEATURE_ID32)
      typedef uint32_t WGPUNonZeroId;
      #endif

      #if !defined(WGPU_FEATURE_ID32)
      typedef uint64_t WGPUNonZeroId;
      #endif

  instead of just spitting out two conflicting definitions of
  `WGPUNonZeroId`.

- The `wgpu_core::hub::IdentityHandlerFactory` trait's `spawn` method
  no longer takes a `min_index` argument. (Our implementations of that
  trait never used that argument anyway, so this was easy to
  accommodate.)

Differential Revision: https://phabricator.services.mozilla.com/D142779
@teoxoy teoxoy mentioned this pull request Dec 5, 2022
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