From ee9d4275262ff66e02dcf2052065161170c06963 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Wed, 5 Apr 2023 16:09:00 -0700 Subject: [PATCH 01/17] hellotriangle can now choose the backend --- samples/hellotriangle.cpp | 65 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 61 insertions(+), 4 deletions(-) diff --git a/samples/hellotriangle.cpp b/samples/hellotriangle.cpp index 8c237c68b15..a36df692201 100644 --- a/samples/hellotriangle.cpp +++ b/samples/hellotriangle.cpp @@ -31,7 +31,11 @@ #include #include +#include + #include +#include + #include "generated/resources/resources.h" @@ -40,6 +44,7 @@ using utils::Entity; using utils::EntityManager; struct App { + Config config; VertexBuffer* vb; IndexBuffer* ib; Material* mat; @@ -62,11 +67,63 @@ static const Vertex TRIANGLE_VERTICES[3] = { static constexpr uint16_t TRIANGLE_INDICES[3] = { 0, 1, 2 }; +static void printUsage(char* name) { + std::string exec_name(utils::Path(name).getName()); + std::string usage( + "HELLOTRIANGLE renders a spinning colored triangle\n" + "Usage:\n" + " SHOWCASE [options]\n" + "Options:\n" + " --help, -h\n" + " Prints this message\n\n" + " --api, -a\n" + " Specify the backend API: opengl, vulkan, or metal\n" + ); + const std::string from("HELLOTRIANGLE"); + for (size_t pos = usage.find(from); pos != std::string::npos; pos = usage.find(from, pos)) { + usage.replace(pos, from.length(), exec_name); + } + std::cout << usage; +} + +static int handleCommandLineArguments(int argc, char* argv[], App* app) { + static constexpr const char* OPTSTR = "ha:"; + static const struct option OPTIONS[] = { + { "help", no_argument, nullptr, 'h' }, + { "api", required_argument, nullptr, 'a' }, + { nullptr, 0, nullptr, 0 } + }; + int opt; + int option_index = 0; + while ((opt = getopt_long(argc, argv, OPTSTR, OPTIONS, &option_index)) >= 0) { + std::string arg(optarg ? optarg : ""); + switch (opt) { + default: + case 'h': + printUsage(argv[0]); + exit(0); + case 'a': + if (arg == "opengl") { + app->config.backend = Engine::Backend::OPENGL; + } else if (arg == "vulkan") { + app->config.backend = Engine::Backend::VULKAN; + } else if (arg == "metal") { + app->config.backend = Engine::Backend::METAL; + } else { + std::cerr << "Unrecognized backend. Must be 'opengl'|'vulkan'|'metal'.\n"; + exit(1); + } + break; + } + } + return optind; +} + int main(int argc, char** argv) { - Config config; - config.title = "hellotriangle"; + App app{}; + app.config.title = "hellotriangle"; + handleCommandLineArguments(argc, argv, &app); - App app; auto setup = [&app](Engine* engine, View* view, Scene* scene) { app.skybox = Skybox::Builder().color({0.1, 0.125, 0.25, 1.0}).build(*engine); scene->setSkybox(app.skybox); @@ -128,7 +185,7 @@ int main(int argc, char** argv) { filament::math::mat4f::rotation(now, filament::math::float3{ 0, 0, 1 })); }); - FilamentApp::get().run(config, setup, cleanup); + FilamentApp::get().run(app.config, setup, cleanup); return 0; } From 944031af14e917e02c63292f6d9d5387de9693c6 Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Thu, 6 Apr 2023 15:09:10 -0700 Subject: [PATCH 02/17] vulkan: add missing index buffer barrier (#6718) --- filament/backend/src/vulkan/VulkanBuffer.cpp | 70 ++++++++------------ 1 file changed, 28 insertions(+), 42 deletions(-) diff --git a/filament/backend/src/vulkan/VulkanBuffer.cpp b/filament/backend/src/vulkan/VulkanBuffer.cpp index 835af73b98a..02fa31c25c3 100644 --- a/filament/backend/src/vulkan/VulkanBuffer.cpp +++ b/filament/backend/src/vulkan/VulkanBuffer.cpp @@ -69,52 +69,38 @@ void VulkanBuffer::loadFromCpu(VulkanContext& context, VulkanStagePool& stagePoo // Firstly, ensure that the copy finishes before the next draw call. // Secondly, in case the user decides to upload another chunk (without ever using the first one) - // we need to ensure that this upload completes first. - + // we need to ensure that this upload completes first (hence + // dstStageMask=VK_PIPELINE_STAGE_TRANSFER_BIT). + VkAccessFlags dstAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT; + VkPipelineStageFlags dstStageMask = VK_PIPELINE_STAGE_TRANSFER_BIT; if (mUsage & VK_BUFFER_USAGE_VERTEX_BUFFER_BIT) { - - VkBufferMemoryBarrier barrier{ - .sType = VK_STRUCTURE_TYPE_BUFFER_MEMORY_BARRIER, - .srcAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT, - .dstAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT | - VK_ACCESS_VERTEX_ATTRIBUTE_READ_BIT | VK_ACCESS_INDEX_READ_BIT, - .srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, - .dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, - .buffer = mGpuBuffer, - .size = VK_WHOLE_SIZE - }; - - vkCmdPipelineBarrier(cmdbuffer, - VK_PIPELINE_STAGE_TRANSFER_BIT, - VK_PIPELINE_STAGE_TRANSFER_BIT | VK_PIPELINE_STAGE_VERTEX_INPUT_BIT, - 0, 0, nullptr, 1, &barrier, 0, nullptr); - + dstAccessMask |= VK_ACCESS_VERTEX_ATTRIBUTE_READ_BIT; + dstStageMask |= VK_PIPELINE_STAGE_VERTEX_INPUT_BIT; + } else if (mUsage & VK_BUFFER_USAGE_INDEX_BUFFER_BIT) { + dstAccessMask |= VK_ACCESS_INDEX_READ_BIT; + dstStageMask |= VK_PIPELINE_STAGE_VERTEX_INPUT_BIT; + } else if (mUsage & VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT) { + dstAccessMask |= VK_ACCESS_UNIFORM_READ_BIT; + // NOTE: ideally dstStageMask would include VERTEX_SHADER_BIT | FRAGMENT_SHADER_BIT, but + // this seems to be insufficient on Mali devices. To work around this we are using a more + // aggressive ALL_GRAPHICS_BIT barrier. + dstStageMask |= VK_PIPELINE_STAGE_ALL_GRAPHICS_BIT; + } else if (mUsage & VK_BUFFER_USAGE_STORAGE_BUFFER_BIT) { + // TODO: implement me } - if (mUsage & VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT) { - // NOTE: ideally dstStageMask would include VERTEX_SHADER_BIT | FRAGMENT_SHADER_BIT, but this - // seems to be insufficient on Mali devices. To work around this we are using a more - // aggressive ALL_GRAPHICS_BIT barrier. - - VkBufferMemoryBarrier barrier{ - .sType = VK_STRUCTURE_TYPE_BUFFER_MEMORY_BARRIER, - .srcAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT, - .dstAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT | VK_ACCESS_UNIFORM_READ_BIT, - .srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, - .dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, - .buffer = mGpuBuffer, - .size = VK_WHOLE_SIZE - }; - - vkCmdPipelineBarrier(cmdbuffer, - VK_PIPELINE_STAGE_TRANSFER_BIT, - VK_PIPELINE_STAGE_TRANSFER_BIT | VK_PIPELINE_STAGE_ALL_GRAPHICS_BIT, - 0, 0, nullptr, 1, &barrier, 0, nullptr); - } + VkBufferMemoryBarrier barrier{ + .sType = VK_STRUCTURE_TYPE_BUFFER_MEMORY_BARRIER, + .srcAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT, + .dstAccessMask = dstAccessMask, + .srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, + .dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED, + .buffer = mGpuBuffer, + .size = VK_WHOLE_SIZE, + }; - if (mUsage & VK_BUFFER_USAGE_STORAGE_BUFFER_BIT) { - // TODO: implement me - } + vkCmdPipelineBarrier(cmdbuffer, VK_PIPELINE_STAGE_TRANSFER_BIT, dstStageMask, 0, 0, nullptr, 1, + &barrier, 0, nullptr); } } // namespace filament::backend From 498a355fb2dfca2ba47f234cd84db2c8531f7810 Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Thu, 6 Apr 2023 17:32:27 -0700 Subject: [PATCH 03/17] vulkan: Fix validation errors (#6717) - Depth attachment layout has generated a lot of error due to it being read-only. But the store-ops for the attachment during the renderpass are all write ops. We set the depth attachment layout as VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL - Enable extra blitting step for SSAO because of the above layout conundrum. - Index buffers did not have a pipeline barriers after loading them. - Remove `assert_invariant(utils::popcount(sampleCount) == 1);` from `reduceSampleCount`. This assert fails when enabling the duplicate pass for SSAO. --- filament/backend/src/vulkan/VulkanBlitter.cpp | 177 ++++++++++-------- filament/backend/src/vulkan/VulkanBlitter.h | 4 - filament/backend/src/vulkan/VulkanDriver.cpp | 90 ++++----- filament/backend/src/vulkan/VulkanFboCache.h | 17 +- .../backend/src/vulkan/VulkanStagePool.cpp | 2 +- filament/backend/src/vulkan/VulkanUtility.cpp | 42 ++--- filament/backend/src/vulkan/VulkanUtility.h | 5 - 7 files changed, 157 insertions(+), 180 deletions(-) diff --git a/filament/backend/src/vulkan/VulkanBlitter.cpp b/filament/backend/src/vulkan/VulkanBlitter.cpp index a273445c642..384309c0ddf 100644 --- a/filament/backend/src/vulkan/VulkanBlitter.cpp +++ b/filament/backend/src/vulkan/VulkanBlitter.cpp @@ -32,11 +32,101 @@ using namespace utils; namespace filament::backend { +namespace { + +inline void blitFast(const VkCommandBuffer cmdbuffer, VkImageAspectFlags aspect, VkFilter filter, + const VkExtent2D srcExtent, VulkanAttachment src, VulkanAttachment dst, + const VkOffset3D srcRect[2], const VkOffset3D dstRect[2]) { + const VkImageBlit blitRegions[1] = {{.srcSubresource = {aspect, src.level, src.layer, 1}, + .srcOffsets = {srcRect[0], srcRect[1]}, + .dstSubresource = {aspect, dst.level, dst.layer, 1}, + .dstOffsets = {dstRect[0], dstRect[1]}}}; + + const VkImageResolve resolveRegions[1] = {{.srcSubresource = {aspect, src.level, src.layer, 1}, + .srcOffset = srcRect[0], + .dstSubresource = {aspect, dst.level, dst.layer, 1}, + .dstOffset = dstRect[0], + .extent = {srcExtent.width, srcExtent.height, 1}}}; + + const VkImageSubresourceRange srcRange = { + .aspectMask = aspect, + .baseMipLevel = src.level, + .levelCount = 1, + .baseArrayLayer = src.layer, + .layerCount = 1, + }; + + const VkImageSubresourceRange dstRange = { + .aspectMask = aspect, + .baseMipLevel = dst.level, + .levelCount = 1, + .baseArrayLayer = dst.layer, + .layerCount = 1, + }; + const VkImageLayout srcLayout = getDefaultImageLayout(src.texture->usage); + transitionImageLayout(cmdbuffer, { + src.getImage(), + srcLayout, + VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, + srcRange, + VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT, + 0, + VK_PIPELINE_STAGE_TRANSFER_BIT, + VK_ACCESS_TRANSFER_READ_BIT, + }); + + transitionImageLayout(cmdbuffer, { + dst.getImage(), + VK_IMAGE_LAYOUT_UNDEFINED, + VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, + dstRange, + VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT, + 0, + VK_PIPELINE_STAGE_TRANSFER_BIT, + VK_ACCESS_TRANSFER_WRITE_BIT, + }); + + if (src.texture->samples > 1 && dst.texture->samples == 1) { + assert_invariant( + aspect != VK_IMAGE_ASPECT_DEPTH_BIT && "Resolve with depth is not yet supported."); + vkCmdResolveImage(cmdbuffer, src.getImage(), VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, + dst.getImage(), VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, 1, resolveRegions); + } else { + vkCmdBlitImage(cmdbuffer, src.getImage(), VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, + dst.getImage(), VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, 1, blitRegions, filter); + } + + VkImageLayout newSrcLayout = getDefaultImageLayout(src.texture->usage); + VkImageLayout const newDestLayout = getDefaultImageLayout(dst.texture->usage); + + // In the case of blitting the depth attachment, we transition the source into GENERAL (for + // sampling) and set the copy as ATTACHMENT_OPTIMAL (to be set as the attachment). + if (any(src.texture->usage & TextureUsage::DEPTH_ATTACHMENT)) { + newSrcLayout = VK_IMAGE_LAYOUT_GENERAL; + } + + transitionImageLayout(cmdbuffer, textureTransitionHelper({ + .image = src.getImage(), + .oldLayout = VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, + .newLayout = newSrcLayout, + .subresources = srcRange, + })); + + transitionImageLayout(cmdbuffer, textureTransitionHelper({ + .image = dst.getImage(), + .oldLayout = VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, + .newLayout = newDestLayout, + .subresources = dstRange, + })); +} + struct BlitterUniforms { int sampleCount; float inverseSampleCount; }; +}// anonymous namespace + void VulkanBlitter::blitColor(BlitArgs args) { const VulkanAttachment src = args.srcTarget->getColor(args.targetIndex); const VulkanAttachment dst = args.dstTarget->getColor(0); @@ -56,8 +146,8 @@ void VulkanBlitter::blitColor(BlitArgs args) { return; } #endif - - blitFast(aspect, args.filter, args.srcTarget->getExtent(), src, dst, + VkCommandBuffer const cmdbuffer = mContext.commands->get().cmdbuffer; + blitFast(cmdbuffer, aspect, args.filter, args.srcTarget->getExtent(), src, dst, args.srcRectPair, args.dstRectPair); } @@ -88,90 +178,11 @@ void VulkanBlitter::blitDepth(BlitArgs args) { args.dstRectPair); return; } - - blitFast(aspect, args.filter, args.srcTarget->getExtent(), src, dst, args.srcRectPair, + VkCommandBuffer const cmdbuffer = mContext.commands->get().cmdbuffer; + blitFast(cmdbuffer, aspect, args.filter, args.srcTarget->getExtent(), src, dst, args.srcRectPair, args.dstRectPair); } -void VulkanBlitter::blitFast(VkImageAspectFlags aspect, VkFilter filter, - const VkExtent2D srcExtent, VulkanAttachment src, VulkanAttachment dst, - const VkOffset3D srcRect[2], const VkOffset3D dstRect[2]) { - const VkImageBlit blitRegions[1] = {{ - .srcSubresource = { aspect, src.level, src.layer, 1 }, - .srcOffsets = { srcRect[0], srcRect[1] }, - .dstSubresource = { aspect, dst.level, dst.layer, 1 }, - .dstOffsets = { dstRect[0], dstRect[1] } - }}; - - const VkImageResolve resolveRegions[1] = {{ - .srcSubresource = { aspect, src.level, src.layer, 1 }, - .srcOffset = srcRect[0], - .dstSubresource = { aspect, dst.level, dst.layer, 1 }, - .dstOffset = dstRect[0], - .extent = { srcExtent.width, srcExtent.height, 1 } - }}; - - const VkImageSubresourceRange srcRange = { - .aspectMask = aspect, - .baseMipLevel = src.level, - .levelCount = 1, - .baseArrayLayer = src.layer, - .layerCount = 1, - }; - - const VkImageSubresourceRange dstRange = { - .aspectMask = aspect, - .baseMipLevel = dst.level, - .levelCount = 1, - .baseArrayLayer = dst.layer, - .layerCount = 1, - }; - - const VkCommandBuffer cmdbuffer = mContext.commands->get().cmdbuffer; - - const VkImageLayout srcLayout = getDefaultImageLayout(src.texture->usage); - - transitionImageLayout(cmdbuffer, { - src.getImage(), - srcLayout, - VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, - srcRange, - VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT, 0, - VK_PIPELINE_STAGE_TRANSFER_BIT, VK_ACCESS_TRANSFER_READ_BIT - }); - - transitionImageLayout(cmdbuffer, { - dst.getImage(), - VK_IMAGE_LAYOUT_UNDEFINED, - VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, - dstRange, - VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT, 0, - VK_PIPELINE_STAGE_TRANSFER_BIT, VK_ACCESS_TRANSFER_WRITE_BIT, - }); - - if (src.texture->samples > 1 && dst.texture->samples == 1) { - assert_invariant(aspect != VK_IMAGE_ASPECT_DEPTH_BIT && "Resolve with depth is not yet supported."); - vkCmdResolveImage(cmdbuffer, src.getImage(), VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, dst.getImage(), - VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, 1, resolveRegions); - } else { - vkCmdBlitImage(cmdbuffer, src.getImage(), VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, dst.getImage(), - VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, 1, blitRegions, filter); - } - - transitionImageLayout(cmdbuffer, blitterTransitionHelper({ - .image = src.getImage(), - .oldLayout = VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, - .newLayout = srcLayout, - .subresources = srcRange - })); - - transitionImageLayout(cmdbuffer, blitterTransitionHelper({ - .image = dst.getImage(), - .oldLayout = VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, - .newLayout = getDefaultImageLayout(dst.texture->usage), - .subresources = dstRange, - })); -} void VulkanBlitter::shutdown() noexcept { if (mContext.device) { diff --git a/filament/backend/src/vulkan/VulkanBlitter.h b/filament/backend/src/vulkan/VulkanBlitter.h index f95b53bb36e..75e0439d6fb 100644 --- a/filament/backend/src/vulkan/VulkanBlitter.h +++ b/filament/backend/src/vulkan/VulkanBlitter.h @@ -53,10 +53,6 @@ class VulkanBlitter { private: void lazyInit() noexcept; - void blitFast(VkImageAspectFlags aspect, VkFilter filter, const VkExtent2D srcExtent, - VulkanAttachment src, VulkanAttachment dst, const VkOffset3D srcRect[2], - const VkOffset3D dstRect[2]); - void blitSlowDepth(VkImageAspectFlags aspect, VkFilter filter, const VkExtent2D srcExtent, VulkanAttachment src, VulkanAttachment dst, const VkOffset3D srcRect[2], const VkOffset3D dstRect[2]); diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 76af3124e5e..756695eb595 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -652,7 +652,26 @@ bool VulkanDriver::isWorkaroundNeeded(Workaround workaround) { // early exit condition is flattened in EASU code return deviceProperties.vendorID == 0x5143; // Qualcomm case Workaround::ALLOW_READ_ONLY_ANCILLARY_FEEDBACK_LOOP: - return true; + // Supporting depth attachment as both sampler and attachment is only possible if we set + // the depth attachment as read-only (e.g. during SSAO pass), however note that the + // store-ops for attachments wrt VkRenderPass only has VK_ATTACHMENT_STORE_OP_DONT_CARE + // and VK_ATTACHMENT_STORE_OP_STORE for versions below 1.3. Only at 1.3 and above do we + // have a true read-only choice VK_ATTACHMENT_STORE_OP_NONE. That means for < 1.3, we + // will trigger a validation sync error if we use the depth attachment also as a + // sampler. See full error below: + // + // SYNC-HAZARD-WRITE-AFTER-READ(ERROR / SPEC): msgNum: 929810911 - Validation Error: + // [ SYNC-HAZARD-WRITE-AFTER-READ ] Object 0: handle = 0x6160000c3680, + // type = VK_OBJECT_TYPE_RENDER_PASS; | MessageID = 0x376bc9df | vkCmdEndRenderPass: + // Hazard WRITE_AFTER_READ in subpass 0 for attachment 1 depth aspect during store with + // storeOp VK_ATTACHMENT_STORE_OP_STORE. Access info (usage: + // SYNC_LATE_FRAGMENT_TESTS_DEPTH_STENCIL_ATTACHMENT_WRITE, prior_usage: + // SYNC_FRAGMENT_SHADER_SHADER_STORAGE_READ, read_barriers: VK_PIPELINE_STAGE_2_NONE, + // command: vkCmdDrawIndexed, seq_no: 177, reset_no: 1) + // + // Therefore we apply the existing workaround of an extra blit until a better + // resolution. + return false; case Workaround::ADRENO_UNIFORM_ARRAY_CRASH: return false; } @@ -871,39 +890,27 @@ void VulkanDriver::beginRenderPass(Handle rth, const RenderPassP VulkanAttachment depth = rt->getSamples() == 1 ? rt->getDepth() : rt->getMsaaDepth(); VulkanDepthLayout initialDepthLayout = fromVkImageLayout(depth.getLayout()); - VulkanDepthLayout renderPassDepthLayout = - fromVkImageLayout(getDefaultImageLayout(TextureUsage::DEPTH_ATTACHMENT)); - VulkanDepthLayout finalDepthLayout = renderPassDepthLayout; - - // Sometimes we need to permit the shader to sample the depth attachment by transitioning the - // layout of all its subresources to a read-only layout. This is especially crucial for SSAO. - // - // We cannot perform this transition using the render pass because the shaders in this render - // pass might sample from multiple miplevels. - // - // We do not use GENERAL here due to the following validation message: - // - // The Vulkan spec states: Image subresources used as attachments in the current render pass - // must not be accessed in any way other than as an attachment by this command, except for - // cases involving read-only access to depth/stencil attachments as described in the Render - // Pass chapter. - // - // https://vulkan.lunarg.com/doc/view/1.2.182.0/mac/1.2-extensions/vkspec.html#VUID-vkCmdDrawIndexed-None-04584) - // - if (params.readOnlyDepthStencil & RenderPassParams::READONLY_DEPTH) { - VkImageSubresourceRange range = { - .aspectMask = VK_IMAGE_ASPECT_DEPTH_BIT, - .baseMipLevel = 0, - .levelCount = depth.texture->levels, - .baseArrayLayer = 0, - .layerCount = depth.texture->depth, - }; - depth.texture->transitionLayout(cmdbuffer, range, VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL); - initialDepthLayout = renderPassDepthLayout = finalDepthLayout = VulkanDepthLayout::READ_ONLY; - } + VulkanDepthLayout renderPassDepthLayout = VulkanDepthLayout::ATTACHMENT; + VulkanDepthLayout finalDepthLayout = VulkanDepthLayout::ATTACHMENT; + TargetBufferFlags clearVal = params.flags.clear; + TargetBufferFlags discardEndVal = params.flags.discardEnd; if (depth.texture) { - depth.texture->trackLayout(depth.level, depth.layer, toVkImageLayout(renderPassDepthLayout)); + if (params.readOnlyDepthStencil & RenderPassParams::READONLY_DEPTH) { + discardEndVal &= ~TargetBufferFlags::DEPTH; + clearVal &= ~TargetBufferFlags::DEPTH; + } + if (initialDepthLayout != VulkanDepthLayout::ATTACHMENT) { + VkImageSubresourceRange subresources{ + .aspectMask = VK_IMAGE_ASPECT_DEPTH_BIT, + .baseMipLevel = 0, + .levelCount = depth.texture->levels, + .baseArrayLayer = 0, + .layerCount = depth.texture->depth, + }; + depth.texture->transitionLayout(cmdbuffer, subresources, + toVkImageLayout(renderPassDepthLayout)); + } } // Create the VkRenderPass or fetch it from cache. @@ -913,9 +920,9 @@ void VulkanDriver::beginRenderPass(Handle rth, const RenderPassP .renderPassDepthLayout = renderPassDepthLayout, .finalDepthLayout = finalDepthLayout, .depthFormat = depth.getFormat(), - .clear = params.flags.clear, + .clear = clearVal, .discardStart = discardStart, - .discardEnd = params.flags.discardEnd, + .discardEnd = discardEndVal, .samples = rt->getSamples(), .subpassMask = uint8_t(params.subpassMask), }; @@ -1070,21 +1077,6 @@ void VulkanDriver::endRenderPass(int) { VulkanRenderTarget* rt = mContext.currentRenderPass.renderTarget; assert_invariant(rt); - // In some cases, depth needs to be transitioned from DEPTH_STENCIL_READ_ONLY_OPTIMAL back to - // GENERAL. We did not do this using the render pass because we need to change multiple mips. - if (mContext.currentRenderPass.params.readOnlyDepthStencil & RenderPassParams::READONLY_DEPTH) { - const VulkanAttachment& depth = rt->getDepth(); - VkImageSubresourceRange range = { - .aspectMask = VK_IMAGE_ASPECT_DEPTH_BIT, - .baseMipLevel = 0, - .levelCount = depth.texture->levels, - .baseArrayLayer = 0, - .layerCount = depth.texture->depth, - }; - depth.texture->transitionLayout(cmdbuffer, range, - getDefaultImageLayout(TextureUsage::DEPTH_ATTACHMENT)); - } - // Since we might soon be sampling from the render target that we just wrote to, we need a // pipeline barrier between framebuffer writes and shader reads. This is a memory barrier rather // than an image barrier. If we were to use image barriers here, we would potentially need to diff --git a/filament/backend/src/vulkan/VulkanFboCache.h b/filament/backend/src/vulkan/VulkanFboCache.h index b8904bddb11..40034e50009 100644 --- a/filament/backend/src/vulkan/VulkanFboCache.h +++ b/filament/backend/src/vulkan/VulkanFboCache.h @@ -30,8 +30,7 @@ namespace filament::backend { // Avoid using VkImageLayout since it requires 4 bytes. enum class VulkanDepthLayout : uint8_t { UNDEFINED, // VK_IMAGE_LAYOUT_UNDEFINED - GENERAL, // VK_IMAGE_LAYOUT_GENERAL - READ_ONLY, // VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL. + ATTACHMENT, // VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL }; // Simple manager for VkFramebuffer and VkRenderPass objects. @@ -134,17 +133,19 @@ class VulkanFboCache { inline VulkanDepthLayout fromVkImageLayout(VkImageLayout layout) { switch (layout) { - case VK_IMAGE_LAYOUT_GENERAL: return VulkanDepthLayout::GENERAL; - case VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL: return VulkanDepthLayout::READ_ONLY; - default: return VulkanDepthLayout::UNDEFINED; + case VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL: + return VulkanDepthLayout::ATTACHMENT; + default: + return VulkanDepthLayout::UNDEFINED; } } inline VkImageLayout toVkImageLayout(VulkanDepthLayout layout) { switch (layout) { - case VulkanDepthLayout::GENERAL: return VK_IMAGE_LAYOUT_GENERAL; - case VulkanDepthLayout::READ_ONLY: return VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL; - default: return VK_IMAGE_LAYOUT_UNDEFINED; + case VulkanDepthLayout::ATTACHMENT: + return VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL; + default: + return VK_IMAGE_LAYOUT_UNDEFINED; } } diff --git a/filament/backend/src/vulkan/VulkanStagePool.cpp b/filament/backend/src/vulkan/VulkanStagePool.cpp index cb4f8b641d8..5ef35b1c890 100644 --- a/filament/backend/src/vulkan/VulkanStagePool.cpp +++ b/filament/backend/src/vulkan/VulkanStagePool.cpp @@ -116,7 +116,7 @@ VulkanStageImage const* VulkanStagePool::acquireImage(PixelDataFormat format, Pi // VK_IMAGE_LAYOUT_PREINITIALIZED or VK_IMAGE_LAYOUT_GENERAL layout. Calling // vkGetImageSubresourceLayout for a linear image returns a subresource layout mapping that is // valid for either of those image layouts." - transitionImageLayout(cmdbuffer, blitterTransitionHelper({ + transitionImageLayout(cmdbuffer, textureTransitionHelper({ .image = image->image, .oldLayout = VK_IMAGE_LAYOUT_UNDEFINED, .newLayout = VK_IMAGE_LAYOUT_GENERAL, diff --git a/filament/backend/src/vulkan/VulkanUtility.cpp b/filament/backend/src/vulkan/VulkanUtility.cpp index 02ba0f0e6c0..1fdc678e09f 100644 --- a/filament/backend/src/vulkan/VulkanUtility.cpp +++ b/filament/backend/src/vulkan/VulkanUtility.cpp @@ -632,11 +632,8 @@ VkImageViewType getImageViewType(SamplerType target) { // exceptions for depth and for transient use of specialized layouts, which is why VulkanTexture // tracks actual layout at the subresource level. VkImageLayout getDefaultImageLayout(TextureUsage usage) { - // Filament sometimes samples from depth while it is bound to the current render target, (e.g. - // SSAO does this while depth writes are disabled) so let's keep it simple and use GENERAL for - // all depth textures. if (any(usage & TextureUsage::DEPTH_ATTACHMENT)) { - return VK_IMAGE_LAYOUT_GENERAL; + return VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL; } // Filament sometimes samples from one miplevel while writing to another level in the same @@ -676,29 +673,8 @@ void transitionImageLayout(VkCommandBuffer cmdbuffer, VulkanLayoutTransition tra nullptr, 1, &barrier); } -VulkanLayoutTransition blitterTransitionHelper(VulkanLayoutTransition transition) { - switch (transition.newLayout) { - case VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL: - case VK_IMAGE_LAYOUT_GENERAL: - transition.srcAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT; - transition.dstAccessMask = VK_ACCESS_SHADER_READ_BIT; - transition.srcStage = VK_PIPELINE_STAGE_TRANSFER_BIT; - transition.dstStage = VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT; - break; - - case VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL: - case VK_IMAGE_LAYOUT_PRESENT_SRC_KHR: - default: - transition.srcAccessMask = VK_ACCESS_TRANSFER_READ_BIT; - transition.dstAccessMask = 0; - transition.srcStage = VK_PIPELINE_STAGE_TRANSFER_BIT; - transition.dstStage = VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT; - break; - } - return transition; -} - VulkanLayoutTransition textureTransitionHelper(VulkanLayoutTransition transition) { + const bool isTransferSrc = transition.oldLayout == VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL; switch (transition.newLayout) { case VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL: transition.srcAccessMask = 0; @@ -714,13 +690,20 @@ VulkanLayoutTransition textureTransitionHelper(VulkanLayoutTransition transition break; case VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL: case VK_IMAGE_LAYOUT_GENERAL: - case VK_IMAGE_LAYOUT_DEPTH_STENCIL_READ_ONLY_OPTIMAL: - transition.srcAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT; + transition.srcAccessMask + = isTransferSrc ? VK_ACCESS_TRANSFER_READ_BIT : VK_ACCESS_TRANSFER_WRITE_BIT; transition.dstAccessMask = VK_ACCESS_SHADER_READ_BIT; transition.srcStage = VK_PIPELINE_STAGE_TRANSFER_BIT; transition.dstStage = VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT; break; - + case VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL: + transition.srcAccessMask + = isTransferSrc ? VK_ACCESS_TRANSFER_READ_BIT : VK_ACCESS_TRANSFER_WRITE_BIT; + transition.dstAccessMask = VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT + | VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT; + transition.srcStage = VK_PIPELINE_STAGE_TRANSFER_BIT; + transition.dstStage = VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT; + break; // We support PRESENT as a target layout to allow blitting from the swap chain. // See also SwapChain::makePresentable(). case VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL: @@ -765,7 +748,6 @@ bool isDepthFormat(VkFormat format) { static uint32_t mostSignificantBit(uint32_t x) { return 1ul << (31ul - utils::clz(x)); } uint8_t reduceSampleCount(uint8_t sampleCount, VkSampleCountFlags mask) { - assert_invariant(utils::popcount(sampleCount) == 1); if (sampleCount & mask) { return sampleCount; } diff --git a/filament/backend/src/vulkan/VulkanUtility.h b/filament/backend/src/vulkan/VulkanUtility.h index 9caa9d256b4..44f9a454c8e 100644 --- a/filament/backend/src/vulkan/VulkanUtility.h +++ b/filament/backend/src/vulkan/VulkanUtility.h @@ -56,11 +56,6 @@ VkShaderStageFlags getShaderStageFlags(ShaderStageFlags stageFlags); void transitionImageLayout(VkCommandBuffer cmdbuffer, VulkanLayoutTransition transition); // Helper function for populating barrier fields based on the desired image layout. -// This logic is specific to blitting. -VulkanLayoutTransition blitterTransitionHelper(VulkanLayoutTransition transition); - -// Helper function for populating barrier fields based on the desired image layout. -// This logic is specific to texturing. VulkanLayoutTransition textureTransitionHelper(VulkanLayoutTransition transition); bool equivalent(const VkRect2D& a, const VkRect2D& b); From 2d1d8a6eec14b3e2115d96d931a5c2b8707c6ceb Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Thu, 6 Apr 2023 17:07:08 -0700 Subject: [PATCH 04/17] add a few asserts --- filament/src/details/ColorGrading.cpp | 21 +++++++-------------- filament/src/details/Texture.cpp | 6 ++++++ 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/filament/src/details/ColorGrading.cpp b/filament/src/details/ColorGrading.cpp index cac9c6962ac..6bd40a41417 100644 --- a/filament/src/details/ColorGrading.cpp +++ b/filament/src/details/ColorGrading.cpp @@ -35,6 +35,7 @@ #include #include +#include namespace filament { @@ -571,21 +572,15 @@ static float3 luminanceScaling(float3 x, // Quality //------------------------------------------------------------------------------ -static void selectLutTextureParams(ColorGrading::LutFormat lutFormat, - TextureFormat& internalFormat, PixelDataFormat& format, PixelDataType& type) noexcept { +static std::tuple + selectLutTextureParams(ColorGrading::LutFormat lutFormat) noexcept { // We use RGBA16F for high quality modes instead of RGB16F because RGB16F // is not supported everywhere switch (lutFormat) { case ColorGrading::LutFormat::INTEGER: - internalFormat = TextureFormat::RGB10_A2; - format = PixelDataFormat::RGBA; - type = PixelDataType::UINT_2_10_10_10_REV; - break; + return { TextureFormat::RGB10_A2, PixelDataFormat::RGBA, PixelDataType::UINT_2_10_10_10_REV }; case ColorGrading::LutFormat::FLOAT: - internalFormat = TextureFormat::RGBA16F; - format = PixelDataFormat::RGBA; - type = PixelDataType::HALF; - break; + return { TextureFormat::RGBA16F, PixelDataFormat::RGBA, PixelDataType::HALF }; } } @@ -670,10 +665,8 @@ FColorGrading::FColorGrading(FEngine& engine, const Builder& builder) { size_t elementSize = sizeof(half4); void* data = malloc(lutElementCount * elementSize); - TextureFormat textureFormat; - PixelDataFormat format; - PixelDataType type; - selectLutTextureParams(builder->format, textureFormat, format, type); + auto [textureFormat, format, type] = selectLutTextureParams(builder->format); + assert_invariant(FTexture::isTextureFormatSupported(engine, textureFormat)); assert_invariant(FTexture::validatePixelFormatAndType(textureFormat, format, type)); void* converted = nullptr; diff --git a/filament/src/details/Texture.cpp b/filament/src/details/Texture.cpp index b68c54d53ca..8add70ae226 100644 --- a/filament/src/details/Texture.cpp +++ b/filament/src/details/Texture.cpp @@ -222,6 +222,9 @@ void FTexture::setImage(FEngine& engine, size_t level, } }; + // this should have been validated already + assert_invariant(isTextureFormatSupported(engine, mFormat)); + ASSERT_PRECONDITION(buffer.type == PixelDataType::COMPRESSED || validatePixelFormatAndType(mFormat, buffer.format, buffer.type), "The combination of internal format=%u and {format=%u, type=%u} is not supported.", @@ -295,6 +298,9 @@ void FTexture::setImage(FEngine& engine, size_t level, } }; + // this should have been validated already + assert_invariant(isTextureFormatSupported(engine, mFormat)); + ASSERT_PRECONDITION(buffer.type == PixelDataType::COMPRESSED || validatePixelFormatAndType(mFormat, buffer.format, buffer.type), "The combination of internal format=%u and {format=%u, type=%u} is not supported.", From 8d3b395e867b9e8f504b86f8e7a13e5bd4581cf1 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Thu, 6 Apr 2023 11:33:23 -0700 Subject: [PATCH 05/17] ProgramBuilder is a relic, rename to Program --- filament/backend/src/opengl/OpenGLProgram.cpp | 12 ++++++------ filament/backend/src/opengl/OpenGLProgram.h | 2 +- filament/src/details/Material.cpp | 6 +++--- filament/src/details/Material.h | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/filament/backend/src/opengl/OpenGLProgram.cpp b/filament/backend/src/opengl/OpenGLProgram.cpp index e0ec9dad0f9..f93ffdeeab3 100644 --- a/filament/backend/src/opengl/OpenGLProgram.cpp +++ b/filament/backend/src/opengl/OpenGLProgram.cpp @@ -44,21 +44,21 @@ OpenGLProgram::OpenGLProgram() noexcept : mInitialized(false), mValid(true), mLazyInitializationData(nullptr) { } -OpenGLProgram::OpenGLProgram(OpenGLDriver& gld, Program&& programBuilder) noexcept - : HwProgram(std::move(programBuilder.getName())), +OpenGLProgram::OpenGLProgram(OpenGLDriver& gld, Program&& program) noexcept + : HwProgram(std::move(program.getName())), mInitialized(false), mValid(true), mLazyInitializationData{ new(LazyInitializationData) } { OpenGLContext& context = gld.getContext(); - mLazyInitializationData->uniformBlockInfo = std::move(programBuilder.getUniformBlockBindings()); - mLazyInitializationData->samplerGroupInfo = std::move(programBuilder.getSamplerGroupInfo()); + mLazyInitializationData->uniformBlockInfo = std::move(program.getUniformBlockBindings()); + mLazyInitializationData->samplerGroupInfo = std::move(program.getSamplerGroupInfo()); // this cannot fail because we check compilation status after linking the program // shaders[] is filled with id of shader stages present. OpenGLProgram::compileShaders(context, - std::move(programBuilder.getShadersSource()), - programBuilder.getSpecializationConstants(), + std::move(program.getShadersSource()), + program.getSpecializationConstants(), gl.shaders, mLazyInitializationData->shaderSourceCode); diff --git a/filament/backend/src/opengl/OpenGLProgram.h b/filament/backend/src/opengl/OpenGLProgram.h index 90095590270..71b37ca136a 100644 --- a/filament/backend/src/opengl/OpenGLProgram.h +++ b/filament/backend/src/opengl/OpenGLProgram.h @@ -38,7 +38,7 @@ class OpenGLProgram : public HwProgram { public: OpenGLProgram() noexcept; - OpenGLProgram(OpenGLDriver& gld, Program&& builder) noexcept; + OpenGLProgram(OpenGLDriver& gld, Program&& program) noexcept; ~OpenGLProgram() noexcept; bool isValid() const noexcept { return mValid; } diff --git a/filament/src/details/Material.cpp b/filament/src/details/Material.cpp index cde6c900e00..76cb4980ff0 100644 --- a/filament/src/details/Material.cpp +++ b/filament/src/details/Material.cpp @@ -383,16 +383,16 @@ void FMaterial::getSurfaceProgramSlow(Variant variant) const noexcept { Variant const vertexVariant = Variant::filterVariantVertex(variant); Variant const fragmentVariant = Variant::filterVariantFragment(variant); - Program pb{ getProgramBuilderWithVariants(variant, vertexVariant, fragmentVariant) }; + Program pb{ getProgramWithVariants(variant, vertexVariant, fragmentVariant) }; createAndCacheProgram(std::move(pb), variant); } void FMaterial::getPostProcessProgramSlow(Variant variant) const noexcept { - Program pb{ getProgramBuilderWithVariants(variant, variant, variant) }; + Program pb{ getProgramWithVariants(variant, variant, variant) }; createAndCacheProgram(std::move(pb), variant); } -Program FMaterial::getProgramBuilderWithVariants( +Program FMaterial::getProgramWithVariants( Variant variant, Variant vertexVariant, Variant fragmentVariant) const noexcept { diff --git a/filament/src/details/Material.h b/filament/src/details/Material.h index 4740e85b698..e19c6ce81a9 100644 --- a/filament/src/details/Material.h +++ b/filament/src/details/Material.h @@ -99,7 +99,7 @@ class FMaterial : public Material { return mCachedPrograms[variant.key]; } - backend::Program getProgramBuilderWithVariants(Variant variant, Variant vertexVariant, + backend::Program getProgramWithVariants(Variant variant, Variant vertexVariant, Variant fragmentVariant) const noexcept; bool isVariantLit() const noexcept { return mIsVariantLit; } From e19011d0e0b546e8b41a13278f5e1abf490eb7c9 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Thu, 6 Apr 2023 13:58:50 -0700 Subject: [PATCH 06/17] use the enum instead of ints everywhere for ShaderModel and ShaderStage --- filament/src/MaterialParser.cpp | 2 +- filament/src/details/Material.cpp | 4 +- filament/src/details/Material.h | 12 ++-- .../filaflat/include/filaflat/MaterialChunk.h | 17 ++++-- libs/filaflat/src/MaterialChunk.cpp | 55 +++++++++++-------- libs/filamat/src/MaterialBuilder.cpp | 32 +++++------ libs/filamat/src/eiff/MaterialSpirvChunk.cpp | 6 +- libs/filamat/src/eiff/MaterialTextChunk.cpp | 6 +- libs/filamat/src/eiff/ShaderEntry.h | 19 ++++--- libs/matdbg/src/ShaderExtractor.cpp | 3 +- libs/matdbg/src/ShaderReplacer.cpp | 41 +++++++------- 11 files changed, 103 insertions(+), 94 deletions(-) diff --git a/filament/src/MaterialParser.cpp b/filament/src/MaterialParser.cpp index 96e5ef73aca..d2601ad993c 100644 --- a/filament/src/MaterialParser.cpp +++ b/filament/src/MaterialParser.cpp @@ -311,7 +311,7 @@ bool MaterialParser::getReflectionMode(ReflectionMode* value) const noexcept { bool MaterialParser::getShader(ShaderContent& shader, ShaderModel shaderModel, Variant variant, ShaderStage stage) noexcept { return mImpl.mMaterialChunk.getShader(shader, - mImpl.mBlobDictionary, uint8_t(shaderModel), variant, uint8_t(stage)); + mImpl.mBlobDictionary, shaderModel, variant, stage); } // ------------------------------------------------------------------------------------------------ diff --git a/filament/src/details/Material.cpp b/filament/src/details/Material.cpp index 76cb4980ff0..79297bf016d 100644 --- a/filament/src/details/Material.cpp +++ b/filament/src/details/Material.cpp @@ -509,6 +509,8 @@ size_t FMaterial::getParameters(ParameterInfo* parameters, size_t count) const n return count; } +#if FILAMENT_ENABLE_MATDBG + // Swaps in an edited version of the original package that was used to create the material. The // edited package was stashed in response to a debugger event. This is invoked only when the // Material Debugger is attached. The only editable features of a material package are the shader @@ -532,8 +534,6 @@ void FMaterial::applyPendingEdits() noexcept { * @{ */ -#if FILAMENT_ENABLE_MATDBG - void FMaterial::onEditCallback(void* userdata, const utils::CString&, const void* packageData, size_t packageSize) { FMaterial* material = downcast((Material*) userdata); diff --git a/filament/src/details/Material.h b/filament/src/details/Material.h index e19c6ce81a9..06ebb123e8c 100644 --- a/filament/src/details/Material.h +++ b/filament/src/details/Material.h @@ -99,9 +99,6 @@ class FMaterial : public Material { return mCachedPrograms[variant.key]; } - backend::Program getProgramWithVariants(Variant variant, Variant vertexVariant, - Variant fragmentVariant) const noexcept; - bool isVariantLit() const noexcept { return mIsVariantLit; } const utils::CString& getName() const noexcept { return mName; } @@ -149,11 +146,11 @@ class FMaterial : public Material { uint32_t generateMaterialInstanceId() const noexcept { return mMaterialInstanceId++; } - void applyPendingEdits() noexcept; - void destroyPrograms(FEngine& engine); #if FILAMENT_ENABLE_MATDBG + void applyPendingEdits() noexcept; + /** * Callback handlers for the debug server, potentially called from any thread. The userdata * argument has the same value that was passed to DebugServer::addMaterial(), which should @@ -187,6 +184,9 @@ class FMaterial : public Material { void prepareProgramSlow(Variant variant) const noexcept; void getSurfaceProgramSlow(Variant variant) const noexcept; void getPostProcessProgramSlow(Variant variant) const noexcept; + backend::Program getProgramWithVariants(Variant variant, Variant vertexVariant, + Variant fragmentVariant) const noexcept; + void createAndCacheProgram(backend::Program&& p, Variant variant) const noexcept; @@ -235,6 +235,7 @@ class FMaterial : public Material { matdbg::MaterialKey mDebuggerId; mutable utils::Mutex mActiveProgramsLock; mutable VariantList mActivePrograms; + std::atomic mPendingEdits = {}; #endif utils::CString mName; @@ -242,7 +243,6 @@ class FMaterial : public Material { const uint32_t mMaterialId; mutable uint32_t mMaterialInstanceId = 0; MaterialParser* mMaterialParser = nullptr; - std::atomic mPendingEdits = {}; }; diff --git a/libs/filaflat/include/filaflat/MaterialChunk.h b/libs/filaflat/include/filaflat/MaterialChunk.h index 5de67d2b2e0..a8ded384b8f 100644 --- a/libs/filaflat/include/filaflat/MaterialChunk.h +++ b/libs/filaflat/include/filaflat/MaterialChunk.h @@ -26,12 +26,17 @@ #include +#include + #include +#include namespace filaflat { class MaterialChunk { public: + using ShaderModel = filament::backend::ShaderModel; + using ShaderStage = filament::backend::ShaderStage; using Variant = filament::Variant; explicit MaterialChunk(ChunkContainer const& container); @@ -43,14 +48,14 @@ class MaterialChunk { // call this as many times as needed // populates "shaderContent" with the requested shader, or returns false on failure. bool getShader(ShaderContent& shaderContent, BlobDictionary const& dictionary, - uint8_t shaderModel, Variant variant, uint8_t stage); + ShaderModel shaderModel, filament::Variant variant, ShaderStage stage); - void visitTextShaders( - utils::Invocable&& visitor) const; + void visitShaders(utils::Invocable&& visitor) const; // These methods are for debugging purposes only (matdbg) // @{ - static void decodeKey(uint32_t key, uint8_t* model, Variant::type_t* variant, uint8_t* stage); + static void decodeKey(uint32_t key, + ShaderModel* outModel, Variant* outVariant, ShaderStage* outStage); const tsl::robin_map& getOffsets() const { return mOffsets; } // @} @@ -63,11 +68,11 @@ class MaterialChunk { bool getTextShader(Unflattener unflattener, BlobDictionary const& dictionary, ShaderContent& shaderContent, - uint8_t shaderModel, Variant variant, uint8_t stage); + ShaderModel shaderModel, filament::Variant variant, ShaderStage shaderStage); bool getSpirvShader( BlobDictionary const& dictionary, ShaderContent& shaderContent, - uint8_t shaderModel, Variant variant, uint8_t stage); + ShaderModel shaderModel, filament::Variant variant, ShaderStage shaderStage); }; } // namespace filamat diff --git a/libs/filaflat/src/MaterialChunk.cpp b/libs/filaflat/src/MaterialChunk.cpp index 8ef7dbf716d..84ec8e533ac 100644 --- a/libs/filaflat/src/MaterialChunk.cpp +++ b/libs/filaflat/src/MaterialChunk.cpp @@ -17,20 +17,27 @@ #include #include +#include + #include namespace filaflat { -static inline uint32_t makeKey(uint8_t shaderModel, filament::Variant variant, uint8_t stage) noexcept { +static inline uint32_t makeKey( + MaterialChunk::ShaderModel shaderModel, + MaterialChunk::Variant variant, + MaterialChunk::ShaderStage stage) noexcept { static_assert(sizeof(variant.key) * 8 <= 8); - return (shaderModel << 16) | (stage << 8) | variant.key; + return (uint32_t(shaderModel) << 16) | (uint32_t(stage) << 8) | variant.key; } -void MaterialChunk::decodeKey(uint32_t key, uint8_t* model, filament::Variant::type_t* variant, - uint8_t* stage) { - *variant = key & 0xff; - *stage = (key >> 8) & 0xff; - *model = (key >> 16) & 0xff; +void MaterialChunk::decodeKey(uint32_t key, + MaterialChunk::ShaderModel* model, + MaterialChunk::Variant* variant, + MaterialChunk::ShaderStage* stage) { + variant->key = key & 0xff; + *model = MaterialChunk::ShaderModel((key >> 16) & 0xff); + *stage = MaterialChunk::ShaderStage((key >> 8) & 0xff); } MaterialChunk::MaterialChunk(ChunkContainer const& container) @@ -61,12 +68,12 @@ bool MaterialChunk::initialize(filamat::ChunkType materialTag) { // Read all index entries. for (uint64_t i = 0 ; i < numShaders; i++) { - uint8_t shaderModelValue; - filament::Variant variant; - uint8_t pipelineStageValue; + uint8_t model; + Variant variant; + uint8_t stage; uint32_t offsetValue; - if (!unflattener.read(&shaderModelValue)) { + if (!unflattener.read(&model)) { return false; } @@ -74,7 +81,7 @@ bool MaterialChunk::initialize(filamat::ChunkType materialTag) { return false; } - if (!unflattener.read(&pipelineStageValue)) { + if (!unflattener.read(&stage)) { return false; } @@ -82,20 +89,21 @@ bool MaterialChunk::initialize(filamat::ChunkType materialTag) { return false; } - uint32_t key = makeKey(shaderModelValue, variant, pipelineStageValue); + uint32_t key = makeKey(ShaderModel(model), variant, ShaderStage(stage)); mOffsets[key] = offsetValue; } return true; } -bool MaterialChunk::getTextShader(Unflattener unflattener, BlobDictionary const& dictionary, - ShaderContent& shaderContent, uint8_t shaderModel, filament::Variant variant, uint8_t ps) { +bool MaterialChunk::getTextShader(Unflattener unflattener, + BlobDictionary const& dictionary, ShaderContent& shaderContent, + ShaderModel shaderModel, Variant variant, ShaderStage shaderStage) { if (mBase == nullptr) { return false; } // Jump and read - uint32_t key = makeKey(shaderModel, variant, ps); + uint32_t key = makeKey(shaderModel, variant, shaderStage); auto pos = mOffsets.find(key); if (pos == mOffsets.end()) { return false; @@ -146,13 +154,13 @@ bool MaterialChunk::getTextShader(Unflattener unflattener, BlobDictionary const& } bool MaterialChunk::getSpirvShader(BlobDictionary const& dictionary, - ShaderContent& shaderContent, uint8_t shaderModel, filament::Variant variant, uint8_t stage) { + ShaderContent& shaderContent, ShaderModel shaderModel, filament::Variant variant, ShaderStage shaderStage) { if (mBase == nullptr) { return false; } - uint32_t key = makeKey(shaderModel, variant, stage); + uint32_t key = makeKey(shaderModel, variant, shaderStage); auto pos = mOffsets.find(key); if (pos == mOffsets.end()) { return false; @@ -162,8 +170,8 @@ bool MaterialChunk::getSpirvShader(BlobDictionary const& dictionary, return true; } -bool MaterialChunk::getShader(ShaderContent& shaderContent, - BlobDictionary const& dictionary, uint8_t shaderModel, filament::Variant variant, uint8_t stage) { +bool MaterialChunk::getShader(ShaderContent& shaderContent, BlobDictionary const& dictionary, + ShaderModel shaderModel, filament::Variant variant, ShaderStage stage) { switch (mMaterialTag) { case filamat::ChunkType::MaterialGlsl: case filamat::ChunkType::MaterialMetal: @@ -175,9 +183,8 @@ bool MaterialChunk::getShader(ShaderContent& shaderContent, } } -void MaterialChunk::visitTextShaders( - utils::Invocable&& visitor) const { - assert_invariant(mMaterialTag != filamat::ChunkType::MaterialSpirv); +void MaterialChunk::visitShaders( + utils::Invocable&& visitor) const { Unflattener unflattener{ mUnflattener }; // make a copy @@ -200,7 +207,7 @@ void MaterialChunk::visitTextShaders( unflattener.read(&pipelineStageValue); unflattener.read(&offsetValue); - visitor(shaderModelValue, variant.key, pipelineStageValue); + visitor(ShaderModel(shaderModelValue), variant, ShaderStage(pipelineStageValue)); } } diff --git a/libs/filamat/src/MaterialBuilder.cpp b/libs/filamat/src/MaterialBuilder.cpp index 1639f585b60..35567278a4f 100644 --- a/libs/filamat/src/MaterialBuilder.cpp +++ b/libs/filamat/src/MaterialBuilder.cpp @@ -742,17 +742,17 @@ bool MaterialBuilder::generateShaders(JobSystem& jobSystem, const std::vector* pSpirv = targetApiNeedsSpirv ? &spirv : nullptr; std::string* pMsl = targetApiNeedsMsl ? &msl : nullptr; - TextEntry glslEntry{0}; - SpirvEntry spirvEntry{0}; - TextEntry metalEntry{0}; + TextEntry glslEntry{}; + SpirvEntry spirvEntry{}; + TextEntry metalEntry{}; - glslEntry.shaderModel = static_cast(params.shaderModel); - spirvEntry.shaderModel = static_cast(params.shaderModel); - metalEntry.shaderModel = static_cast(params.shaderModel); + glslEntry.shaderModel = params.shaderModel; + spirvEntry.shaderModel = params.shaderModel; + metalEntry.shaderModel = params.shaderModel; - glslEntry.variantKey = v.variant.key; - spirvEntry.variantKey = v.variant.key; - metalEntry.variantKey = v.variant.key; + glslEntry.variant = v.variant; + spirvEntry.variant = v.variant; + metalEntry.variant = v.variant; // Generate raw shader code. // The quotes in Google-style line directives cause problems with certain drivers. These @@ -828,14 +828,14 @@ bool MaterialBuilder::generateShaders(JobSystem& jobSystem, const std::vector 0); - metalEntry.stage = uint8_t(v.stage); + metalEntry.stage = v.stage; metalEntry.shader = msl; metalEntries.push_back(metalEntry); #endif @@ -872,10 +872,10 @@ bool MaterialBuilder::generateShaders(JobSystem& jobSystem, const std::vector&& entries) void MaterialSpirvChunk::flatten(Flattener &f) { f.writeUint64(mEntries.size()); for (const SpirvEntry& entry : mEntries) { - f.writeUint8(entry.shaderModel); - f.writeUint8(entry.variantKey); - f.writeUint8(entry.stage); + f.writeUint8(uint8_t(entry.shaderModel)); + f.writeUint8(entry.variant.key); + f.writeUint8(uint8_t(entry.stage)); f.writeUint32(entry.dictionaryIndex); } } diff --git a/libs/filamat/src/eiff/MaterialTextChunk.cpp b/libs/filamat/src/eiff/MaterialTextChunk.cpp index ee708bb7dc3..3b0e45dff62 100644 --- a/libs/filamat/src/eiff/MaterialTextChunk.cpp +++ b/libs/filamat/src/eiff/MaterialTextChunk.cpp @@ -20,9 +20,9 @@ namespace filamat { void MaterialTextChunk::writeEntryAttributes(size_t entryIndex, Flattener& f) const noexcept { const TextEntry& entry = mEntries[entryIndex]; - f.writeUint8(entry.shaderModel); - f.writeUint8(entry.variantKey); - f.writeUint8(entry.stage); + f.writeUint8(uint8_t(entry.shaderModel)); + f.writeUint8(entry.variant.key); + f.writeUint8(uint8_t(entry.stage)); } void compressShader(std::string_view src, Flattener &f, const LineDictionary& dictionary) { diff --git a/libs/filamat/src/eiff/ShaderEntry.h b/libs/filamat/src/eiff/ShaderEntry.h index f5b453b040a..8c0cdb1d1d3 100644 --- a/libs/filamat/src/eiff/ShaderEntry.h +++ b/libs/filamat/src/eiff/ShaderEntry.h @@ -17,26 +17,27 @@ #ifndef TNT_FILAMAT_SHADER_ENTRY_H #define TNT_FILAMAT_SHADER_ENTRY_H +#include + +#include + #include #include -#include - namespace filamat { // TextEntry stores a shader in ASCII text format, like GLSL. struct TextEntry { - uint8_t shaderModel; - filament::Variant::type_t variantKey; - uint8_t stage; + filament::backend::ShaderModel shaderModel; + filament::Variant variant; + filament::backend::ShaderStage stage; std::string shader; }; - struct SpirvEntry { - uint8_t shaderModel; - filament::Variant::type_t variantKey; - uint8_t stage; + filament::backend::ShaderModel shaderModel; + filament::Variant variant; + filament::backend::ShaderStage stage; size_t dictionaryIndex; #ifndef FILAMAT_LITE diff --git a/libs/matdbg/src/ShaderExtractor.cpp b/libs/matdbg/src/ShaderExtractor.cpp index 5de2f5ab97d..3f10b60a014 100644 --- a/libs/matdbg/src/ShaderExtractor.cpp +++ b/libs/matdbg/src/ShaderExtractor.cpp @@ -82,8 +82,7 @@ bool ShaderExtractor::getShader(ShaderModel shaderModel, return false; } - return mMaterialChunk.getShader(shader, blobDictionary, - uint8_t(shaderModel), variant, uint8_t(stage)); + return mMaterialChunk.getShader(shader, blobDictionary, shaderModel, variant, stage); } CString ShaderExtractor::spirvToGLSL(ShaderModel shaderModel, const uint32_t* data, diff --git a/libs/matdbg/src/ShaderReplacer.cpp b/libs/matdbg/src/ShaderReplacer.cpp index 8e63d95b60d..8eb3497eb4c 100644 --- a/libs/matdbg/src/ShaderReplacer.cpp +++ b/libs/matdbg/src/ShaderReplacer.cpp @@ -62,7 +62,7 @@ class ShaderIndex { void writeChunks(ostream& stream); // Replaces the specified shader text with new content. - void replaceShader(backend::ShaderModel shaderModel, Variant variant, + void replaceShader(backend::ShaderModel model, Variant variant, ShaderStage stage, const char* source, size_t sourceLength); bool isEmpty() const { return mShaderRecords.size() == 0; } @@ -299,6 +299,9 @@ size_t ShaderReplacer::getEditedSize() const { ShaderIndex::ShaderIndex(ChunkType dictTag, ChunkType matTag, const filaflat::ChunkContainer& cc) : mDictTag(dictTag), mMatTag(matTag) { + + assert_invariant(matTag != filamat::ChunkType::MaterialSpirv); + filaflat::BlobDictionary stringBlobs; DictionaryReader reader; reader.unflatten(cc, dictTag, stringBlobs); @@ -306,19 +309,17 @@ ShaderIndex::ShaderIndex(ChunkType dictTag, ChunkType matTag, const filaflat::Ch filaflat::MaterialChunk matChunk(cc); matChunk.initialize(matTag); - matChunk.visitTextShaders( - [this, &matChunk, &stringBlobs](uint8_t shaderModel, Variant::type_t variant, - uint8_t stage) { + matChunk.visitShaders([this, &matChunk, &stringBlobs] + (ShaderModel shaderModel, Variant variant, ShaderStage stage) { + ShaderContent content; + UTILS_UNUSED_IN_RELEASE bool success = matChunk.getShader(content, + stringBlobs, shaderModel, variant, stage); - ShaderContent content; - UTILS_UNUSED_IN_RELEASE bool success = matChunk.getShader(content, - stringBlobs, shaderModel, Variant(variant), stage); + std::string source{ content.data(), content.data() + content.size() - 1u }; + assert_invariant(success); - std::string source{content.data(), content.data() + content.size() - 1u}; - assert_invariant(success); - - mShaderRecords.push_back({ shaderModel, variant, stage, std::move(source) }); - }); + mShaderRecords.push_back({ shaderModel, variant, stage, std::move(source) }); + }); } void ShaderIndex::writeChunks(ostream& stream) { @@ -339,12 +340,11 @@ void ShaderIndex::writeChunks(ostream& stream) { stream.write((char*)buffer.get(), bufSize); } -void ShaderIndex::replaceShader(backend::ShaderModel shaderModel, Variant variant, +void ShaderIndex::replaceShader(backend::ShaderModel model, Variant variant, backend::ShaderStage stage, const char* source, size_t sourceLength) { - const uint8_t model = uint8_t(shaderModel); for (auto& record : mShaderRecords) { - if (record.shaderModel == model && record.variantKey == variant.key && - record.stage == uint8_t(stage)) { + if (record.shaderModel == model && record.variant == variant && + record.stage == stage) { record.shader = std::string(source, sourceLength); return; } @@ -365,7 +365,7 @@ BlobIndex::BlobIndex(ChunkType dictTag, ChunkType matTag, const filaflat::ChunkC mShaderRecords.reserve(offsets.size()); for (auto [key, offset] : offsets) { SpirvEntry info; - filaflat::MaterialChunk::decodeKey(key, &info.shaderModel, &info.variantKey, &info.stage); + filaflat::MaterialChunk::decodeKey(key, &info.shaderModel, &info.variant, &info.stage); info.dictionaryIndex = offset; mShaderRecords.emplace_back(info); } @@ -409,13 +409,10 @@ void BlobIndex::writeChunks(ostream& stream) { stream.write((char*)buffer.get() + pad, bufSize - pad); } -void BlobIndex::replaceShader(ShaderModel shaderModel, Variant variant, +void BlobIndex::replaceShader(ShaderModel model, Variant variant, ShaderStage stage, const char* source, size_t sourceLength) { - const uint8_t model = (uint8_t) shaderModel; for (auto& record : mShaderRecords) { - if (record.shaderModel == model && record.variantKey == variant.key && - record.stage == uint8_t(stage)) { - + if (record.shaderModel == model && record.variant == variant && record.stage == stage) { // TODO: because a single blob entry might be used by more than one variant, matdbg // users may unwittingly edit more than 1 variant when multiple variants have the exact // same content before the edit. In practice this is rarely problematic, but we should From 040fc64583e7a5a020841909b36d26e24586c7ed Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Thu, 6 Apr 2023 15:21:54 -0700 Subject: [PATCH 07/17] Improve how we cache shared shaders Some shaders can be shared across all materials (e.g. the depth shaders). We use the filament default material as the "source" of the cache, but until now we relied on an a priori knowledge of which variants were present in the default material. With this change, we now query once the list of variants (of interest) in the default material and reuse that list for caching these variants later. This is better because the cached variants are now entirely driven by the default material (which they depend on anyways). This is also faster because we don't need to query which variant we need each time we create a material. --- filament/src/MaterialParser.h | 2 ++ filament/src/details/Material.cpp | 30 ++++++++++++------- filament/src/details/Material.h | 2 ++ .../filaflat/include/filaflat/MaterialChunk.h | 2 ++ libs/filaflat/src/MaterialChunk.cpp | 7 +++++ 5 files changed, 33 insertions(+), 10 deletions(-) diff --git a/filament/src/MaterialParser.h b/filament/src/MaterialParser.h index e066e547ba4..91b39cd9e72 100644 --- a/filament/src/MaterialParser.h +++ b/filament/src/MaterialParser.h @@ -103,6 +103,8 @@ class MaterialParser { bool getShader(filaflat::ShaderContent& shader, backend::ShaderModel shaderModel, Variant variant, backend::ShaderStage stage) noexcept; + filaflat::MaterialChunk const& getMaterialChunk() const noexcept { return mImpl.mMaterialChunk; } + private: struct MaterialParserDetails { MaterialParserDetails(backend::Backend backend, const void* data, size_t size); diff --git a/filament/src/details/Material.cpp b/filament/src/details/Material.cpp index 79297bf016d..94b7ad18989 100644 --- a/filament/src/details/Material.cpp +++ b/filament/src/details/Material.cpp @@ -281,18 +281,28 @@ FMaterial::FMaterial(FEngine& engine, const Material::Builder& builder) parser->hasCustomDepthShader(&mHasCustomDepthShader); mIsDefaultMaterial = builder->mDefaultMaterial; - // pre-cache the shared variants -- these variants are shared with the default material. - // (note: the default material is unlit, so only unlit variants can be shared) + if (UTILS_UNLIKELY(mIsDefaultMaterial)) { + filaflat::MaterialChunk const& materialChunk{ mMaterialParser->getMaterialChunk() }; + auto variants = FixedCapacityVector::with_capacity(materialChunk.getShaderCount()); + materialChunk.visitShaders([&variants]( + ShaderModel model, Variant variant, ShaderStage stage) { + if (Variant::isValidDepthVariant(variant)) { + variants.push_back(variant); + } + }); + std::sort(variants.begin(), variants.end(), + [](Variant lhs, Variant rhs) { return lhs.key < rhs.key; }); + auto pos = std::unique(variants.begin(), variants.end()); + variants.resize(std::distance(variants.begin(), pos)); + std::swap(mDepthVariants, variants); + } + if (UTILS_UNLIKELY(!mIsDefaultMaterial && !mHasCustomDepthShader)) { - FMaterial const* const pMaterial = engine.getDefaultMaterial(); + FMaterial const* const pDefaultMaterial = engine.getDefaultMaterial(); auto& cachedPrograms = mCachedPrograms; - for (Variant::type_t k = 0, n = VARIANT_COUNT; k < n; ++k) { - const Variant variant(k); - if (Variant::isValidDepthVariant(variant) && - (Variant::filterVariant(variant, false) == variant)) { - pMaterial->prepareProgram(variant); - cachedPrograms[k] = pMaterial->getProgram(variant); - } + for (Variant variant : pDefaultMaterial->mDepthVariants) { + pDefaultMaterial->prepareProgram(variant); + cachedPrograms[variant.key] = pDefaultMaterial->getProgram(variant); } } diff --git a/filament/src/details/Material.h b/filament/src/details/Material.h index 06ebb123e8c..c32a4f2bc68 100644 --- a/filament/src/details/Material.h +++ b/filament/src/details/Material.h @@ -228,6 +228,8 @@ class FMaterial : public Material { BufferInterfaceBlock mUniformInterfaceBlock; SubpassInfo mSubpassInfo; utils::FixedCapacityVector> mUniformBlockBindings; + utils::FixedCapacityVector mDepthVariants; // only populated with default material + SamplerGroupBindingInfoList mSamplerGroupBindingInfoList; SamplerBindingToNameMap mSamplerBindingToNameMap; diff --git a/libs/filaflat/include/filaflat/MaterialChunk.h b/libs/filaflat/include/filaflat/MaterialChunk.h index a8ded384b8f..aa15fa5b2f2 100644 --- a/libs/filaflat/include/filaflat/MaterialChunk.h +++ b/libs/filaflat/include/filaflat/MaterialChunk.h @@ -50,6 +50,8 @@ class MaterialChunk { bool getShader(ShaderContent& shaderContent, BlobDictionary const& dictionary, ShaderModel shaderModel, filament::Variant variant, ShaderStage stage); + uint32_t getShaderCount() const noexcept; + void visitShaders(utils::Invocable&& visitor) const; // These methods are for debugging purposes only (matdbg) diff --git a/libs/filaflat/src/MaterialChunk.cpp b/libs/filaflat/src/MaterialChunk.cpp index 84ec8e533ac..2d73970f5cb 100644 --- a/libs/filaflat/src/MaterialChunk.cpp +++ b/libs/filaflat/src/MaterialChunk.cpp @@ -183,6 +183,13 @@ bool MaterialChunk::getShader(ShaderContent& shaderContent, BlobDictionary const } } +uint32_t MaterialChunk::getShaderCount() const noexcept { + Unflattener unflattener{ mUnflattener }; // make a copy + uint64_t numShaders; + unflattener.read(&numShaders); + return uint32_t(numShaders); +} + void MaterialChunk::visitShaders( utils::Invocable&& visitor) const { From 4a116e679121818c84aee9c2e1440b51e44b7acd Mon Sep 17 00:00:00 2001 From: Romain Guy Date: Fri, 7 Apr 2023 09:34:43 -0700 Subject: [PATCH 08/17] Improve size optimizations when compiling material (#6721) * Improve size optimizations when compiling material This changes the behavior of the size optimizer in matc (-S), but only for GLSL and MSL. With this change we gain a ~65% size reduction on a lit material compiled for OpenGL. To get those gains we generate extra SPIRV debug information to preserve variable names and better utilize the line dictionary. Unfortunately this break the SPIRV optimizer so we skip it and instead rely on a simple DCE pass provided by glslang. We also enhance the whitespace removal pass of the GLSL minifier to move lone { and } to the previous line, which avoids generating an extra index in each shader variant. Each index being at least as big as the character itself, this is quite wasteful. When generating SPIRV for Vulkan, we rely on spirv-opt for size optimizations as before. --- NEW_RELEASE_NOTES.md | 2 ++ libs/filamat/src/GLSLPostProcessor.cpp | 31 +++++++++++++++++++++----- libs/filamat/src/ShaderMinifier.cpp | 10 +++++++-- libs/filamat/src/ShaderMinifier.h | 2 +- 4 files changed, 37 insertions(+), 8 deletions(-) diff --git a/NEW_RELEASE_NOTES.md b/NEW_RELEASE_NOTES.md index c3161a7700b..b746940d782 100644 --- a/NEW_RELEASE_NOTES.md +++ b/NEW_RELEASE_NOTES.md @@ -8,3 +8,5 @@ appropriate header in [RELEASE_NOTES.md](./RELEASE_NOTES.md). ## Release notes for next branch cut +- materials: improved size reduction of OpenGL/Metal shaders by ~65% when compiling materials with + size optimizations (`matc -S`) [⚠️ **Recompile Materials**] diff --git a/libs/filamat/src/GLSLPostProcessor.cpp b/libs/filamat/src/GLSLPostProcessor.cpp index e7086881967..bfb4da07c0e 100644 --- a/libs/filamat/src/GLSLPostProcessor.cpp +++ b/libs/filamat/src/GLSLPostProcessor.cpp @@ -75,7 +75,7 @@ static void collectSibs(const GLSLPostProcessor::Config& config, SibVector& sibs &config.materialInfo->sib); } -}; // namespace msl +} // namespace msl GLSLPostProcessor::GLSLPostProcessor(MaterialBuilder::Optimization optimization, uint32_t flags) : mOptimization(optimization), @@ -395,7 +395,9 @@ bool GLSLPostProcessor::process(const std::string& inputShader, Config const& co if (internalConfig.glslOutput) { *internalConfig.glslOutput = - internalConfig.minifier.removeWhitespace(*internalConfig.glslOutput); + internalConfig.minifier.removeWhitespace( + *internalConfig.glslOutput, + mOptimization == MaterialBuilder::Optimization::SIZE); // In theory this should only be enabled for SIZE, but in practice we often use PERFORMANCE. if (mOptimization != MaterialBuilder::Optimization::NONE) { @@ -470,14 +472,33 @@ void GLSLPostProcessor::fullOptimization(const TShader& tShader, GLSLPostProcessor::Config const& config, InternalConfig& internalConfig) const { SpirvBlob spirv; + bool optimizeForSize = mOptimization == MaterialBuilderBase::Optimization::SIZE; + // Compile GLSL to to SPIR-V SpvOptions options; options.generateDebugInfo = mGenerateDebugInfo; + // This step is required for what we attempt later using spirvbin_t::remap() + if (!internalConfig.spirvOutput && optimizeForSize) { + options.emitNonSemanticShaderDebugInfo = true; + } GlslangToSpv(*tShader.getIntermediate(), spirv, &options); - // Run the SPIR-V optimizer - OptimizerPtr optimizer = createOptimizer(mOptimization, config); - optimizeSpirv(optimizer, spirv); + if (internalConfig.spirvOutput) { + // Run the SPIR-V optimizer + OptimizerPtr optimizer = createOptimizer(mOptimization, config); + optimizeSpirv(optimizer, spirv); + } else { + // When we optimize for size, and we generate text-based shaders, we save much more + // by preserving variable names and running a simple DCE pass instead of using spirv-opt + if (optimizeForSize) { + std::vector whiteListStrings; + spv::spirvbin_t(0).remap( + spirv, whiteListStrings, spv::spirvbin_t::DCE_ALL | spv::spirvbin_t::OPT_ALL); + } else { + OptimizerPtr optimizer = createOptimizer(mOptimization, config); + optimizeSpirv(optimizer, spirv); + } + } if (internalConfig.spirvOutput) { *internalConfig.spirvOutput = spirv; diff --git a/libs/filamat/src/ShaderMinifier.cpp b/libs/filamat/src/ShaderMinifier.cpp index 7097bd2e2e7..d60d5dcad3e 100644 --- a/libs/filamat/src/ShaderMinifier.cpp +++ b/libs/filamat/src/ShaderMinifier.cpp @@ -137,7 +137,7 @@ namespace { * - Remove leading white spaces at the beginning of each line * - Remove empty lines */ -std::string ShaderMinifier::removeWhitespace(const std::string& s) const { +std::string ShaderMinifier::removeWhitespace(const std::string& s, bool mergeBraces) const { size_t cur = 0; std::string r; @@ -155,7 +155,13 @@ std::string ShaderMinifier::removeWhitespace(const std::string& s) const { size_t newPos = s.find_first_not_of(" \t", pos); if (newPos == std::string::npos) newPos = pos; - r.append(s, newPos, len - (newPos - pos)); + // If we have a single { or } on a line, move it to the previous line instead + size_t subLen = len - (newPos - pos); + if (mergeBraces && subLen == 1 && (s[newPos] == '{' || s[newPos] == '}')) { + r.replace(r.size() - 1, 1, 1, s[newPos]); + } else { + r.append(s, newPos, subLen); + } r += '\n'; while (s[cur] == '\n') { diff --git a/libs/filamat/src/ShaderMinifier.h b/libs/filamat/src/ShaderMinifier.h index 737b73c214a..56860414b17 100644 --- a/libs/filamat/src/ShaderMinifier.h +++ b/libs/filamat/src/ShaderMinifier.h @@ -30,7 +30,7 @@ namespace filamat { // This custom minifier is designed for generated code such as uniform structs. class ShaderMinifier { public: - std::string removeWhitespace(const std::string& source) const; + std::string removeWhitespace(const std::string& source, bool mergeBraces = false) const; std::string renameStructFields(const std::string& source); private: From a2beaf0582c4e429ae1393c42feffed26ff52ae3 Mon Sep 17 00:00:00 2001 From: "daemyung jang(danny.jang)" Date: Tue, 11 Apr 2023 01:38:26 +0900 Subject: [PATCH 09/17] Support the external image on macOS (#6689) * Support the external image on macOS Implement CocoaExternalImage. * Fix to take an onwership of the external image * Correct incorrect comments * Rename a function explicitly Make a function name to know copying RECTANGLE to TEXTURE2D. * Do lazy initialization Create CocoaExternalImage::SharedGl when it's needed. * Fix a crash when engine is terminated Destroy the external image shared gl before gl context is destroyed. * Remove an useless variable --- NEW_RELEASE_NOTES.md | 1 + filament/backend/CMakeLists.txt | 1 + .../backend/platforms/PlatformCocoaGL.h | 4 + .../src/opengl/platforms/CocoaExternalImage.h | 90 +++++++ .../opengl/platforms/CocoaExternalImage.mm | 235 ++++++++++++++++++ .../src/opengl/platforms/PlatformCocoaGL.mm | 53 ++++ 6 files changed, 384 insertions(+) create mode 100644 filament/backend/src/opengl/platforms/CocoaExternalImage.h create mode 100644 filament/backend/src/opengl/platforms/CocoaExternalImage.mm diff --git a/NEW_RELEASE_NOTES.md b/NEW_RELEASE_NOTES.md index b746940d782..4d56a0d11d0 100644 --- a/NEW_RELEASE_NOTES.md +++ b/NEW_RELEASE_NOTES.md @@ -10,3 +10,4 @@ appropriate header in [RELEASE_NOTES.md](./RELEASE_NOTES.md). - materials: improved size reduction of OpenGL/Metal shaders by ~65% when compiling materials with size optimizations (`matc -S`) [⚠️ **Recompile Materials**] +opengl: support the external image on macOS diff --git a/filament/backend/CMakeLists.txt b/filament/backend/CMakeLists.txt index 7c7457befbb..c42b95d9b7a 100644 --- a/filament/backend/CMakeLists.txt +++ b/filament/backend/CMakeLists.txt @@ -96,6 +96,7 @@ if (FILAMENT_SUPPORTS_OPENGL AND NOT FILAMENT_USE_EXTERNAL_GLES3 AND NOT FILAMEN list(APPEND SRCS src/opengl/platforms/CocoaTouchExternalImage.mm) elseif (APPLE) list(APPEND SRCS src/opengl/platforms/PlatformCocoaGL.mm) + list(APPEND SRCS src/opengl/platforms/CocoaExternalImage.mm) elseif (WEBGL) list(APPEND SRCS src/opengl/platforms/PlatformWebGL.cpp) elseif (LINUX) diff --git a/filament/backend/include/backend/platforms/PlatformCocoaGL.h b/filament/backend/include/backend/platforms/PlatformCocoaGL.h index f32d0a25bb7..97188852a3d 100644 --- a/filament/backend/include/backend/platforms/PlatformCocoaGL.h +++ b/filament/backend/include/backend/platforms/PlatformCocoaGL.h @@ -57,6 +57,10 @@ class PlatformCocoaGL : public OpenGLPlatform { void destroySwapChain(SwapChain* swapChain) noexcept override; void makeCurrent(SwapChain* drawSwapChain, SwapChain* readSwapChain) noexcept override; void commit(SwapChain* swapChain) noexcept override; + OpenGLPlatform::ExternalTexture* createExternalImageTexture() noexcept override; + void destroyExternalImage(ExternalTexture* texture) noexcept override; + void retainExternalImage(void* externalImage) noexcept override; + bool setExternalImage(void* externalImage, ExternalTexture* texture) noexcept override; private: PlatformCocoaGLImpl* pImpl = nullptr; diff --git a/filament/backend/src/opengl/platforms/CocoaExternalImage.h b/filament/backend/src/opengl/platforms/CocoaExternalImage.h new file mode 100644 index 00000000000..38c3a9d1ff2 --- /dev/null +++ b/filament/backend/src/opengl/platforms/CocoaExternalImage.h @@ -0,0 +1,90 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef FILAMENT_DRIVER_OPENGL_COCOA_EXTERNAL_IMAGE +#define FILAMENT_DRIVER_OPENGL_COCOA_EXTERNAL_IMAGE + +#include +#include + +#include "../gl_headers.h" + +namespace filament::backend { + +class CocoaExternalImage final : public OpenGLPlatform::ExternalTexture { +public: + /** + * GL objects that can be shared across multiple instances of CocoaExternalImage. + */ + class SharedGl { + public: + SharedGl() noexcept; + ~SharedGl() noexcept; + + SharedGl(const SharedGl&) = delete; + SharedGl& operator=(const SharedGl&) = delete; + + GLuint program = 0; + GLuint sampler = 0; + GLuint fragmentShader = 0; + GLuint vertexShader = 0; + }; + + CocoaExternalImage(const CVOpenGLTextureCacheRef textureCache, + const SharedGl& sharedGl) noexcept; + ~CocoaExternalImage() noexcept; + + /** + * Set this external image to the passed-in CVPixelBuffer. + * Afterwards, calling glGetTexture returns the GL texture name backed by the CVPixelBuffer. + */ + bool set(CVPixelBufferRef p) noexcept; + + GLuint getGlTexture() const noexcept; + GLuint getInternalFormat() const noexcept; + GLuint getTarget() const noexcept; + +private: + void release() noexcept; + CVOpenGLTextureRef createTextureFromImage(CVPixelBufferRef image) noexcept; + GLuint encodeCopyRectangleToTexture2D(GLuint rectangle, size_t width, size_t height) noexcept; + + class State { + public: + void save() noexcept; + void restore() noexcept; + + private: + GLint activeTexture = 0; + GLint textureBinding = { 0 }; + GLint samplerBinding = { 0 }; + GLint framebuffer = 0; + GLint viewport[4] = { 0 }; + GLint vertexAttrib = 0; + } mState; + + GLuint mFBO = 0; + const SharedGl& mSharedGl; + GLuint mRgbaTexture = 0; + + const CVOpenGLTextureCacheRef mTextureCache; + CVPixelBufferRef mImage = nullptr; + CVOpenGLTextureRef mTexture = nullptr; +}; + +} // namespace filament::backend + +#endif diff --git a/filament/backend/src/opengl/platforms/CocoaExternalImage.mm b/filament/backend/src/opengl/platforms/CocoaExternalImage.mm new file mode 100644 index 00000000000..4d8e08b9dc5 --- /dev/null +++ b/filament/backend/src/opengl/platforms/CocoaExternalImage.mm @@ -0,0 +1,235 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#define COREVIDEO_SILENCE_GL_DEPRECATION + +#include "CocoaExternalImage.h" +#include +#include "../GLUtils.h" + +namespace filament::backend { + +static const char *s_vertex = R"SHADER(#version 410 core +void main() { + float x = -1.0 + float(((gl_VertexID & 1) <<2)); + float y = -1.0 + float(((gl_VertexID & 2) <<1)); + gl_Position=vec4(x, y, 0.0, 1.0); +} +)SHADER"; + +static const char *s_fragment = R"SHADER(#version 410 core +precision mediump float; + +uniform sampler2DRect rectangle; +layout(location = 0) out vec4 fragColor; + +void main() { + fragColor = texture(rectangle, gl_FragCoord.xy); +} +)SHADER"; + +CocoaExternalImage::SharedGl::SharedGl() noexcept { + glGenSamplers(1, &sampler); + glSamplerParameteri(sampler, GL_TEXTURE_MIN_FILTER, GL_NEAREST); + glSamplerParameteri(sampler, GL_TEXTURE_MAG_FILTER, GL_NEAREST); + glSamplerParameteri(sampler, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE); + glSamplerParameteri(sampler, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE); + glSamplerParameteri(sampler, GL_TEXTURE_WRAP_R, GL_CLAMP_TO_EDGE); + + GLint status; + + vertexShader = glCreateShader(GL_VERTEX_SHADER); + glShaderSource(vertexShader, 1, &s_vertex, nullptr); + glCompileShader(vertexShader); + glGetShaderiv(vertexShader, GL_COMPILE_STATUS, &status); + assert_invariant(status == GL_TRUE); + + fragmentShader = glCreateShader(GL_FRAGMENT_SHADER); + glShaderSource(fragmentShader, 1, &s_fragment, nullptr); + glCompileShader(fragmentShader); + glGetShaderiv(fragmentShader, GL_COMPILE_STATUS, &status); + assert_invariant(status == GL_TRUE); + + program = glCreateProgram(); + glAttachShader(program, vertexShader); + glAttachShader(program, fragmentShader); + glLinkProgram(program); + glGetProgramiv(program, GL_LINK_STATUS, &status); + assert_invariant(status == GL_TRUE); + + // Save current program state. + GLint currentProgram; + glGetIntegerv(GL_CURRENT_PROGRAM, ¤tProgram); + + glUseProgram(program); + GLint samplerLoc = glGetUniformLocation(program, "rectangle"); + glUniform1i(samplerLoc, 0); + + // Restore state. + glUseProgram(currentProgram); +} + +CocoaExternalImage::SharedGl::~SharedGl() noexcept { + glDeleteSamplers(1, &sampler); + glDetachShader(program, vertexShader); + glDetachShader(program, fragmentShader); + glDeleteShader(vertexShader); + glDeleteShader(fragmentShader); + glDeleteProgram(program); +} + +CocoaExternalImage::CocoaExternalImage(const CVOpenGLTextureCacheRef textureCache, + const SharedGl &sharedGl) noexcept : mSharedGl(sharedGl), mTextureCache(textureCache) { + glGenFramebuffers(1, &mFBO); + CHECK_GL_ERROR(utils::slog.e) +} + +CocoaExternalImage::~CocoaExternalImage() noexcept { + glDeleteFramebuffers(1, &mFBO); + release(); +} + +bool CocoaExternalImage::set(CVPixelBufferRef image) noexcept { + // Release references to a previous external image, if we're holding any. + release(); + + if (!image) { + return false; + } + + OSType formatType = CVPixelBufferGetPixelFormatType(image); + ASSERT_POSTCONDITION(formatType == kCVPixelFormatType_32BGRA, + "macOS external images must be 32BGRA format."); + + // The pixel buffer must be locked whenever we do rendering with it. We'll unlock it before + // releasing. + UTILS_UNUSED_IN_RELEASE CVReturn lockStatus = CVPixelBufferLockBaseAddress(image, 0); + assert_invariant(lockStatus == kCVReturnSuccess); + + mImage = image; + mTexture = createTextureFromImage(image); + mRgbaTexture = encodeCopyRectangleToTexture2D(CVOpenGLTextureGetName(mTexture), + CVPixelBufferGetWidth(image), CVPixelBufferGetHeight(image)); + CHECK_GL_ERROR(utils::slog.e) + + return true; +} + +GLuint CocoaExternalImage::getGlTexture() const noexcept { + return mRgbaTexture; +} + +GLuint CocoaExternalImage::getInternalFormat() const noexcept { + if (mRgbaTexture) { + return GL_RGBA8; + } + return 0; +} + +GLuint CocoaExternalImage::getTarget() const noexcept { + if (mRgbaTexture) { + return GL_TEXTURE_2D; + } + return 0; +} + +void CocoaExternalImage::release() noexcept { + if (mImage) { + CVPixelBufferUnlockBaseAddress(mImage, 0); + CVPixelBufferRelease(mImage); + } + if (mTexture) { + CFRelease(mTexture); + } + if (mRgbaTexture) { + glDeleteTextures(1, &mRgbaTexture); + mRgbaTexture = 0; + } +} + +CVOpenGLTextureRef CocoaExternalImage::createTextureFromImage(CVPixelBufferRef image) noexcept { + CVOpenGLTextureRef texture = nullptr; + UTILS_UNUSED_IN_RELEASE CVReturn success = + CVOpenGLTextureCacheCreateTextureFromImage(kCFAllocatorDefault, + mTextureCache, image, nil, &texture); + assert_invariant(success == kCVReturnSuccess); + + return texture; +} + +GLuint CocoaExternalImage::encodeCopyRectangleToTexture2D(GLuint rectangle, + size_t width, size_t height) noexcept { + GLuint texture; + glGenTextures(1, &texture); + + mState.save(); + + // Create a texture to hold the result of the blit image. + glBindTexture(GL_TEXTURE_2D, texture); + glTexStorage2D(GL_TEXTURE_2D, 1, GL_RGBA8, width, height); + CHECK_GL_ERROR(utils::slog.e) + + // source textures + glBindSampler(0, mSharedGl.sampler); + glActiveTexture(GL_TEXTURE0); + glBindTexture(GL_TEXTURE_RECTANGLE, rectangle); + CHECK_GL_ERROR(utils::slog.e) + + // destination texture + glBindFramebuffer(GL_FRAMEBUFFER, mFBO); + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, texture, 0); + CHECK_GL_ERROR(utils::slog.e) + + CHECK_GL_FRAMEBUFFER_STATUS(utils::slog.e, GL_FRAMEBUFFER) + CHECK_GL_ERROR(utils::slog.e) + + // draw + glViewport(0, 0, width, height); + CHECK_GL_ERROR(utils::slog.e) + glUseProgram(mSharedGl.program); + CHECK_GL_ERROR(utils::slog.e) + glDisableVertexAttribArray(0); + glDrawArrays(GL_TRIANGLES, 0, 3); + CHECK_GL_ERROR(utils::slog.e) + + mState.restore(); + CHECK_GL_ERROR(utils::slog.e) + + return texture; +} + +void CocoaExternalImage::State::save() noexcept { + glGetIntegerv(GL_ACTIVE_TEXTURE, &activeTexture); + glGetIntegerv(GL_TEXTURE_BINDING_2D, &textureBinding); + glGetIntegerv(GL_SAMPLER_BINDING, &samplerBinding); + glGetIntegerv(GL_DRAW_FRAMEBUFFER_BINDING, &framebuffer); + glGetIntegerv(GL_VIEWPORT, viewport); + glGetVertexAttribiv(0, GL_VERTEX_ATTRIB_ARRAY_ENABLED, &vertexAttrib); +} + +void CocoaExternalImage::State::restore() noexcept { + glActiveTexture(activeTexture); + glBindTexture(GL_TEXTURE_2D, textureBinding); + glBindSampler(0, samplerBinding); + glBindFramebuffer(GL_FRAMEBUFFER, framebuffer); + glViewport(viewport[0], viewport[1], viewport[2], viewport[3]); + + if (vertexAttrib) { + glEnableVertexAttribArray(0); + } +} + +} // namespace filament::backend diff --git a/filament/backend/src/opengl/platforms/PlatformCocoaGL.mm b/filament/backend/src/opengl/platforms/PlatformCocoaGL.mm index 55d5cb7925e..34a76078aea 100644 --- a/filament/backend/src/opengl/platforms/PlatformCocoaGL.mm +++ b/filament/backend/src/opengl/platforms/PlatformCocoaGL.mm @@ -28,6 +28,7 @@ #include #include +#include "CocoaExternalImage.h" namespace filament::backend { @@ -49,6 +50,8 @@ NSOpenGLContext* mGLContext = nullptr; CocoaGLSwapChain* mCurrentSwapChain = nullptr; std::vector mHeadlessSwapChains; + CVOpenGLTextureCacheRef mTextureCache = nullptr; + std::unique_ptr mExternalImageSharedGl; void updateOpenGLContext(NSView *nsView, bool resetView, bool clearView); }; @@ -159,6 +162,12 @@ int result = bluegl::bind(); ASSERT_POSTCONDITION(!result, "Unable to load OpenGL entry points."); + + UTILS_UNUSED_IN_RELEASE CVReturn success = CVOpenGLTextureCacheCreate(kCFAllocatorDefault, nullptr, + [pImpl->mGLContext CGLContextObj], [pImpl->mGLContext.pixelFormat CGLPixelFormatObj], nullptr, + &pImpl->mTextureCache); + assert_invariant(success == kCVReturnSuccess); + return OpenGLPlatform::createDefaultDriver(this, sharedContext, driverConfig); } @@ -167,6 +176,8 @@ } void PlatformCocoaGL::terminate() noexcept { + CFRelease(pImpl->mTextureCache); + pImpl->mExternalImageSharedGl.reset(); pImpl->mGLContext = nil; bluegl::unbind(); } @@ -256,6 +267,9 @@ void PlatformCocoaGL::commit(Platform::SwapChain* swapChain) noexcept { [pImpl->mGLContext flushBuffer]; + + // This needs to be done periodically. + CVOpenGLTextureCacheFlush(pImpl->mTextureCache, 0); } bool PlatformCocoaGL::pumpEvents() noexcept { @@ -266,6 +280,45 @@ return true; } +OpenGLPlatform::ExternalTexture* PlatformCocoaGL::createExternalImageTexture() noexcept { + if (!pImpl->mExternalImageSharedGl) { + pImpl->mExternalImageSharedGl = std::make_unique(); + } + + ExternalTexture* outTexture = new CocoaExternalImage(pImpl->mTextureCache, + *pImpl->mExternalImageSharedGl); + + // the actual id/target will be set in setExternalImage. + outTexture->id = 0; + outTexture->target = GL_TEXTURE_2D; + return outTexture; +} + +void PlatformCocoaGL::destroyExternalImage(ExternalTexture* texture) noexcept { + auto* p = static_cast(texture); + delete p; +} + +void PlatformCocoaGL::retainExternalImage(void* externalImage) noexcept { + // Take ownership of the passed in buffer. It will be released the next time + // setExternalImage is called, or when the texture is destroyed. + CVPixelBufferRef pixelBuffer = (CVPixelBufferRef) externalImage; + CVPixelBufferRetain(pixelBuffer); +} + +bool PlatformCocoaGL::setExternalImage(void* externalImage, ExternalTexture* texture) noexcept { + CVPixelBufferRef cvPixelBuffer = (CVPixelBufferRef) externalImage; + CocoaExternalImage* cocoaExternalImage = static_cast(texture); + if (!cocoaExternalImage->set(cvPixelBuffer)) { + return false; + } + texture->target = cocoaExternalImage->getTarget(); + texture->id = cocoaExternalImage->getGlTexture(); + // we used to set the internalFormat, but it's not used anywhere on the gl backend side + // cocoaExternalImage->getInternalFormat(); + return true; +} + void PlatformCocoaGLImpl::updateOpenGLContext(NSView *nsView, bool resetView, bool clearView) { NSOpenGLContext* glContext = mGLContext; From ca2f3d76e670098ce76eb6199a73e0a6e3a717a7 Mon Sep 17 00:00:00 2001 From: Ben Doherty Date: Mon, 10 Apr 2023 16:45:38 -0700 Subject: [PATCH 10/17] Metal: work around iPad pipeline error (#6724) --- NEW_RELEASE_NOTES.md | 1 + filament/backend/include/backend/DriverEnums.h | 5 ++++- filament/backend/src/metal/MetalContext.h | 4 ++++ filament/backend/src/metal/MetalDriver.mm | 5 +++++ filament/backend/src/opengl/OpenGLDriver.cpp | 2 ++ filament/backend/src/vulkan/VulkanDriver.cpp | 2 ++ filament/src/details/Material.cpp | 6 +++++- libs/filamat/src/shaders/CodeGenerator.cpp | 4 ++++ shaders/src/light_indirect.fs | 15 ++++++++++++++- 9 files changed, 41 insertions(+), 3 deletions(-) diff --git a/NEW_RELEASE_NOTES.md b/NEW_RELEASE_NOTES.md index 4d56a0d11d0..e5a2fb03f55 100644 --- a/NEW_RELEASE_NOTES.md +++ b/NEW_RELEASE_NOTES.md @@ -10,4 +10,5 @@ appropriate header in [RELEASE_NOTES.md](./RELEASE_NOTES.md). - materials: improved size reduction of OpenGL/Metal shaders by ~65% when compiling materials with size optimizations (`matc -S`) [⚠️ **Recompile Materials**] +- engine: fix potential crash on Metal devices with A8X GPU (iPad Air 2) opengl: support the external image on macOS diff --git a/filament/backend/include/backend/DriverEnums.h b/filament/backend/include/backend/DriverEnums.h index 5fcc2dc223d..8bec4b5beac 100644 --- a/filament/backend/include/backend/DriverEnums.h +++ b/filament/backend/include/backend/DriverEnums.h @@ -1117,7 +1117,10 @@ enum class Workaround : uint16_t { // the whole render pass. ALLOW_READ_ONLY_ANCILLARY_FEEDBACK_LOOP, // for some uniform arrays, it's needed to do an initialization to avoid crash on adreno gpu - ADRENO_UNIFORM_ARRAY_CRASH + ADRENO_UNIFORM_ARRAY_CRASH, + // Workaround a Metal pipeline compilation error with the message: + // "Could not statically determine the target of a texture". See light_indirect.fs + A8X_STATIC_TEXTURE_TARGET_ERROR }; } // namespace filament::backend diff --git a/filament/backend/src/metal/MetalContext.h b/filament/backend/src/metal/MetalContext.h index 097659094b1..7b118f95bc1 100644 --- a/filament/backend/src/metal/MetalContext.h +++ b/filament/backend/src/metal/MetalContext.h @@ -73,6 +73,10 @@ struct MetalContext { uint8_t mac; } highestSupportedGpuFamily; + struct { + bool a8xStaticTextureTargetError; + } bugs; + // sampleCountLookup[requestedSamples] gives a <= sample count supported by the device. std::array sampleCountLookup; diff --git a/filament/backend/src/metal/MetalDriver.mm b/filament/backend/src/metal/MetalDriver.mm index 6b3c872efe1..9a9d9945dbe 100644 --- a/filament/backend/src/metal/MetalDriver.mm +++ b/filament/backend/src/metal/MetalDriver.mm @@ -117,6 +117,9 @@ sc[s] = [mContext->device supportsTextureSampleCount:s] ? s : sc[s - 1]; } + mContext->bugs.a8xStaticTextureTargetError = + [mContext->device.name containsString:@"Apple A8X GPU"]; + mContext->commandQueue = mPlatform.createCommandQueue(mContext->device); mContext->pipelineStateCache.setDevice(mContext->device); mContext->depthStencilStateCache.setDevice(mContext->device); @@ -719,6 +722,8 @@ return true; case Workaround::ADRENO_UNIFORM_ARRAY_CRASH: return false; + case Workaround::A8X_STATIC_TEXTURE_TARGET_ERROR: + return mContext->bugs.a8xStaticTextureTargetError; } return false; } diff --git a/filament/backend/src/opengl/OpenGLDriver.cpp b/filament/backend/src/opengl/OpenGLDriver.cpp index c7f5c2d832c..4be9d0e2d07 100644 --- a/filament/backend/src/opengl/OpenGLDriver.cpp +++ b/filament/backend/src/opengl/OpenGLDriver.cpp @@ -1720,6 +1720,8 @@ bool OpenGLDriver::isWorkaroundNeeded(Workaround workaround) { return mContext.bugs.allow_read_only_ancillary_feedback_loop; case Workaround::ADRENO_UNIFORM_ARRAY_CRASH: return mContext.bugs.enable_initialize_non_used_uniform_array; + default: + return false; } return false; } diff --git a/filament/backend/src/vulkan/VulkanDriver.cpp b/filament/backend/src/vulkan/VulkanDriver.cpp index 756695eb595..79433b28f5b 100644 --- a/filament/backend/src/vulkan/VulkanDriver.cpp +++ b/filament/backend/src/vulkan/VulkanDriver.cpp @@ -674,6 +674,8 @@ bool VulkanDriver::isWorkaroundNeeded(Workaround workaround) { return false; case Workaround::ADRENO_UNIFORM_ARRAY_CRASH: return false; + default: + return false; } return false; } diff --git a/filament/src/details/Material.cpp b/filament/src/details/Material.cpp index 94b7ad18989..02d9d9d208a 100644 --- a/filament/src/details/Material.cpp +++ b/filament/src/details/Material.cpp @@ -462,9 +462,13 @@ Program FMaterial::getProgramWithVariants( } } + const bool staticTextureWorkaround = + mEngine.getDriverApi().isWorkaroundNeeded(Workaround::A8X_STATIC_TEXTURE_TARGET_ERROR); + program.specializationConstants({ { 0, (int)mEngine.getSupportedFeatureLevel() }, - { 1, (int)CONFIG_MAX_INSTANCES } + { 1, (int)CONFIG_MAX_INSTANCES }, + { 2, (bool)staticTextureWorkaround } }); return program; diff --git a/libs/filamat/src/shaders/CodeGenerator.cpp b/libs/filamat/src/shaders/CodeGenerator.cpp index 49f9293c6c7..f3425cca552 100644 --- a/libs/filamat/src/shaders/CodeGenerator.cpp +++ b/libs/filamat/src/shaders/CodeGenerator.cpp @@ -135,6 +135,10 @@ utils::io::sstream& CodeGenerator::generateProlog(utils::io::sstream& out, Shade generateSpecializationConstant(out, "CONFIG_MAX_INSTANCES", 1, (int)CONFIG_MAX_INSTANCES); } + // Workaround a Metal pipeline compilation error with the message: + // "Could not statically determine the target of a texture". See light_indirect.fs + generateSpecializationConstant(out, "CONFIG_STATIC_TEXTURE_TARGET_WORKAROUND", 2, false); + out << '\n'; out << SHADERS_COMMON_DEFINES_GLSL_DATA; diff --git a/shaders/src/light_indirect.fs b/shaders/src/light_indirect.fs index 1c5a7f14191..c5fe00a7a54 100644 --- a/shaders/src/light_indirect.fs +++ b/shaders/src/light_indirect.fs @@ -71,9 +71,22 @@ vec3 Irradiance_RoughnessOne(const vec3 n) { //------------------------------------------------------------------------------ vec3 diffuseIrradiance(const vec3 n) { + // On Metal devices with an A8X chipset, this light_iblSpecular texture sample must be pulled + // outside the frameUniforms.iblSH check. This is to avoid a Metal pipeline compilation error + // with the message: "Could not statically determine the target of a texture". + // The reason for this is unknown, and is possibly a bug that exhibits only on these devices. + vec3 irradianceRoughnessOne; + if (CONFIG_STATIC_TEXTURE_TARGET_WORKAROUND) { + irradianceRoughnessOne = Irradiance_RoughnessOne(n); + } + if (frameUniforms.iblSH[0].x == 65504.0) { #if FILAMENT_QUALITY < FILAMENT_QUALITY_HIGH - return Irradiance_RoughnessOne(n); + if (CONFIG_STATIC_TEXTURE_TARGET_WORKAROUND) { + return irradianceRoughnessOne; + } else { + return Irradiance_RoughnessOne(n); + } #else ivec2 s = textureSize(light_iblSpecular, int(frameUniforms.iblRoughnessOneLevel)); float du = 1.0 / float(s.x); From 6cab8d2cd47b4e3f520cd9195db5fb4d60d98892 Mon Sep 17 00:00:00 2001 From: Ben Doherty Date: Tue, 11 Apr 2023 11:09:53 -0700 Subject: [PATCH 11/17] Expose specialization constants to materials as constant parameters (#6652) --- NEW_RELEASE_NOTES.md | 1 + docs/Materials.md.html | 57 ++++++++++++ .../backend/include/backend/DriverEnums.h | 9 ++ filament/include/filament/Material.h | 28 ++++++ filament/src/MaterialParser.cpp | 40 ++++++++ filament/src/MaterialParser.h | 7 ++ filament/src/details/Material.cpp | 66 +++++++++++-- filament/src/details/Material.h | 2 + .../include/filament/MaterialChunkType.h | 1 + .../include/private/filament/ConstantInfo.h | 38 ++++++++ .../include/private/filament/EngineEnums.h | 4 + libs/filamat/include/filamat/Enums.h | 2 + .../filamat/include/filamat/MaterialBuilder.h | 23 +++++ libs/filamat/src/Enums.cpp | 11 +++ libs/filamat/src/MaterialBuilder.cpp | 64 ++++++++++++- .../src/eiff/MaterialInterfaceBlockChunk.cpp | 15 +++ .../src/eiff/MaterialInterfaceBlockChunk.h | 15 +++ libs/filamat/src/shaders/CodeGenerator.cpp | 2 +- libs/filamat/src/shaders/MaterialInfo.h | 1 + libs/filamat/src/shaders/ShaderGenerator.cpp | 32 +++++++ libs/filamat/src/shaders/ShaderGenerator.h | 2 + libs/filamat/tests/test_filamat.cpp | 49 ++++++++++ libs/matdbg/src/CommonWriter.h | 10 ++ libs/matdbg/src/TextWriter.cpp | 39 ++++++++ tools/matc/src/matc/ParametersProcessor.cpp | 93 +++++++++++++++++++ 25 files changed, 597 insertions(+), 14 deletions(-) create mode 100644 libs/filabridge/include/private/filament/ConstantInfo.h diff --git a/NEW_RELEASE_NOTES.md b/NEW_RELEASE_NOTES.md index e5a2fb03f55..2e27f192b9a 100644 --- a/NEW_RELEASE_NOTES.md +++ b/NEW_RELEASE_NOTES.md @@ -8,6 +8,7 @@ appropriate header in [RELEASE_NOTES.md](./RELEASE_NOTES.md). ## Release notes for next branch cut +- engine: Add support for _constant parameters_, which are constants that can be specialized after material compilation. - materials: improved size reduction of OpenGL/Metal shaders by ~65% when compiling materials with size optimizations (`matc -S`) [⚠️ **Recompile Materials**] - engine: fix potential crash on Metal devices with A8X GPU (iPad Air 2) diff --git a/docs/Materials.md.html b/docs/Materials.md.html index dc45ea39c4f..8db63741d9c 100644 --- a/docs/Materials.md.html +++ b/docs/Materials.md.html @@ -1076,6 +1076,63 @@ } ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +### General: constants + +Type +: array of constant objects + +Value +: Each entry is an object with the properties `name` and `type`, both of `string` type. The name + must be a valid GLSL identifier. Entries also have an optional `default`, which can either be a + `bool` or `number`, depending on the `type` of the constant. The type must be one of the types + described in table [materialConstantsTypes]. + + Type | Description | Default +:----------------------|:-----------------------------------------|:------------------ +int | A signed, 32 bit GLSL int | 0 +float | A single-precision GLSL float | 0.0 +bool | A GLSL bool | false +[Table [materialConstantsTypes]: Material constants types] + +Description +: Lists the constant parameters accepted by your material. These constants can be set, or + "specialized", at runtime when loading a material package. Multiple materials can be loaded from + the same material package with differing constant parameter specializations. Once a material is + loaded from a material package, its constant parameters cannot be changed. Compared to regular + parameters, constant parameters allow the compiler to generate more efficient code. Access + constant parameters from the shader by prefixing the name with `materialConstant_`. For example, + a constant parameter named `myConstant` is accessed in the shader as + `materialConstant_myConstant`. If a constant parameter is not set at runtime, the default is + used. + +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ JSON +material { + constants : [ + { + name : overrideAlpha, + type : bool + }, + { + name : customAlpha, + type : float, + default : 0.5 + } + ], + shadingModel : lit, + blending : transparent, +} + +fragment { + void material(inout MaterialInputs material) { + prepareMaterial(material); + if (materialConstants_overrideAlpha) { + material.baseColor.a = materialConstants_customAlpha; + material.baseColor.rgb *= material.baseColor.a; + } + } +} +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + ### General: variantFilter Type diff --git a/filament/backend/include/backend/DriverEnums.h b/filament/backend/include/backend/DriverEnums.h index 8bec4b5beac..d66ce9f7c4d 100644 --- a/filament/backend/include/backend/DriverEnums.h +++ b/filament/backend/include/backend/DriverEnums.h @@ -279,6 +279,15 @@ enum class UniformType : uint8_t { STRUCT }; +/** + * Supported constant parameter types + */ + enum class ConstantType : uint8_t { + INT, + FLOAT, + BOOL +}; + enum class Precision : uint8_t { LOW, MEDIUM, diff --git a/filament/include/filament/Material.h b/filament/include/filament/Material.h index 6533577383e..602df4da56f 100644 --- a/filament/include/filament/Material.h +++ b/filament/include/filament/Material.h @@ -105,6 +105,34 @@ class UTILS_PUBLIC Material : public FilamentAPI { */ Builder& package(const void* payload, size_t size); + template + using is_supported_constant_parameter_t = typename std::enable_if< + std::is_same::value || + std::is_same::value || + std::is_same::value>::type; + + /** + * Specialize a constant parameter specified in the material definition with a concrete + * value for this material. Once build() is called, this constant cannot be changed. + * Will throw an exception if the name does not match a constant specified in the + * material definition or if the type provided does not match. + * + * @tparam T The type of constant parameter, either int32_t, float, or bool. + * @param name The name of the constant parameter specified in the material definition, such + * as "myConstant". + * @param nameLength Length in `char` of the name parameter. + * @param value The value to use for the constant parameter, must match the type specified + * in the material definition. + */ + template> + Builder& constant(const char* name, size_t nameLength, T value); + + /** inline helper to provide the constant name as a null-terminated C string */ + template> + inline Builder& constant(const char* name, T value) { + return constant(name, strlen(name), value); + } + /** * Creates the Material object and returns a pointer to it. * diff --git a/filament/src/MaterialParser.cpp b/filament/src/MaterialParser.cpp index d2601ad993c..ae876630684 100644 --- a/filament/src/MaterialParser.cpp +++ b/filament/src/MaterialParser.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include @@ -177,6 +178,13 @@ bool MaterialParser::getSamplerBlockBindings( pSamplerGroupInfoList, pSamplerBindingToNameMap); } +bool MaterialParser::getConstants(utils::FixedCapacityVector* value) const noexcept { + auto [start, end] = mImpl.mChunkContainer.getChunkRange(filamat::MaterialConstants); + if (start == end) return false; + Unflattener unflattener(start, end); + return ChunkMaterialConstants::unflatten(unflattener, value); +} + bool MaterialParser::getDepthWriteSet(bool* value) const noexcept { return mImpl.getFromSimpleChunk(ChunkType::MaterialDepthWriteSet, value); } @@ -546,4 +554,36 @@ bool ChunkSamplerBlockBindings::unflatten(Unflattener& unflattener, return true; } +bool ChunkMaterialConstants::unflatten(filaflat::Unflattener& unflattener, + utils::FixedCapacityVector* materialConstants) { + assert_invariant(materialConstants); + + // Read number of constants. + uint64_t numConstants = 0; + if (!unflattener.read(&numConstants)) { + return false; + } + + materialConstants->reserve(numConstants); + materialConstants->resize(numConstants); + + for (uint64_t i = 0; i < numConstants; i++) { + CString constantName; + uint8_t constantType = 0; + + if (!unflattener.read(&constantName)) { + return false; + } + + if (!unflattener.read(&constantType)) { + return false; + } + + (*materialConstants)[i].name = constantName; + (*materialConstants)[i].type = static_cast(constantType); + } + + return true; +} + } // namespace filament diff --git a/filament/src/MaterialParser.h b/filament/src/MaterialParser.h index 91b39cd9e72..154cb743f66 100644 --- a/filament/src/MaterialParser.h +++ b/filament/src/MaterialParser.h @@ -43,6 +43,7 @@ namespace filament { class BufferInterfaceBlock; class SamplerInterfaceBlock; struct SubpassInfo; +struct MaterialConstant; class MaterialParser { public: @@ -71,6 +72,7 @@ class MaterialParser { bool getUniformBlockBindings(utils::FixedCapacityVector>* value) const noexcept; bool getSamplerBlockBindings(SamplerGroupBindingInfoList* pSamplerGroupInfoList, SamplerBindingToNameMap* pSamplerBindingToNameMap) const noexcept; + bool getConstants(utils::FixedCapacityVector* value) const noexcept; bool getDepthWriteSet(bool* value) const noexcept; bool getDepthWrite(bool* value) const noexcept; @@ -170,6 +172,11 @@ struct ChunkSamplerBlockBindings { SamplerBindingToNameMap* pSamplerBindingToNameMap); }; +struct ChunkMaterialConstants { + static bool unflatten(filaflat::Unflattener& unflattener, + utils::FixedCapacityVector* materialConstants); +}; + } // namespace filament #endif // TNT_FILAMENT_MATERIALPARSER_H diff --git a/filament/src/details/Material.cpp b/filament/src/details/Material.cpp index 02d9d9d208a..f6ef4c68e1f 100644 --- a/filament/src/details/Material.cpp +++ b/filament/src/details/Material.cpp @@ -32,8 +32,11 @@ #include #include +#include #include +#include + namespace filament { using namespace backend; @@ -71,6 +74,7 @@ struct Material::BuilderDetails { size_t mSize = 0; MaterialParser* mMaterialParser = nullptr; bool mDefaultMaterial = false; + std::unordered_map> mConstantSpecializations; }; FMaterial::DefaultMaterialBuilder::DefaultMaterialBuilder() : Material::Builder() { @@ -91,6 +95,17 @@ Material::Builder& Material::Builder::package(const void* payload, size_t size) return *this; } +template +Material::Builder& Material::Builder::constant(const char* name, size_t nameLength, T value) { + ASSERT_PRECONDITION(name != nullptr, "name cannot be null"); + mImpl->mConstantSpecializations[{name, nameLength}] = value; + return *this; +} + +template Material::Builder& Material::Builder::constant(const char*, size_t, int32_t); +template Material::Builder& Material::Builder::constant(const char*, size_t, float); +template Material::Builder& Material::Builder::constant(const char*, size_t, bool); + Material* Material::Builder::build(Engine& engine) { std::unique_ptr materialParser{ createParser( downcast(engine).getBackend(), mImpl->mPayload, mImpl->mSize) }; @@ -179,6 +194,48 @@ FMaterial::FMaterial(FEngine& engine, const Material::Builder& builder) mSubpassInfo.isValid = false; } + utils::FixedCapacityVector constants; + // Older materials won't have a constants chunk, but that's okay. + parser->getConstants(&constants); + + // Verify that all the constant specializations exist in the material and that their types match. + // The first specialization constants are defined internally by Filament. + // The subsequent constants are user-defined in the material. + mSpecializationConstants.reserve(constants.size() + CONFIG_MAX_RESERVED_SPEC_CONSTANTS); + mSpecializationConstants.push_back({0, (int)mEngine.getSupportedFeatureLevel()}); + mSpecializationConstants.push_back({1, (int)CONFIG_MAX_INSTANCES}); + const bool staticTextureWorkaround = + mEngine.getDriverApi().isWorkaroundNeeded(Workaround::A8X_STATIC_TEXTURE_TARGET_ERROR); + mSpecializationConstants.push_back({2, (bool)staticTextureWorkaround}); + for (const auto& [name, value] : builder->mConstantSpecializations) { + auto found = std::find_if( + constants.begin(), constants.end(), [name = name](const auto& constant) { + return strncmp(constant.name.data(), name.data(), name.length()) == 0; + }); + ASSERT_PRECONDITION(found != constants.end(), + "The material %s does not have a constant parameter named %s.", mName.c_str_safe(), name.c_str()); + const char* const types[3] = {"an int", "a float", "a bool"}; + const char* const errorMessage = + "The constant parameter %s on material %s is of type %s, but %s was " + "provided."; + switch (found->type) { + case ConstantType::INT: + ASSERT_PRECONDITION(std::holds_alternative(value), errorMessage, + name.c_str(), mName.c_str_safe(), "int", types[value.index()]); + break; + case ConstantType::FLOAT: + ASSERT_PRECONDITION(std::holds_alternative(value), errorMessage, + name.c_str(), mName.c_str_safe(), "float", types[value.index()]); + break; + case ConstantType::BOOL: + ASSERT_PRECONDITION(std::holds_alternative(value), errorMessage, + name.c_str(), mName.c_str_safe(), "bool", types[value.index()]); + break; + } + uint32_t index = std::distance(constants.begin(), found) + CONFIG_MAX_RESERVED_SPEC_CONSTANTS; + mSpecializationConstants.push_back({index, value}); + } + parser->getShading(&mShading); parser->getMaterialProperties(&mMaterialProperties); parser->getBlendingMode(&mBlendingMode); @@ -462,14 +519,7 @@ Program FMaterial::getProgramWithVariants( } } - const bool staticTextureWorkaround = - mEngine.getDriverApi().isWorkaroundNeeded(Workaround::A8X_STATIC_TEXTURE_TARGET_ERROR); - - program.specializationConstants({ - { 0, (int)mEngine.getSupportedFeatureLevel() }, - { 1, (int)CONFIG_MAX_INSTANCES }, - { 2, (bool)staticTextureWorkaround } - }); + program.specializationConstants(mSpecializationConstants); return program; } diff --git a/filament/src/details/Material.h b/filament/src/details/Material.h index c32a4f2bc68..a9bc190bbb8 100644 --- a/filament/src/details/Material.h +++ b/filament/src/details/Material.h @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -232,6 +233,7 @@ class FMaterial : public Material { SamplerGroupBindingInfoList mSamplerGroupBindingInfoList; SamplerBindingToNameMap mSamplerBindingToNameMap; + utils::FixedCapacityVector mSpecializationConstants; #if FILAMENT_ENABLE_MATDBG matdbg::MaterialKey mDebuggerId; diff --git a/libs/filabridge/include/filament/MaterialChunkType.h b/libs/filabridge/include/filament/MaterialChunkType.h index eed5dea0792..5d3e95ee3b2 100644 --- a/libs/filabridge/include/filament/MaterialChunkType.h +++ b/libs/filabridge/include/filament/MaterialChunkType.h @@ -48,6 +48,7 @@ enum UTILS_PUBLIC ChunkType : uint64_t { MaterialSamplerBindings = charTo64bitNum("MAT_SAMP"), MaterialUniformBindings = charTo64bitNum("MAT_UNIF"), MaterialProperties = charTo64bitNum("MAT_PROP"), + MaterialConstants = charTo64bitNum("MAT_CONS"), MaterialName = charTo64bitNum("MAT_NAME"), MaterialVersion = charTo64bitNum("MAT_VERS"), diff --git a/libs/filabridge/include/private/filament/ConstantInfo.h b/libs/filabridge/include/private/filament/ConstantInfo.h new file mode 100644 index 00000000000..04ceaf15beb --- /dev/null +++ b/libs/filabridge/include/private/filament/ConstantInfo.h @@ -0,0 +1,38 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef TNT_FILAMENT_CONSTANTINFO_H +#define TNT_FILAMENT_CONSTANTINFO_H + +#include + +#include + +namespace filament { + +struct MaterialConstant { + using ConstantType = backend::ConstantType; + + utils::CString name; + ConstantType type; + + MaterialConstant() = default; + MaterialConstant(const char* name, ConstantType type) : name(name), type(type) {} +}; + +} + +#endif // TNT_FILAMENT_CONSTANTINFO_H diff --git a/libs/filabridge/include/private/filament/EngineEnums.h b/libs/filabridge/include/private/filament/EngineEnums.h index f2113344f72..39558be756e 100644 --- a/libs/filabridge/include/private/filament/EngineEnums.h +++ b/libs/filabridge/include/private/filament/EngineEnums.h @@ -60,6 +60,10 @@ enum class SamplerBindingPoints : uint8_t { constexpr size_t CONFIG_MAX_LIGHT_COUNT = 256; constexpr size_t CONFIG_MAX_LIGHT_INDEX = CONFIG_MAX_LIGHT_COUNT - 1; +// The number of specialization constants that Filament reserves for its own use. These are always +// the first constants (from 0 to CONFIG_MAX_RESERVED_SPEC_CONSTANTS - 1). +constexpr size_t CONFIG_MAX_RESERVED_SPEC_CONSTANTS = 8; + // The maximum number of shadowmaps. // There is currently a maximum limit of 128 shadowmaps. // Factors contributing to this limit: diff --git a/libs/filamat/include/filamat/Enums.h b/libs/filamat/include/filamat/Enums.h index ea626e814a0..04b5ff8bbfc 100644 --- a/libs/filamat/include/filamat/Enums.h +++ b/libs/filamat/include/filamat/Enums.h @@ -34,6 +34,7 @@ using ParameterPrecision = MaterialBuilder::ParameterPrecision; using OutputTarget = MaterialBuilder::OutputTarget; using OutputQualifier = MaterialBuilder::VariableQualifier; using OutputType = MaterialBuilder::OutputType; +using ConstantType = MaterialBuilder::ConstantType; // Convenience methods to convert std::string to Enum and also iterate over Enum values. class Enums { @@ -77,6 +78,7 @@ class Enums { static std::unordered_map mStringToOutputTarget; static std::unordered_map mStringToOutputQualifier; static std::unordered_map mStringToOutputType; + static std::unordered_map mStringToConstantType; }; template diff --git a/libs/filamat/include/filamat/MaterialBuilder.h b/libs/filamat/include/filamat/MaterialBuilder.h index 8cbeba4728a..3f668063c93 100644 --- a/libs/filamat/include/filamat/MaterialBuilder.h +++ b/libs/filamat/include/filamat/MaterialBuilder.h @@ -39,6 +39,7 @@ #include #include #include +#include #include #include @@ -238,6 +239,7 @@ class UTILS_PUBLIC MaterialBuilder : public MaterialBuilderBase { using SpecularAmbientOcclusion = filament::SpecularAmbientOcclusion; using UniformType = filament::backend::UniformType; + using ConstantType = filament::backend::ConstantType; using SamplerType = filament::backend::SamplerType; using SubpassType = filament::backend::SubpassType; using SamplerFormat = filament::backend::SamplerFormat; @@ -290,6 +292,15 @@ class UTILS_PUBLIC MaterialBuilder : public MaterialBuilderBase { MaterialBuilder& parameter(const char* name, size_t size, UniformType type, ParameterPrecision precision = ParameterPrecision::DEFAULT) noexcept; + //! Add a constant parameter to this material. + template + using is_supported_constant_parameter_t = typename std::enable_if< + std::is_same::value || + std::is_same::value || + std::is_same::value>::type; + template> + MaterialBuilder& constant(const char *name, ConstantType type, T defaultValue = 0); + /** * Add a sampler parameter to this material. * @@ -652,6 +663,16 @@ class UTILS_PUBLIC MaterialBuilder : public MaterialBuilderBase { int location; }; + struct Constant { + utils::CString name; + ConstantType type; + union { + int32_t i; + float f; + bool b; + } defaultValue; + }; + static constexpr size_t MATERIAL_PROPERTIES_COUNT = filament::MATERIAL_PROPERTIES_COUNT; using Property = filament::Property; @@ -679,6 +700,7 @@ class UTILS_PUBLIC MaterialBuilder : public MaterialBuilderBase { using ParameterList = Parameter[MAX_PARAMETERS_COUNT]; using SubpassList = Parameter[MAX_SUBPASS_COUNT]; using BufferList = std::vector>; + using ConstantList = std::vector; // returns the number of parameters declared in this material uint8_t getParameterCount() const noexcept { return mParameterCount; } @@ -763,6 +785,7 @@ class UTILS_PUBLIC MaterialBuilder : public MaterialBuilderBase { PropertyList mProperties; ParameterList mParameters; + ConstantList mConstants; SubpassList mSubpasses; VariableList mVariables; OutputList mOutputs; diff --git a/libs/filamat/src/Enums.cpp b/libs/filamat/src/Enums.cpp index 8049fdc9fe4..01c4011c7ef 100644 --- a/libs/filamat/src/Enums.cpp +++ b/libs/filamat/src/Enums.cpp @@ -160,4 +160,15 @@ std::unordered_map& Enums::getMap() n return mStringToSamplerFormat; }; +std::unordered_map Enums::mStringToConstantType = { + { "int", ConstantType::INT }, + { "float", ConstantType::FLOAT }, + { "bool", ConstantType::BOOL }, +}; + +template <> +std::unordered_map& Enums::getMap() noexcept { + return mStringToConstantType; +}; + } // namespace filamat diff --git a/libs/filamat/src/MaterialBuilder.cpp b/libs/filamat/src/MaterialBuilder.cpp index 35567278a4f..08a6ebeaaf9 100644 --- a/libs/filamat/src/MaterialBuilder.cpp +++ b/libs/filamat/src/MaterialBuilder.cpp @@ -45,6 +45,7 @@ #include #include #include +#include #include #include @@ -212,6 +213,53 @@ MaterialBuilder& MaterialBuilder::parameter(const char* name, SamplerType sample return parameter(name, samplerType, SamplerFormat::FLOAT, precision); } +template +MaterialBuilder& MaterialBuilder::constant(const char* name, ConstantType type, T defaultValue) { + auto result = std::find_if(mConstants.begin(), mConstants.end(), [name](const Constant& c) { + return c.name == utils::CString(name); + }); + ASSERT_POSTCONDITION(result == mConstants.end(), + "There is already a constant parameter present with the name %s.", name); + Constant constant { + .name = CString(name), + .type = type, + }; + auto toString = [](ConstantType t) { + switch (t) { + case ConstantType::INT: return "INT"; + case ConstantType::FLOAT: return "FLOAT"; + case ConstantType::BOOL: return "BOOL"; + } + }; + + const char* const errMessage = + "Constant %s was declared with type %s but given %s default value."; + if constexpr (std::is_same_v) { + ASSERT_POSTCONDITION( + type == ConstantType::INT, errMessage, name, toString(type), "an int"); + constant.defaultValue.i = defaultValue; + } else if constexpr (std::is_same_v) { + ASSERT_POSTCONDITION( + type == ConstantType::FLOAT, errMessage, name, toString(type), "a float"); + constant.defaultValue.f = defaultValue; + } else if constexpr (std::is_same_v) { + ASSERT_POSTCONDITION( + type == ConstantType::BOOL, errMessage, name, toString(type), "a bool"); + constant.defaultValue.b = defaultValue; + } else { + assert_invariant(false); + } + + mConstants.push_back(constant); + return *this; +} +template MaterialBuilder& MaterialBuilder::constant( + const char* name, ConstantType type, int32_t defaultValue); +template MaterialBuilder& MaterialBuilder::constant( + const char* name, ConstantType type, float defaultValue); +template MaterialBuilder& MaterialBuilder::constant( + const char* name, ConstantType type, bool defaultValue); + MaterialBuilder& MaterialBuilder::buffer(BufferInterfaceBlock bib) noexcept { ASSERT_POSTCONDITION(mBuffers.size() < MAX_BUFFERS_COUNT, "Too many buffers"); mBuffers.emplace_back(std::make_unique(std::move(bib))); @@ -699,10 +747,10 @@ bool MaterialBuilder::generateShaders(JobSystem& jobSystem, const std::vector(ChunkType::MaterialHasCustomDepthShader, needsStandardDepthProgram()); @@ -1163,7 +1211,7 @@ bool MaterialBuilder::needsStandardDepthProgram() const noexcept { std::string MaterialBuilder::peek(backend::ShaderStage stage, const CodeGenParams& params, const PropertyList& properties) noexcept { - ShaderGenerator sg(properties, mVariables, mOutputs, mDefines, + ShaderGenerator sg(properties, mVariables, mOutputs, mDefines, mConstants, mMaterialFragmentCode.getResolved(), mMaterialFragmentCode.getLineOffset(), mMaterialVertexCode.getResolved(), mMaterialVertexCode.getLineOffset(), mMaterialDomain); @@ -1222,6 +1270,12 @@ void MaterialBuilder::writeCommonChunks(ChunkContainer& container, MaterialInfo& // User Material SIB container.push(info.sib); + // User constant parameters + utils::FixedCapacityVector constantsEntry(mConstants.size()); + std::transform(mConstants.begin(), mConstants.end(), constantsEntry.begin(), + [](Constant const& c) { return MaterialConstant(c.name.c_str(), c.type); }); + container.push(std::move(constantsEntry)); + // TODO: should we write the SSBO info? this would only be needed if we wanted to provide // an interface to set [get?] values in the buffer. But we can do that easily // with a c-struct (what about kotlin/java?). tbd. diff --git a/libs/filamat/src/eiff/MaterialInterfaceBlockChunk.cpp b/libs/filamat/src/eiff/MaterialInterfaceBlockChunk.cpp index d3a23d9b3c3..b3ebd9e0b56 100644 --- a/libs/filamat/src/eiff/MaterialInterfaceBlockChunk.cpp +++ b/libs/filamat/src/eiff/MaterialInterfaceBlockChunk.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include @@ -89,6 +90,20 @@ void MaterialSubpassInterfaceBlockChunk::flatten(Flattener& f) { // ------------------------------------------------------------------------------------------------ +MaterialConstantParametersChunk::MaterialConstantParametersChunk( + utils::FixedCapacityVector constants) + : Chunk(ChunkType::MaterialConstants), mConstants(std::move(constants)) {} + +void MaterialConstantParametersChunk::flatten(Flattener& f) { + f.writeUint64(mConstants.size()); + for (const auto& constant : mConstants) { + f.writeString(constant.name.c_str()); + f.writeUint8(static_cast(constant.type)); + } +} + +// ------------------------------------------------------------------------------------------------ + MaterialUniformBlockBindingsChunk::MaterialUniformBlockBindingsChunk( utils::FixedCapacityVector> list) : Chunk(ChunkType::MaterialUniformBindings), diff --git a/libs/filamat/src/eiff/MaterialInterfaceBlockChunk.h b/libs/filamat/src/eiff/MaterialInterfaceBlockChunk.h index df42347556c..aa287096d47 100644 --- a/libs/filamat/src/eiff/MaterialInterfaceBlockChunk.h +++ b/libs/filamat/src/eiff/MaterialInterfaceBlockChunk.h @@ -28,6 +28,7 @@ class SamplerBindingMap; class SamplerInterfaceBlock; class BufferInterfaceBlock; struct SubpassInfo; +struct MaterialConstant; } // namespace filament namespace filamat { @@ -71,6 +72,20 @@ class MaterialSubpassInterfaceBlockChunk final : public Chunk { // ------------------------------------------------------------------------------------------------ +class MaterialConstantParametersChunk final : public Chunk { +public: + explicit MaterialConstantParametersChunk( + utils::FixedCapacityVector constants); + ~MaterialConstantParametersChunk() final = default; + +private: + void flatten(Flattener&) final; + + utils::FixedCapacityVector mConstants; +}; + +// ------------------------------------------------------------------------------------------------ + class MaterialUniformBlockBindingsChunk final : public Chunk { public: explicit MaterialUniformBlockBindingsChunk( diff --git a/libs/filamat/src/shaders/CodeGenerator.cpp b/libs/filamat/src/shaders/CodeGenerator.cpp index f3425cca552..bf6d3851565 100644 --- a/libs/filamat/src/shaders/CodeGenerator.cpp +++ b/libs/filamat/src/shaders/CodeGenerator.cpp @@ -119,7 +119,7 @@ utils::io::sstream& CodeGenerator::generateProlog(utils::io::sstream& out, Shade out << "precision lowp sampler3D;\n"; } - // specification constants + // Filament-reserved specification constants (limited by CONFIG_MAX_RESERVED_SPEC_CONSTANTS) out << '\n'; generateSpecializationConstant(out, "BACKEND_FEATURE_LEVEL", 0, 1); diff --git a/libs/filamat/src/shaders/MaterialInfo.h b/libs/filamat/src/shaders/MaterialInfo.h index 22af34cb28c..9536dcc7041 100644 --- a/libs/filamat/src/shaders/MaterialInfo.h +++ b/libs/filamat/src/shaders/MaterialInfo.h @@ -28,6 +28,7 @@ #include #include +#include namespace filamat { diff --git a/libs/filamat/src/shaders/ShaderGenerator.cpp b/libs/filamat/src/shaders/ShaderGenerator.cpp index e7d6bf6f4fb..406820f3f7b 100644 --- a/libs/filamat/src/shaders/ShaderGenerator.cpp +++ b/libs/filamat/src/shaders/ShaderGenerator.cpp @@ -126,11 +126,36 @@ static void appendShader(io::sstream& ss, } } +static void generateUserSpecConstants( + const CodeGenerator& cg, io::sstream& fs, MaterialBuilder::ConstantList constants) { + // Constants 0 to CONFIG_MAX_RESERVED_SPEC_CONSTANTS - 1 are reserved by Filament. + size_t index = CONFIG_MAX_RESERVED_SPEC_CONSTANTS; + for (const auto& constant : constants) { + std::string fullName = std::string("materialConstants_") + constant.name.c_str(); + switch (constant.type) { + case ConstantType::INT: + cg.generateSpecializationConstant( + fs, fullName.c_str(), index++, constant.defaultValue.i); + break; + case ConstantType::FLOAT: + cg.generateSpecializationConstant( + fs, fullName.c_str(), index++, constant.defaultValue.f); + break; + case ConstantType::BOOL: + cg.generateSpecializationConstant( + fs, fullName.c_str(), index++, constant.defaultValue.b); + break; + } + } +} + + ShaderGenerator::ShaderGenerator( MaterialBuilder::PropertyList const& properties, MaterialBuilder::VariableList const& variables, MaterialBuilder::OutputList const& outputs, MaterialBuilder::PreprocessorDefineList const& defines, + MaterialBuilder::ConstantList const& constants, CString const& materialCode, size_t lineOffset, CString const& materialVertexCode, size_t vertexLineOffset, MaterialBuilder::MaterialDomain materialDomain) noexcept { @@ -151,6 +176,7 @@ ShaderGenerator::ShaderGenerator( mMaterialVertexLineOffset = vertexLineOffset; mMaterialDomain = materialDomain; mDefines = defines; + mConstants = constants; if (mMaterialFragmentCode.empty()) { if (mMaterialDomain == MaterialBuilder::MaterialDomain::SURFACE) { @@ -203,6 +229,8 @@ std::string ShaderGenerator::createVertexProgram(ShaderModel shaderModel, cg.generateProlog(vs, ShaderStage::VERTEX, material); + generateUserSpecConstants(cg, vs, mConstants); + cg.generateQualityDefine(vs, material.quality); CodeGenerator::generateDefine(vs, "FLIP_UV_ATTRIBUTE", material.flipUV); @@ -342,6 +370,8 @@ std::string ShaderGenerator::createFragmentProgram(ShaderModel shaderModel, io::sstream fs; cg.generateProlog(fs, ShaderStage::FRAGMENT, material); + generateUserSpecConstants(cg, fs, mConstants); + cg.generateQualityDefine(fs, material.quality); CodeGenerator::generateDefine(fs, "GEOMETRIC_SPECULAR_AA", material.specularAntiAliasing && lit); @@ -554,6 +584,8 @@ std::string ShaderGenerator::createComputeProgram(filament::backend::ShaderModel cg.generateProlog(s, ShaderStage::COMPUTE, material); + generateUserSpecConstants(cg, s, mConstants); + cg.generateQualityDefine(s, material.quality); cg.generateUniforms(s, ShaderStage::COMPUTE, diff --git a/libs/filamat/src/shaders/ShaderGenerator.h b/libs/filamat/src/shaders/ShaderGenerator.h index 2d7a63bd9e7..d1e35a293cf 100644 --- a/libs/filamat/src/shaders/ShaderGenerator.h +++ b/libs/filamat/src/shaders/ShaderGenerator.h @@ -42,6 +42,7 @@ class ShaderGenerator { MaterialBuilder::VariableList const& variables, MaterialBuilder::OutputList const& outputs, MaterialBuilder::PreprocessorDefineList const& defines, + MaterialBuilder::ConstantList const& constants, utils::CString const& materialCode, size_t lineOffset, utils::CString const& materialVertexCode, @@ -88,6 +89,7 @@ class ShaderGenerator { MaterialBuilder::OutputList mOutputs; MaterialBuilder::MaterialDomain mMaterialDomain; MaterialBuilder::PreprocessorDefineList mDefines; + MaterialBuilder::ConstantList mConstants; utils::CString mMaterialFragmentCode; // fragment or compute code utils::CString mMaterialVertexCode; size_t mMaterialLineOffset; diff --git a/libs/filamat/tests/test_filamat.cpp b/libs/filamat/tests/test_filamat.cpp index 95b25f25ebc..6280ca3c42b 100644 --- a/libs/filamat/tests/test_filamat.cpp +++ b/libs/filamat/tests/test_filamat.cpp @@ -802,6 +802,55 @@ TEST_F(MaterialCompiler, CustomSurfaceShadingHasFunction) { EXPECT_TRUE(result.isValid()); } +TEST_F(MaterialCompiler, ConstantParameter) { + std::string shaderCode(R"( + void material(inout MaterialInputs material) { + prepareMaterial(material); + if (materialConstants_myBoolConstant) { + material.baseColor.rgb = float3(materialConstants_myFloatConstant); + int anInt = materialConstants_myIntConstant; + } + } + )"); + std::string vertexCode(R"( + void materialVertex(inout MaterialVertexInputs material) { + int anInt = materialConstants_myIntConstant; + bool aBool = materialConstants_myBoolConstant; + float aFloat = materialConstants_myFloatConstant; + } + )"); + filamat::MaterialBuilder builder; + builder.constant("myFloatConstant", ConstantType::FLOAT, 1.0f); + builder.constant("myIntConstant", ConstantType::INT, 123); + builder.constant("myBoolConstant", ConstantType::BOOL, true); + builder.constant("myOtherBoolConstant", ConstantType::BOOL); + + builder.shading(filament::Shading::LIT); + builder.material(shaderCode.c_str()); + builder.materialVertex(vertexCode.c_str()); + filamat::Package result = builder.build(*jobSystem); + EXPECT_TRUE(result.isValid()); +} + +TEST_F(MaterialCompiler, ConstantParameterSameName) { +#if !defined(NDEBUG) && defined(GTEST_HAS_DEATH_TEST) + EXPECT_THROW({ + filamat::MaterialBuilder builder; + builder.constant("myFloatConstant", ConstantType::FLOAT, 1.0f); + builder.constant("myFloatConstant", ConstantType::FLOAT, 1.0f); + }, utils::PostconditionPanic); +#endif +} + +TEST_F(MaterialCompiler, ConstantParameterWrongType) { +#if !defined(NDEBUG) && defined(GTEST_HAS_DEATH_TEST) + EXPECT_THROW({ + filamat::MaterialBuilder builder; + builder.constant("myFloatConstant", ConstantType::FLOAT, 10); + }, utils::PostconditionPanic); +#endif +} + int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); diff --git a/libs/matdbg/src/CommonWriter.h b/libs/matdbg/src/CommonWriter.h index a7113197087..6d2afc1ee4e 100644 --- a/libs/matdbg/src/CommonWriter.h +++ b/libs/matdbg/src/CommonWriter.h @@ -236,6 +236,16 @@ const char* toString(backend::SamplerFormat format) noexcept { return "--"; } +inline +const char* toString(backend::ConstantType type) noexcept { + switch (type) { + case backend::ConstantType::FLOAT: return "float"; + case backend::ConstantType::INT: return "int"; + case backend::ConstantType::BOOL: return "bool"; + } + return "--"; +} + // Returns a human-readable variant description. // For example: DYN|DIR std::string formatVariantString(Variant variant, MaterialDomain domain) noexcept; diff --git a/libs/matdbg/src/TextWriter.cpp b/libs/matdbg/src/TextWriter.cpp index 737458ab0c4..1d889cb186f 100644 --- a/libs/matdbg/src/TextWriter.cpp +++ b/libs/matdbg/src/TextWriter.cpp @@ -259,6 +259,42 @@ static bool printParametersInfo(ostream& text, const ChunkContainer& container) return true; } +static bool printConstantInfo(ostream& text, const ChunkContainer& container) { + if (!container.hasChunk(ChunkType::MaterialConstants)) { + return true; + } + + auto [startConstants, endConstants] = container.getChunkRange(ChunkType::MaterialConstants); + Unflattener constants(startConstants, endConstants); + + uint64_t constantsCount; + constants.read(&constantsCount); + + text << "Constants:" << endl; + + for (uint64_t i = 0; i < constantsCount; i++) { + CString fieldName; + uint8_t fieldType; + + if (!constants.read(&fieldName)) { + return false; + } + + if (!constants.read(&fieldType)) { + return false; + } + + text << " " + << setw(alignment) << fieldName.c_str() + << setw(shortAlignment) << toString(ConstantType(fieldType)) + << endl; + } + + text << endl; + + return true; +} + static bool printSubpassesInfo(ostream& text, const ChunkContainer& container) { // Subpasses are optional. @@ -406,6 +442,9 @@ bool TextWriter::writeMaterialInfo(const filaflat::ChunkContainer& container) { if (!printParametersInfo(text, container)) { return false; } + if (!printConstantInfo(text, container)) { + return false; + } if (!printSubpassesInfo(text, container)) { return false; } diff --git a/tools/matc/src/matc/ParametersProcessor.cpp b/tools/matc/src/matc/ParametersProcessor.cpp index f1745e4b2c1..8a5c8239263 100644 --- a/tools/matc/src/matc/ParametersProcessor.cpp +++ b/tools/matc/src/matc/ParametersProcessor.cpp @@ -246,6 +246,98 @@ static bool processParameters(MaterialBuilder& builder, const JsonishValue& v) { return ok; } +static bool processConstant(MaterialBuilder& builder, const JsonishObject& jsonObject) noexcept { + const JsonishValue* typeValue = jsonObject.getValue("type"); + if (!typeValue) { + std::cerr << "constants: entry without key 'type'." << std::endl; + return false; + } + if (typeValue->getType() != JsonishValue::STRING) { + std::cerr << "constants: type value must be STRING." << std::endl; + return false; + } + + const JsonishValue* nameValue = jsonObject.getValue("name"); + if (!nameValue) { + std::cerr << "constants: entry without 'name' key." << std::endl; + return false; + } + if (nameValue->getType() != JsonishValue::STRING) { + std::cerr << "constants: name value must be STRING." << std::endl; + return false; + } + + auto typeString = typeValue->toJsonString()->getString(); + auto nameString = nameValue->toJsonString()->getString(); + const JsonishValue* defaultValue = jsonObject.getValue("default"); + + if (Enums::isValid(typeString)) { + auto type = Enums::toEnum(typeString); + switch (type) { + case ConstantType::INT: { + int32_t intDefault = 0; + if (defaultValue) { + if (defaultValue->getType() != JsonishValue::NUMBER) { + std::cerr << "constants: INT constants must have NUMBER default value" + << std::endl; + return false; + } + // FIXME: Jsonish doesn't distinguish between integers and floats. + intDefault = (int32_t)defaultValue->toJsonNumber()->getFloat(); + } + builder.constant(nameString.c_str(), type, intDefault); + break; + } + case ConstantType::FLOAT: { + float floatDefault = 0.0f; + if (defaultValue) { + if (defaultValue->getType() != JsonishValue::NUMBER) { + std::cerr << "constants: FLOAT constants must have NUMBER default value" + << std::endl; + return false; + } + floatDefault = defaultValue->toJsonNumber()->getFloat(); + } + builder.constant(nameString.c_str(), type, floatDefault); + break; + } + case ConstantType::BOOL: + bool boolDefault = false; + if (defaultValue) { + if (defaultValue->getType() != JsonishValue::BOOL) { + std::cerr << "constants: BOOL constants must have BOOL default value" + << std::endl; + return false; + } + boolDefault = defaultValue->toJsonBool()->getBool(); + } + builder.constant(nameString.c_str(), type, boolDefault); + break; + } + } else { + std::cerr << "constants: the type '" << typeString + << "' for constant with name '" << nameString << "' is not a valid constant " + << "parameter type." << std::endl; + return false; + } + + return true; +} + +static bool processConstants(MaterialBuilder& builder, const JsonishValue& v) { + auto jsonArray = v.toJsonArray(); + + bool ok = true; + for (auto value : jsonArray->getElements()) { + if (value->getType() == JsonishValue::Type::OBJECT) { + ok &= processConstant(builder, *value->toJsonObject()); + continue; + } + std::cerr << "constants must be an array of OBJECTs." << std::endl; + return false; + } + return ok; +} static bool processBufferField(filament::BufferInterfaceBlock::Builder& builder, const JsonishObject& jsonObject) noexcept { @@ -1059,6 +1151,7 @@ ParametersProcessor::ParametersProcessor() { mParameters["name"] = { &processName, Type::STRING }; mParameters["interpolation"] = { &processInterpolation, Type::STRING }; mParameters["parameters"] = { &processParameters, Type::ARRAY }; + mParameters["constants"] = { &processConstants, Type::ARRAY }; mParameters["buffers"] = { &processBuffers, Type::ARRAY }; mParameters["subpasses"] = { &processSubpasses, Type::ARRAY }; mParameters["variables"] = { &processVariables, Type::ARRAY }; From 3603aaafa62cb7bd3d00e661f0d7399196460181 Mon Sep 17 00:00:00 2001 From: Benjamin Doherty Date: Tue, 11 Apr 2023 11:16:42 -0700 Subject: [PATCH 12/17] Release Filament 1.32.3 --- NEW_RELEASE_NOTES.md | 5 ----- README.md | 4 ++-- RELEASE_NOTES.md | 8 ++++++++ android/gradle.properties | 2 +- ios/CocoaPods/Filament.podspec | 4 ++-- web/filament-js/package.json | 2 +- 6 files changed, 14 insertions(+), 11 deletions(-) diff --git a/NEW_RELEASE_NOTES.md b/NEW_RELEASE_NOTES.md index 2e27f192b9a..c3161a7700b 100644 --- a/NEW_RELEASE_NOTES.md +++ b/NEW_RELEASE_NOTES.md @@ -8,8 +8,3 @@ appropriate header in [RELEASE_NOTES.md](./RELEASE_NOTES.md). ## Release notes for next branch cut -- engine: Add support for _constant parameters_, which are constants that can be specialized after material compilation. -- materials: improved size reduction of OpenGL/Metal shaders by ~65% when compiling materials with - size optimizations (`matc -S`) [⚠️ **Recompile Materials**] -- engine: fix potential crash on Metal devices with A8X GPU (iPad Air 2) -opengl: support the external image on macOS diff --git a/README.md b/README.md index c39f43a73e0..d844e6ef721 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ repositories { } dependencies { - implementation 'com.google.android.filament:filament-android:1.32.2' + implementation 'com.google.android.filament:filament-android:1.32.3' } ``` @@ -50,7 +50,7 @@ Here are all the libraries available in the group `com.google.android.filament`: iOS projects can use CocoaPods to install the latest release: ``` -pod 'Filament', '~> 1.32.2' +pod 'Filament', '~> 1.32.3' ``` ### Snapshots diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 0459a8731c9..10dc2e20376 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -7,6 +7,14 @@ A new header is inserted each time a *tag* is created. Instead, if you are authoring a PR for the main branch, add your release note to [NEW_RELEASE_NOTES.md](./NEW_RELEASE_NOTES.md). +## v1.32.4 + +- engine: Add support for _constant parameters_, which are constants that can be specialized after material compilation. +- materials: improved size reduction of OpenGL/Metal shaders by ~65% when compiling materials with + size optimizations (`matc -S`) [⚠️ **Recompile Materials**] +- engine: fix potential crash on Metal devices with A8X GPU (iPad Air 2) [⚠️ **Recompile Materials**] +- opengl: support the external image on macOS + ## v1.32.3 - fog: added an option to disable the fog after a certain distance [⚠️ **Recompile Materials**]. diff --git a/android/gradle.properties b/android/gradle.properties index d96821341b2..ad772facd67 100644 --- a/android/gradle.properties +++ b/android/gradle.properties @@ -1,5 +1,5 @@ GROUP=com.google.android.filament -VERSION_NAME=1.32.2 +VERSION_NAME=1.32.3 POM_DESCRIPTION=Real-time physically based rendering engine for Android. diff --git a/ios/CocoaPods/Filament.podspec b/ios/CocoaPods/Filament.podspec index 39c77fcad0f..f54a7ec6597 100644 --- a/ios/CocoaPods/Filament.podspec +++ b/ios/CocoaPods/Filament.podspec @@ -1,12 +1,12 @@ Pod::Spec.new do |spec| spec.name = "Filament" - spec.version = "1.32.2" + spec.version = "1.32.3" spec.license = { :type => "Apache 2.0", :file => "LICENSE" } spec.homepage = "https://google.github.io/filament" spec.authors = "Google LLC." spec.summary = "Filament is a real-time physically based rendering engine for Android, iOS, Windows, Linux, macOS, and WASM/WebGL." spec.platform = :ios, "11.0" - spec.source = { :http => "https://github.com/google/filament/releases/download/v1.32.2/filament-v1.32.2-ios.tgz" } + spec.source = { :http => "https://github.com/google/filament/releases/download/v1.32.3/filament-v1.32.3-ios.tgz" } # Fix linking error with Xcode 12; we do not yet support the simulator on Apple silicon. spec.pod_target_xcconfig = { diff --git a/web/filament-js/package.json b/web/filament-js/package.json index d354faa43b6..8e441828cb7 100644 --- a/web/filament-js/package.json +++ b/web/filament-js/package.json @@ -1,6 +1,6 @@ { "name": "filament", - "version": "1.32.2", + "version": "1.32.3", "description": "Real-time physically based rendering engine", "main": "filament.js", "module": "filament.js", From e187bc442d2fc988cb53d507069214e379364b1b Mon Sep 17 00:00:00 2001 From: Benjamin Doherty Date: Tue, 11 Apr 2023 11:19:53 -0700 Subject: [PATCH 13/17] Bump version to 1.32.4 --- README.md | 4 ++-- android/gradle.properties | 2 +- ios/CocoaPods/Filament.podspec | 4 ++-- web/filament-js/package.json | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index d844e6ef721..7e6177721d1 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ repositories { } dependencies { - implementation 'com.google.android.filament:filament-android:1.32.3' + implementation 'com.google.android.filament:filament-android:1.32.4' } ``` @@ -50,7 +50,7 @@ Here are all the libraries available in the group `com.google.android.filament`: iOS projects can use CocoaPods to install the latest release: ``` -pod 'Filament', '~> 1.32.3' +pod 'Filament', '~> 1.32.4' ``` ### Snapshots diff --git a/android/gradle.properties b/android/gradle.properties index ad772facd67..fdcf41ac21a 100644 --- a/android/gradle.properties +++ b/android/gradle.properties @@ -1,5 +1,5 @@ GROUP=com.google.android.filament -VERSION_NAME=1.32.3 +VERSION_NAME=1.32.4 POM_DESCRIPTION=Real-time physically based rendering engine for Android. diff --git a/ios/CocoaPods/Filament.podspec b/ios/CocoaPods/Filament.podspec index f54a7ec6597..5d51eec81e5 100644 --- a/ios/CocoaPods/Filament.podspec +++ b/ios/CocoaPods/Filament.podspec @@ -1,12 +1,12 @@ Pod::Spec.new do |spec| spec.name = "Filament" - spec.version = "1.32.3" + spec.version = "1.32.4" spec.license = { :type => "Apache 2.0", :file => "LICENSE" } spec.homepage = "https://google.github.io/filament" spec.authors = "Google LLC." spec.summary = "Filament is a real-time physically based rendering engine for Android, iOS, Windows, Linux, macOS, and WASM/WebGL." spec.platform = :ios, "11.0" - spec.source = { :http => "https://github.com/google/filament/releases/download/v1.32.3/filament-v1.32.3-ios.tgz" } + spec.source = { :http => "https://github.com/google/filament/releases/download/v1.32.4/filament-v1.32.4-ios.tgz" } # Fix linking error with Xcode 12; we do not yet support the simulator on Apple silicon. spec.pod_target_xcconfig = { diff --git a/web/filament-js/package.json b/web/filament-js/package.json index 8e441828cb7..93b16a6aefb 100644 --- a/web/filament-js/package.json +++ b/web/filament-js/package.json @@ -1,6 +1,6 @@ { "name": "filament", - "version": "1.32.3", + "version": "1.32.4", "description": "Real-time physically based rendering engine", "main": "filament.js", "module": "filament.js", From 0d63fa02ee47259597a5c163ed24a78b37e9d4e2 Mon Sep 17 00:00:00 2001 From: Benjamin Doherty Date: Tue, 11 Apr 2023 15:36:24 -0700 Subject: [PATCH 14/17] Fix build when exceptions disabled --- libs/filamat/tests/test_filamat.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/filamat/tests/test_filamat.cpp b/libs/filamat/tests/test_filamat.cpp index 6280ca3c42b..0102c34160e 100644 --- a/libs/filamat/tests/test_filamat.cpp +++ b/libs/filamat/tests/test_filamat.cpp @@ -833,7 +833,7 @@ TEST_F(MaterialCompiler, ConstantParameter) { } TEST_F(MaterialCompiler, ConstantParameterSameName) { -#if !defined(NDEBUG) && defined(GTEST_HAS_DEATH_TEST) +#ifdef __EXCEPTIONS EXPECT_THROW({ filamat::MaterialBuilder builder; builder.constant("myFloatConstant", ConstantType::FLOAT, 1.0f); @@ -843,7 +843,7 @@ TEST_F(MaterialCompiler, ConstantParameterSameName) { } TEST_F(MaterialCompiler, ConstantParameterWrongType) { -#if !defined(NDEBUG) && defined(GTEST_HAS_DEATH_TEST) +#ifdef __EXCEPTIONS EXPECT_THROW({ filamat::MaterialBuilder builder; builder.constant("myFloatConstant", ConstantType::FLOAT, 10); From b77aac43eae81418b90b0596a3a5c7747acb7aeb Mon Sep 17 00:00:00 2001 From: Ben Doherty Date: Tue, 11 Apr 2023 15:55:04 -0700 Subject: [PATCH 15/17] Fix float spec constant formatting (#6731) --- libs/filamat/src/shaders/CodeGenerator.cpp | 27 +++++++++++++--------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/libs/filamat/src/shaders/CodeGenerator.cpp b/libs/filamat/src/shaders/CodeGenerator.cpp index bf6d3851565..023cff92020 100644 --- a/libs/filamat/src/shaders/CodeGenerator.cpp +++ b/libs/filamat/src/shaders/CodeGenerator.cpp @@ -612,22 +612,27 @@ io::sstream& CodeGenerator::generateIndexedDefine(io::sstream& out, const char* return out; } +struct SpecializationConstantFormatter { + std::string operator()(int value) noexcept { return std::to_string(value); } + std::string operator()(float value) noexcept { return std::to_string(value); } + std::string operator()(bool value) noexcept { return value ? "true" : "false"; } +}; + utils::io::sstream& CodeGenerator::generateSpecializationConstant(utils::io::sstream& out, const char* name, uint32_t id, std::variant value) const { + + std::string constantString = std::visit(SpecializationConstantFormatter(), value); + static const char* types[] = { "int", "float", "bool" }; if (mTargetLanguage == MaterialBuilderBase::TargetLanguage::SPIRV) { - std::visit([&](auto&& arg) { - out << "layout (constant_id = " << id << ") const " - << types[value.index()] << " " << name << " = " << arg << ";\n\n"; - }, value); + out << "layout (constant_id = " << id << ") const " + << types[value.index()] << " " << name << " = " << constantString << ";\n\n"; } else { - std::visit([&](auto&& arg) { - out << "#ifndef SPIRV_CROSS_CONSTANT_ID_" << id << '\n' - << "#define SPIRV_CROSS_CONSTANT_ID_" << id << " " << arg << '\n' - << "#endif" << '\n' - << "const " << types[value.index()] << " " << name << " = SPIRV_CROSS_CONSTANT_ID_" << id - << ";\n\n"; - }, value); + out << "#ifndef SPIRV_CROSS_CONSTANT_ID_" << id << '\n' + << "#define SPIRV_CROSS_CONSTANT_ID_" << id << " " << constantString << '\n' + << "#endif" << '\n' + << "const " << types[value.index()] << " " << name << " = SPIRV_CROSS_CONSTANT_ID_" << id + << ";\n\n"; } return out; } From 89dc43f36109ff52c5f83ab56310eca298694af5 Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Tue, 11 Apr 2023 14:25:25 -0700 Subject: [PATCH 16/17] vulkan: fix spec constant bool size --- filament/backend/src/vulkan/VulkanHandles.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/filament/backend/src/vulkan/VulkanHandles.cpp b/filament/backend/src/vulkan/VulkanHandles.cpp index 5a0a0ab4bbd..5c97db4eb5c 100644 --- a/filament/backend/src/vulkan/VulkanHandles.cpp +++ b/filament/backend/src/vulkan/VulkanHandles.cpp @@ -93,7 +93,9 @@ VulkanProgram::VulkanProgram(VulkanContext& context, const Program& builder) noe pEntries[i] = { .constantID = specializationConstants[i].id, .offset = offset, - .size = sizeof(arg) + // Turns out vulkan expects the size of bool to be 4 (verified through + // validation layer). So all expected types are of 4 bytes. + .size = 4, }; T* const addr = (T*)((char*)pData + offset); *addr = arg; From 6623dcbebffe08d6c9a127bd2169816ad0835eda Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Tue, 11 Apr 2023 15:51:33 -0700 Subject: [PATCH 17/17] fix specification constant injection in glsl - boolean where handled as int - always cast float to float() --- filament/backend/src/opengl/OpenGLProgram.cpp | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/filament/backend/src/opengl/OpenGLProgram.cpp b/filament/backend/src/opengl/OpenGLProgram.cpp index f93ffdeeab3..b7a6f158d6c 100644 --- a/filament/backend/src/opengl/OpenGLProgram.cpp +++ b/filament/backend/src/opengl/OpenGLProgram.cpp @@ -40,6 +40,18 @@ static void logCompilationError(utils::io::ostream& out, static void logProgramLinkError(utils::io::ostream& out, const char* name, GLuint program) noexcept; +static inline std::string to_string(bool b) noexcept { + return b ? "true" : "false"; +} + +static inline std::string to_string(int i) noexcept { + return std::to_string(i); +} + +static inline std::string to_string(float f) noexcept { + return "float(" + std::to_string(f) + ")"; +} + OpenGLProgram::OpenGLProgram() noexcept : mInitialized(false), mValid(true), mLazyInitializationData(nullptr) { } @@ -109,7 +121,7 @@ void OpenGLProgram::compileShaders(OpenGLContext& context, for (auto const& sc : specializationConstants) { specializationConstantString += "#define SPIRV_CROSS_CONSTANT_ID_" + std::to_string(sc.id) + ' '; specializationConstantString += std::visit([](auto&& arg) { - return std::to_string(arg); + return to_string(arg); }, sc.value); specializationConstantString += '\n'; }