[Impeller] Add support for VK_EXT_blend_operation_advanced. #47131
Conversation
chinmaygarde
changed the title
impeller/compiler/compiler.cc
Outdated
| @@ -159,6 +159,12 @@ static CompilerBackend CreateGLSLCompiler(const spirv_cross::ParsedIR& ir, | |||
| source_options.type == SourceType::kFragmentShader) { | |||
| gl_compiler->remap_ext_framebuffer_fetch(0, 0, true); | |||
| } | |||
| if (source_options.type == SourceType::kFragmentShader) { | |||
There was a problem hiding this comment.
We discussed on Discord how this is causing issues because we target ES100. We'll need to target a modern ES variant as well. Instead, we are going to focus on Vulkan.
impeller/core/formats.h
Outdated
| @@ -472,14 +479,15 @@ struct ColorAttachmentDescriptor { | |||
| src_alpha_blend_factor == o.src_alpha_blend_factor && // | |||
| alpha_blend_op == o.alpha_blend_op && // | |||
| dst_alpha_blend_factor == o.dst_alpha_blend_factor && // | |||
| write_mask == o.write_mask; | |||
| write_mask == o.write_mask && // | |||
| advanced_blend_override == o.advanced_blend_override; | |||
There was a problem hiding this comment.
Does optional equality also do a deep compare?
| @@ -106,6 +106,11 @@ CapabilitiesGLES::CapabilitiesGLES(const ProcTableGLES& gl) { | |||
| gl.GetDescription()->HasExtension(kOESTextureBorderClampExt)) { | |||
| supports_decal_sampler_address_mode_ = true; | |||
| } | |||
|
|
|||
| supports_native_advanced_blend_ = | |||
There was a problem hiding this comment.
Back this out to focus on VK.
| @@ -4,9 +4,12 @@ | |||
|
|
|||
| #include "impeller/renderer/backend/gles/render_pass_gles.h" | |||
|
|
|||
| #include "GLES3/gl3.h" | |||
There was a problem hiding this comment.
Not that it matters since its going to be backed out. Include the single umbrella header for all gl. I believe it is //impeller/toolkit/gles/gles.h.
| @@ -34,20 +37,93 @@ void RenderPassGLES::OnSetLabel(std::string label) { | |||
| label_ = std::move(label); | |||
| } | |||
|
|
|||
| #define MULTIPLY_KHR 0x9294 | |||
There was a problem hiding this comment.
IMO, if we redefine macros, we should have our own prefix.
There was a problem hiding this comment.
So, IMPELLER_MULTIPLY_KHR etc..
| case BlendMode::kMultiply: | ||
| return vk::BlendOp::eMultiplyEXT; | ||
| case BlendMode::kHue: | ||
| return vk::BlendOp::eHslHueEXT; // Is this right? |
There was a problem hiding this comment.
Re, "Is this right?". hue, sat, lumninosity are HSL components. So I guess so. In fact, I like the Vulkan name better as its clearer.
|
We need to do something with VK_ACCESS_COLOR_ATTACHMENT_READ_NONCOHERENT_BIT_EXT and memory barriers ... something! |
|
Actually to land this one I need to set up subpass self-dependency in a similar manner to the framebuffer ext version, so i'm going to land that one first instead of working on both simultaneously |
Advanced blends without framebuffer fetch.