Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@jonahwilliams
Copy link
Contributor

@jonahwilliams jonahwilliams commented Oct 19, 2023

Advanced blends without framebuffer fetch.

jonahwilliams added 2 commits October 20, 2023 08:22
@chinmaygarde chinmaygarde changed the title [Impeller] add support for VK_EXT_blend_operation_advanced [Impeller] Add support for VK_EXT_blend_operation_advanced. Oct 21, 2023
source_options.type == SourceType::kFragmentShader) {
gl_compiler->remap_ext_framebuffer_fetch(0, 0, true);
}
if (source_options.type == SourceType::kFragmentShader) {
Copy link
Member

Choose a reason for hiding this comment

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

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.

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;
Copy link
Member

Choose a reason for hiding this comment

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

Does optional equality also do a deep compare?

supports_decal_sampler_address_mode_ = true;
}

supports_native_advanced_blend_ =
Copy link
Member

Choose a reason for hiding this comment

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

Back this out to focus on VK.


#include "impeller/renderer/backend/gles/render_pass_gles.h"

#include "GLES3/gl3.h"
Copy link
Member

Choose a reason for hiding this comment

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

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.

label_ = std::move(label);
}

#define MULTIPLY_KHR 0x9294
Copy link
Member

Choose a reason for hiding this comment

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

IMO, if we redefine macros, we should have our own prefix.

Copy link
Member

Choose a reason for hiding this comment

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

So, IMPELLER_MULTIPLY_KHR etc..

case BlendMode::kMultiply:
return vk::BlendOp::eMultiplyEXT;
case BlendMode::kHue:
return vk::BlendOp::eHslHueEXT; // Is this right?
Copy link
Member

Choose a reason for hiding this comment

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

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.

@jonahwilliams
Copy link
Contributor Author

We need to do something with VK_ACCESS_COLOR_ATTACHMENT_READ_NONCOHERENT_BIT_EXT and memory barriers ... something!

@jonahwilliams
Copy link
Contributor Author

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

@jonahwilliams
Copy link
Contributor Author

@jonahwilliams jonahwilliams added the Work in progress (WIP) Not ready (yet) for review! label Dec 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

e: impeller Work in progress (WIP) Not ready (yet) for review!

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants