src/libANGLE/renderer/vulkan/SurfaceVk.cpp


Log

Author Commit Date CI Message
Charlie Lao ecc378cc 2025-03-03T16:43:33 Reland "Dont use Subject/Observer for SwapchainImageChanged" This is reland of the following CL https://chromium-review.googlesource.com/c/angle/angle/+/6319893 Because we do deferred ANI (VkAcquireNextImage) call until image is needed, we need a way to force Context to go through FramebufferVk::syncState call (FramebufferVk::syncState calls WindowSurfaceVk::getAttachmentRenderTarget, which end up calling ANI. Right now we uses subject/observer mechanism, by sending angle::SubjectMessage::SwapchainImageChanged to all observers of WindowSurfaceVk. In this case it is egl::Surface. Then eglSurface redirects this message to its observers, which are all gl::Framebuffer's attachments: color, depth, stencil. Even though only color attachment needs to be notified, but because we don't have a separate list of observers, depth/stencil attachment also receive the notification and they early out. Then gl::Framebuffer sets DIRTY_BIT_COLOR_BUFFER_CONTENTS_0 dirty bit and send the angle::SubjectMessage::DirtyBitsFlagged to Context, which dirty DrawFBO and ReadFBO and dirty cached state. Note that this is specific for swap image changed case, there is no surface property change (surface property change will still trigger the subject/observer message with SubjectMessage::SubjectChanged message, but this occurs rarely). This gets worse for apps that uses multiple contexts, for the example pokemon_masters_ex has three contexts, each context has its own default frame buffer that attach to the same surface, and we never remove non-current context from the observer list. This end up with egl::Surface has 12 observers and for every frame, it loop over the list of 12 observers and send message (virtual function call) to each of them. Color attachment also ends up sending two messages to Context, one for Read FBO and another for Draw FBO. There are total 21 virtual function calls. Even for single context usage, you have 6 virtual function calls, for every frame. EGL spec says "an EGLSurface must be current on only one thread at a time", any other context must call EGLMakeCurrent in order to use this surface, which will add all necessary dirty bits at that time. So we really only need to notify current context. In this CL, SwapchainImageChanged no longer uses subject/observer mechanism, so this message is removed. This CL still uses subject/observer mechanism to send DirtyBitsFlagged from Framebuffer back to context. We could call setDrawFramebufferDirty and setReadFramebufferDirty directly, but that will require to remove the "const" decoration out of gl::Context which generates too much code diff, so onStateChange(angle::SubjectMessage::DirtyBitsFlagged) is still used. Bug: angleproject:400711938 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6319893 Commit-Queue: Charlie Lao <cclao@google.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Yuxin Hu <yuxinhu@google.com> Change-Id: I017b0e8934b5194a520828fa5c4af1d6e3ef9aac Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6404621
Igor Nazarov 8cf89716 2025-03-14T20:17:07 Vulkan: Remove perFrameWindowSizeQuery feature Feature was enabled for all platforms in order for surface to be resized before acquire next image (not only after swap). Remove it, as if it's always enabled to simplify the code. Bug: angleproject:397848903 Bug: angleproject:42262287 Bug: angleproject:42262286 Bug: angleproject:40096601 Change-Id: I768772e30f5f38f68992e5b82c84430732aa77d9 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6354166 Commit-Queue: Igor Nazarov <i.nazarov@samsung.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Igor Nazarov 081934f6 2025-03-12T15:22:31 Vulkan: Add avoidInvisibleWindowSwapchainRecreate feature There is a bug on some NVIDIA drivers on Linux X11 when `vkAcquireNextImageKHR()` constantly returns `VK_ERROR_OUT_OF_DATE_KHR` if recreate the swapchain while window is not visible. If not recreate the swapchain after window resize - nothing happens, acquire and present works without errors. But if recreate the swpahchain to the `VkSurfaceCapabilitiesKHR::currentExtent` the next `vkAcquireNextImageKHR()` will return `VK_ERROR_OUT_OF_DATE_KHR`. ANGLE will retry to recreate the swapchain one more time and fail the call. Enabling the "avoidInvisibleWindowSwapchainRecreate" feature will avoid swapchain recreation when window size changes while it is not visible. Test: angle_end2end_tests --gtest_filter=EGLSurfaceTest.ResizeInvisibleWindow/* Bug: angleproject:397848903 Bug: angleproject:42264022 Bug: angleproject:42263074 Change-Id: I48588bf467d15c0e84b923092e06a42c22084dcc Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6348739 Reviewed-by: Charlie Lao <cclao@google.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com>
Igor Nazarov caf14998 2025-02-25T17:28:05 Vulkan: Fix window surface resize and size query Added `mSizeState` enum member. When it is not `Resolved`, surface size is not yet resolved and may change any time in response to native window resize. Otherwise, surface size is resolved and will not change until next swap. Updated `getUserWidth()`/`getUserHeight()` methods. They return current window size when surface size is not resolved, or current surface size, otherwise. Window size is queried either by `getCurrentWindowSize()`, when surface is sized by swapchain, or by querying vulkan surface capabilities. The "perFrameWindowSizeQuery" feature controls when `mSizeState` is updated. Initially it is not resolved. Marked as not resolved in `invalidateSwapchain()`, and in `deferAcquireNextImage()` when the feature is enabled. It is marked as resolved after swapchain is created when feature is disabled, or in `AcquireNextImageUnlocked()` function after acquire next image, otherwise. Enabled some tests that were previously failing. Replaced old resize tests with new versions. Test: angle_end2end_tests --gtest_filter=EGLSurfaceTest.Resize*/* Bug: angleproject:397848903 Bug: angleproject:373659619 Bug: angleproject:153329980 Bug: angleproject:42266013 Bug: angleproject:42265843 Bug: angleproject:42265529 Bug: angleproject:42264022 Bug: angleproject:42263074 Bug: angleproject:42261800 Bug: angleproject:40096826 Change-Id: I3ad836960a68229fab6c94624022f1a0aaf2c3e5 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6300645 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com> Reviewed-by: Charlie Lao <cclao@google.com>
Igor Nazarov e44357c6 2025-03-25T15:50:15 Vulkan: Do not defer ANI in shared present mode Calling ANI in shared present mode is not necessary after the initial acquire. Skip calling `deferAcquireNextImage()` in that mode. For code simplicity and as a preparation for (anglebug.com/400711938), the `invalidateSwapchain()` method is now also defers ANI and notifies `SubjectMessage::SurfaceChanged` to the parent class. `Framebuffer` message handling was updated to also reset color buffer dirty bit and to notify `SubjectMessage::DirtyBitsFlagged` if needed, as in the `SubjectMessage::SwapchainImageChanged` message case (which will be removed in future CL). This way, single `SubjectMessage::SurfaceChanged` message is enough to handle swapchain recreate. The `VK_SUBOPTIMAL_KHR` is no longer treaded as OUT_OF_DATE when in shared present mode. Added for consistency (since we are already skipping "perFrameWindowSizeQuery" checks), to preserve content, and to match the Android native GLES driver behavior. Call `invalidateSwapchain()` when swapchain operations fail to avoid repeated swapchain use and to be able to recover from the error. The `checkForOutOfDateSwapchain()` was split into two methods: - checkForOutOfDateSwapchain(): - Called only after present. - Checks present out of date result and present mode compatibility. - Invalidates the swapchain and updates the present mode if the above checks fails. - prepareSwapchainForAcquireNextImage(): - Calls `queryAndAdjustSurfaceCaps()` and `recreateSwapchain()` if swapchain is invalid. - Calls `queryAndAdjustSurfaceCaps()` and checks surface properties when "perFrameWindowSizeQuery" is enabled. Then calls `recreateSwapchain()` if something changed. Other changes: - The `prepareForAcquireNextSwapchainImage()` method was replaced with `prepareSwapchainForAcquireNextImage()`. - Removed `doDeferredAcquireNextImageWithUsableSwapchain()` and `postProcessUnlockedAcquire()` methods because of redundancy. - Move image invalidation code into `acquireNextSwapchainImage()` to make `doDeferredAcquireNextImage()` simpler. - Convert `resizeSwapchainImages()` into `createSwapchainImages()` for simplicity. Updated old and added new tests. Test: angle_end2end_tests --gtest_filter=EGLSurfaceTest.ReadFramebufferBindingSyncState/* Test: angle_end2end_tests --gtest_filter=EGLSingleBufferTest.OnCreateWindowSurface/* Test: angle_end2end_tests --gtest_filter=EGLSingleBufferTest.OnSetSurfaceAttrib/* Test: angle_end2end_tests --gtest_filter=EGLSingleBufferTest.WindowResize/* Test: angle_end2end_tests --gtest_filter=EGLSingleBufferTest.WindowRotation/* Bug: angleproject:400711938 Bug: angleproject:397848903 Bug: angleproject:42262606 Change-Id: I2247417aa8b7b5afc10a8420083aeb845895aec9 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6387920 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com>
Yuly Novikov 3cfc0ce2 2025-03-24T07:41:33 Revert "Vulkan:Dont use Subject/Observer for SwapchainImageChanged" This reverts commit 48103cb2f2b292cb50cc5a29546b358b2e47fd29. Reason for revert: assert fails https://ci.chromium.org/ui/p/angle/builders/ci/android-arm64-exp-test/7085/overview I 22:27:33.697 77.533s _RunTestsOnDevice(17221FDF6000A4) [ RUN ] EGLAndroidAutoRefreshTest.SwapCPUThrottling/ES3_Vulkan_NoFixture INFO:root:ERR: SurfaceVk.cpp:3165 (getCurrentFramebuffer): ! Assert failed in getCurrentFramebuffer (../../src/libANGLE/renderer/vulkan/SurfaceVk.cpp:3165): mAcquireOperation.state == ImageAcquireState::Ready Original change's description: > Vulkan:Dont use Subject/Observer for SwapchainImageChanged > > Because we do deferred ANI (VkAcquireNextImage) call until image is > needed, we need a way to force Context to go through > FramebufferVk::syncState call (FramebufferVk::syncState calls > WindowSurfaceVk::getAttachmentRenderTarget, which end up calling ANI. > Right now we uses subject/observer mechanism, by sending > angle::SubjectMessage::SwapchainImageChanged to all observers of > WindowSurfaceVk. In this case it is egl::Surface. Then eglSurface > redirects this message to its observers, which are all gl::Framebuffer's > attachments: color, depth, stencil. Even though only color attachment > needs to be notified, but because we don't have a separate list of > observers, depth/stencil attachment also receive the notification and > they early out. Then gl::Framebuffer sets > DIRTY_BIT_COLOR_BUFFER_CONTENTS_0 dirty bit and send the > angle::SubjectMessage::DirtyBitsFlagged to Context, which dirty DrawFBO > and ReadFBO and dirty cached state. Note that this is specific for swap > image changed case, there is no surface property change (surface > property change will still trigger the subject/observer message with > SubjectMessage::SubjectChanged message, but this occurs rarely). This > gets worse for apps that uses multiple contexts, for the example > pokemon_masters_ex has three contexts, each context has its own default > frame buffer that attach to the same surface, and we never remove > non-current context from the observer list. This end up with > egl::Surface has 12 observers and for every frame, it loop over the list > of 12 observers and send message (virtual function call) to each of > them. Color attachment also ends up sending two messages to Context, one > for Read FBO and another for Draw FBO. There are total 21 virtual > function calls. Even for single context usage, you have 6 virtual > function calls, for every frame. > > EGL spec says "an EGLSurface must be current on only one thread at a > time", any other context must call EGLMakeCurrent in order to use this > surface, which will add all necessary dirty bits at that time. So we > really only need to notify current context. In this CL, > SwapchainImageChanged no longer uses subject/observer mechanism, so this > message is removed. > > This CL still uses subject/observer mechanism to send DirtyBitsFlagged > from Framebuffer back to context. We could call setDrawFramebufferDirty > and setReadFramebufferDirty directly, but that will require to remove > the "const" decoration out of gl::Context which generates too much code > diff, so onStateChange(angle::SubjectMessage::DirtyBitsFlagged) is still > used. > > Bug: angleproject:400711938 > Change-Id: I61354516fd0aa307714b7abd30c6b6e45ff7b496 > Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6319893 > Commit-Queue: Charlie Lao <cclao@google.com> > Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> > Reviewed-by: Yuxin Hu <yuxinhu@google.com> Bug: angleproject:400711938 Change-Id: Ib7899d1ac63a1f86af0953a1d25922578c470fc9 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6387755 Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Commit-Queue: Yuly Novikov <ynovikov@chromium.org>
Igor Nazarov 1166eec4 2025-03-20T02:02:50 Move TRACE_EVENT inside acquireNextSwapchainImage() The `acquireNextSwapchainImage()` is no longer called from surface initialize, which allows to move the trance event inside the function. Change also uses loop to handle repeated ANI retry instead of call duplication. This is done to further reduce code duplication in the next CL. Additionally, fixed assert that was missed in recently merged CL: Vulkan: Remove recreateSwapchain ContextVk dependency https://crrev.com/c/angle/angle/+/6298733 Bug: angleproject:397848903 Bug: angleproject:42261625 Change-Id: Iae2ce35018d4fe105430271b56804af8c74152c2 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6372565 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Charlie Lao 48103cb2 2025-03-03T16:43:33 Vulkan:Dont use Subject/Observer for SwapchainImageChanged Because we do deferred ANI (VkAcquireNextImage) call until image is needed, we need a way to force Context to go through FramebufferVk::syncState call (FramebufferVk::syncState calls WindowSurfaceVk::getAttachmentRenderTarget, which end up calling ANI. Right now we uses subject/observer mechanism, by sending angle::SubjectMessage::SwapchainImageChanged to all observers of WindowSurfaceVk. In this case it is egl::Surface. Then eglSurface redirects this message to its observers, which are all gl::Framebuffer's attachments: color, depth, stencil. Even though only color attachment needs to be notified, but because we don't have a separate list of observers, depth/stencil attachment also receive the notification and they early out. Then gl::Framebuffer sets DIRTY_BIT_COLOR_BUFFER_CONTENTS_0 dirty bit and send the angle::SubjectMessage::DirtyBitsFlagged to Context, which dirty DrawFBO and ReadFBO and dirty cached state. Note that this is specific for swap image changed case, there is no surface property change (surface property change will still trigger the subject/observer message with SubjectMessage::SubjectChanged message, but this occurs rarely). This gets worse for apps that uses multiple contexts, for the example pokemon_masters_ex has three contexts, each context has its own default frame buffer that attach to the same surface, and we never remove non-current context from the observer list. This end up with egl::Surface has 12 observers and for every frame, it loop over the list of 12 observers and send message (virtual function call) to each of them. Color attachment also ends up sending two messages to Context, one for Read FBO and another for Draw FBO. There are total 21 virtual function calls. Even for single context usage, you have 6 virtual function calls, for every frame. EGL spec says "an EGLSurface must be current on only one thread at a time", any other context must call EGLMakeCurrent in order to use this surface, which will add all necessary dirty bits at that time. So we really only need to notify current context. In this CL, SwapchainImageChanged no longer uses subject/observer mechanism, so this message is removed. This CL still uses subject/observer mechanism to send DirtyBitsFlagged from Framebuffer back to context. We could call setDrawFramebufferDirty and setReadFramebufferDirty directly, but that will require to remove the "const" decoration out of gl::Context which generates too much code diff, so onStateChange(angle::SubjectMessage::DirtyBitsFlagged) is still used. Bug: angleproject:400711938 Change-Id: I61354516fd0aa307714b7abd30c6b6e45ff7b496 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6319893 Commit-Queue: Charlie Lao <cclao@google.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Yuxin Hu <yuxinhu@google.com>
Igor Nazarov daaae2bd 2025-02-24T19:36:39 Vulkan: Remove surface caps query from createSwapChain This is mainly a refactoring with minor bug fixes. The `createSwapChain()` is called at initialize and at swapchain recreate. Before recreate there is always `queryAndAdjustSurfaceCaps()` call, which queries current surface sizes and min image count. However, surface capabilities may be queried again from `createSwapChain()` to get compatible present modes and the min image count. Problem is that surface caps `currentExtent` may change after the second query in the `createSwapChain()`, while the `extents` parameter is already calculated. Such situation will violate the specification. To eliminate the possibility of the above problem and to simplify the code, query of compatible present modes is moved to the `queryAndAdjustSurfaceCaps()`. To save performance, `mCompatiblePresentModes` is only updated if swapchain is already invalid (recreate is imminent). Function is now also handles `mEmulatedPreTransform` to further simplify the code. The `mSurfaceCaps` member was replaced with local variable. The `mIsSurfaceSizedBySwapchain` boolean is added instead to check it in the `getUserWidth()`/`getUserHeight()` methods. This also fixes a possible bug (anglebug.com/168327817) on Fuchsia when this expression could be false: mSurfaceCaps.currentExtent.width == kSurfaceSizedBySwapchain This is because previously `queryAndAdjustSurfaceCaps()` updated `mSurfaceCaps` with the values from `getCurrentWindowSize()`. It seems that the bug never happened on practice, otherwise the ASSERT in the same function would fail. The `prepareForAcquireNextSwapchainImage()` is used at initialize instead of `createSwapChain()` to remove code duplication. Fixed (anglebug.com/168327817) checking `mPreTransform` without enabled "enablePreRotateSurfaces" feature. Bug: angleproject:397848903 Bug: angleproject:168327817 Change-Id: I02e8e3b815b30350e12476853d8dcffed3fe7c38 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6298736 Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Igor Nazarov ac251fa8 2025-02-24T17:31:58 Vulkan: Remove doDeferredAcquireNextImage ContextVk dependency This is the continuation of the previous CL. After this change `lockSurface()` calls `doDeferredAcquireNextImage()` as the rest of the code do. The `ImageHelper::invalidateSubresourceContentImpl()` required `ContextVk` pointer before this change. The `ContextVk` is only used to print the performance warning when `layerIndex` exceed the maximum count, which is not possible in case of a `WindowSurfaceVk`. Added `layerLimitReachedOut` pointer instead of writing the warning. It is processed in `invalidateSubresource[Stencil]Content()`. Added `invalidateEntireLevel[Stencil]Content()` which do not require `ContextVk` to use in the `WindowSurfaceVk`. Bug: angleproject:397848903 Bug: angleproject:42264593 Change-Id: I58451a4818ad56fa196c3c971df3a5f7793f2bfe Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6298735 Reviewed-by: Charlie Lao <cclao@google.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com>
Igor Nazarov 2f5a91ba 2025-02-24T17:25:10 Vulkan: Remove checkForOutOfDateSwapchain ContextVk dependency This is the continuation of the previous CL. After this CL it is now possible to call `prepareForAcquireNextSwapchainImage()` from `lockSurface()` that will also do necessary swapchain recreation in case of window resize. These methods required `ContextVk` pointer before this change: - RenderTargetVk::releaseImageAndViews() - replaced with `releaseSwapchainImage()` that does not release framebuffer because Window Surface framebuffers are not managed by the cache. - added `release()` method that does release the framebuffer. - WindowSurfaceVk::releaseSwapchainImages() - use `Renderer::collectGarbage()` instead of `ContextVk::addGarbage()`. - use `ImageHelper::releaseImage()` instead of `ImageHelper::releaseImageFromShareContexts()`. The `finalizeImageLayoutInShareContexts()` was not required since renderpass must be already ended, because swapchain recreate is only possible after present. Removal of `addToPendingImageGarbage()` is not going to cause OOM problems, because repeated swapchain recreate calls are not possible without swap (submissions). Bug: angleproject:397848903 Bug: angleproject:42264593 Change-Id: Iacfa3a144aa980659569b7100be25a44ebb9f0a0 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6298734 Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Igor Nazarov e6a27504 2025-03-07T20:14:50 Vulkan: Swapchain recreate workaround for shared present mode On Android, `vkCreateSwapchainKHR` may return `VK_ERROR_NATIVE_WINDOW_IN_USE_KHR` if use `mLastSwapchain` as an `oldSwapchain` when in shared present mode. This happens on three different devices: SM-G996B (Mali-G78), SM-S921B (Xclipse 940), SM-S921U (Adreno 940). Bug: angleproject:397848903 Bug: angleproject:42262606 Change-Id: I1541c06c254f3e8231b165fb0c53eff5d810e8bd Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6329488 Commit-Queue: Igor Nazarov <i.nazarov@samsung.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Igor Nazarov 354a5463 2025-02-24T16:52:06 Vulkan: Remove recreateSwapchain ContextVk dependency In the upcoming change, swapchain will be invalidated in swap, while recreate deferred until ANI. After such invalidation, `lockSurface()` wold not be possible. Even that currently swapchain may be invalid only after previous recreate failure, this change adds `recreateSwapchain()` call into `lockSurface()` because it is now possible and as a preparation for the upcoming change. The `recreateSwapchain()` is split into `invalidateSwapchain()` and `recreateSwapchain()` methods. The invalidate must be called before the recreate. Bug: angleproject:397848903 Bug: angleproject:42264593 Change-Id: Ida4a98a985f9fbb527cdf8c81c088ae1210f409c Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6298733 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com>
Igor Nazarov 79faadf7 2025-02-19T16:04:19 Vulkan: Simple WindowSurfaceVk refactoring Changes: - Update `ImageAcquireState` enumeration. - Make `getCurrentWindowSize()` const. Some implementations are now using local `surfaceCaps` variable instead of `mSurfaceCaps`, however this will not affect reset of the logic. - Remove `extentsOut` parameter from `createSurfaceVk()` and call `getCurrentWindowSize()` explicitly only when needed. - Remove `forceSwapchainRecreate` parameter since it is always has false as an argument. - Remove redundant `impl::` namespace. - Make `queryAndAdjustSurfaceCaps()` const. Bug: angleproject:397848903 Change-Id: I955be15fb4709e137f2ad8a165fd04a3fe626fbf Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6298732 Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com>
Igor Nazarov 668a7a2e 2025-02-19T14:55:49 Revert "Vulkan: Return cached width and height for eglQuerySurface" This reverts commit db833869131eddb36e695717a823a540e3cdc8ba. Reason for revert: Native GLES driver behavior on Android is different from what was tested by "EGLPreRotationSurfaceTest.CheckSurfaceCapabilities" test. More precisely, behavior is different depending if native Window is resided using `ANativeWindow_setBuffersGeometry()` or resized by the user (screen rotation or resizing of a floating window). In case of `ANativeWindow_setBuffersGeometry()` resize (used by tests), surface will have size of the Window up until image is acquired (dequeued). In other words, size will be fixed after draw and will match current Window size until draw. In case of resizing by the user, surface size will be fixed until the next swap. This is the behavior that was tested by the reverted test. Bug: angleproject:397848903 Bug: angleproject:153329980 Change-Id: I8fc8d194c1591aff714dd255acd25392b4963e0d Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6298731 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com>
Igor Nazarov e70d1298 2025-02-18T14:32:35 Vulkan: Rework SurfaceVk::mUse tracking logic Original commit that introduced `mUse` had a bug and was then reverted: Vulkan: SurfaceVk should only wait for GPU work that uses it https://crrev.com/c/angle/angle/+/4220723 The reland fixed the original issue by adding "contextVk::mLastSubmitQueueSerial" into "Surface::mUse" before calling finish: Reland "Vulkan: SurfaceVk should only wait for GPU work that uses it" https://crrev.com/c/angle/angle/+/4406891 However, the real issue was that the `finish()` call (called because of the "waitIdleBeforeSwapchainRecreation" feature) was done after `releaseSwapchainImages()`, which resets all resource uses. Because of this bug, `finish()` ignored all uses of swapchain images, causing GPU crash on Mali. Crash was not because of semaphores that was not tracked by resource use. Adding "mLastSubmitQueueSerial" fixed the crash simply because it accounts for all uses ever made by the Context. This change fixes this issue by calling `mergeImageResourceUses()` at the beginning of `releaseSwapchainImages()`. Adds missing retain when transitioning into the `ImageLayout::Present`. Updates one-off submit tracking by updating use of the image, rather than `SurfaceVk::mUse` directly. Explicitly tracks present semaphore submission. All these changes allow to remove adding `ContextVk::mSubmittedResourceUse` before calling `finish()` and in the `unMakeCurrent()`. I confirmed that this fix works by forcing `oldSwapchain` destruction (as if it was never used) and "waitIdleBeforeSwapchainRecreation" enabled. GPU crash happens on SM-S921B if remove addition of `mSubmittedResourceUse` before `finish()` call, and fixed again by calling `mergeImageResourceUses()` in the `releaseSwapchainImages()`. So the `mergeImageResourceUses()` fixes the issue without potentially waiting for unrelated submissions from the `mSubmittedResourceUse`. After this change, surface use tracking should work according to the original intent. Bug: angleproject:397848903 Bug: b/267806287 Change-Id: Ic8197e33d0c19aad274b5f9da6f9c7377a166b60 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6277446 Reviewed-by: Charlie Lao <cclao@google.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com>
Igor Nazarov 318d4038 2025-02-18T13:19:28 Vulkan: Fix present optimization w.r.t shared present mode Since the implementation of "EGL_KHR_mutable_render_buffer" mode (angleproject:42262606), the renderpass optimization for present (b/153885625) was not working correctly. It sill tries to transition into `ImageLayout::Present`, but instead of entirely skipping the transition, it inserts the barrier even when renderpass is still opened. When both "supportsSharedPresentableImageExtension" and "preferDynamicRendering" are enabled, code will hit ASSERT in `flushToPrimary()` when attempting to record `ImageLayout::Present` barrier into the primary command buffer. Above issue is fixed by skipping the transitioning into `ImageLayout::Present` when in shared present mode. Other changes and fixes: - removed renderpass flush when resolving with renderpass, since it is not necessary (angleproject:42265256). - above change reveled a bug in `finalizeImageLayout(&mColorImageMS)` call. This call reverts the layout in the previous finalize call from Present to ColorWrite. So it must have been inserted before finalizing the swapchain image. Issue fixed by removing both finalize calls. - updated condition to skip invalidate w.r.t shared present mode (b/229689340), that was missed during implementation of "EGL_ANDROID_front_buffer_auto_refresh" (angleproject:42265697). Test: angle_end2end_tests --gtest_filter=EGLSurfaceTest.PresentLayoutTransitionWithMSAA/* Bug: b/153885625 Bug: angleproject:42262606 Bug: angleproject:42265256 Bug: b/229689340 Bug: angleproject:42265697 Change-Id: Ifad8aea8548fa7bfac27941812c435b2af655309 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6277445 Reviewed-by: Charlie Lao <cclao@google.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com>
Yuxiang Qian ca8f072e 2025-02-13T16:46:30 Implement EGL_LARGEST_PBUFFER When EGL_LARGEST_BUFFER is set to true during creating a EGL pbuffer surface, it requires the largest available pixel buffer surface when the allocation would otherwise fail. This means if width/height is bigger than config's maxPbufferWidth/maxPbufferHeight, it should be clamp to maxPbufferWidth/maxPbufferHeight. An end2end test is also added. Bug: angleproject:397315935 Change-Id: Ic160d9693478c87b59ece0a421d3acd5d6699ddc Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6277912 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Geoff Lang <geofflang@chromium.org>
Igor Nazarov 323c49a3 2025-02-20T15:44:08 Vulkan: Fix swap optimization after staged clear for MSAA If the MSAA image only had a clear update and the update wasn't going to be applied to the swapchain image, the clear update was accidentally dropped because `flushStagedUpdates` wouldn't see the clear update which is already extracted in `deferredClearValues`. Test: angle_end2end_tests --gtest_filter=EGLSingleBufferTest.StagedClearResolveOnSwap/* Bug: angleproject:382006939 Change-Id: I53ae26eeb8a7f38b7758f86bf97982a90b1990dd Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6285627 Reviewed-by: Yuxin Hu <yuxinhu@google.com> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Igor Nazarov e5b87adc 2025-02-20T15:32:16 Vulkan: Fix WindowSurfaceVk::hasStagedUpdates w.r.t MSAA The old `hasStagedUpdates()` checked swapchain image for staged updates. However in case of MSAA, staged updates are added to the `mColorImageMS`. Universal solution is to check staged updates on the renderpass image from `mColorRenderTarget`, which will be `mColorImageMS` in case of MSAA surface, and the currently acquired swapchain image otherwise. No automated test is possible, since `glReadPixels()` still works as expected. Other reason is that `ANGLE_VK_TRACE_EVENT_AND_MARKER` before acquire inserts commands into the outside renderpass command buffer causing flush to happen. In other words, the debug marker interfering with the normal ANGLE behavior, so even visual test is not possible. Bug: angleproject:237449314 Change-Id: I237514a640a23a33d894109856711c1eb2dcbb29 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6285626 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com>
Igor Nazarov 03bdf1ca 2025-02-18T16:37:49 Vulkan: Use correct actual FormatID for MSAA swapchain image Implementation of "overrideSurfaceFormatRGB8ToRGBA8" feature missed updating actual FormatID for the MSAA swapchain image. This bug makes possible to initialize RGBA8 swapchain image and RGB8 MSAA image. However, on practice MSAA is still RGBA8 because RGB8 renderable is generally emulated on Android. VUID-vkCmdResolveImage-srcImage-01386: srcImage and dstImage must have been created with the same image format. This fix is cosmetic and is not expected to fix any real bugs (because RGB8 is generally emulated with RGBA8). Bug: angleproject:42265147 Change-Id: Ief4b74ac31e93a655f5088b7927e970585255f25 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6275968 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com>
Igor Nazarov 4eed3a0c 2025-02-18T20:57:38 Vulkan: Remove EGL_WIDTH/HEIGHT from WindowSurfaceVk initialize The EGL_WIDTH and EGL_HEIGHT are only applicable when "EGL_ANGLE_window_fixed_size" is supported, which is currently only exposed by D3D backends. Bug: angleproject:40096349 Change-Id: I45886185d4740804709f274b70b73728b2a2e59d Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6276201 Commit-Queue: Igor Nazarov <i.nazarov@samsung.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Roman Lavrov cfb430c8 2025-02-10T13:19:05 Remove angle::ErrorStream helper Most uses of the helper either use just the code or a fixed string, which compiles to a few instructions. Using this helper adds 200+ bytes of assembly to each use, due to the unneeded instantiation of ostringstream which allocates a buffer etc. The combined effect of this CL on an Android perf build is ~12KB (0.2%) reduction in size. The cases where the message is actually formatted are converted to an explicit use of ostringstream. Removing the helper so that the new code is explicit about the intent to use ostringstream, or an alternative way to format the message. Discovered accidentally while looking into size reduction due to __builtin_unreachable() Semi-automated code change, risk of copy-paste mistakes should be minimal. Bug: angleproject:394129077 Change-Id: I47c2642d750d31416b08a1cfa435d5463c294e35 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6250078 Commit-Queue: Roman Lavrov <romanl@google.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com>
Igor Nazarov ae84e067 2025-02-10T19:31:37 Vulkan: Use correct feature in queryAndAdjustSurfaceCaps() Check "supportsSurfaceMaintenance1" feature instead of "supportsSwapchainMaintenance1", because `VkSurfacePresentModeEXT` structure requires "VK_EXT_surface_maintenance1" extension and not "VK_EXT_swapchain_maintenance1". Some platforms may expose one but not the other, causing inconsistent `GetMinImageCount()` calculations and swapchain recreation on each frame if "perFrameWindowSizeQuery" feature is enabled. Bug: angleproject:42267111 Change-Id: Id17bec8017799a7c175e6a4b03583665faf5dddd Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6249841 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Charlie Lao 6eb9789e 2025-02-06T11:59:31 Vulkan: Reserve enough storage for mSwapchainImages Investigation shows that when running traces in back to back mode, mSwapchainImages end up with max size of 6 or 7. This CL reserve the enough capacity (8 to be power of 2) so that no storage reallocation related data copy will occur. Similar storage is reserved for mSwapchainImageBindings as well. Bug: b/293297177 Change-Id: I1e216b6eaeed72554a20d27963b797b00ca5fd07 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6240592 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Charlie Lao <cclao@google.com>
Igor Nazarov fb118e41 2025-01-22T13:26:31 Vulkan: Always defer acquireNextSwapchainImage() call This change is a preparation before the follow up EGL_WIDTH/EGL_HEIGH eglQuerySurface rework. There is no need to acquire initial swapchain image after surface initialization. Skipping ANI may speed-up apps startup in some cases. The operation is also removed from swap (in case of out-of-date swapchain) for consistency and as a preparation for the follow up change. Change also enables "perFrameWindowSizeQuery" for NVIDIA on Windows because of the "SurfaceDimensionsChangeAndFragCoord" test failure. Test fails because `vkQueuePresentKHR()` does not return `VK_ERROR_OUT_OF_DATE_KHR` when presenting old image size after the window resize, even despite that surface caps currentExtent has already new size. Issue is not permanent, and may "fix" itself if add few empty swap buffers before starting the test or do ANI in the initialize (which one will work depends on GPU). Above issue is observed on the driver 546.01 (currently used by bots). Old 531.68 and new 566.36 does not have this issue (tested on RTX 3060 Laptop). The 546.01 driver, however, fails on that same system even on the base CL (without this change). Bug: angleproject:153329980 Bug: angleproject:42264593 Change-Id: I261e1f573a6b892f1ba4282dd17f17289c2f4184 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6182599 Commit-Queue: Igor Nazarov <i.nazarov@samsung.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Igor Nazarov b5809efa 2025-01-16T17:56:47 Delay eglSwapInterval() effect until swap ... and fix resetting effect of `eglSwapInterval()` call by changing the `EGL_RENDER_BUFFER` attribute, and resetting desired shared present mode when calling `eglSwapInterval()`. Bug: angleproject:390333526 Change-Id: I473c7d4c5929e2179bbcd378b3695d10213644de Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6169922 Commit-Queue: Igor Nazarov <i.nazarov@samsung.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com>
Charlie Lao fbd230f5 2025-01-23T12:59:06 Vulkan: Split ErrorContext into ErrorContext and Context ErrorContext continue to be context for error handling. vk::Context is added to serve as common base class for ContextVk and CLContextVk. Bug: angleproject:390443243 Change-Id: Ifac0b1d2d714ce610693ce60a35459c6c9cddf1a Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6191438 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi c1214ec2 2025-01-22T14:22:56 Vulkan: Rename Context to ErrorContext In preparation for adding another Context (derived by GL and CL contexts), which includes logic that pertains to command recording (such as barrier tracking). Bug: angleproject:390443243 Change-Id: Idf495b62e63fb9aa901a2f16447fdaf3c2acd90b Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6191248 Auto-Submit: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Charlie Lao <cclao@google.com> Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Yuxiang Qian 308cfc62 2025-01-13T14:54:18 Do not reset buffer age after query The spec doesn't mention that we need to reset the buffer age if no rendering after last call of eglQuerySurface to query EGL_BUFFER_AGE_KHR. Age was also reset after first time of query. Remove it to align with spec. New end2end test is also added. Bug: angleproject:391039188 Change-Id: I00c96e3a71ea2c9abcb86ebaf520243408dedd52 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6185135 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Geoff Lang <geofflang@chromium.org>
Igor Nazarov d8a95bf7 2024-12-19T17:35:05 Vulkan: Protect OneOff commands recording with pool mutex Bug: angleproject:384940864 Change-Id: I518c54ae4b0fc5da0e58d330f8c531bc8d65529e Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6108843 Commit-Queue: Igor Nazarov <i.nazarov@samsung.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com>
Igor Nazarov 16d8325f 2025-01-16T13:30:07 Vulkan: Make WindowSurfaceVk::setAutoRefreshEnabled thread safe Bug: angleproject:379762019 Change-Id: I20feb72c4777a8dbd2058ced8a958d08b6c218ab Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6175131 Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com>
Igor Nazarov 6793c5ee 2025-01-16T13:46:48 Vulkan: Simplify WindowSurfaceVk::mMinImageCount tracking `mMinImageCount` is updated in the `WindowSurfaceVk::createSwapChain()` method when "VK_EXT_surface_maintenance1" is enabled, which makes updates outside of this method irrelevant. This CL removes all these updates and makes update in `createSwapChain()` unconditional. The `mMinImageCount` is now only accessed in the `checkForOutOfDateSwapchain()` method outside of the `createSwapChain()` method (without this check `mMinImageCount` might be a local variable). Shared present mode and "forceContinuousRefreshOnSharedPresent" handling is moved just before swapchain creation to properly handle cases when "VK_EXT_surface_maintenance1" is disabled. Bug: b/289274676 Change-Id: I963eab2dfbbb3ebf563183c47167cfd4b60e2f38 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6175130 Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi f72428bb 2025-01-10T22:24:07 Vulkan: Fix MSAA swapchain resolve out of render pass .. vs the optimization that transitions the swapchain image to PRESENT_SRC as part of the render pass. When the swapchain image is the resolve attachment, its layout is decided when the render pass is closed (or through manual calls to finalizeImageLayout()). If the render area forbids the swapchain image from becoming the resolve attachment, it must be resolved after the render pass. Prior to this change, the swapchain image was still marked for transition to PRESENT_SRC by the render pass. This is inefficient, as the following out-of-render-pass resolve would have to transition the image back out of PRESENT_SRC. Nevertheless, this also had a bug exposed by an ASSERT in the dynamic rendering path: * Before the out-of-render-pass resolve, the image layouts are forcefully finalized. * The code in finalizeImageLayout() checks which image is being finalized; if the image is not any of the ones in the render pass, it was silently ignored. * The image marked for transition to PRESENT_SRC (mImageOptimizeForPresent) is not separately checked, as it is expected to be an attachment as well. The code that optimized the final render pass always marked the swapchain image for optimization, even if it was not going to become the resolve attachment. This change makes sure this optimization is done only if the image is definitely an attachment of the render pass. Bug: angleproject:389048224 Change-Id: I9f451d2698944111ac96bd97fefd6efa23859b7f Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6168388 Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Yuxin Hu <yuxinhu@google.com>
Mohan Maiya db833869 2025-01-08T06:14:22 Vulkan: Return cached width and height for eglQuerySurface eglQuerySurface will return extents as of the most recent eglSwapBuffers call Bug: angleproject:153329980 Test: EGLPreRotationSurfaceTest.CheckSurfaceCapabilities* Change-Id: Ifc9e84ed92bdc645afd12814a3a5539cc8b55da3 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6158264 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Ian Elliott <ianelliott@google.com>
Yuxin Hu febba52a 2024-12-12T17:26:59 Optimize for swap after clear Bug: angleproject:382006939 Change-Id: Ia6b9a53042a1d188dbd5a5f6436f17ca1e389a4e Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6072416 Reviewed-by: Charlie Lao <cclao@google.com> Reviewed-by: Cody Northrop <cnorthrop@google.com> Commit-Queue: Yuxin Hu <yuxinhu@google.com>
Yuxin Hu c0ee7b20 2024-12-12T16:49:40 Swap getWidth() and getHeight() if the swapchain is 90 emulate rotated When checking if we need to recreate swapchain, we should swap the getWidth() and getHeight() if Is90DegreeRoration(mEmulatedPreTransform) is true. This is because: When creating swapchain, if Is90DegreeRoration(mEmulatedPreTransform) is true, we store swapped mSurfaceCaps.currentExtent.width and mSurfaceCaps.currentExtent.height in getWidth() and getHeight(), but we use the original mSurfaceCaps.currentExtent.width and mSurfaceCaps.currentExtent.height to create the swapchain. On next acquire, to check if the swapchain property changes, we should swap getWidth() and getHeight() if if Is90DegreeRoration(mEmulatedPreTransform) is true, otherwise we are recreating swapchains when width and height are unchanged. Bug: b/382006939 Change-Id: I1cbe9da2ff5e76602a90963514d2d0d5fbf677e7 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6090199 Commit-Queue: Yuxin Hu <yuxinhu@google.com> Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com> Reviewed-by: Charlie Lao <cclao@google.com>
Igor Nazarov 1287e19e 2024-12-13T15:28:59 Vulkan: Further improve recreateSwapchain() error handling Fixed problem in cases if something fails after assigning `mSwapchain = newSwapChain` and before setting `mAcquireOperation.state = impl::ImageAcquireState::NeedToAcquire`. In the above case we will already have `mSwapchain` pointer with potentially undefined state of related members, while `mAcquireOperation.state` may remain `NeedToProcess`. Returning in case of a failure without assigning `mSwapchain` we will stay in the "invalid swapchain" state while still have new swapchain in the `mLastSwapchain`. This state is already handled by existing code and will try to call `recreateSwapchain()` again until it succeeds. Bug: angleproject:380234227 Change-Id: I465115deef4c06f35d40b90efc192b51e7b9fd44 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6089081 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com>
Yuxiang Qian a1be7e61 2024-11-26T14:39:18 Implement EGL_EXT_surface_compression This patch adds implementation of EGL_EXT_surface_compression to ANGLE, including new API eglQuerySupportedCompressionRatesEXT and adding EGL_SURFACE_COMPRESSION_EXT in EGLQuerySurface and EGLCreateWindowSurface/EGLCreatePlatformWindowSurface. Angle end2end test is added to verify the extension. Bug: angleproject:375496226 Change-Id: I06926930d94485a378fc831d552cf55fe7938a57 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6073355 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi c75bd915 2024-12-10T23:01:44 Vulkan: Remove asyncCommandQueue It's been years and it never showed an advantage. In the meantime, performance without this feature seems close to native drivers (i.e. the feature has lost its appeal) and it's frequently a source of complication and bugs. Bug: angleproject:42262955 Bug: angleproject:42265241 Bug: angleproject:42265934 Bug: angleproject:42265368 Bug: angleproject:42265738 Bug: angleproject:42266015 Bug: angleproject:377503738 Bug: angleproject:42265678 Bug: angleproject:173004081 Change-Id: Id8d7588fdbc397c28c1dd18aafa1f64cbe77806f Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6084760 Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com> Reviewed-by: mohan maiya <m.maiya@samsung.com> Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Robic Sun f51170b3 2024-11-21T16:30:40 Enable GL_KHR_texture_compression_astc_hdr Vulkan supports GL_KHR_texture_compression_astc_hdr, so this extension can be enabled in Angle. Bug: angleproject:379186304 Change-Id: I438a120c3f884a7eefcd883ad71abf68f81cb473 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6038457 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Neil Zhang 01dee1cb 2024-10-14T15:04:28 Add implementation for GL_EXT_texture_storage_compression Bug: angleproject:352364583 Change-Id: I3dab4c68d5d0206d681e165e991217bd3de8eeb6 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6011055 Auto-Submit: Neil Zhang <Neil.Zhang@arm.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Igor Nazarov cecefe53 2024-11-21T22:11:17 Vulkan: Improve recreateSwapchain() error handling The `WindowSurfaceVk::recreateSwapchain()` method may fail in many points. After failing, state of `WindowSurfaceVk` may become invalid, causing undefined behavior. Invalid state examples: - Skip adding `cleanupData` into the `mOldSwapchains` while `mPresentHistory already cleared (resource leak). - `mSwapchain` may remain `VK_NULL_HANDLE` after the failure. - `lastSwapchain` may be not retired, causing subsequent swapchain creation to fail. Change adds `mLastSwapchain` member to continue tracking last created swapchain during the `mSwapchain` recreation process until it is retired. This process now may span multiple API calls. The `mSwapchain` pointer is invalidated as soon as recreate starts - to indicate that we do not have usable swapchain and still need to create one. Notable difference with the old code is that old swapchain and present history are now collected into the `mOldSwapchains` after new swapchain is created (or old swapchain is retired in case of a failure). Because of this, code can now be simplified and easily refactored into a separate method. The `kMaxOldSwapchains` is now also checked after old swapchain is collected - this is to prevent resource leak if `finish()` fails. Added `cleanUpOldSwapchains()` call to reduce a chance of hitting the `kMaxOldSwapchains` limit. Change also renamed `presentOutOfDate` parameter where it does not always make sense. As a side effect, fixed a bug when calling `prepareForAcquireNextSwapchainImage()` with true, is then replaced with false in the `computePresentOutOfDate()` call. Bug: angleproject:380234227 Change-Id: Ie6d85c1b9760cda68c8fc9368235756659a9bdac Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6040159 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Igor Nazarov f458f865 2024-11-12T16:52:42 Vulkan: Update AcquireNextImageUnlocked() implementation Main goal of this change is to simplify acquire next image operation state change tracking. Key members before this change: 1. `mAcquireOperation.needToAcquireNextSwapchainImage` 2. `unlockedAcquireResult.acquireSemaphore` Possible states: 1: 2: false NULL -> image is acquired or no need true ANY -> need to prepare for and do unlocked ANI false HANDLE -> need to process ANI result Only `mAcquireOperation.state` is used after this change. Possible states: Ready -> image is acquired or no need NeedToAcquire -> need to prepare for and do unlocked ANI NeedToProcessResult -> need to process ANI result Value of `unlockedAcquireResult.acquireSemaphore` is now only used as semaphore in the post process and not as boolean, indicating that result needs processing. Change adds a lot of ASSERTs to protect from invalid scenarios. In order for these ASSERTs to work as expected, `deferAcquireNextImage()` is now called regardless if swapchain is out-of-date or not. Another functional change is that `postProcessUnlockedAcquire()` does not mark image as processed at the start and then defers ANI in case of a failure (so the next ANI fail again), but instead keeps the current state (NeedToProcessResult) allowing failure to be processed again without repeating ANI call in case if there will be no swapchain recreation. State is set to `Ready` only after processing is successful. Bug: angleproject:42265346 Bug: angleproject:42266579 Change-Id: I4a6a66fa3b6f25a788a8959eff5848b7ec5bdd27 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6018094 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com>
Igor Nazarov ea86503c 2024-11-20T19:47:49 Vulkan: Remove vkAcquireNextImageKHR multi-threading support Currently, it is not possible to call `TryAcquireNextImageUnlocked()` from multiple threads. However, original implementation added multi-threading support for future use. Possible future use: - First thread (where Surface is current) calls `eglPrepareSwapBuffersANGLE()` which will call `TryAcquireNextImageUnlocked()` in the unlocked tail call (without the share group lock). - Other thread (where shared Context is current) calls some EGL/GLES API under the share group lock, that may cause calling `doDeferredAcquireNextImage()` on the Surface from the first thread. Calling this method may call `TryAcquireNextImageUnlocked()` under the share group lock, while it is also called from the first thread in the unlocked tail call. Calling `doDeferredAcquireNextImage()` may also cause swapchain recreation. Taking into account above scenario, current implementation has following bugs: 1. Possibility to recreate the swapchain from other thread, while first thread is going to or calling the `TryAcquireNextImageUnlocked()` function: a) Other thread may call `prepareForAcquireNextSwapchainImage()` because `NeedToProcessAcquireNextImageResult()` is still false until first thread finishes `TryAcquireNextImageUnlocked()` call. Also, checking `NeedToProcessAcquireNextImageResult()` from other thread is not thread safe. b) Other thread finished `TryAcquireNextImageUnlocked()` first, but with `VK_ERROR_OUT_OF_DATE_KHR` error. First thread still did not get the chance to call this function because of OS's thread scheduling delays. Other thread will set `needToAcquireNextSwapchainImage = true` and will start to recreate a swapchain. At this time, first thread will start executing `TryAcquireNextImageUnlocked()`, and because bool is true - it will actually perform ANI during recreate from other thread. 2. Possibility to call `TryAcquireNextImageUnlocked()` with old swapchain. This is similar to (1), but this time call is delayed after swapchain is recreated. Since above scenario currently is not possible and the described future scenario is unlikely to ever happen, instead of fixing above problems this CL removes the multi-threading support to simplify the code and make these bugs N/A. Change removes word "try" from structures, members, and functions because without multithreading there is no need for that and just adds unnecessary complication. It also removes most of the "share group lock" mentions because now this information is irrelevant and may cause confusion. What is relevant is that `AcquireNextImageUnlocked()` MUST only be called from a thread where Surface is current and only CAN access members that is only ever accessed from this thread (regardless if the lock is taken or not). For example, if in the future `unlockedAcquireResult` will be accessed from other thread but with the share group lock held, while `AcquireNextImageUnlocked()` will still only be called from the current thread - then this will be a race condition. Bug: angleproject:42265346 Bug: angleproject:42266579 Change-Id: Ie9be408c0a3b3c1f37ec80727ce5ad2cb8932dad Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6018093 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Jiawei Gu 6b9b8239 2024-11-21T15:56:40 Vulkan: fix un-initialized mCompatiblePresentModes When VK_EXT_swapchain_maintenance1 is supported but VK_EXT_surface_maintenance1 is not, mCompatiblePresentModes has no chance to be initialized. So need to set it up in such case. Bug: None Change-Id: I38ac6d9da3d6cebf2effcdcd74084a53d68a32c2 Signed-off-by: Jiawei Gu <gujiawei.will@bytedance.com> Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6040834 Reviewed-by: Yuxin Hu <yuxinhu@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Charlie Lao 70d1ef67 2024-11-20T11:34:39 Vulkan: Ensure onFramebufferBoundary is called for offscreen There is peak memory regression observed from crrev.com/c/6022549. What I suspect happening is that for offscreen or single buffered case, glFlush/glFinish is called but bail out because it already submitted or deferred. So we end up not calling onFramebufferBoundary(). This CL ensures we always call onFramebufferBoundary from these two functions for single buffer or offscreen. Also fixed a bug when onSharedPresentContextFlush is called we may end up calling onFramebufferBoundary. To make API names consistent, existing flushImpl() is renamed to flushAndSubmitCommands() and a new flushIMpl is added to wrap around most logic inside flush(). Bug: angleproject:372268711 Change-Id: I54eed8a81f4153d52ab962f213cacc87a73b89ac Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6037491 Reviewed-by: Yuxin Hu <yuxinhu@google.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Charlie Lao <cclao@google.com>
Igor Nazarov 17904b43 2024-11-19T16:37:43 Vulkan: Restrict EGL_ANDROID_front_buffer_auto_refresh support Original functionality supported scenario where `VK_PRESENT_MODE_SHARED_DEMAND_REFRESH_KHR` and `VK_PRESENT_MODE_SHARED_CONTINUOUS_REFRESH_KHR` modes may be incompatible, requiring a call to `deferAcquireNextImage()` method in order to cause swapchain recreation without swap. The `eglSurfaceAttrib()` may be called from any thread, this means that `deferAcquireNextImage` may be also called from any thread - which is not thread safe. This CL restricts exposing the extension only when VK_EXT_swapchain_maintenance1 extension is also supported, where present modes expected to be compatible on Android, so no need for swapchain recreation and therefore - `deferAcquireNextImage()` call. Not requiring to call `deferAcquireNextImage()` improves thread safety of `WindowSurfaceVk::setAutoRefreshEnabled()` method. There is still race condition accessing `mDesiredSwapchainPresentMode` but it will be addressed in a separate CL. Bug: angleproject:42265697 Bug: angleproject:379762019 Change-Id: I4631c736188eb52a5476e1a2bed3439d49cf12d4 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6035187 Commit-Queue: Igor Nazarov <i.nazarov@samsung.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Charlie Lao 74f74b63 2024-11-14T10:07:34 Vulkan: Add ContextVk::onFramebufferBoundary() function This makes a more formal API to track frame boundary. Also adds a uint32_t mCurrentFrameCount to track the total number of frames rendered so that we could use for heuristic purpose. Bug: angleproject:372268711 Change-Id: I153497403ed0d8fde18f1786186ce600df60c514 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6022549 Reviewed-by: Yuxin Hu <yuxinhu@google.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Charlie Lao <cclao@google.com>
Igor Nazarov bf07bcb7 2024-11-11T17:23:43 Vulkan: Remove dead code since mSwapchainImages always valid All elements of the `mSwapchainImages` always contain valid images. This fact makes code calling `acquireNextSwapchainImage()` in the `WindowSurfaceVk::lockSurface()` a dead code. Probably, instead of `acquireNextSwapchainImage()` it should call `doDeferredAcquireNextImage()` if `needsAcquireImageOrProcessResult()` is true, but this is another issue. This CL simply removes a dead code. Bug: angleproject:42264593 Bug: angleproject:42262606 Change-Id: Ic7bda330d74e1ccad149ffab723954f45dab700b Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6018092 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi 08c1724f 2024-10-11T14:29:00 Vulkan: Support GL_ARM_shader_framebuffer_fetch_depth_stencil Bug: angleproject:352364582 Change-Id: I63fd78314fa7ebccbf366c252e309a9c0f09c8c1 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5938150 Reviewed-by: Charlie Lao <cclao@google.com> Reviewed-by: Yuxin Hu <yuxinhu@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi 65fcf9c4 2024-10-26T10:53:18 Vulkan: Remove redundant dependent feature checks Since [1], when a feature is overriden, the dependent features automatically take the override into account. Tests no longer need to account for dependent features, neither does the logic in the code. [1]:https://chromium-review.googlesource.com/c/angle/angle/+/4749524 Bug: angleproject:42266725 Change-Id: I5440aba4a89cffbe710e26ad7de4cfee783e9bdf Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5967414 Reviewed-by: Yuxin Hu <yuxinhu@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com>
Shahbaz Youssefi 1608d0be 2024-10-10T16:53:15 Vulkan: Isolate framebuffer fetch no-RP-break optim from DR Prior to [1], changes to framebuffer fetch usage by shaders caused a render pass break. This was due to a limitation of render pass compatibility rules. It also caused other headache, such as needing to clear the render pass cache, recreating pipelines etc. [1]:https://chromium-review.googlesource.com/c/angle/angle/+/3697308 In [1] an important optimization was implemented for tiling GPUs where ANGLE permanently switched to framebuffer fetch mode on first encountering framebuffer fetch use. From that point on, ANGLE would always make every render pass framebuffer fetch compatible. In reality, the render pass break was unnecessary, which became apparent with dynamic rendering (for example that whether the render pass includes input attachments has no bearing on a pipeline that doesn't use input attachments at all). In [2], dynamic rendering kept the render pass break + permanent switch behavior for simplicity. [2]:https://chromium-review.googlesource.com/c/angle/angle/+/5637155 This change untangles the optimization done for legacy render passes from dynamic rendering, allowing dynamic rendering to start every render pass without framebuffer fetch and enable it later if a framebuffer fetch program is used. This is in preparation for supporting depth/stencil framebuffer fetch, where a perma-switch is troublesome (for example in combination with read-only depth/stencil feedback loops). Bug: angleproject:352364582 Change-Id: I31221cf22a28d58b9b2bf188e9c0b786cd0fe3d2 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5923120 Reviewed-by: Yuxin Hu <yuxinhu@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com>
Roman Lavrov fdec6935 2024-09-20T16:29:03 Workaround supportsSurfaceMaintenance1 on Linux with llvmpipe Presence of multiple ICD appears to confuse the loader and we end up with support reported but vkGetPhysicalDeviceSurfaceCapabilities2KHR not actually working (see bug for more detials) Bug: angleproject:368647924 Change-Id: Idc813b99f873643d067cc4b4e38f1ed5249500b4 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5875015 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Roman Lavrov <romanl@google.com>
Mohan Maiya 1b4d6185 2024-09-12T09:18:46 Vulkan: Cleanup sRGB related code Image and image view code is littered with sRGB related enums, even in places that don't deal with sRGB. Remove sRGB related parameters from initLayerImageView and getLevelLayerDrawImageView methods, which now assume default values. Add dedicated methods that allow overriding sRGB state values. Also introduce ColorspaceState struct that consolidates all sRGB related states, this will be used in follow up changes to track and infer colorspace of image views Bug: angleproject:40644776 Change-Id: Ifb366db48043e376f9ff6c30c852c44dd96562a1 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5860808 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: mohan maiya <m.maiya@samsung.com>
Hailin Zhang 8c12874c 2024-09-12T11:26:29 vulkan: add double buffer swapchain for fifo mode. Double buffer swapchain usually drop the performance. But at B* the default fps cap for silver device still at 30fps. This change is for reduce latency. Bug: b/311022968 Change-Id: Ida4044f439bbe3f235d53f5d1d2f945533cbb094 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5858255 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Igor Nazarov 177d15b3 2024-08-23T19:34:34 Vulkan: Simplify WindowSurfaceVk::prepareSwap() method In case of swapchain recreate inside `prepareSwap()`, perform unlocked ANI call as usual. ANI processing will be done late in `swap()` or `getBufferAge()`. This is possible, because surface `getWidth/Height()` is no longer require valid `mColorRenderTarget`. Because of the above, `swapchainRecreatedOut` parameter was removed. Additionally, `getBufferAge()` now uses `doDeferredAcquireNextImageWithUsableSwapchain()` in order to process possible VK_ERROR_OUT_OF_DATE_KHR error and recreate the swapchain, instead of failing. Test: angle_end2end_tests --gtest-filter=EGLSingleBufferTest.AcquireImageFromSwapImpl* Bug: angleproject:6851 Bug: angleproject:8133 Change-Id: Ieb375781da2b1108d0a5e11dbb012885a946688e Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5806335 Reviewed-by: Charlie Lao <cclao@google.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com>
Igor Nazarov 636d838e 2024-08-27T16:30:04 Vulkan: Decouple Window Surface width/height from color RT `mColorRenderTarget` is invalidated in `releaseSwapchainImages()` method, when swapchain is recreated. It remains invalid until ANI result is processed in the `postProcessUnlockedTryAcquire()` method. In case if swapchain recreate and processing of the results are done in different API calls (prepareSwap and swap), other thread way query invalid render target size using `eglQuerySurface()` API. Currently this should not be possible, but may change in the future. Another way to have invalid render target, is if ANI fails after swapchain recreate. To avoid above problems, surface size is cached after swapchain is recreated. Implementation is moved to the `SurfaceVk` base class. The `WindowSurfaceVk::getRotatedWidth/Height()` methods were unused. `mColorRenderTarget` is now only acessed after ANI result is processed. Bug: angleproject:1319 Change-Id: Ibc007f1e8d30b6281be93c4561e05dd2f14f44d3 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5816834 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com> Reviewed-by: Charlie Lao <cclao@google.com>
Igor Nazarov 74816e9e 2024-08-29T15:58:22 Vulkan: Cleanup RenderTargetVk release/destroy methods Current implementation of `release()` and `destroy()` methods does not actually invalidate state of the object. All they do is manage the Framebuffer cache. The `release()` method renamed to `releaseFramebuffers()` to match its behavior. Added new `releaseImageAndViews()` method that calls `releaseFramebuffers()` and also null the pointers in order to catch invalid usage in ASSSERTs (for example: usage of `SurfaceVk::mColorRenderTarget` after swapchain recreation but before ANI processing). `destroy()` is updated to also call `reset()`. `releaseFramebuffers()` is only used in `TextureVk` class. In case of `releaseImageViews()`, arrays are cleared, so there is no point calling `releaseImageAndViews()`. In case of `refreshImageViews()`, render targets may be reused, since all pointers remain valid. `RenderbufferVk` and `WindowSurfaceVk` are using new `releaseImageAndViews()` and updated `destroy()` methods. Other changes: - Replace clearing of RT arrays in `TextureVk::setImageHelper()` with ASSERTs. - Fix ASSERT in `TextureVk::releaseImageViews()`, but it seems that this code path is impossible. Bug: b/234769934 Change-Id: I431d25b81dd4dd343149c12e680e5c997aa18436 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5822575 Reviewed-by: Charlie Lao <cclao@google.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com>
Shahbaz Youssefi 1db80b88 2024-07-10T12:47:42 Reland "Vulkan: Use VK_KHR_dynamic_rendering[_local_read]" This is a reland of commit c379ff48043a47e444c388c45270db40d3172d50 Original change's description: > Vulkan: Use VK_KHR_dynamic_rendering[_local_read] > > Bug: angleproject:42267038 > Change-Id: I1f4eb0f309992a9c1c287a69520dadf5eff23b26 > Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5637155 > Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> > Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com> > Reviewed-by: Charlie Lao <cclao@google.com> Bug: angleproject:42267038 Change-Id: I083e6963b5421386695e49a9872edbb2016c9763 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5691342 Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
Shahbaz Youssefi 584fbcee 2024-07-10T12:43:34 Vulkan: Rework swap-time barrier logic Avoids unnecessary transitions when overlay is enabled Bug: angleproject:42267038 Change-Id: I0534911c0142c5e94cf3be112283fb98fcde0f6c Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5691346 Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com>
Shahbaz Youssefi 373ac541 2024-07-10T11:14:47 Vulkan: Make surface RP check independent from framebuffer object With dynamic rendering, there is no framebuffer object, so checking whether the currently open render pass belongs to the window surface (at swap time) is made independent from these objects. Bug: angleproject:42267038 Change-Id: I408e2376ba865b64fa1e8890316e8f57c08c695f Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5691345 Reviewed-by: Charlie Lao <cclao@google.com> Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com> Commit-Queue: Charlie Lao <cclao@google.com> Auto-Submit: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi 7d461b21 2024-07-10T14:11:53 Revert "Vulkan: Use VK_KHR_dynamic_rendering[_local_read]" This reverts commit c379ff48043a47e444c388c45270db40d3172d50. Reason for revert: Regresses CPU perf and memory when _not_ using DR Original change's description: > Vulkan: Use VK_KHR_dynamic_rendering[_local_read] > > Bug: angleproject:42267038 > Change-Id: I1f4eb0f309992a9c1c287a69520dadf5eff23b26 > Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5637155 > Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> > Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com> > Reviewed-by: Charlie Lao <cclao@google.com> Bug: angleproject:42267038 Change-Id: I3865f0d86813f0eeb9085a92875a33bd449b907f Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5691337 Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Igor Nazarov 6052d0f4 2024-07-03T15:17:23 Vulkan: Fix EGL_EXT_buffer_age with single buffered surface The specification: 4) What is the buffer age of a single buffered surface? RESOLVED: 0. This falls out implicitly from the buffer age calculations, which dictate that a buffer's age starts at 0, and is only incremented by frame boundaries. Since frame boundary functions do not affect single buffered surfaces, their age will always be 0. Test: angle_end2end_tests --gtest_filter=EGLBufferAgeTest.SingleBuffer* Bug: angleproject:3529 Change-Id: Iab5da26d1bce56d928398cc250d900465fc5d261 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5675986 Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi c379ff48 2024-06-10T22:01:57 Vulkan: Use VK_KHR_dynamic_rendering[_local_read] Bug: angleproject:42267038 Change-Id: I1f4eb0f309992a9c1c287a69520dadf5eff23b26 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5637155 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com> Reviewed-by: Charlie Lao <cclao@google.com>
Charlie Lao 8c546d35 2024-06-25T12:49:40 Vulkan: Limit VkEvent for usage matters for Manhattan31 only If we use VkEvent to track all image operations causes performance regression on some app traces, including manhattan10 trace. This mainly because of CPU overhead comes with VkCmdSetEvent, mostly inside vulkan driver. These app traces likely not benefit from VkEvent because the specific bubble (false dependency) does not manifest on these app traces, but the CPU overhead takes a performance toll on it. In order to strike a balance between benefit and overhead, this CL removes most of VkEvent usage and only leaves the ones that matters for manhattan31. The only we still keeps are generateMipmap, dispatchCompute, texture sampling. We can always add more if more beneficial usage cases comes up and no regression in other traces. Bug: b/336844257 Change-Id: I346fe70bc33e57edf04e933a2db0f79738c4481d Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5654737 Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Charlie Lao <cclao@google.com>
Shahbaz Youssefi d193d51b 2024-06-17T22:46:08 Replace issue ids post migration to new issue tracker This change replaces anglebug.com/NNNN links. Bug: None Change-Id: I8ac3aec8d2a8a844b3d7b99fc0a6b2be8da31761 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5637912 Reviewed-by: Geoff Lang <geofflang@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi e29d643e 2024-05-07T13:40:18 Vulkan: Fix missing per-present-mode query ... if VK_EXT_surface_maintenance1 is supported but VK_EXT_swapchain_maintenance1 isn't. The code mistakenly made the appropriate query conditional to the presence of swapchain_maint1 instead of surface_maint1. On devices where surface_maint1 was available but swapchain_maint1 wasn't, this caused a VVL error that is now fixed. Bug: angleproject:8680 Change-Id: Ide9a87f0e50887572f693d741d5476361320ea19 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5522756 Reviewed-by: Cody Northrop <cnorthrop@google.com> Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi c3a1cae4 2024-04-15T14:58:55 Use angle::SimpleMutex everywhere in libGLESv2 Only cases left that use std::mutex are: - Share group and the context ErrorSet mutexes as they need try_lock() - Anywhere mutexes are used in conjunction with std::condition_variables (as they explicitly require std::mutex) Bug: angleproject:8667 Change-Id: Ib6d68938b0886f9e7c43e023162557990ecfb300 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5453294 Reviewed-by: Roman Lavrov <romanl@google.com> Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Charlie Lao a96e9197 2024-04-25T10:35:02 Vulkan: Add RefCountedEvent class and VkCmdSetEvent call This CL defines RefCountedEvent class that adds reference counting to VkEvent. CommandBufferHelper and ImageHelper each holds one reference count to the event. Every time an event is added to the command buffer, the corresponding RefCountedEvent will be added to the garbage list which tracks the GPU completion using ResourceUse. That event garbage's reference count will not decremented until GPU is finished, thus ensures we never destroy a VkEvent until GPU is completed. For images used by RenderPassCommands, As RenderPassCommandBufferHelper::imageRead and imageWrite get called, an event with that layout gets created and added to the image. That event is saved in RenderPassCommandBufferHelper::mRefCountedEvents and that VkCmdSetEvents calls are issued from RenderPassCommandBufferHelper::flushToPrimary(). For renderPass attachments, the events are created and added to image when attachment image gets finalized. For images used in OutsideRenderPassCommands, The events are inserted as needed as we generates commands that uses image. We do not wait until commands gets flushed to issue VkCmdSetEvent calls. A convenient function trackImageWithEvent() is added to create and setEvent and add event to image all in one call. You can add this call after the image operation whenever we think it benefits, which gives us better control. (Note: Even if forgot to insert the trackImageWithEvent call, it is still okay since every time barrier is inserted, the event gets released. Next time when we inserts barrier again we will fallback to pipelineBarrier since there is no event associated with it. But that is next CL's content). This CL only adds the VkCmdSetEvent call when feature flag is enabled. The feature flag is still disabled and no VkCmdWaitEvent is used in this CL (will be added in later CL). Bug: b/336844257 Change-Id: Iae5c4d2553a80f0f74cd6065d72a9c592c79f075 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5490203 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Geoff Lang <geofflang@chromium.org> Commit-Queue: Charlie Lao <cclao@google.com>
Charlie Lao c74d2634 2024-04-19T12:19:51 Vulkan: Update mSurfaceCaps with per-present mode caps. Vulkan spec says "The per-present mode image counts may be less-than or greater-than the image counts returned when VkSurfacePresentModeEXT is not provided". Riht now we are using the generic surfaceCaps information which has smaller minImageCount. This triggers VVL error VUID-VkSwapchainCreateInfoKHR-presentMode-02839.. This CL updates mSurfaceCaps and mMinImageCount with per-present mode caps and create swapChain with proper minImageCount. Bug: angleproject:8672 Change-Id: I6061d2db8e98032dcb2c3225f2976013a99336a9 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5467850 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Yuly Novikov <ynovikov@chromium.org> Commit-Queue: Charlie Lao <cclao@google.com>
Shahbaz Youssefi 9475ac40 2023-11-15T10:25:06 Vulkan: Make efficient MSAA resolve possible Prior to this change, using a resolve attachment to implement resolve through glBlitFramebuffer was done by temporarily modifying the source FramebufferVk's framebuffer description. This caused a good deal of complexity; enough to require the render pass to be immediately closed after this optimization. The downsides to this are: - Only one attachment can be efficiently resolved - There is no chance for the MSAA attachment to be invalidated In this change, resolve attachments that are added because of glBlitFramebuffer are stored in the command buffer, with the FramebufferVk completely oblivious to them. When the render pass is closed, either the FramebufferVk's original framebuffer object is used (if no resolve attachments are added) or a temporary one is created to include those resolve attachments. With the above method, the render pass is able to accumulate many resolve attachments as well as have its MSAA attachments be invalidated before it is flushed. For a FramebufferVk that is resolved in this way, there used to be two framebuffers created each time and thrown away as the code alternated between starting a render pass without a resolve attachment and then closing with one. With this change, there is now one framebuffer (without resolve attachments) that is cached in FramebufferVk (and is not recreated every time), and only the framebuffer with resolve attachments is recreated every time. Ultimatley, when VK_KHR_dynamic_rendering is implemented in ANGLE, there would be no framebuffers to create and destroy, and this change paves the way for that support too. WindowSurfaceVk framebuffers are still imagefull. Making them imageless adds unnecessary complication with no benefit. ----------------- To achieve efficient MSAA rendering on tiling hardware, applications should do the following: ``` glBindFramebuffer(GL_FRAMEBUFFER, msaaFBO); // Clear the framebuffer to avoid a load // Or invalidate, if not needed to load: // glInvalidateFramebuffer(GL_DRAW_FRAMEBUFFER, ...); glClear(...); // Draw calls // Resolve into the single sampled framebuffer glBindFramebuffer(GL_DRAW_FRAMEBUFFER, resolveFBO); glBlitFramebuffer(...); // Immediately discard the contents of the MSAA buffer, to avoid store glInvalidateFramebuffer(GL_READ_FRAMEBUFFER, ...); ``` The above would translate to the following Vulkan render pass: - MSAA LOAD_OP_CLEAR/DONT_CARE - MSAA STORE_OP_DONT_CARE - Resolve LOAD_OP_DONT_CARE - Resolve STORE_OP_STORE This makes sure the MSAA data doesn't leave the tile memory and greatly reduces bandwidth usage. Once anglebug.com/4892 is fixed, this would also allow the MSAA image to never be allocated either. Bug: angleproject:7551 Bug: angleproject:8625 Change-Id: Ia9f4d20863d76a013d8495033f95c7b39f77e062 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5388492 Reviewed-by: Yuxin Hu <yuxinhu@google.com> Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Mohan Maiya 318e5e02 2024-03-24T08:17:56 Vulkan: Update EGL_EXT_buffer_age implementation 1. Buffer age is always 1 when swap behavior is EGL_BUFFER_PRESERVED. 2. WindowSurfaceVk::getBufferAge no longer acquires a swapchain image. See commit: b46cf6989f6fe8db5f0759001f633681a96fadde 3. It is valid to pass attributes of eglQuerySurface API to eglQuerySurface64KHR API 4. Add deadlock fix to eglQuerySurface64KHR Bug: angleproject:3529 Bug: angleproject:6851 Tests: EGLLockSurface3Test.QuerySurfaceAndQuerySurface64Parity* Tests: EGLBufferAgeTest.QueryBufferAge* Change-Id: Idf3c4fc08364f671fb02e99111be2beb7a1d9f3d Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5389461 Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com> Commit-Queue: mohan maiya <m.maiya@samsung.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Yiwei Zhang e3aac00b 2024-03-18T18:18:36 Vulkan: only request OPAQUE compositeAlpha on Android if no alpha Android used to only advertise INHERIT bit with the expectations that a Vulkan swapchain is only connected with SurfaceFlinger so that WM takes full control of the composition. That works fine till an encoder surface with EGL client (via ANGLE + KHR_swapchain) hits a suboptimal code path if the backing storage is RGBA but not RGBX. This change prepares for Android KHR_swapchain advertising OPAQUE bit for such surfaces as a hint to get RGBX backing. Bug: b/328125698, b/328122401 Test: encoder cts is passing, and no regression to ImageReader use cases Change-Id: Ia7f1c69935690f20efac6295eb824bee0d03eb07 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5378134 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi 914fe61b 2024-03-15T13:20:49 Vulkan: Rename RendererVk.* to vk_renderer.* Done in a separate CL from the move to namespace vk to avoid possible rebase-time confusion with the file name change. Bug: angleproject:8564 Change-Id: Ibab79029834b88514d4466a7a4c076b1352bc450 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5370107 Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com> Commit-Queue: Amirali Abdolrashidi <abdolrashidi@google.com>
Shahbaz Youssefi 60aaf4a0 2024-03-14T12:58:56 Vulkan: Move renderer to namespace vk This class is agnostic of EGL. This change moves it to namespace vk for use with the OpenCL implementation Bug: angleproject:8564 Change-Id: I57f7807d6af8b3d5d7f8efbaf8b5d537a930f881 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5371324 Reviewed-by: Austin Annestrand <a.annestrand@samsung.com> Reviewed-by: Geoff Lang <geofflang@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi aba3705b 2024-03-02T21:58:34 Vulkan: Completely remove egl::Display from RendererVk This class is now independent of EGL. The only mentions of EGL is egl::ContextPriority, which is just an enum and is tolerable for now. OpenCL can now instantiate RendererVk without having to create a temp egl::Display. Bug: angleproject:8564 Change-Id: Ia78cfcb3a48c97f397441cf7cda71d74cfaddd8f Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5335581 Reviewed-by: Geoff Lang <geofflang@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Auto-Submit: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi 545e3f6e 2024-03-01T23:27:03 Vulkan: Decouple RendererVk from egl::BlobCache The new vk::GlobalOps class abstracts access to egl::BlobCache. This is a step towards decoupling RendererVk from egl::Display for direct use with OpenCL. Bug: angleproject:8564 Change-Id: I7b3910254430df74b889759639da1749735584a7 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5332082 Reviewed-by: Geoff Lang <geofflang@chromium.org> Commit-Queue: Geoff Lang <geofflang@chromium.org> Auto-Submit: Shahbaz Youssefi <syoussefi@chromium.org>
Chris Forbes 6367f541 2024-01-25T10:16:04 Vulkan: supply YcbcrConversionDesc earlier Previously, the AHB import path would allow ImageHelper to build a bogus YcbcrConversionDesc (in initExternal) and then later overwrite it with what it wanted. The intermediate state was not necessarily valid, and could cause assertion failures and VVL errors. Instead, have ImageHelper clients provide the conversion they want upfront. In the non-external case, build an appropriate conversion for formats which need them, before delegating to initExternal. Bug: b/315387961 Change-Id: Icc8f561bb2de0289ceec56d41978b8c4651a47a2 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5232769 Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: Chris Forbes <chrisforbes@google.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi 341906d5 2024-01-22T10:19:25 Vulkan: Never delay device and queue selection Previously, when multiple queue families where eligible, ANGLE delayed the decision on which queue family to use and delayed device creation until a window surface was created. This assumption was never correct. The application may start rendering to pbuffers or using EGL_KHR_surfaceless_context long before it creates a window surface. So a queue may need to be chosen regardless of its surface present capabilities. Once done, ANGLE has no mechanism to change that queue should a window surface require a different queue. This change drops the pretense to support multiple queue families properly and makes ANGLE always choose the first graphics queue family it encounters. So far, only MoltenVk seems to expose multiple graphics queue families, and choosing the first one ultimately correct for the purposes of presenting to a MacOS surface. Bug: angleproject:8478 Change-Id: I8efbfe0c8036be5f9cee01eb657f83f85a4864fd Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5225081 Reviewed-by: Charlie Lao <cclao@google.com> Reviewed-by: Yuxin Hu <yuxinhu@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Shahbaz Youssefi b46cf698 2023-11-30T11:00:18 Vulkan: Fix Android deadlock with querying buffer age Similar to eglSwapBuffers, eglQuerySurface with EGL_BUFFER_AGE_EXT can cause a call to vkAcquireNextImageKHR. Bug: angleproject:6851 Bug: b/313975825 Change-Id: If3f0521219cab9aba2aeb2b70958bf0f197bc96a Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5077406 Reviewed-by: Igor Nazarov <i.nazarov@samsung.com> Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Charlie Lao f7eb7efb 2023-11-28T11:20:57 Vulkan: Fix camera panorama hang When EGL attributes does not speficy color space, right now we pick VK_COLOR_SPACE_SRGB_NONLINEAR_KHR. This overwrites the color space people set via ANativeWindow. This causes camera goes into CSC conversion and errors out and stops media codec pipeline and causes camera to hang. This CL enables mapUnspecifiedColorSpaceToPassThrough feature flag for all platform that supports VK_EXT_swapchain_colorspace so that it will pick VK_COLOR_SPACE_PASS_THROUGH_EXT and vulkan swapchain code will try to keep whatever color space surface already has. The feature flag is still kept here in case there is regression and we need to experiment this feature flag again. We can remove it once everything settles down. This CL also adds updateColorSpace() and getActualFormatID() to de-duplicate the logic of R8G8B8_UNORM overriding and use of VK_COLOR_SPACE_PASS_THROUGH_EXT. Bug: b/309480316 Bug: b/302196568 Change-Id: I0952fe78c2bfd59446391c553745b2b5cb152a9d Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5066801 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Chris Forbes <chrisforbes@google.com> Commit-Queue: Charlie Lao <cclao@google.com>
Shahbaz Youssefi c5eb810b 2023-11-14T15:01:25 Remove angle::Result::Incomplete entirely Bug: angleproject:8414 Change-Id: I8d607614614360b65c530e3e7647d8a7ba38b18e Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5031191 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Geoff Lang <geofflang@chromium.org>
Amirali Abdolrashidi e7418836 2023-08-16T14:25:52 Vulkan: Add context flushing as OOM fallback * As a new fallback for out-of-memory errors, if an allocation results in device OOM, the context is flushed and the allocation is retried. * Functions related to buffer/image allocations now return a VkResult value instead of angle::Result, which will be bubbled up to a higher level for safer handling. * The OOM is no longer handled at the level where the allocation happens, but is moved up to the context. * Added two functions to ContextVk for allocating memory for images and buffer suballocations, which also include the fallback options. * initBufferAllocation(): Uses BufferHelper::initSuballocation() * initImageAllocation(): Uses ImageHelper::initMemory() * Moved initNonZeroMemory() out of the following functions: * BufferHelper::initSuballocation() * Moved to ContextVk::initBufferAllocation(). * ImageHelper::initMemory() * Moved to ContextVk::initImageAllocation(). * Also moved to new function: ImageHelper::initMemoryAndNonZeroFillIfNeeded(). This function replaced the rest of initMemory() usages outside initImageAllocation(). * New macros for memory allocation * VK_RESULT_TRY() * If the output of the command inside it is not VK_SUCCESS, it will return with the error result from the command. * VK_RESULT_CHECK() * If the output of the command inside it is not VK_SUCCESS, it will return with the input error. * Added a test in which allocation would fail due to too much pending garbage without the fix on some platforms. The test ends once there has been a submission. * New suite: UniformBufferMemoryTest * Added a similar test for flushing texture-related pending garbage. * New suite: Texture2DMemoryTestES3 Bug: b/280304441 Change-Id: I60248ce39eae80b5a8ffe4723d8a1c5641087f23 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4787949 Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: Amirali Abdolrashidi <abdolrashidi@google.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Mohan Maiya 9740b01b 2023-09-08T16:30:08 Enhance UnlockedTailCall run method UnlockedTailCall::CallType is now std::function<void(void *)> This is in preparation for upcoming changes where unlocked tail calls need access to objects outside block and namespace scope. Bug: angleproject:8340 Tests: UnlockedTailCall* Change-Id: Ida6822b701c5c11ce4b8f6e3aae53108755e2cad Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4852021 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: mohan maiya <m.maiya@samsung.com>
Cody Northrop 00daa451 2023-08-28T16:30:23 Vulkan: Include minImageCount in swapchain check The following VVL error has been firing for traces run in landscape mode: [ VUID-VkSwapchainCreateInfoKHR-presentMode-02839 ] vkCreateSwapchainKHR(): pCreateInfo.minImageCount 4, which is outside the bounds returned by vkGetPhysicalDeviceSurfaceCapabilitiesKHR() (i.e. minImageCount = 5, maxImageCount = 64). On Android, rotation can cause minImageCount to change. We need to detect this as an out of date swapchain. Test: angle_trace_test --gtest_filter=TraceTest.among_us Bug: b/289274676 Change-Id: Ie75adec5f5318b73c0c27efc134f10f53485692d Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4819790 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Cody Northrop <cnorthrop@google.com> Reviewed-by: Charlie Lao <cclao@google.com>
Mohan Maiya 0d67d1bb 2023-08-29T11:19:35 Vulkan: Support EGL_EXT_gl_colorspace_bt2020 Add support for following 3 BT2020 colorspace extensions 1. EGL_EXT_gl_colorspace_bt2020_hlg 2. EGL_EXT_gl_colorspace_bt2020_linear 3. EGL_EXT_gl_colorspace_bt2020_pq Bug: angleproject:8330 Tests: EGLSurfaceTest.CreateWithEGLConfig1010102Support* Change-Id: I3a6ca0daa9ad4400da3834f43678b0db816ba6e4 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4824045 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: mohan maiya <m.maiya@samsung.com>
Jason Macnak 8138ae9b 2023-07-07T14:16:07 Vulkan: EGL's DISPLAY_P3_PASSTHROUGH -> VK's DISPLAY_P3_NONLINEAR From https://registry.khronos.org/EGL/extensions/EXT/EGL_EXT_gl_colorspace_display_p3_passthrough.txt If its value is EGL_GL_COLORSPACE_DISPLAY_P3_PASSTHROUGH_EXT, then a non-linear, sRGB encoded Display-P3 color space is assumed Bug: b/289125521 Test: cvd start Test: cts -m CtsViewTestCases -t android.view.cts.TextureViewTest#testGetBitmap_8888_PassthroughP3 Change-Id: I07c77c86bbc0d82923ad7435cd2a5558770cd2e3 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4673910 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Chris Forbes <chrisforbes@google.com> Commit-Queue: Jason Macnak <natsu@google.com>
Hailin Zhang 6eea5ff4 2023-07-18T12:52:37 Vulkan: fix default MSAA framebuffer clear issue. Bug: b/290813597 Change-Id: I134c5a99382ca30dbd885a17dfa3c7ac227480ff Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4698113 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Hailin Zhang <hailinzhang@google.com>
Steven Noonan ac263582 2023-06-19T12:26:44 Vulkan: add workaround for VK_EXT_full_screen_exclusive on AMD On outdated (but recent) AMD drivers, the Windows-only Vulkan extension VK_EXT_full_screen_exclusive appeared to be implicitly enabled and set to VK_FULL_SCREEN_EXCLUSIVE_APPLICATION_CONTROLLED_EXT mode. Even though ANGLE did not enable or interact with this extension at all, the driver was incorrectly returning VK_ERROR_FULL_SCREEN_EXCLUSIVE_MODE_LOST_EXT error codes on various swapchain operations when the full screen window focus was lost (i.e. alt-tab out and back in). Naturally, ANGLE was not expecting these error codes and did not know how to handle them. Depending on where the errors occurred, ANGLE might crash or retry creating the swapchain repeatedly. Treating the unexpected VK_ERROR_FULL_SCREEN_EXCLUSIVE_MODE_LOST_EXT error code as VK_ERROR_OUT_OF_DATE_KHR/VK_SUBOPTIMAL_KHR was not sufficient, because the driver would repeat the error on every swapchain operation, apparently expecting the error to be handled by a vkAcquireFullScreenExclusiveModeEXT call (even though that would make no sense, since the extension was not enabled). The incorrect driver behavior was reported to AMD and was fixed in recent driver releases. The earliest driver I've tested and know to be working is AMD's Adrenaline driver version 23.5.2 (VkPhysicalDeviceProperties calls this driverVersion 2.0.262/0x800106). The last known bad version was 0x8000e9. The simplest workaround on these older AMD graphics drivers is to explicitly enable the extension, but set it to VK_FULL_SCREEN_EXCLUSIVE_DISALLOWED_EXT mode. On newer drivers we do not need to do anything with the extension and can ignore it. Bug: angleproject:8215 Change-Id: I7c58d47a0350f4b0bc1a77f200c1e2f72fcde8d8 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4627279 Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com>
Shahbaz Youssefi 86e21fa2 2023-06-12T13:36:21 Vulkan: Refactor angle::Format depth/stencil checks Bug: b/246008627 Change-Id: If0a2992c5bd66adf27c6866aea04e54ba465a522 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4608489 Reviewed-by: Roman Lavrov <romanl@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Yuxin Hu <yuxinhu@google.com>
Igor Nazarov dbffa5d3 2023-05-30T15:43:45 Vulkan: Update present history logic Keeping old Swapchain's `ImagePresentOperation`s with Fences can lead to multiple "old swapchains" lists. This may cause situations exceeding the `kMaxOldSwapchains` limit. Instead of marking `ImagePresentOperation` as corresponding to an older swapchain, accumulate all present history in the `mOldSwapchains`. Since Fences are only used when VK_EXT_swapchain_maintenance1 is supported, instead of attaching "old swapchains" to the first present operation, they are destroyed when present Fences are signaled in the new `cleanUpOldSwapchains()` method. Also this CL fixes a bug: if (mPresentHistory.empty() || mPresentHistory.back().imageIndex == kInvalidImageIndex) The above condition will always pass when VK_EXT_swapchain_maintenance1 is supported. Bug: angleproject:7847 Change-Id: I1a41944456756b8af740f1903c873823323a2e0b Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4505952 Reviewed-by: Charlie Lao <cclao@google.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com>
Igor Nazarov 95f57b19 2023-01-17T19:24:04 Vulkan: Fix Shared Present Mode "acquireNextSwapchainImage()" - no need to call "vkAcquireNextImageKHR()" in Shared Present Mode. - proper VkResult processing (VK_SUBOPTIMAL_KHR replaced with VK_SUCCESS). Bug: angleproject:6878 Change-Id: Ib20ae1f5de3a3cc13bfc4ef034076e3402f93d31 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4489464 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com>
Igor Nazarov 552e7468 2023-05-26T18:39:55 Vulkan: Do not use ANI Fence in present semaphore recycling Fences from `vkAcquireNextImageKHR()` has many problems on some Intel drivers, and one of them does not have a workaround. Instead of using ANI Fence, this change will use `swapSerial`, that has execution dependency on ANI semaphore. As result, semaphore recycling will be delayed a little longer, until the Device finises rendering to the image. On practice, this will not significantly affect the total number of allocated semaphores. As a bonus, this change also fixes a bug with Shared Present Mode: Current logic assumes that PE will only signal image's ANI fence after waiting on previous present semaphore with this image. In reality, PE returns same image index and fence that is always signaled. Additionally, ANI call is not required at all. There is no 100% safe solution when not using: VkSwapchainPresentFenceInfoEXT This CL will recycle present semaphores only after next present semaphore is signaled. This not only ensures, that present semaphores are not reused inflight, but also gives time to PE to finish waiting on the semaphore until next present. Bug: angleproject:8155 Bug: angleproject:7847 Change-Id: Ia112190d77ea7e04f8f9755cf56f5387744dc599 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4572426 Reviewed-by: Charlie Lao <cclao@google.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com>
Igor Nazarov 53b2db67 2023-05-26T13:34:39 Vulkan: Reset Swapchain Fence before recycling Depending on the Intel GPU hardware and driver version, crash may happen when accessing ANI Fence after Swapchain destruction. This access includes call to: `vkGetFenceStatus()`, `vkWaitForFences()`, `vkResetFences()`, and `vkDestroyFence()`. Resetting Fence before recycling will "detach" it from the Swapchain, allowing safe usage/destruction of the Fence after that Swapchain destruction. For other devices and drivers, this change is harmless. Only difference is that there will be extra reset calls for items that were recycled but never used before (number of such cases extremely low, compared to cases when Fences are reused). Additionally, this change adds `ASSERT(garbageObject.valid())` into the `Recycler<>`. Before this change, it was possible to recycle invalid fence in the `ImagePresentOperation::destroy()` method. Luckily, this happened only in `WindowSurfaceVk::destroy()`, where invalid recycled fence is not used (it is safe to destroy invalid fence). Bug: angleproject:8155 Change-Id: I0adbaf63333536523004a077977eb68cfaccd6f7 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4572425 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com> Reviewed-by: Charlie Lao <cclao@google.com>
Igor Nazarov bcccb6c4 2023-05-30T18:21:06 Vulkan: Minor fix in doDeferredAcquireNextImage() method If call `doDeferredAcquireNextImage()` after `prepareSwap()`, the `prepareForAcquireNextSwapchainImage()` method will be called again, and may possible recreate the Swapchain while there is ANI result that needs processing. In the current code the above is not possible (except if call `prepareSwap` directly from the application side). This may be possible in the future, when ANI is deferred until RenderPass end, that may happen in the `flushImpl()` of the `swapImpl()`. This change skips `prepareForAcquireNextSwapchainImage()` if there is ANI result that needs processing. Additionally, `doDeferredAcquireNextImageWithUsableSwapchain()` call in `swapImpl()` replaced with `doDeferredAcquireNextImage()` for simplicity. Possible call sequences after this change: prepareSwap: prepareForAcquireNextSwapchainImage() doDeferredAcquireNextImageWithUsableSwapchain() or prepareSwap: prepareForAcquireNextSwapchainImage() TryAcquireNextImageUnlocked() other API: doDeferredAcquireNextImage(): // skipped - prepareForAcquireNextSwapchainImage() doDeferredAcquireNextImageWithUsableSwapchain() or other API: doDeferredAcquireNextImage(): prepareForAcquireNextSwapchainImage() doDeferredAcquireNextImageWithUsableSwapchain() Bug: angleproject:6851 Bug: angleproject:8133 Change-Id: I4f973c05821fc9e43769cc2621f4467790118f23 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4572424 Reviewed-by: Charlie Lao <cclao@google.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com>
Igor Nazarov 7b07818e 2023-05-29T19:46:40 Vulkan: Call resetAcquireNextImageSemaphore() only when required Currently resetAcquireNextImageSemaphore() is called for each swapchain image when swapchain is recreated and when EGLSurface is destroyed. Releasing semaphores in case of swapchain recreation is not necessary, since swapchain should not be recreated while there still image that needs presenting. Call removed to reveal possible bugs, when recreate happens without such present. In case of EGLSurface destruction there may be only one image that has semaphore. Call moved from `destroySwapChainImages()` to `destroy()`. Bug: b/275624771 Change-Id: Ia4ce678f2b73785818150dfb276b19a5f5b7d25f Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4567550 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com>
Igor Nazarov da41e7d9 2023-05-29T19:17:48 Vulkan: Account for queueSubmitOneOff() in SurfaceVk::mUse There is a possibility to destroy EGLSurface without waiting for `queueSubmitOneOff()` submission. One possible way to reproduce the problem is to query `EGL_BUFFER_AGE_EXT` without any rendering in the EGL_SINGLE_BUFFER mode. This is a regression from: Reland "Vulkan: SurfaceVk should only wait for GPU work that uses it" https://chromium-review.googlesource.com/c/angle/angle/+/4406891 Test: angle_end2end_tests --gtest_filter=EGLSingleBufferTest.WaitOneOffSubmission* Bug: b/267806287 Change-Id: I9478fcc4fd64b38747cbd80dea51137da9ef5f21 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4567549 Reviewed-by: Charlie Lao <cclao@google.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Igor Nazarov e24f4519 2023-01-19T02:30:39 Vulkan: Add externalFence into submitCommands() Currently one-off fence in the `queueSubmitOneOff()` is used only in `SyncHelperNativeFence::initializeWithFd()` to submit external fence. Other `queueSubmitOneOff()` calls may use `QueueSerial` instead of a fence. Providing `fence` into `queueSubmitOneOff()` prevents tracking that submission with `QueueSerial`. Therefore using `mUse` to collecting `mFenceWithFd` as garbage will not work as intended. This CL removes `fence` from `queueSubmitOneOff()` and adds optional `externalFence` into `submitCommands()` instead. Providing `externalFence` will cause additional `vkQueueSubmit()` call: - first submission will submit everything as usual except using the `externalFence`. - second, will only submit internal `CommandQueue` fence for `QueueSerial` tracking. As the result of this CL, call to `initializeWithFd()` will always produce two (2) `vkQueueSubmit()` calls. Previously it may be one (1) or two (2) submissions. Future CL will reduce submission count to one (1). If add additional submission into `queueSubmitOneOff()` instead of `submitCommands()`, then maximum number of submissions will be three (3). Bug: angleproject:8117 Change-Id: I6f1ec12682aaab71bfc871e665fec2659df96b26 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4392877 Reviewed-by: Charlie Lao <cclao@google.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com>
Shahbaz Youssefi 7fd01d03 2023-04-19T00:54:24 Vulkan: Throttle the CPU without holding the global lock CPU throttling goes through CommandQueue and is thread-safe. Performing it in an unlocked tail call allows other unrelated EGL calls to go through. Bug: angleproject:8135 Change-Id: Idb3841be5d8ea8c4b76217f6707be26b28ea39c2 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4444027 Reviewed-by: Igor Nazarov <i.nazarov@samsung.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Charlie Lao <cclao@google.com>