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

Expose alpha_mode, support non-opaque mode on metal and vk backends #2836

Merged
merged 6 commits into from Sep 19, 2022

Conversation

jinleili
Copy link
Contributor

@jinleili jinleili commented Jul 2, 2022

Checklist

  • Run cargo clippy.
  • Run RUSTFLAGS=--cfg=web_sys_unstable_apis cargo clippy --target wasm32-unknown-unknown if applicable.
  • Add change to CHANGELOG.md. See simple instructions inside file.

Connections
#687

Description

To get a transparent background Canvas, there are different cases on the user side:
  • If use winit, have to set window_builder.with_transparent(true) ;
  • If use custom SurfaceView on Android, the only way to set SurfaceView background color to transparent is:
val sv = WGPUSurfaceView(ctx)
sv.setZOrderOnTop(true)  
sv.holder.setFormat(PixelFormat.TRANSPARENT)
  • If call create_surface_from_core_animation_layer on metal to create surface, have to ensure that the background color of the layer is nil or clearColor
Known issues:

Testing
Tested on macOS(metal, vulkan-portability, angle), iOS, Nexus 5x(Android 8.1, vk and gl backends).

macOS android

jinleili added a commit to jinleili/wgpu-in-app that referenced this pull request Jul 2, 2022
jinleili added a commit to jinleili/wgpu-in-app that referenced this pull request Jul 2, 2022
@geertbleyen
Copy link
Contributor

geertbleyen commented Aug 10, 2022

@cwfitzgerald, is something preventing this PR from being completed (besides the merge conflicts)? This particular change has been under discussion for quite a while now in #687
We've been doing private patches for this for a while now, and having the alpha mode exposed means we can stop using a fork.

@cwfitzgerald
Copy link
Member

No, just my highly split attention. I'll take a look soon. Thank you for reviewing!

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.

Code generally looks good - one comment. We should also add a function to query what modes are actually available and enforce that the user only uses an available mode (similar to how we do present modes now).

wgpu-hal/src/dx12/adapter.rs Outdated Show resolved Hide resolved
@Hoverbear
Copy link

I had the opportunity to test this patch atop v0.13.1 in Bevy as part of exploring the cause of bevyengine/bevy#5779. It seemed functional and working in x86_64 Linux on a Vulkan backend with a AMD RADV NAVI10 adapter running inside Wayland (Gnome).

@geertbleyen
Copy link
Contributor

@jinleili , do you intend to continue work on this PR?

@jinleili
Copy link
Contributor Author

@jinleili , do you intend to continue work on this PR?

Yes, I still want to get it done.

@jinleili
Copy link
Contributor Author

Code generally looks good - one comment. We should also add a function to query what modes are actually available and enforce that the user only uses an available mode (similar to how we do present modes now).

Is it acceptable to simply match the WebGPU Spec and provide only normal Opaque and PreMultiplied options?

Copy link
Member

@jimblandy jimblandy left a comment

Choose a reason for hiding this comment

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

It looks to me like @jinleili has implemented all of @cwfitzgerald's requested changes.

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.

Still need a way to query which transparency modes are supported and validation that the requested mode is supported.

@jinleili jinleili changed the title Expose alpha_mode, support non-opaque mode on metal vk and gl backends Expose alpha_mode, support non-opaque mode on metal vk backends Sep 3, 2022
jinleili added a commit to jinleili/wgpu-in-app that referenced this pull request Sep 3, 2022
@jinleili
Copy link
Contributor Author

jinleili commented Sep 3, 2022

@cwfitzgerald Added surface_get_supported_alpha_modes fn, extract the same code from surface_get_supported_xxx into fetch_adapter_and_surface fn, also renamed get_supported_modes to get_supported_present_modes.

@jinleili jinleili changed the title Expose alpha_mode, support non-opaque mode on metal vk backends Expose alpha_mode, support non-opaque mode on metal and vk backends Sep 3, 2022
@jinleili
Copy link
Contributor Author

jinleili commented Sep 4, 2022

Since alpha_mode is now strictly validated, Opaque is no longer guaranteed to be supported, because the vk backend on Android devices tends to only support Inherit.

So, use get_supported_alpha_modes to set the alpha_mode to keep examples portability.

@geertbleyen
Copy link
Contributor

@cwfitzgerald , these changes are now ok to merge?

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.

Great stuff - sorry for the delay on merging!

@cwfitzgerald cwfitzgerald merged commit 44399bb into gfx-rs:master Sep 19, 2022
@codeart1st
Copy link

Great work @jinleili
Looking forward this change in the next release👍

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.

None yet

6 participants