src/libANGLE/renderer/vulkan/FramebufferVk.cpp


Log

Author Commit Date CI Message
Shahbaz Youssefi a0b5299b 2022-04-12T00:38:50 Vulkan: Fix resolve with subpass into smaller framebuffer The condition to optimize resolve with subpass did not take into account that the resolve area must match the render pass are, neither did it disallow flipping and rotation. Bug: angleproject:7196 Bug: chromium:1314383 Change-Id: I57e50da4d6e04dfebcce3c0a5061015e5ee8773b Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3581055 Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Amirali Abdolrashidi 607d398e 2022-03-14T16:32:21 Vulkan: Optimize resolve of multisample swapchains * Resolves the multisampled image if the last render pass draws into the default framebuffer. * Added test to check the number of resolves in the optimization subpass (credit: Xinyi He) * Added test to check the number of resolves outside the subpass. * Added disabled test to see if the subpass resolve works. Bug: angleproject:6762 Change-Id: I86a8db3387851ab97d5f7a3d8a0ff26961254c14 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3523062 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Amirali Abdolrashidi <abdolrashidi@google.com>
Shahbaz Youssefi 256bbb8a 2022-04-04T22:24:55 Vulkan: Don't invalidate resolve attachment except with MSRTT Previously, resolve attachments were only used with MSRTT emulation. As such, when an attachment was invalidated, its corresponding resolve attachment was also invalidated implicitly. This changed when glBlitFramebuffer was optimized to resolve attachments, though the render pass is immediately closed and no chance is currently given to invalidation. An upcoming change needs to invalidate the multisampled attachment independently from the resolve attachment. That is fixed in this change so that the implicit invalidation is done only for MSRTT emulation. Bug: angleproject:6762 Change-Id: Ia730d4bea1f4229c8068a41b151a7af95649b606 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3569483 Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com> Auto-Submit: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi 36a051d8 2022-03-28T22:53:38 Vulkan: Move mid-RP color clear to loadOp if content undefined Instead of using vkCmdClearAttachments, if the color attachment has not been written to, modify the loadOp of the currently open renderpass to CLEAR. This is an adaptation of commit cfe5a1735a934cc83133bb6c69d19aa27278a270 The difference with that commit is that, with the prior changes that added tracking of color attachment access in the render pass, this change is greatly simplified by being able to immediately know if clear can be moved to the beginning of the render pass. Bug: angleproject:5048 Change-Id: I72b3613ad08ff869b71aced7e1f4e9be916d7b49 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3557815 Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Steven Noonan 1205a7e1 2022-03-24T22:15:44 Vulkan: always respect deferred clears in render pass I had a situation which was like this: - glBindFramebuffer(fbo) - glClear(color + depth) - series of depth-only draws: - glDrawBuffers() - disable all color buffers - glColorMask() - all false - glDrawElements/glDrawArrays - glBindFramebuffer(0) Even though the glClear happened before glDrawBuffers/glColorMask, it only got executed on the first glDraw* call. And since the draw buffers got disabled before it decided to act on the Clear, it thought it couldn't touch the color buffer in the render pass. So it ended up doing: vkCmdClearColorImage() on the color buffer vkCmdBeginRenderPass() with LoadOp C=Load, D=Clear before the draw instead of: vkCmdBeginRenderPass() with LoadOp C=Clear, D=Clear Bug: angleproject:7127 Change-Id: Ibc3b55b0c7815defcf6d711fa876eff43ba29d40 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3551298 Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi 041c4c6d 2022-03-25T16:30:03 Vulkan: Track color attachment usage like D/S in render pass That is in preparation for optimizing mid-render-pass clears, which requires an answer to the following query: "has this color image been read from / written to so far in the render pass?" With this change, a future CL will also be able to optimize color attachment invalidates, which currently break the render pass unconditionally, the same way depth/stencil is optimized. Bug: angleproject:5048 Change-Id: I3d3ee40d8444e6861c06340d5d52b17f5ee895b4 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3542989 Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Lingfeng Yang <lfy@google.com> Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi 3f331fae 2022-03-22T15:23:38 Vulkan: Dirty bits for depth/stencil access and feedback loop In preparation for doing the same for color, the depth/stencil render pass access and feedback loop modes are now updated with ContextVk dirty bits. This change also fixes clear after read-only depth/stencil feedback loop. The render pass wasn't broken in that case. Bug: angleproject:5048 Change-Id: I40f9b49593f9e6f35f42408e41c9d6267edb375e Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3542988 Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi 20e7bbb7 2022-03-18T17:03:18 Vulkan: Fix invalidate of attachments with emulated format Some image formats may be emulated such that the emulated format has more channels than the original. ANGLE clears the image once so that these extra channels contain valid values, and carefully ensures they are never modified. For swapchain images with such formats, as they are automatically invalidated at the end of the frame, a workaroud was added to make sure they are re-cleared in the beginning of the next frame. This however doesn't fix the issue of glInvalidateFramebuffer resulting in the contents of attachments with such formats to be discarded (even if the following render pass clears it, the contents are invalid in between). This change instead makes sure invalidate of images with emulated formats that have extra channels are handled appropriately: - On IMR hardware, the invalidate is dropped altogether as it provides little to no benefit. - On TBR hardware, a clear is automatically staged on the invalidated image. The latter replaces the workaround that was added to make the following render pass use loadOp=CLEAR, by adding a clear that's respected regardless of what the future usage is. This change also paves the way for a future change where the invalidate of color attachments is tracked in render passes similarly to how depth/stencil currently is. With this change, the image is no longer in an inconsistent state where its contents are considered invalid, even though some channels are meant to remain valid. Bug: angleproject:6860 Change-Id: Iec5b4854dfbe3a0bf93cd5aa82c19fe116065744 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3536389 Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Jamie Madill e8ee847d 2022-02-23T12:25:39 Vulkan: Add UpdateDescriptorSetsBuilder. This helper class encapsulates the vkUpdateDescriptorSets caching. As part of the refactor, we switch passing a ContextVk to passing a vk::Context with some mutable variables. This helps encapsulate ContextVk. Since we use the perf counters in many places, this CL moves the perf counters to vk::Context, so we can access them everywhere. Refactoring change only. Bug: angleproject:6776 Change-Id: Id529962b2f425bece6f9b3bd0cd1698c692e58cb Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3484980 Reviewed-by: Charlie Lao <cclao@google.com> Reviewed-by: Yuxin Hu <yuxinhu@google.com> Commit-Queue: Jamie Madill <jmadill@chromium.org>
Jamie Madill a956162c 2022-03-01T13:05:29 Vulkan: Expose performance counters via extension. This CL rewrites the Vulkan perf counters test to work in the angle_end2end_test suite using the newly exposed AMD extension. Note that we implement only a subset of the extension. Instead of generating monitors and starting/stopping them we simply read back all performance counter data at once using the special montior value "0". The CL also enables these tests on SwiftShader. Bug: angleproject:4918 Change-Id: I5d8f6eecb1ccff448657cbdb65b51a225dfb90c0 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3497538 Reviewed-by: Charlie Lao <cclao@google.com> Reviewed-by: Yuxin Hu <yuxinhu@google.com> Commit-Queue: Jamie Madill <jmadill@chromium.org>
Tim Van Patten 5749ec7d 2022-03-01T02:14:42 Revert "Vulkan: Move mid-RP color clear to loadOp if content undefined" This reverts commit cfe5a1735a934cc83133bb6c69d19aa27278a270. Reason for revert: https://bugs.chromium.org/p/angleproject/issues/detail?id=5048#c7 @timvp That change just caused a regression in my project. I clear the color + depth buffer before drawing, but initially draw only to the depth buffer. It seems that it decided to ignore the color buffer clear as a result of that. Original change's description: > Vulkan: Move mid-RP color clear to loadOp if content undefined > > Instead of using vkCmdClearAttachments, if the color attachment has not > been written to, modify the loadOp of the currently open renderpass to > CLEAR. > > Bug: angleproject:5048 > Test: VulkanPerformanceCounterTest.MidRenderpassClear > Change-Id: Ida47e6ac7d0f29e2c49bdf2e74c1d876a5d7c223 > Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3381912 > Reviewed-by: Jamie Madill <jmadill@chromium.org> > Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> > Commit-Queue: Tim Van Patten <timvp@google.com> Bug: angleproject:5048 Change-Id: Iec5c73632429a80f955f7d659cf670f9cbb6c9b7 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3496662 Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Commit-Queue: Tim Van Patten <timvp@google.com>
Tim Van Patten cfe5a173 2022-01-11T19:26:42 Vulkan: Move mid-RP color clear to loadOp if content undefined Instead of using vkCmdClearAttachments, if the color attachment has not been written to, modify the loadOp of the currently open renderpass to CLEAR. Bug: angleproject:5048 Test: VulkanPerformanceCounterTest.MidRenderpassClear Change-Id: Ida47e6ac7d0f29e2c49bdf2e74c1d876a5d7c223 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3381912 Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Tim Van Patten <timvp@google.com>
Shahbaz Youssefi 71d06198 2022-02-07T11:16:41 Vulkan: Fix vkCmdClearAttachments vs multiview Bug: angleproject:6262 Change-Id: I5a24693bb1b35201213c2e3fa25f7864609357b3 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3442255 Reviewed-by: Tim Van Patten <timvp@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi 02ad19e3 2022-02-07T13:46:46 Vulkan: Fix vkCmdResolveImage offsets glBlitFramebuffer takes identical regions for src and dst when resolving. vkCmdResolveImage should use the clipped area instead of using the actual offsets passed to this function. Bug: chromium:1292537 Change-Id: I13b91a4e14bdb3fcbf8f01edb36d7fb4110429ea Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3444340 Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Cody Northrop 7d7cca47 2022-01-17T18:00:25 Vulkan: Update default FBO when fetch in use If the fetch mode of the default framebuffer changes, lazily create and use a new set of framebuffers (one per swapchain image) that are setup for fetch (i.e. have a matching renderpass). Test: FramebufferFetchES31.DefaultFramebufferTest Test: FramebufferFetchES31.DefaultFramebufferMixedProgramsTest Bug: angleproject:6893 Change-Id: Iff2b73d7c34b9b8ca9429c3f24aa700c2746cc81 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3401933 Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Cody Northrop <cnorthrop@google.com>
Shahbaz Youssefi 33427a4b 2022-01-31T12:07:43 Vulkan: Fix vkCmdResolveImage extents The source framebuffer's extents were accidentally used instead of the blit area extents. Bug: chromium:1288020 Change-Id: Ib723db50d9687fee0453d027141a94ea26d8a4b8 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3427561 Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi 9d11a341 2021-12-16T13:47:04 Vulkan: Fix Vulkan SCB vs multiple subpasses vkCmdNextSubpass must be called on the primary command buffer, so the render pass command buffers need to be split on subpass boundaries. This is only done when using Vulkan secondary command buffers. Bug: angleproject:6811 Change-Id: I087fff305c757c78e87bfde4410e7de6bd1a6ba6 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3344774 Reviewed-by: Tim Van Patten <timvp@google.com> Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi acd8fc76 2021-12-16T01:05:02 Vulkan: Distinguish RP and outside-RP command buffer types What goes inside and outside a render pass command buffer is largely mutually exclusive. Moreover, the size and frequency of allocations is different between the two. This change distinguishes the C++ types used for inside and outside render pass command buffers: - The type now documents which command buffer a function is able to receive. - `isRenderPass` flag passing, checking and asserting is largely removed. - A follow up change experiments with using different (Vulkan vs ANGLE) secondary command buffers for inside and outside RP command buffers. - A future change could specialize the pool behaviors per command buffer type. Bug: angleproject:6811 Change-Id: Ia4bc669d26ac7e94e8a0dfb9b361666c82f42cc3 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3344373 Reviewed-by: Tim Van Patten <timvp@google.com> Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Charlie Lao 6da1ac81 2021-12-22T10:13:43 Vulkan: Switch ReadPixels from DynamicBuffer to shared pool FramebufferVk::readPixelsImpl() and ImageHelper::copyImageDataToBuffer() use per FramebufferVk DynamicBuffer. This CL removes this and uses shared buffer pool to allocate a temporary staging buffer for readPixels as needed and frees it immediately afterwards. Bug: b/208323792 Change-Id: I65ddf9bf9f1f14578d9def63f5287cb1a4121dff Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3354038 Reviewed-by: Tim Van Patten <timvp@google.com> Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Charlie Lao <cclao@google.com>
Shahbaz Youssefi c1598640 2021-11-30T23:44:30 Vulkan: Improve mid-RP clear warning message One app does: - Draw - Draw - Clear depth, then disable it - Draw In this case, the message generated for mid-RP clear, suggesting the clear be done earlier is not meaningful as the previous draw calls did use the depth/stencil image. The message now includes an alternative suggestion to invalidate the depth/stencil image instead of clearing it. Note that the app may still legitimately do multiple passes in one render pass where depth/stencil is cleared in between, so the warning is not applicable in all cases. It's still useful to notice issues in more common scenarios. Bug: angleproject:2472 Change-Id: I3abbecf8c83b7b856c2430675e69b1471e91c0c4 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3308920 Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Ian Elliott <ianelliott@google.com>
Shahbaz Youssefi cf8ac00a 2021-12-20T11:57:25 Vulkan: Fix MSRTT w.r.t stencil-only unresolve The stencil bit was being silently masked out in a bitset in FramebufferDesc that tracked whether the framebuffer requires any unresolve. If only stencil needs unresolving, this mask was zero, leading to an incorrect framebuffer getting pulled from the cache. A follow up change will add an ASSERT in BitSetT to catch such errors in the future. This issue was only reproducible on SwiftShader and AMD as the only implementers of VK_EXT_shader_stencil_export. Bug: angleproject:6840 Bug: angleproject:6324 Change-Id: I4f055982ebd75f621ec1e34b0d60eaa497c27b17 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3349979 Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi 55840e90 2021-12-03T15:24:00 Vulkan: Fix deferred flush vs UtilsVk Take the following scenario: 1. Draw 2. Flush (this is deferred) 3. Get image view (this is retain()ed) 4. Pass view to a draw-based UtilsVk function 5. Flush 6. Delete image view At step 4, UtilsVk may start a new render pass and use the image view from step 3. Since the flush at step 2 is deferred, it will be performed at this step, and so the serial of the image view is set to the previous submission. When step 4 uses this view, it doesn't retain it. Step 5 submits the new command buffer using this image view. At step 6, if the previous submission has finished, it will destroy the view immediately even though it's in use by the new submission. One solution could have been to make sure render pass closure originating from UtilsVk doesn't incur a flush. However, due to the current design where the render pass is immediately recorded in RendererVk's primary command buffer, it's possible that an unrelated context would perform the flush anyway. This change makes sure instead that the render pass is closed before any views are allocated/retained to be used by UtilsVk. Bug: chromium:1272266 Change-Id: I5bdefb34e03c368511c4c174cf7965fda158d2b8 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3315976 Reviewed-by: Tim Van Patten <timvp@google.com> Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi dbc0c646 2021-11-06T01:09:26 Vulkan: Output the reason for RP closure in command buffer To make it easier when viewing the command buffer in a graphics debugger, this change inserts a marker just before closing the render pass that specifies why the render pass was closed. Bug: angleproject:2472 Change-Id: I862e500cd58332d6e199c853315c560fe6a73dc2 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3265609 Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Tim Van Patten <timvp@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi d2d3a546 2021-11-11T12:22:04 Vulkan: Write perf warnings in command buffer It's much easier to understand what command the perf warning refers to when it's visible in the command buffer using a graphics API debugger. This change creates ANGLE_VK_PERF_WARNING which gives the warning both to the application (through ANGLE_PERF_WARNING) and inserts it in the command buffer. Bug: angleproject:2472 Change-Id: Ie84feed53eca5cda93e1f2bc653fcbf9bcd57b56 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3275839 Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Tim Van Patten <timvp@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Brandon Schade 2aa62964 2021-11-11T13:24:27 Vulkan: Reset mFramebuffer when mFramebufferCache is cleared The mFramebuffer pointer becomes stale when mFramebufferCache is cleared. Set mFramebuffer to nullptr when this happens. Test: --deqp-surface-type=fbo --deqp-case=KHR-GLES31.* Bug: angleproject:6682 Change-Id: I5fd21a64f0f935de04e2934e794c915ccf880c16 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3276701 Reviewed-by: Tim Van Patten <timvp@google.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Mohan Maiya <m.maiya@samsung.com> Commit-Queue: Brandon Schade <b.schade@samsung.com>
Shahbaz Youssefi ef93b32c 2021-11-09T00:26:13 Vulkan: Fix deferred clears vs invalidate In this scenario: - Clear color - Invalidate depth - Clear color The invalidate step flushed the deferred color clear, but the following clear did not expect an open render pass without any draw calls in it. This change fixes this issue, while simultaneously optimizing invalidate by making sure the clears accumulated during syncState() are redeferred instead of flushed. This issue was discovered in https://chromium-review.googlesource.com/c/angle/angle/+/3266176 where, as part of an unrelated fix, an accidental render pass closure is removed. Bug: chromium:1267424 Change-Id: Icfc0a53dbf84e6339ee23960ed847444830054e6 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3266178 Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Jamie Madill 3a9f18f1 2021-10-18T10:44:38 Refactor program pipeline handling. In preparation for moving more code from gl::Program to gl::ProgramExecutable so it can be shared with ProgramPipeline. Bug: angleproject:6566 Change-Id: Icb7ecccb37ae8e0d7d5fef8968f0dd7ef6fe6150 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3226305 Reviewed-by: Tim Van Patten <timvp@google.com> Reviewed-by: Cody Northrop <cnorthrop@google.com> Commit-Queue: Jamie Madill <jmadill@chromium.org>
Jamie Madill aac5d678 2021-10-20T11:48:57 Make "isCompute" private to ProgramExecutable. This eliminates uses of "isCompute" is the Vulkan back-end. Instead of checking the state flag, we can use the context of the current command to determine if we're running a compute or a graphics command. This will eventually lead to us being able to compile the program pipeline objects before we run a draw or dispatch command. Changes the driver uniforms descriptor desc to bind to both graphics and compute shader stages to simplify the code. This could have theoretical but low-risk performance implications. Bug: angleproject:6595 Change-Id: Ie30d419b6ece5b33f5066a034d3805fe96519b36 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3233903 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Tim Van Patten <timvp@google.com> Commit-Queue: Jamie Madill <jmadill@chromium.org>
Shahbaz Youssefi e637e4c9 2021-10-18T13:54:00 Vulkan: Optimize updating blend state in pipeline desc Updating blend funcs and equations always updated all 8 slots. Now that's only done for the attachments that are present. Bug: angleproject:6298 Change-Id: I58fa7e4dfa27d05fef54cc9d56c7b2aa5ef43dd8 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3202550 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Tim Van Patten <timvp@google.com>
Charlie Lao 6cb599f9 2021-10-06T13:07:27 Vulkan: Change dest to dst for consistency Cleanup only, no functional change. dst aligns better with src. Bug: angleproject:6502 Change-Id: I69821b1aae50a7ce647c7cc876468b6de309eec8 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3208514 Commit-Queue: Charlie Lao <cclao@google.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Cody Northrop <cnorthrop@google.com>
Shahbaz Youssefi 4ad55d07 2021-10-05T15:45:05 Vulkan: Turn 1-view multiview FBOs into no-multiview When only 1 view is enabled, ANGLE continued to append a VkRenderPassMultiviewCreateInfo struct to the render pass. VVL produced an error when transform feedback was used in this case, quoting that multiview and transform feedback cannot mix. This change makes sure that 1-view framebuffers are threated as if they were not multiview. Bug: angleproject:6478 Change-Id: If079c9a052f822342a49a9cc880be2577a356b64 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3206269 Reviewed-by: Charlie Lao <cclao@google.com> Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi eb1d5ca5 2021-01-29T15:34:49 Vulkan: Enable sync validation Based on a change by tobine@google.com Bug: angleproject:5290 Change-Id: Ieae1be5a29f0dcb3ea8aaa04e77fc402380a08b1 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3171432 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com>
Shahbaz Youssefi 371542cd 2021-08-23T23:10:09 Vulkan: Use VK_EXT_load_store_op_none With read-only depth/stencil attachments, ANGLE utilizes storeOp=NONE to optimize memory bandwidth (by avoiding write back of tile memory at the end of the render pass). Simultaneoulsy, this avoids a synchronization hazard with the next write to that depth/stencil image. If a framebuffer contains a depth/stencil attachment but it's unused, ANGLE utilizes loadOp=NONE/storeOp=NONE to effectively remove any memory bandwidth wasted on the attachment. Bug: angleproject:5371 Change-Id: I76cbadbf1194041532ac4b690ffe087298f2de51 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3114232 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com>
Charlie Lao 8ea87a67 2021-08-17T18:46:36 Vulkan: Avoid texture format fallback when possible Some texture formats are not renderable on some hardware. For example, R4G4B4A4 are not renderable on nvidia and not blendable on ARM. R5G5B5A1 are also not blendable on nvidia. Right now when we generate format table, we are being most conservative, picking an actual format that is always renderable and blendable. This means when R4G4B4A4 is used on one of these GPUs, we are always falling back to R8G8B8A8 regardless if the texture is actually being used as color attachment or not. This CL adds a actualRenderableImageFormatID field in vk::Format. Initially we will pick actualImageFormatID which only ensures texture sample capability. If later on the texture is being attached to FBO, then we will switch to actualRenderableImageFormatID and do data copy if necessary. This way we save memory and reduce texture bandwidth for most usage of these textures. For renderBuffer and surfaces and EGLImages, we always pick the renderable textures. Bug: b/196456356 Change-Id: I02eec3365c2a317b0d1bad6dbdc3e741114c5bba Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3104514 Reviewed-by: Ian Elliott <ianelliott@google.com> Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Charlie Lao <cclao@google.com>
Charlie Lao 6977fa6f 2021-08-31T09:47:59 Vulkan: Store actualFormatID (not intendedFormat) in RenderPassDesc Today we store intendedFormatID in the RenderPassDesc. At unpack time, we convert intendedFormatID to vk::Format and then get the actual VkFormat. This is a bit complex and unnecessarily confusing. And this will be very error prone in the future when vk::Format has two actual image formats. This CL packed actualFormatID into RenderPassDesc and converts to VkFormat directly. Once packed in the RenderPassDesc, we never needs to reference to intendedFormat or vk::Format since all these does not matter. The only format matters is actualFormatID once you packed into the RenderPassDesc. This simplifies the logic and prepare for the future CLs. Bug: b/196456356 Change-Id: Ia282115c824e3ec446d2be15b40b1e2974b99afa Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3133761 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Ian Elliott <ianelliott@google.com> Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Charlie Lao <cclao@google.com>
Tim Van Patten 6eeab192 2021-08-27T16:26:25 Vulkan: Don't defer clear for read render target We shouldn't collect deferred clears for the read render target because they get applied to the draw render target. This CL flushes stages updates to the read render target only if it's not the same as the draw render target. This can happen when the read render target is bound to another surface. Bug: b/192327017 Change-Id: I7c9e804f4eff10728aed7aeeaf41ef3869c9bdbd Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3125462 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Tim Van Patten <timvp@google.com>
Charlie Lao e98539f0 2021-08-17T14:42:26 Vulkan: Add ImageHelper::getIntendedFormatID() This adds helper API to return intendedFormat directly from ImageHelper object instead of vk::Format, to make API symmetrical. It is also necessary. It is also needed in some places where we no longer have access to vk::Format any more due to refactoring. Bug: b/196456356 Change-Id: Ie0502793623138ded28c3f01320c57ffea2d93df Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3101925 Reviewed-by: Ian Elliott <ianelliott@google.com> Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Charlie Lao <cclao@google.com>
Charlie Lao cf24931a 2021-08-17T10:48:23 Vulkan: Add ImageHelper::getActualFormat() This is preparation for future CLs. In the future vk::Format may not tell you what actual format is. This CL adds a new method of ImageHelper::getActualFormatID() and ImageHelper::getActualFormat() so that we can use these two APIs and avoid using vk::Format, thus reduce reliance on vk::Format. Bug: b/196456356 Change-Id: Ic50e664e033feb5e066f40269c33cffe96024172 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3100319 Commit-Queue: Charlie Lao <cclao@google.com> Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Ian Elliott <ianelliott@google.com>
Jason Macnak 0be285c6 2021-07-20T11:36:22 Vulkan: Flush read RT if color attachment is the read buffer Prior to this change, the following sequence: * eglMakeCurrent(..., draw=surface1, read=surface1, ...); * glClear(...); * eglMakeCurrent(..., draw=surface2, read=surface1, ...); * glBlitFramebuffer(...); * eglMakeCurrent(..., draw=surface1, read=surface1, ...); * glReadPixels(...); would end up with the `vkCmdClearColorImage()` on surface1 occuring after the `vkCmdBlitImage()`. This CL updates flushColorAttachmentUpdates() to flush any staged updates to both the read and draw attachments, since they can be different if different read and draw surfaces are bound. Adds a test which is a small repro of android.opengl.cts.FramebufferTest#testBlitFramebuffer failure. Bug: b/192327017 Test: EGLSurfaceTest.BlitBetweenSurfaces/* Change-Id: Iabad26dfcd8633e9dcfcee2fb16ba352bc3931d5 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3078980 Commit-Queue: Tim Van Patten <timvp@google.com> Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Geoff Lang <geofflang@chromium.org>
Shahbaz Youssefi cf090996 2021-08-05T16:20:39 Vulkan: Call onFramebufferChange from a single place When sRGB control mode was dirty, onFramebufferChange was called which may not have been necessary if the actualy state hadn't changed. This removes a call to onFramebufferChange() in this path, and leaves it to be naturally called if the framebuffer desc actually changes. Bug: angleproject:5075 Change-Id: I177572a3cb819d7e1ecd589f46e03da4b967529e Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3076619 Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Tim Van Patten <timvp@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi 5b314268 2021-06-15T17:37:45 Vulkan: Support OVR_multiview and OVR_multiview2 Multiview is supported in Vulkan simply by specifying the number of views in the render pass, and creating the appropriate image views. A number of changes to the way image views and render targets are stored are made to support those that don't cover the entire range of layers. One particular detail that is not implemented in this change is the use of queries in combination with multiview. Vulkan specifies that N queries are actually produced (N being the number of views) which must be summed by the application, but this is not currently done. Bug: angleproject:6048 Change-Id: I1d4a9894c232d3a93d7a97c9fa0eedc334e57469 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2967625 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com>
Ian Elliott ebf75698 2021-06-10T18:55:04 Vulkan: Fix AGI clear hierarchy bug for clear commands This approach properly handles outside-render-pass clears. Bug: b/190622922 Change-Id: Ia4a9d6ec13d7da8c4a445af1127e82c03f37e8b2 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2951960 Reviewed-by: Mark Lobodzinski <mark@lunarg.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Ian Elliott <ianelliott@google.com>
Mark Lobodzinski 4bafc503 2021-06-01T10:59:17 Vulkan: Fix AGI hierarchy for clear commands Treat mid-render-pass glClear* commands the same as glDraw* commands, generating a hierarchy. This results in vkCmdClearAttachment commands being nested under glClear, instead of being a peer of glDraw* commands. Bug: b/183547523 Change-Id: Ibc6900b0485fd174d79c8fe6c94ea17dbefa520b Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2941364 Reviewed-by: Mark Lobodzinski <mark@lunarg.com> Reviewed-by: Mike Schuchardt <mikes@lunarg.com> Reviewed-by: Ian Elliott <ianelliott@google.com> Commit-Queue: Ian Elliott <ianelliott@google.com>
Shahbaz Youssefi bcb678a5 2021-05-27T16:50:55 Vulkan: Fix deferred clears vs 3D textures Two bugs are fixed in this change. One is that framebuffer attachments to 3D textures should not attempt to defer clears. The clear staged for the 3D texture applies to all slices, not just the slice the framebuffer is attached to (and that would get cleared through deferred clears). Secondly, when clearing an attachment to a 3D texture, the clear must be applied immediately through a render pass loadOp to affect only the slice that's attached. This was already handled for layered framebuffers where the clear was applied immediately if the 3D texture render target had more layers than the framebuffer. The condition for this is generalized to check whether the 3D texture has more slices (regardless of whether the render target is layered or not). Test is based on one written by Austin Eng <enga@chromium.org> Bug: angleproject:5967 Change-Id: I43cf5fc24673323eda8390021641e2238be6e375 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2923785 Reviewed-by: Charlie Lao <cclao@google.com> Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Geoff Lang bccb0d56 2021-05-11T13:53:05 Add messages for framebuffer completeness errors. This also creates a common code path for all framebuffer completeness errors (FramebufferStatus::Incomplete) which helps for adding a debug breakpoint. Bug: angleproject:5949 Change-Id: Ib102dbf86e020777e56c6dc6b78dda8ebdba2127 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2888110 Commit-Queue: Geoff Lang <geofflang@chromium.org> Reviewed-by: Jonah Ryan-Davis <jonahr@google.com> Reviewed-by: Jamie Madill <jmadill@chromium.org>
Charlie Lao a37d9748 2021-04-13T14:52:31 Vulkan: Add support for FBO with unequal sized attachments OpenGLES 3.0 allows FBO with unequal sized attachments. This CL removes assertion that all attachment must have equal size from vulkan backend, and uses common intersect area to create VkFramebuffer object. Bug: b/181800403 Change-Id: Icbb12a26784b184ebd91740855672013f64b889d Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2824760 Commit-Queue: Charlie Lao <cclao@google.com> Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Tim Van Patten <timvp@google.com>
Mohan Maiya f28d63e9 2021-03-24T11:14:24 Vulkan: Enable glBlitFramebuffer for EXT_sRGB_write_control Implements support for the glBlitFramebuffer edge case in the EXT_sRGB_write_control spec, and fully exposes this extension. Bug: angleproject:5075 Test: SRGBFramebufferTest*.*Vulkan* Change-Id: I05f044abbc5cb272825d1fc4b9028217f18e63c2 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2785641 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Mohan Maiya <m.maiya@samsung.com>
Shahbaz Youssefi d1462228 2021-03-09T11:25:36 Vulkan: Use VK_EXT_multisampled_render_to_single_sampled Additionally, makes the emulation path not require independentResolveNone. This was only used to select the NONE resolve mode when the attachment format doesn't have either of depth or stencil aspects, but it's ok to specify the same resolve mode for both aspects even if one aspect is missing. Bug: chromium:1088005 Change-Id: Ifc37cbf5331145179c5927853b996a0d62b871ee Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2743666 Reviewed-by: David Reveman <reveman@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Tim Van Patten <timvp@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Charlie Lao da437f26 2021-03-08T19:08:14 Vulkan: Defer color image layout changes at endRenderPass time Right now color render target's image's layout change are done at beginRenderPass time. The problem is that the layout also depends on whether texture is also being used as a sampler or not. That information is not known when renderpass starts. We did some special treatment for depth stencil attachment so that its layout determination is deferred until endRenderPass time. This CL expands that same mechanism to color attachment as well. Right now the color attachment will still pick the same ImageLayout::ColorAttachment layout since the logic to detect it is also used for texture sampling is not there yet. Bug: b/175584609 Change-Id: Id7486174d475f894461578b31d0d40fdd90e808a Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2744121 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Tim Van Patten <timvp@google.com> Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Charlie Lao <cclao@google.com>
Mohan Maiya 81dcf078 2021-03-08T11:21:31 Reland "Vulkan: Support EXT_sRGB_write_control" This is a reland of 6073af536cf627742696823edc82c9b0a481a8bc with 2 changes - 1. Don't enable the extension even in nonConformant mode 2. Don't enable VK_KHR_image_format_list for swiftshader Original change's description: > Vulkan: Support EXT_sRGB_write_control > > Implement support for EXT_sRGB_write_control. This extension > requires VK_KHR_image_format_list to be supported. > > The spec requires this functionality to work with glBlitFramebuffer > as well but support for that will be added in a follow up change. > As such, this extension is only exposed in non-conformant mode. > > Bug: angleproject:5075 > Tests: SRGBFramebufferTest.*Vulkan* > Change-Id: I59b38f6cd810a3d0d67ec29f4f19c25f65f70862 > Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2617243 > Commit-Queue: Mohan Maiya <m.maiya@samsung.com> > Reviewed-by: Jamie Madill <jmadill@chromium.org> > Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Bug: angleproject:5075 Change-Id: I8e149d196a39c3c4769bfa8690792f3c53831299 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2762647 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Mohan Maiya <m.maiya@samsung.com>
Peng Huang b27740f3 2021-03-09T16:15:15 Revert "Vulkan: Support EXT_sRGB_write_control" This reverts commit 6073af536cf627742696823edc82c9b0a481a8bc. Reason for revert: crbug.com/1186140 Original change's description: > Vulkan: Support EXT_sRGB_write_control > > Implement support for EXT_sRGB_write_control. This extension > requires VK_KHR_image_format_list to be supported. > > The spec requires this functionality to work with glBlitFramebuffer > as well but support for that will be added in a follow up change. > As such, this extension is only exposed in non-conformant mode. > > Bug: angleproject:5075 > Tests: SRGBFramebufferTest.*Vulkan* > Change-Id: I59b38f6cd810a3d0d67ec29f4f19c25f65f70862 > Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2617243 > Commit-Queue: Mohan Maiya <m.maiya@samsung.com> > Reviewed-by: Jamie Madill <jmadill@chromium.org> > Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Bug: angleproject:5075,chromium:1186140 Change-Id: Ib0d4d60fe7434fb950f99db2c210aab9af7d2d0e No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2743663 Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Commit-Queue: Peng Huang <penghuang@chromium.org>
Mohan Maiya 6073af53 2021-03-08T11:21:31 Vulkan: Support EXT_sRGB_write_control Implement support for EXT_sRGB_write_control. This extension requires VK_KHR_image_format_list to be supported. The spec requires this functionality to work with glBlitFramebuffer as well but support for that will be added in a follow up change. As such, this extension is only exposed in non-conformant mode. Bug: angleproject:5075 Tests: SRGBFramebufferTest.*Vulkan* Change-Id: I59b38f6cd810a3d0d67ec29f4f19c25f65f70862 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2617243 Commit-Queue: Mohan Maiya <m.maiya@samsung.com> Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi e366e2c3 2021-02-27T01:00:02 Vulkan: Keep dynamic buffer's free list trimmed ContextVk's staging buffer never gets a chance to free its free buffer list. During application load time, a large amount of memory may be allocated from this buffer to stage texture updates and they would remain throughout the life of the application. This change ensures that the free buffer list doesn't grow unbounded. In the Manhattan trace, this saves >1GB of memory on Linux. There are now three policies for vk::DynamicBuffer: - Always reuse buffers: This is useful for dynamic buffers that make frequent small allocations, such as default uniforms, driver uniforms, default vertex attributes and UBO updates. - Never reuse buffers: This is for situations where the buffer is unlikely to be used after some initial usage, such as texture data upload or vertex format emulation (as the conversion result is cached, so it's never redone). - Limited reuse of buffers: For the staging buffer in the context which is shared by all immutable texture data uploads, it's useful to keep a limited number of buffers (1 in this change) to support future texture streaming while allowing a large number of buffers allocated in a burst to be discarded. Bug: angleproject:5690 Change-Id: Ic39ce61e6beb3165dbce4b668e1d3984a2b35986 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2725499 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Tim Van Patten <timvp@google.com> Reviewed-by: Charlie Lao <cclao@google.com> Reviewed-by: Jamie Madill <jmadill@chromium.org>
Ian Elliott 6af0c03f 2021-02-25T18:15:21 Vulkan: Always write to swapchain alpha channel This fixes an issue with the "Ragnarok M: Eternal Love" game, which uses a GL_RGB8 format for its window, which is actually backed by a GL_RGBA8 format (a.k.a. "emulated alpha"). The game does no explicit clear per frame. Therefore, ANGLE selects a render pass loadOp=DONT_CARE, which leaves the alpha channel undefined (0.0 on a Pixel 4 XL). That causes SurfaceFlinger to show (blend or alternate vsyncs) what should have been covered up by the game (e.g. the Android launcher and live wallpaper). The solution is to prevent loadOp=DONT_CARE for emulated alpha. Test: ClearTest.ChangeFramebufferAttachmentFromRGBAtoRGB Test: dEQP-GLES2.functional.fbo.render.stencil_clear.tex2d_rgb_stencil_index8 Bug: b/180139027 Change-Id: Ied97b57c93d41326cb3294ff246691e09f316791 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2704949 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Tim Van Patten <timvp@google.com> Commit-Queue: Ian Elliott <ianelliott@google.com>
Mohan Maiya 550f2a3e 2021-02-24T09:49:42 Vulkan: Shader support for EXT_shader_framebuffer_fetch_non_coherent Translator can accept gl_LastFragData and 'inout' variable to gain access to framebuffer attachment data. The Vulkan translator replaces it with the SubpassInput type variable. Note that this works only for the noncoherent version of the extension. Bug: angleproject:5454 Test: *EXTShaderFramebufferFetchNoncoherent*.* Change-Id: I392f84ee3ad3eb9fbd09d0b7ff83731a9a3f33f6 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2598060 Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Mohan Maiya <m.maiya@samsung.com>
Shahbaz Youssefi 39d7fc18 2021-02-17T00:18:41 Vulkan: Don't break the render pass on dispatch calls The only reason a dispatch call may need to break the render pass implicitly is for read-after-writes where the write originates from the render pass but is not through a storage buffer/image. There are only two such scenrios possible: - Framebuffer attachment write -> texture sample - Transform feedback write -> ubo read All other uses of the buffers and textures that require breaking the render pass are handled by `glMemoryBarrier`. Bug: angleproject:5070 Change-Id: I92b50d69d8782097ee8ff477ac57da6209c326a1 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2698998 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Tim Van Patten <timvp@google.com>
Mohan Maiya 907a3cee 2021-02-17T08:07:45 Vulkan: Add support for EXT_shader_framebuffer_fetch_non_coherent EXT_shader_framebuffer_fetch_non_coherent is implemented using subpass input attachments. The extension will be enabled in a follow up change that adds required changes to the Vulkan translator. Bug: angleproject:5454 Test: FramebufferFetchNonCoherentES31.*Vulkan Change-Id: Ic73c66a476c4a21db5269431166a198841f1dc0c Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2598059 Commit-Queue: Mohan Maiya <m.maiya@samsung.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org>
Shahbaz Youssefi 30622479 2021-02-16T12:33:40 Vulkan: Fix crash with deferred clears and MSRTT The following scenario was mishandled: - MSRTT draw with an unresolve operation (i.e. has two subpasses) - Deferred clear - Flush deferred clear with MSRTT framebuffer not needing unresolve (i.e. has one subpass) Bug: chromium:1178693 Change-Id: If3548e99897d698d61dfafbe9f86193723d06e5a Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2697648 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org>
Shahbaz Youssefi 45a493ea 2021-02-05T13:48:48 Vulkan: Use a dirty bit to start the render pass Some dirty bits need to run before the render pass starts. An upcoming change for example needs to break the render pass when the program pipeline is changed while transform feedback is active. Another upcoming change may need to do the same based on a preceding glMemoryBarrier. This change adds a new dirty bit to start the render pass after some dirty bits have already been processed. Bug: angleproject:5528 Change-Id: I993c9efefed4c8fee268b218a8dd66a582d4e7cd Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2678863 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Tim Van Patten <timvp@google.com>
Jamie Madill 2e9706d8 2021-01-08T17:29:42 Vulkan: Use angle::FormatID instead of VkFormat. This change switches the internal enums we pass around from VkFormat to FormatID. The end goal of the refactor is to allow the Vulkan back-end to store packed tables indexed by FormatID. Because VkFormat has large gaps in its enum space we'd otherwise need to use unordered data structures like unordered_map. The change removes the redundant VkFormat storage from vk::Format and uses a new table query to return the VkFormat that 1:1 matches an angle::FormatID. We also include a reverse mapping for use with native Vulkan get functions for Android. Also moves sRGB conversion functions into renderer_utils. A couple sRGB formats that don't exist in GL are no longer handled by the sRGB conversion functions. These formats should be extremely rare. Bug: angleproject:5438 Change-Id: Id8b49773ca0c556f9f5a6a10fcf0d9762b93bbea Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2618204 Commit-Queue: Jamie Madill <jmadill@chromium.org> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Ian Elliott <ianelliott@google.com>
Stephan Hartmann d21d682d 2021-01-11T12:59:53 libstdc++: fix incomplete type for FramebufferCache libstdc++ does not allow incomplete type for T2 with std::pair<T1,T2> and fails with: .../../src/libANGLE/renderer/vulkan/vk_cache_utils.h:1570:64: required from here /usr/lib/gcc/x86_64-pc-linux-gnu/9.3.0/include/g++-v9/bits/stl_pair.h:215:11: error: std::pair<_T1, _T2>::second has incomplete type 215 | _T2 second; /// @c second is a copy of the second object | ^~~~~~ https://chromium-review.googlesource.com/c/angle/angle/+/2580111 added class FramebufferCache with incomplete type in |mPayload| to vk::FramebufferHelper. Changing include order is not an option. However, FramebufferCache is only used in FramebufferVk and we can make it local there. Bug: chromium:957519 Change-Id: I5fbdca23adbb9f4aecc266988c02fb0d051504cb Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2621473 Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Jamie Madill <jmadill@chromium.org>
Jamie Madill e91097bf 2020-12-29T14:05:56 Vulkan: Clean up "actual"/"intended" naming. Clarifies that the GL internal format is an "intended" format and the Vulkan formats are "actual" formats. This makes all the format fields use the same consistent naming pattern. Bug: angleproject:5438 Change-Id: I935a49895109e9e06eae5ef98d5614dfd1128ff8 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2605728 Reviewed-by: Tim Van Patten <timvp@google.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Jamie Madill <jmadill@chromium.org>
Mohan Maiya 98b56e60 2020-12-12T16:28:21 Vulkan: Accumulate internal cache stats in renderer The CacheStats of all internal caches are accumulated by the renderer. In order to see the hit ratios of all caches, the following GN args must be enabled: is_debug = true angle_enable_perf_counter_output = true Bug: angleproject:5447 Test: Manual verification with angle_end2end_tests Change-Id: Iaca3249192e9e4e130d8291b7759c459d79b06ee Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2588430 Commit-Queue: Mohan Maiya <m.maiya@samsung.com> Reviewed-by: Tim Van Patten <timvp@google.com> Reviewed-by: Jamie Madill <jmadill@chromium.org>
Mohan Maiya 46eaba7f 2020-12-12T10:31:26 Vulkan: Add support for internal cache hit and miss counts Add a CacheStats class that provides cache hit and miss bookkeeping. All internal caches make use of this class to keep track of its stats. This provides a means to profile cache hit ratios a.k.a Vulkan object reuse for any application. Bug: angleproject:5447 Test: Manual verification with angle_end2end_tests Change-Id: I44eeb0c2b9b291ec1cdd156fb2be4a5fe80d2848 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2580111 Commit-Queue: Mohan Maiya <m.maiya@samsung.com> Reviewed-by: Tim Van Patten <timvp@google.com> Reviewed-by: Jamie Madill <jmadill@chromium.org>
Mohan Maiya da8c2261 2020-12-12T16:39:55 Vulkan: Enable FramebufferVk cache on Apple This effectively reverts ff60aba. The crash no longer occurs on Apple. Bug: angleproject:4442 Change-Id: I4aa745c80a482eb99311f3810e34124afe950cfe Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2588429 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Mohan Maiya <m.maiya@samsung.com>
Shahbaz Youssefi 56330564 2020-12-10T00:46:04 Vulkan: Support layered framebuffers This feature is introduced by geometry shaders, where all the layers of a texture can be attached to a framebuffer. The geometry shader would use gl_Layer to decide which layer the primitive should be rendered to. Bug: angleproject:3571 Change-Id: Ib2ae8e227b226295f9e2f62f6b230839070bc95c Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2582711 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Tim Van Patten <timvp@google.com>
Shahbaz Youssefi 0d2de6f0 2020-12-03T04:29:02 Vulkan: Fix precision issue in blit math Bug: chromium:1154759 Change-Id: If31ef7ebecdfa2a0cba91e917870ea0bdfd9b9db Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2570464 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jonah Ryan-Davis <jonahr@google.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi 46107d3e 2020-11-18T22:03:22 Vulkan: Delay defining D/S content to endRenderPass Take the following situation: 1. Start RP with D/S undefined: loadOp = DONT_CARE, storeOp = STORE * At this point, onDepthStencilWrite calls image->onWrite, setting depth/stencil contents defined. 2. At endRP, observe depth/stencil is not used: storeOp = DONT_CARE 3. Start another RP with D/S: loadOp = LOAD, storeOp = STORE Because the call to image->onWrite was done at startRP, the contents of the depth/stencil image is marked as defined, and the next render pass is loading these data. This change moves image->onWrite to endRenderPass, and only calls it if storeOp = STORE, taking advantage of all the opportunistic optimizations that try to set storeOp to another value. Bug: angleproject:4836 Change-Id: I9858e5caa6b1f67f841a5c6356e66927356ef469 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2548319 Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi 86d7e4d8 2020-11-10T15:55:34 Vulkan: Support texture buffers Texture buffers are placed in the same descriptor set with the rest of the textures. However, the different code paths that handle textures have special cases for texture buffers as they create a different descriptor type (texel buffer instead of combined image sampler). Image view serials are used to track the buffer view serials as well so the texture descriptor cache can handle texture buffers as well. This CL is missing storage texel buffer support. Bug: angleproject:3573 Change-Id: Iff80ca22ff9b9957a0c9a3c7aaada1fa54b24ec8 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2532653 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Tim Van Patten <timvp@google.com>
Amy Liu b10a0dfc 2020-11-12T14:51:05 Fix flip info of colorBlit with pre-rotation. Get wrong colorBlit results on android if there is flip operation added by glBlitFramebuffer API. Fix the implementation and add related end2end tests. Bug: angleproject:5044 Change-Id: I797f8858b3943b5effe27261e954ca1405960ef0 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2534210 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Ian Elliott <ianelliott@google.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi 9e7f08fc 2020-11-06T16:55:51 Vulkan: Fix incorrect reordering of barriers Take the following situation, a simple copy from image to buffer: ANGLE_TRY(contextVk->onBufferTransferWrite(buffer)); ANGLE_TRY(contextVk->onImageTransferRead(aspectFlags, image)); CommandBuffer &commandBuffer = contextVk->getOutsideRenderPassCommandBuffer(); commandBuffer.copyImageToBuffer(imageHandle, layout, bufferHandle, 1, regions); Both `onBufferTransferWrite` and `onImageTransferRead` may flush either the outsideRP or insideRP command buffers. If buffer is not previously used, but image is used: - onBufferTransferWrite: buffer usage is recorded in outsideRP1 - onImageTransferREad: outsiderRP1 is flushed, outsideRP2 is started - copyImageToBuffer: recorded on outsideRP2, but buffer usage not recorded there - A following command that uses the buffer and requires barrier doesn't close outsideRP2 as it believes it was not used there Bug: angleproject:5319 Change-Id: Ib8994083fbc21969a538cda3784adee57b089415 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2523388 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Tim Van Patten <timvp@google.com>
Shahbaz Youssefi 3dfaeeb0 2020-10-30T16:57:44 Vulkan: Don't flush deferred clears on READ framebuffer sync Deferred clears are not collected when syncing the READ framebuffer. Prior to this change, we had in FramebufferVk::syncState: if (READ && deferredClears.any()) { flushDeferredClears(); } However, this is impossible / unnecessary: - Every operation whose syncState (for the DRAW framebuffer) collects deferred clears will flush the deferred clears. In fact, it's an error for the next operation's syncState to encounter pre-existing deferred clears. - The READ framebuffer is synced before the DRAW framebuffer. This makes it impossible for there to be deferred clears when READ is synced. It may be necessary to swap the order in which the READ and DRAW framebuffers are synced. See http://anglebug.com/5266. In that case, if the READ and DRAW framebuffers are identical: - The DRAW framebuffer's sync will collect deferred clears. The READ framebuffer's sync will see deferred clears, but it must not flush them! Bug: angleproject:4988 Change-Id: I179002d739608ccb8bda95d4379dc6d54e2bf4bb Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2511372 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Tim Van Patten <timvp@google.com> Reviewed-by: Jamie Madill <jmadill@chromium.org>
Shahbaz Youssefi 2d964a47 2020-10-30T16:46:30 Vulkan: Defer clears even if following command is scissored Take the following scenario: 1. glClear 2. glScissor(half of framebuffer) 3. glDrawArrays The clear in step 1 is deferred. When FramebufferVk::syncState is called in step 3, the deferred clear was applied using vkCmdClearColorImage because the draw call is scissored. This causes loadOp=LOAD to be used after the clear because the render pass is started too small (the same size as the scissor). This change makes scissored operations also take advantage of loadOp=LOAD with deferred clears. A number of changes are made to this effect: - FramebufferVk::syncState no longer limits collecting deferred clears to no-scissor. - FramebufferVk::startNewRenderPass automatically expands the render area to full size if it's clearing any attachment. - A number of bugs are fixed where FramebufferVk::flushDeferredClears is called with the scissor area. Instead, flushDeferredClears now unconditionally uses the complete render area. Note that these bugs didn't have symptoms as "scissor" and "deferred clears" were mutually exclusive. Bug: angleproject:4988 Change-Id: I24fc3d88bf9c8998869b36c863692d0f0acce994 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2511371 Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Tim Van Patten <timvp@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi a0e91016 2020-10-30T10:01:36 Vulkan: Don't break the render pass on scissor change Prior to this change, the render area was decided when the render pass was started, and remained fixed. If a small scissor was initially used, this created a render pass with a small area. If then the scissor region was expanded, the render pass was broken. This change instead expands the render area on scissor change to avoid breaking the render pass. If glInvalidateSubFramebuffer previously successfully resulted in storeOp=DONT_CARE, this optimization may need to undo that. As a result, the invalidate area is stored in the render pass and if the render area grows beyond that, invalidate is undone. Bug: angleproject:4988 Change-Id: I4e8039dec53a95a193a97cb40db3f71e397568d6 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2508983 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Tim Van Patten <timvp@google.com> Reviewed-by: Jamie Madill <jmadill@chromium.org>
Shahbaz Youssefi 265c5fa9 2020-11-02T21:50:25 Vulkan: Fix scissor update in FramebufferVk::syncState A previous change [1] made FramebufferVk::syncState update scissor and rasterization samples only when the DRAW framebuffer is synced. This is incorrect as the READ framebuffer is synced before the DRAW framebuffer, and if the two are the same, the latter is discarded. Very few functions sync both READ and DRAW framebuffers when they are identical. A test is tailored to expose this bug. [1]: https://chromium-review.googlesource.com/c/angle/angle/+/2510013 Bug: angleproject:4988 Change-Id: I6123ac18dded938171bc90a04d4d81f1b42a1694 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2515742 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Tim Van Patten <timvp@google.com>
Shahbaz Youssefi 8afe3f17 2020-11-03T10:55:31 Vulkan: Fix use of uninitialized data in staged clears When depth/stencil data are staged, only the depthStencil field of VkClearValue is initialized. However, when comparing staged clears, memcmp is used which also compares the extra bytes in the aliasing color field. Bug: chromium:1144491 Change-Id: Ic384ba792e9fd199d8e9c3e534ccdc6ea65ee9b8 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2517244 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org>
Shahbaz Youssefi 7dc92430 2020-10-30T13:05:05 Noop clear of non-existing attachments. Bug: angleproject:4988 Change-Id: Ib6ff9756ec7ae5aa2b11f4d12932829fe05656d6 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2511369 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Tim Van Patten <timvp@google.com>
Shahbaz Youssefi 42e10d3e 2020-10-30T16:36:45 Noop empty-scissor clears Bug: angleproject:4988 Change-Id: I64909292927e20c65141302c9bf5e7ef09af84b7 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2511370 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Tim Van Patten <timvp@google.com>
Shahbaz Youssefi 57f7c9b5 2020-10-29T16:31:46 Vulkan: Fix prerotation bug with glInvalidateSubFramebuffer The area passed to FramebufferVk::invalidateSub is not rotated, but was being compared with the rotated framebuffer dimensions / render area. Bug: angleproject:5264 Change-Id: I2de181bf77ad650418b757a3848395bbdab13d8a Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2508978 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Tim Van Patten <timvp@google.com> Reviewed-by: Ian Elliott <ianelliott@google.com> Reviewed-by: Jamie Madill <jmadill@chromium.org>
Jamie Madill 97843bda 2020-10-30T17:03:36 Vulkan: Fix EGL Surface robust init. The error here was related to using a single cache variable for the robust init setting for all the surfaces in a DisplayVk. Fix this by passing down the robust init setting from the SurfaceVk to image init. Bug: angleproject:5274 Change-Id: I9bc9c20990268d1d5166411fb53f8f2593fd1971 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2510694 Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Jamie Madill <jmadill@chromium.org>
Shahbaz Youssefi 9c66a092 2020-10-30T09:56:19 Vulkan: Update scissor only in DRAW framebuffer's syncState Bug: angleproject:4988 Change-Id: I16f9268cdc221c84f962bbb9bd06ef5b19a6ac05 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2510013 Reviewed-by: Tim Van Patten <timvp@google.com> Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi 8f36b846 2020-10-29T15:13:55 Vulkan: Optimize glFramebufferSubInvalidate If glFramebufferSubInvalidate() is called with an area that covers the whole framebuffer, behave as if glFramebufferInvalidate() is called. This allows deferred clears to be removed for example, and attachment contents to be marked undefined. Bug: angleproject:4988 Change-Id: Iff3f291ea6c07abccc2740174d0451b432ac5da8 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2508977 Reviewed-by: Charlie Lao <cclao@google.com> Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Alexey Knyazev 1c7f0284 2020-10-21T01:40:08 Vulkan: Fix invalid clamping of ES3 clear stencil values Added 3 new end2end tests Bug: angleproject:5202 Change-Id: I95f9ffd989105f5bd3283676d6fa46e904503369 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2488481 Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Alexey Knyazev <lexa.knyazev@gmail.com>
Shahbaz Youssefi c55cd6b4 2020-10-14T23:06:27 Vulkan: Remove dead path in clear If the clear is not mid render pass, clearWithLoadOp was used to either: - modify the current render pass loadOps, assuming no rendering has been done, or - defer the clears by staging them in the attachment images. The former path however is dead code. It's impossible to start the render pass without recording any commands. In other words, if the render pass has already started, the clear must be mid RP. Bug: angleproject:4836 Change-Id: Idb1cb37b8a0e56b897ac69cf435f9a52be4bd2f4 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2473764 Reviewed-by: Charlie Lao <cclao@google.com> Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Tim Van Patten 16102e8b 2020-10-17T01:15:05 Reland "Vulkan: Fold deferred clears into current clears" This reverts commit 37c400146e59f718b516eb58e16fa53c8a88bf21. Reason for revert: Need to clear the package cache when bisecting. Original change's description: > Revert "Vulkan: Fold deferred clears into current clears" > > This reverts commit e416c92a81c9ef01d633ec5c05e81c2551b6c5d6. > > Reason for revert: Reverted parent: > https://chromium-review.googlesource.com/c/angle/angle/+/2481612 > > Original change's description: > > Vulkan: Fold deferred clears into current clears > > > > If there are clears prior to a glClear() call, those clears were > > flushed (starting a new render pass) and then the clear call's clears > > would be applied (essentially modifying the loadOps of said render > > pass). > > > > The main downside of the above is that the current glClear() clears > > don't get a chance to be deferred. This was observed in Chrome which > > clears an attachment with an emulated format, then switches > > framebuffers. > > > > Additionally, if the render pass had already been started, the deferred > > clears could have become inlined instead of breaking the render pass. > > Although, it's unlikely for there to be deferred clears when the render > > pass is already open. > > > > This change first identifies which clears need to go through the draw > > path (scissored, masked or as workaround for driver bug). It merges the > > rest of the clears (that don't need the draw path) with the deferred > > clears. It then checks deferred clears and applies them by either: > > > > - vkCmdClearAttachments if mid RP > > - Start a new render pass and use loadOps, if any draw-based clear needs > > to follow. > > - Modify current RP loadOps / defer the clear > > > > Afterwards, the draw-based clears are applied. > > > > Bug: angleproject:4836 > > Change-Id: Id4992c78983b199734508c9d4bb18ed3195c91ec > > Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2455167 > > Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> > > Reviewed-by: Jamie Madill <jmadill@chromium.org> > > Reviewed-by: Charlie Lao <cclao@google.com> > > TBR=syoussefi@chromium.org,jmadill@chromium.org,cclao@google.com > > Change-Id: I85733b3594409df9b96e3d5b34933522c97c42cf > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: angleproject:4836 > Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2481613 > Reviewed-by: Tim Van Patten <timvp@google.com> > Commit-Queue: Tim Van Patten <timvp@google.com> TBR=timvp@google.com,syoussefi@chromium.org,jmadill@chromium.org,cclao@google.com # Not skipping CQ checks because this is a reland. Bug: angleproject:4836 Change-Id: I702cd510f39ee46feab27d4efbf61ae5da10d4e2 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2481856 Reviewed-by: Tim Van Patten <timvp@google.com> Commit-Queue: Tim Van Patten <timvp@google.com>
Tim Van Patten 37c40014 2020-10-16T22:19:20 Revert "Vulkan: Fold deferred clears into current clears" This reverts commit e416c92a81c9ef01d633ec5c05e81c2551b6c5d6. Reason for revert: Reverted parent: https://chromium-review.googlesource.com/c/angle/angle/+/2481612 Original change's description: > Vulkan: Fold deferred clears into current clears > > If there are clears prior to a glClear() call, those clears were > flushed (starting a new render pass) and then the clear call's clears > would be applied (essentially modifying the loadOps of said render > pass). > > The main downside of the above is that the current glClear() clears > don't get a chance to be deferred. This was observed in Chrome which > clears an attachment with an emulated format, then switches > framebuffers. > > Additionally, if the render pass had already been started, the deferred > clears could have become inlined instead of breaking the render pass. > Although, it's unlikely for there to be deferred clears when the render > pass is already open. > > This change first identifies which clears need to go through the draw > path (scissored, masked or as workaround for driver bug). It merges the > rest of the clears (that don't need the draw path) with the deferred > clears. It then checks deferred clears and applies them by either: > > - vkCmdClearAttachments if mid RP > - Start a new render pass and use loadOps, if any draw-based clear needs > to follow. > - Modify current RP loadOps / defer the clear > > Afterwards, the draw-based clears are applied. > > Bug: angleproject:4836 > Change-Id: Id4992c78983b199734508c9d4bb18ed3195c91ec > Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2455167 > Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> > Reviewed-by: Jamie Madill <jmadill@chromium.org> > Reviewed-by: Charlie Lao <cclao@google.com> TBR=syoussefi@chromium.org,jmadill@chromium.org,cclao@google.com Change-Id: I85733b3594409df9b96e3d5b34933522c97c42cf No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: angleproject:4836 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2481613 Reviewed-by: Tim Van Patten <timvp@google.com> Commit-Queue: Tim Van Patten <timvp@google.com>
Shahbaz Youssefi e416c92a 2020-10-06T23:29:02 Vulkan: Fold deferred clears into current clears If there are clears prior to a glClear() call, those clears were flushed (starting a new render pass) and then the clear call's clears would be applied (essentially modifying the loadOps of said render pass). The main downside of the above is that the current glClear() clears don't get a chance to be deferred. This was observed in Chrome which clears an attachment with an emulated format, then switches framebuffers. Additionally, if the render pass had already been started, the deferred clears could have become inlined instead of breaking the render pass. Although, it's unlikely for there to be deferred clears when the render pass is already open. This change first identifies which clears need to go through the draw path (scissored, masked or as workaround for driver bug). It merges the rest of the clears (that don't need the draw path) with the deferred clears. It then checks deferred clears and applies them by either: - vkCmdClearAttachments if mid RP - Start a new render pass and use loadOps, if any draw-based clear needs to follow. - Modify current RP loadOps / defer the clear Afterwards, the draw-based clears are applied. Bug: angleproject:4836 Change-Id: Id4992c78983b199734508c9d4bb18ed3195c91ec Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2455167 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com>
Shahbaz Youssefi 39b5e771 2020-10-09T11:06:10 Vulkan: Fix vector size issue with clearWithCommand gl::DrawBuffersVector was used to hold the clear values, but that didn't have enough space for depth/stencil clear values if MAX draw buffers where used and cleared. The added test in this change exposes the vkCmdClearAttachment Qualcomm bug (previously presumed to affect color clears only) with depth/stencil buffers, so the workaround is expanded to avoid vkCmdClearAttachment entirely. Bug: b/159808300 Change-Id: I27c58d9b534bce0bdd27cc53fc64e139f1363c1a Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2455166 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Courtney Goeltzenleuchter <courtneygo@google.com>
Shahbaz Youssefi df8f71d1 2020-10-09T15:27:28 Vulkan: Don't break the render pass on scissored clears clearImmediatelyWithRenderPassOp is removed and the draw path is used for the scissor. That path was added to avoid creating a large number of graphics pipelines due to the scissor state. This is now done by using dynamic state for scissor in the draw path for clear. Running the following dEQP tests without and with dynamic state for scissor: dEQP-GLES3.functional.fragment_ops.depth_stencil.stencil_ops.* the number of graphics pipelines is reduced from 95392 to 16. Bug: angleproject:4617 Bug: angleproject:4836 Change-Id: Ib373d8cd23ca2b67e6fd26aa2a1103f281f7e473 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2463985 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com> Reviewed-by: Jamie Madill <jmadill@chromium.org>
Charlie Lao df6b7298 2020-10-12T13:39:09 Vulkan: Use StoreOp_None_QCOM for read only depth stencil buffer For read only depth stencil buffers, there is no need to store depth or stencil value. But we can not use DontCare for storeOp because vulkan core spec says DontCare indicates data is undefined after this. VK_QCOM_render_pass_store_ops extension introduces a new store op that will leave data defined but skip the store. This CL utilize this if the extension is available. Bug: angleproject:5055 Change-Id: I104f3d01eb342a2d0cc900f342430e901bde1bff Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2462604 Commit-Queue: Charlie Lao <cclao@google.com> Reviewed-by: Tim Van Patten <timvp@google.com> Reviewed-by: Jamie Madill <jmadill@chromium.org>
Shahbaz Youssefi 68bd685a 2020-10-10T22:58:41 Reland: "4 Vulkan content defined CLs." Reland "Vulkan: Avoid content restore by detecting no-op stencil" This relands commit 243d0f899e443cd931c78aba7489382dff79edbb. Reland "Vulkan: Restore at the end of RP if write-after-invalidate" This relands commit e5d52ac3b9a00656acdd912ee8cd62dd14784075. Reland "Vulkan: Invalidate/restore depth/stencil separately." This relands commit 61fa0878964a796f6d3b3c13bc3a3849403ecdbd. Reland "Vulkan: Move content-defined tracking to ImageHelper" This relands commit 2392e6b34c0ddfbfd7b4c3cb67323ba463e11a57. Reason for revert: Caused crashes in Fuchsia x64 and on ARM. Reland fixes content defined for external images. Original CL message: Content-defined tracking was done in render targets prior to this change. This had multiple drawbacks: - When a framebuffer attachment is changed (including the first time it's set), it's unknown whether the contents of the attachment is defined. - Invalidate takes effect at the end of render pass, at which point the render target objects may be gone. Attachment ImageHelpers are however correctly tracked. This change moves content-defined tracking to the ImageHelper itself, and tracks it per subresource. ImageHelper::onWrite() now receives the subresource that is being written, and marks it as having defined content. A future optimization can make use of this change to ImageHelper::onWrite to track "dirty" subresources. This can lead to the removal of unnecessary barriers when same-kind writes are done on different subresources of the image. See http://anglebug.com/3347#c15 Bug: b/167275320 Bug: angleproject:4836 Bug: angleproject:5159 Change-Id: If5c1ae7152657fd7c94db7d55bea4fb9ddf835ba Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2464825 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Jamie Madill d6b1c17b 2020-10-10T14:29:15 Revert 4 Vulkan content defined CLs. Revert "Vulkan: Avoid content restore by detecting no-op stencil" This reverts commit 243d0f899e443cd931c78aba7489382dff79edbb. Revert "Vulkan: Restore at the end of RP if write-after-invalidate" This reverts commit e5d52ac3b9a00656acdd912ee8cd62dd14784075. Revert "Vulkan: Invalidate/restore depth/stencil separately." This reverts commit 61fa0878964a796f6d3b3c13bc3a3849403ecdbd. Revert "Vulkan: Move content-defined tracking to ImageHelper" This reverts commit 2392e6b34c0ddfbfd7b4c3cb67323ba463e11a57. Causes crashes in Fuchsia x64 and on ARM. Original CL message: Content-defined tracking was done in render targets prior to this change. This had multiple drawbacks: - When a framebuffer attachment is changed (including the first time it's set), it's unknown whether the contents of the attachment is defined. - Invalidate takes effect at the end of render pass, at which point the render target objects may be gone. Attachment ImageHelpers are however correctly tracked. This change moves content-defined tracking to the ImageHelper itself, and tracks it per subresource. ImageHelper::onWrite() now receives the subresource that is being written, and marks it as having defined content. A future optimization can make use of this change to ImageHelper::onWrite to track "dirty" subresources. This can lead to the removal of unnecessary barriers when same-kind writes are done on different subresources of the image. See http://anglebug.com/3347#c15 Bug: b/167275320 Bug: angleproject:4836 Bug: angleproject:5159 Change-Id: I93d9dfe973caa7ce70aefa46b5b7d04a8637efb3 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2464822 Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Jamie Madill <jmadill@chromium.org>
Shahbaz Youssefi 6c1c3bd9 2020-10-09T11:46:04 Vulkan: Clear depth by shader if depthClamp not supported This will avoid breaking the render pass when clearing depth through clearWithDraw if the depthClamp Vulkan feature is not present. Bug: angleproject:4836 Change-Id: I845fd5074dd95f6896da89f9e119ebc5000a5688 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2462719 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com>
Shahbaz Youssefi e5d52ac3 2020-10-08T14:26:22 Vulkan: Restore at the end of RP if write-after-invalidate If a depth/stencil attachment is invalidated, but subsequently drawn to in the same render pass, undo the invalidate when the render pass is closed. Adapted from https://chromium-review.googlesource.com/c/angle/angle/+/2386478. Bug: b/167275320 Bug: angleproject:4836 Change-Id: I17a35bfd692ddc403ceaa6ec44b5c4f16ff9eed6 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2461464 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Ian Elliott <ianelliott@google.com>
Shahbaz Youssefi f8070feb 2020-10-09T11:03:29 Vulkan: Use depthClamp to clear depth where available This will avoid breaking render pass if clearing depth in clearWithDraw. Bug: angleproject:4836 Change-Id: I50242d1115efc91059923143f6ae5fd25fb3d36f Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2462717 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com> Reviewed-by: Jamie Madill <jmadill@chromium.org>
Shahbaz Youssefi 61fa0878 2020-10-08T11:35:48 Vulkan: Invalidate/restore depth/stencil separately. Depth/stencil content defined is already tracked separately in the ImageHelper. This change exposes this tracking from RenderTargetVk. Bug: b/167275320 Bug: angleproject:4836 Change-Id: Ie6520e7a4ab557eb233c60c6ab0d4a8f8f098bf6 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2462039 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Ian Elliott <ianelliott@google.com>
Tim Van Patten a3d5a6e3 2020-10-07T15:03:40 Vulkan: Call onColorDraw in resolveColorWithSubpass We are currently calling onImageRenderPassWrite() on the read render target within resolveColorWithSubpass(). We need to instead call onColorDraw() on the draw render target, since that's what's actually being written. Bug: b/159903491 Test: CQ Change-Id: I577381d91228e132950455d2e872fbb9b066d0c8 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2458850 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: Tim Van Patten <timvp@google.com>
Shahbaz Youssefi 2392e6b3 2020-10-07T23:59:43 Vulkan: Move content-defined tracking to ImageHelper Content-defined tracking was done in render targets prior to this change. This had multiple drawbacks: - When a framebuffer attachment is changed (including the first time it's set), it's unknown whether the contents of the attachment is defined. - Invalidate takes effect at the end of render pass, at which point the render target objects may be gone. Attachment ImageHelpers are however correctly tracked. This change moves content-defined tracking to the ImageHelper itself, and tracks it per subresource. ImageHelper::onWrite() now receives the subresource that is being written, and marks it as having defined content. A future optimization can make use of this change to ImageHelper::onWrite to track "dirty" subresources. This can lead to the removal of unnecessary barriers when same-kind writes are done on different subresources of the image. See http://anglebug.com/3347#c15 Bug: b/167275320 Bug: angleproject:4836 Change-Id: Iabd1dace4eae9eb379453a9eb7ec6eafc9db1aef Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2462036 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Reviewed-by: Ian Elliott <ianelliott@google.com>
Shahbaz Youssefi 1a87f1f0 2020-10-06T17:15:34 Add a test for deferred clears and 0-sized scissor clears The Vulkan backend no-ops such clears before flushing deferred clears. Deferred clears are actually not gathered when doing a scissored clear, so this is not an issue. An ASSERT is added along with a regression test. Bug: angleproject:4836 Change-Id: I5ea5bab499ced41e13023ffb6b821e3caefb9ab2 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2453466 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com>
Charlie Lao 22ed1e59 2020-10-05T17:59:27 Vulkan: Remove depth stencil access out of RenderPassDesc Vulkan spec says that image layout is not counted toward render pass compatibility: "Two render passes are compatible if their corresponding color, input, resolve, and depth/stencil attachment references are compatible and if they are otherwise identical except for: Initial and final image layout in attachment descriptions Image layout in attachment references" This CL removes the depth stencil access mode information out of RenderPassDesc structure. It is essentially partially reverted the change from crrev.com/c/2354280 Bug: b/170134600 Change-Id: Iada4d89c3249489b47db3046952e7cb10f252891 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2451597 Commit-Queue: Charlie Lao <cclao@google.com> Reviewed-by: Tim Van Patten <timvp@google.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org>