|
41bc2afa
|
2025-10-02T16:22:33
|
|
Vulkan: Remove mBindingDirtyBitsRequiresPipelineUpdate
mBindingDirtyBitsRequiresPipelineUpdate and
mAttribDirtyBitsRequiresPipelineUpdate are used to avoid
GraphicsPipelineDesc::updateVertexInput() call. This function is not
being used when supportsVertexInputDynamicState is enabled, which almost
all recent drivers do. We could potentially do similar optimizations
when supportsVertexInputDynamicState is enabled to avoid
RenderPassCommands::setVertexInput() call. But the logic is complex
enough not really worth it (See crrev.com/c/6961186 for draft CL). If
any, simply compare the new value and old value probably is as good as
checking the dirty bits. In this CL, all these dirty bits checking are
removed so that we do not waste CPU cycles to track these dirty bits
while not being used at all when supportsVertexInputDynamicState.enabled
is true.
Bug: b/439073246
Bug: b/442636174
Change-Id: I7d71d4fc9388612e7c6ccc50a2e781325fe953bc
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/7007241
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
|
|
|
8f8f0439
|
2025-10-02T15:44:46
|
|
Group the class methods together
In previous many CLs, I intentionally not move around the code for ease
of code review. Now everything has settled down, it is the time to
regroup class methods together.
Also renamed some various in VertexArrayVk to be consistent with each
other.
No functional change is expected in this CL.
Bug: b/439073246
Bug: b/433331119
Change-Id: I84f9a2ff9ea20f359e2f546ecb4e3e503b805748
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/7007472
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
|
62b00f86
|
2025-10-02T12:19:23
|
|
Vulkan: Further minimize VertexArrayVk::syncDirtyEnabledAttrib
This CL splits the streaming handling into its own function:
syncDirtyEnabledAttrib is now splitted into
syncDirtyEnabledNonStreamingAttrib and syncDirtyEnabledStreamingAttrib.
This minimize the condition check inside syncDirtyEnabledAttrib.
VertexArrayVk::syncDirtyEnabledAttrib() function has a check of
bufferGL->getSize() > 0 as well as hasAtLeastOneVertex. And if either of
them are false, they simply point to empty buffer. This CL merges these
this fallout case into hasAtLeastOneVertex and added ASSERT to ensure
that if buffer size is 0, hasAtLeastOneVertex is false.
Bug: b/439073246
Change-Id: I5e3592c31469d15f5321208eb0278b10e447118c
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6987341
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
7093e400
|
2025-09-29T14:31:06
|
|
Vulkan: Fix buffer size for vertex array
* In syncDirtyEnabledAttrib() in VertexArrayVk, the array buffer size
used for binding (BindVertexBuffers2()) has been changed to that of
bufferHelper.
Bug: angleproject:448047351
Change-Id: I852e4839cba698c01f92644cf32fadf366c0b54f
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6996428
Commit-Queue: Cody Northrop <cnorthrop@google.com>
Reviewed-by: Charlie Lao <cclao@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
|
1f40285c
|
2025-09-24T11:54:44
|
|
Vulkan: Bypass buffer for VA if Bind*Buffers2
Due to added support for BindVertexBuffers2 and BindIndexBuffer2,
there would be no need to allocate a buffer for vertex arrays via
getBufferForVertexArray() if the related features are supported. It
should be possible to directly reference the original buffer and the
proper offset instead without errors.
* The buffer handles and offsets in VertexArrayVk now get the original
buffer handle and offset instead of using getBufferForVertexArray().
* getBufferForVertexArray() will now be used only when the following
feature is disabled:
* useVertexInputBindingStrideDynamicState
* (which is used as condition for vkCmdBindVertexBuffers2())
* Renamed the function for index buffer: getIndexBufferForVertexArray()
This will be used when the following feature is disabled:
* supportsMaintenance5
* (which is used as condition for vkCmdBindIndexBuffer2()))
* Moved the rest of the code in getBufferForVertexArray() to ~Impl(),
which is used by both the functions above.
* Removed redundant condition from the vertex input binding stride
dynamic state feature.
Bug: chromium:40059200
Bug: angleproject:394337110
Change-Id: I665611f92058048a9778aa4b823fabfad7c96c84
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6980316
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Amirali Abdolrashidi <abdolrashidi@google.com>
Reviewed-by: Charlie Lao <cclao@google.com>
|
|
|
91263bbb
|
2025-09-18T14:55:02
|
|
Vulkan:Improve ContextVk::handleDirtyGraphicsDefaultAttribs
ContextVk::handleDirtyGraphicsDefaultAttribs() calls
updateDefaultAttrib(), which basically does the work
ContextVk::onVertexArrayChange() is doing. This CL simplify the code by
calling onVertexArrayChange directly from
handleDirtyGraphicsDefaultAttribs. onVertexAttributeChange is removed.
This also has the advantage that onVertexArrayChange put the for loop
inside if check, versus current implementation does all the if check
inside the for loop.
Bug: b/439073246
Change-Id: Ic7b7f531f2f831d16f34794e23fefc9569a3e142
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6967318
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
|
|
|
75f6faf1
|
2025-09-05T17:01:43
|
|
Vulkan: Use size in BindVertexBuffers2
* Added the following attribute array to VertexArrayVk:
mCurrentArrayBufferSizes
* Updated during syncing/updating the vertex array attributes.
* Added the pointer for buffer sizes to the following call:
vkCmdBindVertexBuffers2EXT()
Bug: chromium:40059200
Change-Id: I754741d12a729d42dab7e0f0b166a174c8d86181
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6917838
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Charlie Lao <cclao@google.com>
Commit-Queue: Amirali Abdolrashidi <abdolrashidi@google.com>
|
|
|
b1fc47c7
|
2025-09-17T18:21:02
|
|
Vulkan: add check for vertexAttributeInstanceRateZeroDivisor
Allow zero divisor for vertex attributes when
mVertexAttributeDivisorFeaturesmVertexAttributeDivisorFeatures.vertexAttributeInstanceRateZeroDivisor
is true.
Bug: b/439073246
Change-Id: I68705faffcefb38360f6fbcf5d937d1d902180aa
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6961233
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
|
17c0d100
|
2025-09-16T13:59:36
|
|
Vulkan: Remove VertexArrayVk::updateActiveAttribInfo
This function get called from DIRTY_BIT_VERTEX_ARRAY_BINDING context
dirty bit processing. It is doing exact same work as
ContextVk::onVertexArrayChange(), which was already been optimized in
earlier CL. In this CL VertexArrayVk::updateActiveAttribInfo() is
deleted and onVertexArrayChange() is being used, which does minimum
things when supportsVertexInputDynamicState is enabled.
Bug: b/439073246
Change-Id: I6b388a8f89a63e654d290c6f8d2212052b8c0d1f
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6945076
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
Commit-Queue: Charlie Lao <cclao@google.com>
|
|
|
cba665f3
|
2025-09-11T17:59:41
|
|
Vulkan: Add fast path for supportsVertexInputDynamicState
In earlier CLs, we have stored stride/offset/format/divisor in Vulkan
structs directly in VertexArray, this CL try to send these structs to
vulkan driver directly without making another copy. divisor code has
been modified to update inputRate as well as divisor in VertexArrayVk.
WHen program and VertexArray's attribute types matches we use
VertexArray's mVertexInputBindingDesc and mVertexInputAttribDesc without
any data copy. If attribType mismatch then we make a copy and patch up
the stride/format.
Bug: b/439073246
Change-Id: I905b1e6d0609ffc4eb63b47e11a84f8617e06c29
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6898416
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
|
|
|
7201e17d
|
2025-09-16T11:04:05
|
|
Vulkan: Move divisor>MaxVertexAttribDivisor to VA::syncState
Another preparation for use mVertexInputBindingDesc directly. Instead of
checking divisor > renderer->getMaxVertexAttribDivisor() in ContextVk
code, this CL take care of them in VertexArrayVk::syncState, so that all
other code do not need to take care of it.
Bug: b/439073246
Change-Id: Id938c168a1490e072e6f4bb08b85b60b55968ddf
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6955256
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
|
|
|
130bdaa9
|
2025-09-10T12:51:37
|
|
Vulkan: Store VkFormat in VkVertexInputBindingDescription2EXT
In preparation for next CL that uses VkVertexInputBindingDescription2EXT
directly, this CL also stores the VkFormat in
VkVertexInputBindingDescription2EXT directly. mDefaultAttribFormatIDs is
added to store the angle::FormatID for the default attribute (used when
attrib is disabled in VertexArray)
Bug: b/439073246
Change-Id: Ia36758b258be23676b9b12c0c7cec7adefe1036a
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6935270
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
|
46444b3d
|
2025-09-09T16:13:58
|
|
Vulkan: Use VkVertexInputBindingDescription2EXT to store stride
This is preparation for next CL. Since most driver supports
VK_EXT_vertex_input_dynamic_state, which uses
VkVertexInputBindingDescription2EXT and
VkVertexInputAttributeDescription2EXT to send stride/divisor/offset to
the vulkan driver, why not store these directly in these structs instead
of store gl::AttribArray<> and copy then into
VkVertexInputBindingDescription2EXT/VkVertexInputBindingDescription2EXT
later. This CL only replaced mCurrentArrayBufferRelativeOffsets,
mCurrentArrayBufferStrides, mCurrentArrayBufferDivisors with
mVertexInputBindingDesc and mVertexInputAttribDesc. It still does the
data copy, which means this CL is mostly a mechanical change. But it
makes the next CL diff smaller.
Bug: b/439073246
Change-Id: Ie3c2034df07ea5e973b07a15f715fdb7c73ec04d
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6933260
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
|
c292f292
|
2025-09-10T14:34:14
|
|
Vulkan: Remove compressVertexData feature
This feature was added for performance reason. It was used years ago to
improve performance of lego legacy. That entire game dashboard feature
was disabled a few years ago. So this code path is no longer been used
now, and not been tested on bots as well. This CL deletes this feature
and related code path so that we don't just leave it bit rotten.
Bug: b/167404532
Bug: b/439073246
Change-Id: I384fc97021592da57d38e8c1771892071ae68a89
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6935271
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
|
|
|
dcbe926e
|
2025-09-10T17:17:58
|
|
Vulkan: Avoid redundant updateVertexInput on disabled attribs
When supportsVertexInputDynamicState feature is disabled, we are calling
updateVertexInput right from VertexArray::syncState for both enabled and
disabled attributes. But disabled attributes we are also calling
contextVk->invalidateDefaultAttribute(attribIndex), which end up with
VertexArrayVk::updateDefaultAttrib(), which calls updateVertexInput().
This means updateVertexInput() are called twice for disabled attributes.
This CL skips onVertexArrayChange for disabled attributes.
Bug: b/439073246
Change-Id: Icf9c08d1a920d9112ef4080b95d0451f6230c6dd
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6937213
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
|
|
|
36ff2883
|
2025-09-10T11:32:12
|
|
Vulkan: Remove redundant setDefaultPackedInput call
VertexArrayVk::syncDirtyDisabledAttrib() is calling
contextVk->invalidateDefaultAttribute(attribIndex), which will set
DIRTY_BIT_DEFAULT_ATTRIBS, which triggers
ContextVk::handleDirtyGraphicsDefaultAttribs() call, which calls
updateDefaultAttrib(), which calls setDefaultPackedInput(). So the
setDefaultPackedInput() call in VertexArrayVk::syncDirtyDisabledAttrib()
is not needed. Further, when program changes we also
invalidateDefaultAttributes(), which dirty all active default attribs,
so we can rely on DIRTY_BIT_DEFAULT_ATTRIBS.
Bug: b/439073246
Change-Id: Iae985697ace757462d1dbe24e5fdd967863e8674
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6935268
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
|
|
|
fecb02db
|
2025-08-28T12:08:34
|
|
Vulkan: Reduce onVertexBufferChange/onVertexAttributeChange
When we loop each attribute we end up calling onVertexBufferChange or
onVertexAttributeChange. For most drivers
supportsVertexInputDynamicState is enabled. This means we are repeatedly
check feature bit and set DIRTY_BIT_VERTEX_BUFFERS repeatedly for each
dirty attributes. This CL moves these calls out of attribIndex for loop.
ContextVk::onVertexArrayChange() now get called directly from
VertexArrayVk::syncState() so that we only go through most logic only
once if supportsVertexInputDynamicState is enabled.
Bug: b/439073246
Change-Id: Ib1316560ef686222e72b4d7ad32c63b043dfbaa5
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6896934
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Charlie Lao <cclao@google.com>
|
|
|
8bae2565
|
2025-08-22T14:38:23
|
|
Vulkan: Improve VertexArrayVk::syncState
VertexArrayVk::syncState often time shows up the biggest single API in
simpleperf. For example, in tower_of_fantasy it is 7.9% of all CPU time
in libANGLE. This function also uses macros which made it hard to debug.
This CL removes the usage of macros which makes code much easier to
handle. The other real problem is that we are repeatedly calling
syncDirtyAttrib() function for disabled attributes. This CL breaks the
dirty bits into bindingDirtyBits and bufferDataDiryBits and
attribDirtyBits. Only attribDirtyBits will end up doing the actual state
sync. All other dirty bits will just turn them into attribDirtyBits.
Also disabled attributes will be looped separately. This simplification
makes it impossible to have duplicate state syncs since we only call
sync*Attrib at the end of function. By splitting syncDirtyAttrib into
syncDirtyEnabledAttrib/syncDirtyDIsabledAttrib/syncNeedsConversionAttrib,
we also moved the if check from syncDirtyAttrib (which is called within
for loop) to syncState.
With this CL, simpleperf shows this function has reduced from 7.9% to
5.9%.
Bug: b/439073246
Change-Id: I99b5ff0b34a5992e31541d2e9cd81ff5c9dda716
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6876527
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
Commit-Queue: Charlie Lao <cclao@google.com>
|
|
25390156
|
2025-08-21T00:13:19
|
|
Suppress unsafe buffers on a file-by-file basis in src/ [1 of N]
In this CL, we suppress many files but stop short of actually
enabling the warning by not removing the line from the
unsafe_buffers_paths.txt file. That will happen in a follow-on
CL, along with resolving any stragglers missed here.
This is mostly a manual change so as to familiarize myself with
the kinds of issues faced by the Angle codebase when applying buffer
safety warnings.
-- Re-generate affected hashes.
-- Clang-format applied to all changed files.
-- Add a few missing .reserve() calls to vectors as noticed.
-- Fix some mismatches between file names and header comments.
-- Be more consistent with header comment format (blank lines and
trailing //-only lines when a filename comment adjoins license
boilerplate).
Bug: b/436880895
Change-Id: I3bde5cc2059acbe8345057289214f1a26f1c34aa
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6869022
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
|
fa29f604
|
2025-07-02T13:23:31
|
|
Remove sharedContextLock from {Enable|Disable}VertexAttribArray
VertexArray objects are per context objects. In theory they do not need
to protected by shared context lock. The reason we are taking locks
because all these functions end up accessing Buffer object which are
shared. In prior CLs we have removed subject observer usage from
VertexArray which means VertexArray no longer accessed from other
thread. In prior CLs we also split VertexArray into two classes:
VertexArrayPrivate which has no buffer, and VertexArray which is
subclass from VertexArrayPrivate and owns buffer. In this CL,
glEnableVertexAttribArray and glDisableVertexAttribArray calls no longer
take shared context lock. ContextPrivateEnableVertexAttribArray and
ContextPrivateDisableVertexAttribArray are called from these two APIs
and they only have access to StatePrivate. State Private holds a
VertexArrayPrivate pointer, which means they do not have anyway to
access buffer objects. The main challenge I run into here is
mCachedActiveClientAttribsMask, mCachedActiveBufferedAttribsMask,
mCachedActiveDefaultAttribsMask, mCachedHasAnyEnabledClientAttrib,
mCachedNonInstancedVertexElementLimit,
mCachedInstancedVertexElementLimit. These StateCache variable needs to
be updated when these two APIs are called, and calculating these
variable needs access to buffer object. The solution here is adding a
bool mIsCachedActiveAttribMasksValid in the PrivateStateCache so that
instead of immediately update these mCached* variable, we just set
mIsCachedActiveAttribMasksValid to false. Then whenever any of these
mCached* variable is needed, we will check
mIsCachedActiveAttribMasksValid and calculate these cached variables. It
adds one if check when accessing these caches, but the other benefit is
that we may have avoided duplicated calculation when multiple states
changed.
Bug: b/433331119
Change-Id: I3227c72bc40501712db93fb3d540b835f07150b5
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4514436
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
|
|
|
b4d84458
|
2025-05-23T18:08:19
|
|
Move Buffer from VertexBinding to VertexArray
In later CL we will not taking shared context lock for certain
VertexArray API calls. VertexArray itself is per context, so this sounds
reasonable to do. The main challenge here is a lot of VertexArray
function end up accessing gl::Buffer object, which could be modified by
other shared contexts. In order to safely not taking the shared context
lock, we need to separate out Buffer object out of VertexArray itself so
that these lockless APIs will take VertexArray that does not have access
to buffer.
In this CL, VertexArray is split into two classes: VertexArrayPrivate is
everything in VertexArray except buffers. VertexArray is a subclass of
VertexArrayPrivate and owns all the buffers. Buffer is removed from
gl::VertexBinding class. In order to let back end access to buffers,
VertexArrayImpl holds a weak reference to
VertexArray::mVertexArrayBuffers (which is a vector of buffers).
Further, VertexArrayBufferBindingMask mBufferBindingMask is moved from
VertexArrayState into VertexArray class well, since it tracks which
index has a non-null buffer. The bulk of change are due to the
VertexARrayImpl constructor change, since it now takes
vertexArrayBuffers argument. Other bulk of changes are due to
VertexBinding no long has the buffer, but you need to get it directly
from VertexArray or VertexArrayImpl.
This CL also reverts some of the change in crrev.com/c/6758215 that
mVertexBindings no longer contains kElementArrayBufferIndex.
BYPASS_LARGE_CHANGE_WARNING
Bug: b/433331119
Change-Id: I15f4576f7c5c8d8f4d9c9c07d38a60ce539bfeea
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6774702
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
Commit-Queue: Charlie Lao <cclao@google.com>
|
|
|
8dca0efe
|
2025-07-21T15:29:10
|
|
Replace VertexArray::DIRTY_BIT_LOST_OBSERVATION with API call
This dirty bit was added so that back end can inspect buffers and set
proper VertexArray::DirtyBitType. The same thing can achieved by add a
virtual function on VertexArrayImpl class. The advantage of virtual
function on VertexArrayImpl is that all back end essentially have the
same implementation and we can just implemented in VertexArrayImpl
instead of duplicate in each back end. The other advantage is after this
CL DIRTY_BIT_BINDING_n and DIRTY_BIT_BUFFER_DATA_n will be well aligned
instead of offset by 1 caused by DIRTY_BIT_LOST_OBSERVATION. The other
motivation of this change is in later CL I want to move
mBufferBindingMask out of VertexArrayState, which means back end will
not have access to it. By using VertexArrayImpl API, I can pass
mBufferBindingMask directly to the back end via function parameter. So,
this CL removed DIRTY_BIT_LOST_OBSERVATION, added
VertexArrayImpl::checkBufferForDirtyBits().
Bug: b/433331119
Change-Id: I5c8cbc9bace63db416e86c2ae3631f74a12b20b8
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6775986
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
|
2ac49bb6
|
2025-07-01T12:11:12
|
|
Reland "Vulkan: Move VertexArray::ElementBuffer away from observer"
This reverts commit 79ac1a8cd767a32cce6401203e20c4bd4ca4d539.
Reason for revert: the regression bug is fixed in PS6
The regression bug with the original CL is caused by when we bind a
vertex array without element buffer rebind, we missed to reset
mIndexRangeInlineCache. The other bug is that VertexArray::mDiryBits is
64 bit but VertexArrayBufferBindingMask is 16 bit, in
VertexArray::setDependentDirtyBits(), bufferBindingMask.to_ulong() <<
DIRTY_BIT_BUFFER_DATA_0 is only producing the 32 bit value on windows
platform due to unsigned long is 32 bit value. bits() is used and bit
shift is operated on to uint64_t here to avoid dropping high bits on
windows.
Two tests are added that reproduce the regression bug caused by
the original CL.
Bug: angleproject:400711938
Original change's description:
> Revert "Vulkan: Move VertexArray::ElementBuffer away from observer"
>
> This reverts commit 3f012a43ee2c101543785720eedfeaa80708479d.
>
> Reason for revert: https://issues.chromium.org/427064102
>
> Bug: angleproject:400711938
> Original change's description:
> > Vulkan: Move VertexArray::ElementBuffer away from observer
> >
> > Right now, VertexArray's element buffer is always observing buffer's
> > change. In previous CLs, we have moved vertex array away from
> > subject/observer usage. This CL moves element buffer away from
> > subject/observer as well. Since the gl::Buffer tracks buffer's binding
> > to each context's current vertex array's binding point,
> > kElementArrayBufferIndex is added to VertexArrayBufferBindingMask bits
> > so that the element buffer is tracked exactly the same as other vertex
> > array buffer bindings. The VerextArray code has been modified to handle
> > this special bit, since element buffer has its own binding point
> > VertexArrayState::mElementArrayBuffer as opposed to
> > VertexArrayState::mVertexBindings. After this CL, VertexArray object
> > should be completely off subject/observer usages.
> >
> > Bug: angleproject:400711938
> > Change-Id: I662ddfabc95034bdc7734939c944ab033f41801c
> > Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6552160
> > Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
> > Reviewed-by: Geoff Lang <geofflang@chromium.org>
> > Commit-Queue: Charlie Lao <cclao@google.com>
>
> Bug: angleproject:400711938
> Change-Id: I9487ba8b108baaeda1c8a27189dba64f77616774
> Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6663539
> Commit-Queue: Charlie Lao <cclao@google.com>
> Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
Bug: angleproject:400711938
Change-Id: I3f47ad1238c41f12b5cbd7a59b84be3fce1e9562
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6664004
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
Commit-Queue: Charlie Lao <cclao@google.com>
|
|
|
937cf23c
|
2025-05-13T16:11:47
|
|
Vulkan: Remove VertexArrayBufferContentsObservers
When vertex array needs to convert buffer's data, right now it uses
Subject/Observer to subscribe notifications from buffers about the data
change. Since we always dirty all binding point when we bind
VertexArray, or app has to rebind buffer to vertex array if its on the
other context, this notification really is only needed for the current
context's vertex array. In prior CLs we already moved notification from
buffer to the current context's vertex array away from Subject/Observer
usage pattern. This CL did similar things to
VertexArrayBufferContentsObservers::mContentsObservers.
VertexArrayBufferContentsObservers has been deleted in this CL. Each
VertexArrayImpl now tracks the need of content observer with a bit mask
of each bindingIndex (which is tracked by
mContentsObserverBindingsMask). When a buffer's content changes,
gl::Buffer will retrieve this bit mask from backend and pass it to the
current gl::Context, which sends to current VertexArray object, which
then set proper DATA dirty bits on VertexArray based on the binding bit
mask. If back end think it does not need any data conversion, then the
bit mask is zero and nothing will be done. This further removes
dependence on subject observer, which enables us to avoid taking shared
context lock for glEnableVertexAttribArray and
glDisableVertexAttribArray.
Bug: angleproject:400711938
Change-Id: Ieb0c09c042a560dd121242b63ec24478482399b3
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6549157
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
|
|
|
3bbdee0f
|
2025-03-28T11:55:24
|
|
Vulkan: Remove Subject/Observer from BufferImpl
Right now the gl::Buffer back end send message to the front end and then
gl::Buffer propagate the message to the observers (vertex array,
textures, transform feedback). We are seeing many of these kind of
message passing (mainly to vertexArray), and each message is a virtual
function call. The message call also lacks of context information that
we can not do certain optimizations. This CL adopts the new API feedback
argument approach for buffer APIs from the back end to the front end.
The only difficulty I ran into is D3D backend where the message could be
delivered from draw calls. For now the subject/observer code path is
still kept in the gl::Buffer, but no back end will use it except D3D11.
That will be removed in the later CL when D3D11 switch to use feedback
mechanism.
BYPASS_LARGE_CHANGE_WARNING
Bug: angleproject:400711938
Change-Id: I5fb3b660fd4260b9ba691239ad777b575b31e2ab
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6408892
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
|
|
4be3143c
|
2025-04-10T15:18:52
|
|
Vulkan: Vertex attribute hole crash workaround
When the start vertex index of glDrawArrays is not 0, this hole
case will crash again.
* Added a condition to bypass the attribute update if the
attribute pointer is null.
* Added a glDrawArrays(GL_TRIANGLES, 3, 3) in unit test
VertexAttribPointerCopyBufferFromInvalidAddress
Bug: angleproject:359729255
Change-Id: I1d7f50dc56080ef0b4e48c4c3c983189a26171c1
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6445172
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Cody Northrop <cnorthrop@google.com>
Auto-Submit: Mavis Deng <mavis.deng@arm.com>
Commit-Queue: Cody Northrop <cnorthrop@google.com>
|
|
ce394c49
|
2025-04-10T10:33:57
|
|
Vulkan: Fix undefined behavior accessing empty vertex buffer
This fixes undefined behavior detected by VVL:
[ SYNC-HAZARD-READ-AFTER-WRITE ] vkCmdDraw():
READ_AFTER_WRITE hazard detected. vkCmdDraw reads vertex
VkBuffer 0xe000000000e, which was previously written by
vkCmdBeginRenderingKHR.
No sufficient synchronization is present to ensure that a read
(VK_ACCESS_2_VERTEX_ATTRIBUTE_READ_BIT) at
VK_PIPELINE_STAGE_2_VERTEX_ATTRIBUTE_INPUT_BIT
does not conflict with a prior write
(VK_ACCESS_2_COLOR_ATTACHMENT_WRITE_BIT) at
VK_PIPELINE_STAGE_2_COLOR_ATTACHMENT_OUTPUT_BIT.
This is regression from the old CL:
https://crrev.com/c/angle/angle/+/3297026
Vulkan: Don't attempt to convert vertices in empty buffers
Above change made `if (bufferVk->getSize() == 0)` branch a dead code.
In the current code this branch is `if (numVertices == 0)` and is no
longer a dead code. It sets zero stride to prevent reading beyond the
empty buffer (which is only 16 bytes).
This fix also sets `mCurrentArrayBufferStrides[attribIndex]` to zero
when it is not a streaming attribute (in case of zero sized buffer).
Bug: chromium:1271671
Change-Id: I3460201ae7366b8de1089efef6fbefe359d28460
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6440439
Commit-Queue: Igor Nazarov <i.nazarov@samsung.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
|
1d25be59
|
2025-01-09T11:09:22
|
|
Vulkan: Pass Context instead of Renderer to BufferHelper APIs
This is preparation CL for later CL. In later CL we need to access
context argument in BufferHelper's barrier related functions. release()
also preferred to have context argument so that the events can be
recycled within share group. Because of this, a lot of functions has to
propagate back to pass context as argument instead of renderer.
Bug: angleproject:360274928
Change-Id: I13e930666eeeefbcff7b542d0e3126f3b07ce286
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/6164686
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
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>
|
|
|
91391c06
|
2024-10-02T16:07:02
|
|
Vulkan: Vertex attribute hole crash workaround
* Added condition to bypass reading from a streaming attribute
if the source pointer is null.
* Added unit test that crash if a vertex pointer is not defined
for an enabled vertex attribute.
* VertexAttribPointerCopyBufferFromInvalidAddress
* Credit for the original test: tingwei.guo
Bug: angleproject:359729255
Change-Id: I2592fed66f0eba8c7003ec02cc8ca802833f23b3
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5899978
Commit-Queue: Amirali Abdolrashidi <abdolrashidi@google.com>
Reviewed-by: Charlie Lao <cclao@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
|
878e1c92
|
2024-10-07T16:34:54
|
|
Vulkan: Fix line-loop draw arrays after elements
Currently, when drawing line-loop arrays in Vulkan, an index
buffer is created to assist in drawing (since Vulkan does not
natively include a line-loop draw mode). However, when LL array
draw calls are mixed with non-LL element draw calls, it can lead
to some rendering issues due to the fact that the proper index
buffer is not obtained.
* In VertexArrayVk::handleLineLoop(), if the cached indices are the
same as the last draw, the same index buffer is obtained from the
LineLoopHelper object.
* (Using the newly added getCurrentIndexBuffer())
* Added unit test in which a triangle element draw is places between
two LL array draws. Before the fix, the second LL draw would result
in an incorrect line draw.
Bug: chromium:40911000
Change-Id: Ibba9a0cb2b77a2b6ae2c1e9230afe3d16b70cb63
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5908694
Reviewed-by: Charlie Lao <cclao@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Amirali Abdolrashidi <abdolrashidi@google.com>
|
|
|
67a5ea45
|
2024-09-23T16:09:12
|
|
Vulkan: Fix the error from multiple lineloop draws
Since Vulkan does not support line-loop draws natively, such a
draw call requires the conversion of the related buffers to prepare
them for this operation. For glDrawElementsIndirect(), the index
and the indirect buffers would need conversion.
However, what currently happens in this case is that the original
buffer pointer is overwritten after the conversion, removing the
link to the original buffer. Therefore, if there is a second line-loop
call just after the first, it will try to use the converted buffer as
the new source, which leads to errors due the buffer already being in
use.
The index buffer for the draw is bound when the related dirty bit is
handled. Therefore, instead of using the draw index buffer directly
for handling the line-loop scenario, we can use the index buffer in
the form of a local pointer passed between functions. Then, in order
to reconcile line-loop with the other cases, the draw index buffer is
set just before setting up the indexed draw.
* Functions handling line-loop draws do not modify the element array
buffer in VertexArrayVk directly, but use local buffer pointers to
pass the current element array pointer to further processing and
drawing.
* Added mCurrentElementArrayBuffer for ContextVk to be bound to the
index buffer to used for draw instead of the one from its vertex
array object.
* Before the indexed draw, mCurrentElementArrayBuffer is set to the
last destination index buffer.
* Added unit test that makes a line-loop draw and then a non-LL call
using the same element array.
Bug: angleproject:360758685
Change-Id: I6d6328f6326c1a1f9f80e5ef346aa077c867d344
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5878764
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Amirali Abdolrashidi <abdolrashidi@google.com>
Reviewed-by: Charlie Lao <cclao@google.com>
|
|
|
c9d55051
|
2024-09-06T10:56:07
|
|
Vulkan: Consolidate dirtyRanges before vertex conversion
Detect two ranges overlap or are continuous and merge them. This reduces
number of dispatch calls as well as avoids redundant conversion.
Bug: b/357622380
Change-Id: I06b73a1e9fd573d79af985b247f4d66bf97f756e
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5851642
Auto-Submit: Charlie Lao <cclao@google.com>
Commit-Queue: Yuxin Hu <yuxinhu@google.com>
Reviewed-by: Yuxin Hu <yuxinhu@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
|
10d56e63
|
2024-09-06T10:56:07
|
|
Vulkan: use mEntireBufferDirty if range covers entire buffer
Detect the dirtyRange covers entire buffer and set mEntireBufferDirty
instead of add range into mDirtyRanges. This will get into a much
simpler code path that will not have any redundant conversion. This also
removes quite big portion of overlapped ranges cases without much of
overhead.
Bug: b/357622380
Change-Id: Iedaa3662a6fc52257e71d39ab75baddf6ad3e41b
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5840476
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Yuxin Hu <yuxinhu@google.com>
Auto-Submit: Charlie Lao <cclao@google.com>
|
|
|
53476d6f
|
2024-09-04T14:34:11
|
|
Vulkan: Do vertex conversion with fine grain dirtyRange
Right now when we do vertex conversion with multiple dirty ranges, we
are merging dirty ranges into single range and then
UtilsVk::convertVertexBuffer() is called for the merged dirty range. If
there is big gap between two ranges, the merged range could be very big.
This means we end up doing many unnecessary conversion. This CL tracks
individual dirty ranges and issues dispatchCompute for each dirty range,
thus minimize the unnecessary conversions.
On S24, this change further reduces TraceTest.gangstar_vegas frame time
from ~6.0ms to ~3.8ms, almost parity to native GLES's ~3.6ms.
Bug: b/357622380
Change-Id: Ia103f3963bdb5996ff3f95164c955a3e4f33f311
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5787633
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Yuxin Hu <yuxinhu@google.com>
|
|
|
e4aab5cc
|
2024-08-13T14:54:19
|
|
Vulkan: Reuse the same conversion buffer by adjusting offset
In some usage case where app calls bufferSubData and then calls
glVertexAttribPointer to source vertex from different offset, we could
still just reuse the existing VertexConversionBuffer instead of create a
new one. The benefit with this is that all previous converted data are
still valid, thus reduce the overall conversion, and reduce the number
of conversion buffers and saves memory.
Bug: b/357622380
Change-Id: Ifcf626427e2ed500d6ab617541b78a60839d9acd
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5785670
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Yuxin Hu <yuxinhu@google.com>
|
|
|
38d5b4d6
|
2024-08-21T15:49:33
|
|
Vulkan: Clean up the CovertVertexBuffer* API
Some minor clean up of the API so that the arguments we passed into
these two functions CovertVertexBufferGPU and CovertVertexBufferCPU are
symmetrical.
This CL also factor out the common logic in these two functions into a
helper function CalculateOffsetAndVertexCountForConversion().
Bug: b/357622380
Change-Id: I5339850c99d733dfea5039ba74734290f8326bd3
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5804529
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Yuxin Hu <yuxinhu@google.com>
|
|
|
8fc6d8a8
|
2024-08-13T09:49:46
|
|
Vulkan: Limit vertexBufferConversion to the data within dirtyRange
Right now when we do vertex buffer conversion, we always convert from
the binding.offset all the way to the end of the buffer, because we do
not know who much vertex will be drawn. According to the comment, if we
choose to only convert the number of vertices this draw call will be
using, then we may end up issuing a lot of conversions in the case that
there are many draw calls with small vertex numbers. This CL intends to
keep this behavior (i.e, still do the conversion of the data from
binding offset to end of the buffer), but apply the optimization that
pays attention of mDirtyRange and only convert the portion of data that
has actually been modified. Note that the mDirtyRange is the union of
all data ranges that has been modified, so may include the unmodified
data between two modified ranges.
Bug: b/357622380
Change-Id: I91f3dbf4cacb17d3fa4d8d599519865be765557f
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5785860
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Yuxin Hu <yuxinhu@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
|
86508e20
|
2024-08-16T14:56:37
|
|
Vulkan: Make VertexConversionBuffer a class
And wrap the cache key (i.e, formatID/stride/offset) into a CacheKey
struct so that we can easily add more data members. This CL also changes
ConversionBuffer from struct to class to have better encapsulation. No
functional changes is expected here.
Bug: b/357622380
Change-Id: Ieecf5c922b95a940137c8e54657ef3f458c55fc9
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5793921
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Yuxin Hu <yuxinhu@google.com>
|
|
|
d353f2ce
|
2024-05-31T16:57:41
|
|
Vulkan: Check any robustness instead of EXT only
* Updated checks for robustnessEXT with robustnessAny(), which also
includes robustnessKHR.
Bug: angleproject:42262244
Change-Id: Ia79bc7f1a5ead29417eec0a5663b70d79c34ad56
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5587992
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
dc643d9b
|
2024-04-01T15:59:46
|
|
Vulkan: add feature control for client buffer merge
add feature control for client attribute buffer
merge. default is off to reduce memeory usage for
some application.
Bug: b/328301788
Change-Id: I5bfd39fb1ea656ebb29bd2dc21726b60bbc1a8d2
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5410942
Commit-Queue: Hailin Zhang <hailinzhang@google.com>
Reviewed-by: Charlie Lao <cclao@google.com>
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
|
|
|
efd41bd2
|
2024-03-15T13:25:03
|
|
Vulkan: Rename ResourceVk.* to vk_resource.*
This file adds helpers to namespace vk, so its name is changed for
consistency with other namespace vk files.
Bug: angleproject:8564
Change-Id: I6525e7609eb9385f2a3eecaa7c52b7417fda7f12
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5370108
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
Auto-Submit: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
|
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>
|
|
|
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>
|
|
6607a2b9
|
2024-01-17T15:58:20
|
|
Vulkan: Add support for VK_EXT_vertex_input_dynamic_state
Hook into VK_EXT_vertex_input_dynamic_state so pipeline states
that differ only in vertex input state can reuse existing
pipelines.
Bug: angleproject:7162
Tests: StateChangeTestES3.Vertex*
Change-Id: Icd3134dee93fc5fc2e9d284fcfa8c674b62faec8
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5207462
Commit-Queue: mohan maiya <m.maiya@samsung.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
3fe678c9
|
2023-11-30T12:15:57
|
|
Avoid malloc in VertexArrayVk::mergeClientAttribsRange
Replace temporary std::vector with angle::FixedVector
Bug: b/300968773
Change-Id: I002233afc99c0eb03a5ad11ab7a5bfd85626b3a0
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5074625
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
|
67222ef4
|
2023-10-20T22:56:09
|
|
Reland "Vulkan: merge client buffer data."
This is a reland of commit 563569acfcaf56ea87916d2ab5d50f09c8e0094e
Original change's description:
> Vulkan: merge client buffer data.
>
> some old fashion game still use lots of interleaved
> client buffer. instead of copy each attrib alone.
> this cl try to merge all the attrib ranges. reduce
> allocated memory and do whole range memcpy.
>
> Bug: b/306763053
> Change-Id: I493d7f0e1ef593fb7059c36ae0ed2149c4595e42
> Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4960642
> Commit-Queue: Hailin Zhang <hailinzhang@google.com>
> Reviewed-by: Charlie Lao <cclao@google.com>
Bug: b/306763053
Change-Id: If079ab055b7b7a2d14235bee3311fd628f657f35
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4997325
Reviewed-by: Charlie Lao <cclao@google.com>
Commit-Queue: Hailin Zhang <hailinzhang@google.com>
|
|
46817856
|
2023-11-01T04:38:40
|
|
Revert "Vulkan: merge client buffer data."
This reverts commit 563569acfcaf56ea87916d2ab5d50f09c8e0094e.
Reason for revert: Broke Android build, see aosp/2812590
Original change's description:
> Vulkan: merge client buffer data.
>
> some old fashion game still use lots of interleaved
> client buffer. instead of copy each attrib alone.
> this cl try to merge all the attrib ranges. reduce
> allocated memory and do whole range memcpy.
>
> Bug: b/306763053
> Change-Id: I493d7f0e1ef593fb7059c36ae0ed2149c4595e42
> Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4960642
> Commit-Queue: Hailin Zhang <hailinzhang@google.com>
> Reviewed-by: Charlie Lao <cclao@google.com>
Bug: b/306763053
Change-Id: I31ab923af16bf3f9e99033ebd6ea4c5e5b310d71
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4995494
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Peiyong Lin <lpy@google.com>
Auto-Submit: Peiyong Lin <lpy@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
|
|
|
563569ac
|
2023-10-20T22:56:09
|
|
Vulkan: merge client buffer data.
some old fashion game still use lots of interleaved
client buffer. instead of copy each attrib alone.
this cl try to merge all the attrib ranges. reduce
allocated memory and do whole range memcpy.
Bug: b/306763053
Change-Id: I493d7f0e1ef593fb7059c36ae0ed2149c4595e42
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4960642
Commit-Queue: Hailin Zhang <hailinzhang@google.com>
Reviewed-by: Charlie Lao <cclao@google.com>
|
|
|
80e2d8d7
|
2023-10-19T13:53:32
|
|
Vulkan: Update PipelineProgram if mStreamingVertexAttribsMask dirty
When mStreamingVertexAttribsMask bit chnages, we are going to switch
between a internal buffer and user's buffer. We must update graphics
pipeline program, even if bufferOnly is true.
Bug: b/303219048
Change-Id: I0536cb7bcd6ed8f1de39aaea5bd6cd6eef61c5e1
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4957193
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
|
91ef1f3c
|
2023-09-08T16:39:53
|
|
Move buffer suballocation callers to ContextVk
* Moved the following functions from BufferHelper to ContextVk.
* initBufferForBufferCopy()
* initBufferForImageCopy()
* initBufferForVertexConversion()
Bug: b/280304441
Change-Id: I890f4396b00b0c20feb44f0ad113c55924ce1014
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4854760
Commit-Queue: Amirali Abdolrashidi <abdolrashidi@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Charlie Lao <cclao@google.com>
|
|
|
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>
|
|
7f6e5354
|
2023-07-31T13:02:35
|
|
Vulkan: Skip vertex conversion if the draw has 0 vertices.
If the draw call start vertex is beyond the end of the buffer, vertex
conversion will no-op and no conversion buffer is created. Just skip
the entire conversion process in this case and bind the empty buffer.
Fix GetVertexCount not taking 0 stride into account.
Bug: chromium:1464690
Change-Id: Iaffcd329595c3319fe9cd5317aef2402f9db6b1e
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4734811
Commit-Queue: Geoff Lang <geofflang@chromium.org>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
|
b0e9bbd7
|
2023-05-31T14:23:40
|
|
Vulkan: Split features for dynamic state
When a driver bug with dynamic state is encountered, it is hard to debug
which dynamic state exactly is causing an issue, due to the current
granularity of disabling all entire state from an extension. With this
change, every dynamic state gets its own ANGLE feature, and can be
toggled as necessary.
Disabling the supportsExtendedDynamicState* features implicitly
disables all dependent features.
Bug: b/285124778
Bug: b/275210062
Bug: fuchsia:107106
Bug: angleproject:5906
Change-Id: Ic291279872df2d0eb58618ff364ab118bdcc4a9f
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4577553
Reviewed-by: Cody Northrop <cnorthrop@google.com>
Auto-Submit: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Yuxin Hu <yuxinhu@google.com>
|
|
|
76fa3806
|
2023-05-04T10:19:35
|
|
Vulkan: Expand BufferOnly path for VertexArray binding change
VertexArrayVk has a fast code path for attribute change where the only
change is buffer (i.e, no format or relativeOffset change). It will pass
in bufferOnly to syncDirtyAttrib() call and will avoid invalidate
graphics pipeline. This CL expands DIRTY_BIT_BINDING_n change and will
also try to detect the bufferOnly case.
This CL and crrev.com/c/4507978 together seeing Gfxbench driver overhead
score improves 1.48% (from average 6804 before CLs to 6905 after CLs) on
pixel 7 pro.
Bug: b/277644512
Change-Id: I71da1b886bb26ba2629b83af3aeaba4d45c3d3c2
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4504919
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
Commit-Queue: Charlie Lao <cclao@google.com>
Auto-Submit: Charlie Lao <cclao@google.com>
|
|
|
0561884e
|
2023-04-26T17:26:42
|
|
Vulkan: Dirty VertexArray binding bit if buffer storage change
In crrev.com/c/3669603, we did optimization for black_desert_mobile that
when vertex array is unbound, we remove vertex array from buffer's
observer list to reduce overhead of observer notifications when buffer
is been modified. To compensate for the lost notification, when vertex
array is bound, we always assume every buffer that is bound to vertex
array has been dirtied, for the simplicity at that time. This CL further
the optimization of that CL. In this CL, I moved the dirty bit set into
backend and improves vulkan backend by checking buffer's serial number
and only dirty the binding if the serial has changed. Given this, now we
can also remove all the non-current vertex array from buffer's observer
list (previously it is heuristic based with a hard coded observer count
limit). This and the previous CL improves asphalt_9 by ~1%.
Bug: b/277644512
Change-Id: Ibc3f8e3df9fe70c6879e0b2bca86d8487a9dba73
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4481241
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Yuxin Hu <yuxinhu@google.com>
Commit-Queue: Charlie Lao <cclao@google.com>
|
|
|
67ad3ddc
|
2023-03-06T16:44:36
|
|
Vulkan: Relax size limit for dynamicBuffer to pick buddy algorithm
If glBufferData's usage is one of the dynamic usage, app may keep
calling glBufferData frequently, which means get into suballocation code
frequently. There are two suballocation algorithms today: buddy
algorithm (faster) and generic (slower). Right now the decision of which
algorithm (i.e, which pool) to use is purely based on size or memory
type. This CL also utilize usage information so that dynamic usage will
pick buddy algorithm with bigger size threshold. mSmallBufferPool is
removed and replaced with the BufferPoolPointerArray that gets picked
based on allocation algorithm.
This CL reduces average frame time of efootball_pes_2021 from 7.518 ms
to 4.670 ms on pixel 7 Pro.
Bug: b/271915956
Change-Id: I1c2f270ac49f56e6f405501d20691cfbab49e7eb
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4313685
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
|
fbd7d5fa
|
2022-10-17T17:20:09
|
|
Move thread pool classes to common/
In preparation for access by image_util files.
Bug: b/250688943
Change-Id: I24777269a5071eae9a60f939635d01ed7246461f
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3961454
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
|
2587e508
|
2022-05-26T17:44:47
|
|
Vulkan: Remove redundant vertex array dirty bit processing
When BINDING0+n and ATTRIB0+n dirty bits are both set, we are going
down syncState twice. What BINDING0+n covers all the work needed for
attribute and DATA0+n, so we should remove the ATTRIB0+n and DATA0+n.
Similarly if we see DATA0+n, we should skip ATTRIBU0+n as well.
Bug: b/235277703
Change-Id: I5e03c18bac3df30a14f3b6652caff2aff33f2fe6
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3669608
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
|
|
|
58fd3157
|
2022-05-12T00:01:19
|
|
Vulkan: Dynamic state for vertex stride
Bug: angleproject:5906
Change-Id: I73b7e004fc25bf3777982736412adc1ca57504b9
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3644856
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Yuxin Hu <yuxinhu@google.com>
|
|
|
fe28a429
|
2022-03-30T15:34:49
|
|
Vulkan: Create buffer for vertex array if robust enabled
If robust access is enabled (i.e., chrome), we want to ensure vulkan
driver never access beyond that OpenGL buffer boundary. But with
suballocation from BufferPool, we are using the same VkBuffer for all
suballocations from the same BufferBlock. this combined with the fact
that there is no size information in the vkCmdBindVertexBuffers, it
means vulkan driver can not properly ensure vertex access not go beyond
the subrange. It can only guarantee not access beyond the entire
VkBuffer size. This CL creates a dedicated vkBuffer object and bind it
to the suballocation of the vkDeviceMemory so that vulkan driver will
see the exact range of the subrange instead of entire buffer. Since we
may allocated more memory than actual requested size and the extra
paddings are not zero filled , user size is used to create this
vkBuffer. This is only enabled when robust access is enabled.
This CL also ported webgl conformance test
out-of-bounds-index-buffers.html and out-of-bounds-array-buffers.html
to end2end test.
Bug: chromium:1310038
Change-Id: I3499ae600028149b1039082e5011232b3e4e5e80
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3553940
Reviewed-by: Yuxin Hu <yuxinhu@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Charlie Lao <cclao@google.com>
|
|
|
349636a0
|
2022-03-15T09:39:36
|
|
Vulkan: Update mCurrentElementArrayBuffersync based on dirty bit
The previous fix crrev.com/c/3513553 has run into corner case that
requires more follow up change crrev.com/c/3522565. But with that, there
is report that now we are hitting assertion in
handleDirtyGraphicsIndexBuffer(). This becomes a bit fragile This new
fix relies on the DIRTY_BIT_INDEX_BUFFER dirty bit and should be more
reliable as long as the dirty bit is set properly (if not, then we have
other bug that it won't even send down vulkan command to bind the
correct element buffer). We could further optimize the code path and
create a fast path for most common usages in the future.
Bug: chromium:1299261
Change-Id: Ifa8f86d431798c9ca4c128ed71a3e9e0a3537ccb
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3526021
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Charlie Lao <cclao@google.com>
|
|
|
d4ddd0c1
|
2022-03-10T16:31:00
|
|
Vulkan: Handle the case where the bound buffer is empty
If vertex attribute is enabled and buffer is bound, but buffer size is
0, we should not crash. This CL skips mapImpl and data copy all together
if size is 0 to avoid crash when calling mapImpl while buffer is
invalid.
This CL also added a test for this.
Bug: chromium:1296467
Change-Id: I79af348f133e1d3b4427f044e370652d0875dc91
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3516700
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Charlie Lao <cclao@google.com>
|
|
|
b97aab3f
|
2022-03-09T17:36:24
|
|
Vulkan: resync mCurrentElementArrayBuffer when out of lineloop
When glDrawElements is called with GL_UNSIGNED_BYTE type or LineLoop
mode, we will internally allocate an element buffer and copy data to it.
But when we switch out of that mode, we must re-sync
mCurrentElementArrayBuffer to what it should be based on VertexArray
buffer binding. This CL fix the bug that we were previously not updating
it and end up using the wrong element buffer.
Also added three tests:
DrawWithSameBufferButDifferentTypes: that uses GL_UNSIGNED_BYTE data and
GL_UNSIGNED_SHORT data in the same buffer and switch between these two
data types without incurring buffer change.
DrawWithSameBufferButDifferentModes: draw line mode followed by triangle
without the same element buffer.
DrawArraysLineLoopFollowedByDrawElementsTriangle: draw line mode with
glDrawArrays and then followed by DrawElements.
Bug: chromium:1299261
Change-Id: I5c471117d300e9fac9127a9d8fa66d48ac312f03
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3513553
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Amirali Abdolrashidi <abdolrashidi@google.com>
Commit-Queue: Charlie Lao <cclao@google.com>
|
|
|
38723c28
|
2022-02-15T16:29:36
|
|
Vulkan: Allocate space for default attrib only if it is enabled
When context's default attributes is dirty, we allocate space for the
default attribute, regardless it is enabled or not. Then we call into
VertexArrayVk::updateDefaultAttrib() which only update its state if the
attribute is enabled. This causes a use-after-free scenario that if it
is disabled, the vertex array may have a pointer to the buffer that is
now becomes inflight which may gets deleted when DynamicBuffer code
think the size no longer matches etc.
Bug: chromium:1296467
Change-Id: Ib9ec8e60ebdb326f9bbfb215b3711c37631fce4b
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3466776
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Charlie Lao <cclao@google.com>
|
|
|
52045876
|
2022-02-07T15:08:15
|
|
Vulkan: StreamVertexDataWithDivisor write beyond buffer boundary
StreamVertexDataWithDivisor() function is advancing dst with dstStride,
but then later on it is treating dst as if it never advanced, thus
result in write out of buffer boundary. This was hidden by VMA's memory
suballocation, which means it may result in some rendering artifacts.
Bug: angleproject:6990
Change-Id: Ic91e917cedd247dfe85b12a69ae26b21b7a4e67a
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3445528
Reviewed-by: Roman Lavrov <romanl@google.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Charlie Lao <cclao@google.com>
|
|
|
1608a956
|
2022-02-02T13:54:53
|
|
Vulkan: Revert client vertex data streaming to use DynamicBuffer
In early CL crrev.com/c/3352489, I switched client vertex data streaming
from using DynamicBuffer to sub-allocating from the buffer pool. That
caused CPU overhead regression due to extra cost of handling the
suballocation object creating and garbage collection etc. Even after all
other optimizations I did since then that significantly improved garbage
collection performance, there is still 6% CPU time regression as
measured with gardenscape. This CL moves StreamVertexData() back to use
DynamicBuffer. In order to do that, I have cleaned up DynamicBuffer
interface to be consistent with suballocation interface by storing the
current allocated offset/size in the suballocation object. With that,
the BufferHelper object that returned from DynamicBuffer will be able to
pass around and referenced exactly like it comes from suballocation code
path, and you can retrieve offset/size from that BufferHelper object
instead of having to pass offset around between various function calls.
Given that streaming vertex data from client memory is only possible for
default vertex array and there is only one default vertex array for each
context, this stream vertex data dynamic buffer is essentially a per
context object. So the other change I made here is that I have merged
mDynamicVertexData with default attribute (which uses per context
dynamic buffers) code to use the same sets of dynamic buffers, since you
will only use one or the other but not both.
Bug: b/205337962
Change-Id: I0ceca5b854069f00afdb9544ee86953b9b773821
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3434645
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Charlie Lao <cclao@google.com>
|
|
|
293c0b51
|
2022-01-21T15:53:38
|
|
Vulkan: Cache commonly used 6 ushorts stream index array data
Looking at all app traces that we currently have, 16 out of 100 apps are
making glDrawElements calls without element buffer. And among these
usages, most of them are calling glDrawElements with 6 unsigned shorts,
which makes sense for drawing a quad. This CL caches first four
BufferHelper objects with 6 uint16_t indices in a buffer and reuse them
if the data matches. With this we avoid create/destroy suballocations,
we even save the time of data copy and set DIRTY_BIT_INDEX_BUFFER when
called with same set of indices, which is the case for almost all apps
that uses glDrawElements based on app traces research.
In order to test the effect, I modified the `--minimize-gpu-work` to
keep glDrawElements calls with (count=6, tye=ushort) to pass down
count/type into angle, and only change the mode to point. That way this
new optimization will gets activated with `--minimze-gpu-work` command
line option (see crrev.com/c/3421377). With that, this CL sees cpu
overhead reduced from 2.54ms to 2.37ms on Pixel6 with
vulkan_offscreen_gardenscape.
Bug: b/215768827
Change-Id: I9b682868978e3bef7b5b9d1a596500ead2738d3e
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3404677
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Charlie Lao <cclao@google.com>
|
|
|
5f0badf4
|
2022-01-18T20:05:55
|
|
Vulkan: Prevent out of bounds read in divisor emulation path.
Split the replicated part of StreamVertexData out to
StreamVertexDataWithDivisor, there is only a partial argument
overlap between the two.
Bug: chromium:1285885
Change-Id: Ibf6ab3efc6b12b430b1d391c6ae61bd9668b4407
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3398816
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Roman Lavrov <romanl@google.com>
|
|
|
d2354968
|
2022-01-20T10:59:05
|
|
Vulkan: Rename BufferHelper::initFor* to allocateFor*
Simply name change per feedback from other CL's review.
Bug: b/205337962
Change-Id: Ieb53ed9a2922d09716a1219eb340fe273e5f1807
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3402882
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Charlie Lao <cclao@google.com>
|
|
|
3263eb01
|
2021-12-28T10:10:50
|
|
Vulkan: Switch GPU translated vertex buffer to NonHostVisible
When we translate using GPU, there is no need to use host visible
memory. Use device local memory instead.
Bug: b/205337962
Change-Id: Ic76dcb28bde2f079f6ac406d846518bf5f369d74
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3340553
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Tim Van Patten <timvp@google.com>
Commit-Queue: Charlie Lao <cclao@google.com>
|
|
|
ad27d5d6
|
2021-12-21T11:22:30
|
|
Reland "Vulkan: Consolidate all vertex conversion buffers to shared pool"
This is a reland of cca412cd8b349b7281727c50f2a59d115fd90a05
Further inspection shows it was red-herring. The original CL does not
have the un-intended diff that I saw in the commit email. This is
try to reland the original CL without any modification.
Original change's description:
> Vulkan: Consolidate all vertex conversion buffers to shared pool
>
> There are various conversion buffers that holds converted vertex or
> element or index data. They are DynamicBuffer for now. This CL switches
> them to use the shared group buffer pool. With this change, all
> allocation is represented by a BufferHelper object instead of an offset.
> I am able to remove the offset arguments from a lot of APIs.
>
> Bug: b/208323792
> Change-Id: Ib611beb0c16cddbdd9ddf7b8961c439da9fa5180
> Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3352489
> Reviewed-by: Tim Van Patten <timvp@google.com>
> Reviewed-by: Jamie Madill <jmadill@chromium.org>
> Commit-Queue: Charlie Lao <cclao@google.com>
Bug: b/208323792
Change-Id: I90852ad38c2b9ac423800bb6854757bcc17cd166
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3370602
Reviewed-by: Tim Van Patten <timvp@google.com>
Commit-Queue: Charlie Lao <cclao@google.com>
|
|
|
4e85bdd9
|
2022-01-06T17:06:25
|
|
Revert "Vulkan: Consolidate all vertex conversion buffers to shared pool"
This reverts commit cca412cd8b349b7281727c50f2a59d115fd90a05.
Reason for revert: There is accidental code merge bug left in.
Original change's description:
> Vulkan: Consolidate all vertex conversion buffers to shared pool
>
> There are various conversion buffers that holds converted vertex or
> element or index data. They are DynamicBuffer for now. This CL switches
> them to use the shared group buffer pool. With this change, all
> allocation is represented by a BufferHelper object instead of an offset.
> I am able to remove the offset arguments from a lot of APIs.
>
> Bug: b/208323792
> Change-Id: Ib611beb0c16cddbdd9ddf7b8961c439da9fa5180
> Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3352489
> Reviewed-by: Tim Van Patten <timvp@google.com>
> Reviewed-by: Jamie Madill <jmadill@chromium.org>
> Commit-Queue: Charlie Lao <cclao@google.com>
Bug: b/208323792
Change-Id: I18bba207d1d8bb76dff32d9855a744dba93bc6d6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3370601
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Charlie Lao <cclao@google.com>
|
|
|
cca412cd
|
2021-12-21T11:22:30
|
|
Vulkan: Consolidate all vertex conversion buffers to shared pool
There are various conversion buffers that holds converted vertex or
element or index data. They are DynamicBuffer for now. This CL switches
them to use the shared group buffer pool. With this change, all
allocation is represented by a BufferHelper object instead of an offset.
I am able to remove the offset arguments from a lot of APIs.
Bug: b/208323792
Change-Id: Ib611beb0c16cddbdd9ddf7b8961c439da9fa5180
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3352489
Reviewed-by: Tim Van Patten <timvp@google.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Charlie Lao <cclao@google.com>
|
|
|
6c894e82
|
2021-11-04T14:49:41
|
|
Vulkan: Replace BufferVk::getBufferAndOffset() with getBuffer()
Now BufferHelper class already keeps offset information. There is no
reason for BufferVk to have that information any more.
Bug: b/205337962
Change-Id: I6e014fb480bfcd5018ef9231b0fb87a50021f179
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3266147
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Charlie Lao <cclao@google.com>
|
|
|
83a670ab
|
2021-10-29T09:12:26
|
|
Vulkan: Implement BufferPool using VMA's virtual allocator
VMA's allocation calls used to be sub-allocating a pool of memory. What
we really want is sub-allocate a VkBuffer object. VMA recently added
support to expose the underlying range allocation algorithm via APIs,
which user can use it to sub-allocate any object. This CL uses that new
virtual allocation API to sub-allocate from a pool of VkBuffers.
In this CL we only switched BufferVk::mBuffer to sub-allocate from the
BufferPool object.
Bug: b/205337962
Change-Id: Ia6ef00c22e58687e375b31bc12ac515fd89f3488
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3266146
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Tim Van Patten <timvp@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Charlie Lao <cclao@google.com>
|
|
|
4ae30a63
|
2021-11-22T16:25:11
|
|
Vulkan: Don't attempt to convert vertices in empty buffers
Bug: chromium:1271671
Change-Id: I869f30fd9c8a52c07263bb7a72978a31f2aceb9a
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3297026
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
|
dbc0c646
|
2021-11-06T01:09:26
|
|
Vulkan: Output the reason for RP closure in command buffer
To make it easier when viewing the command buffer in a graphics
debugger, this change inserts a marker just before closing the render
pass that specifies why the render pass was closed.
Bug: angleproject:2472
Change-Id: I862e500cd58332d6e199c853315c560fe6a73dc2
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3265609
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Tim Van Patten <timvp@google.com>
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
|
d2d3a546
|
2021-11-11T12:22:04
|
|
Vulkan: Write perf warnings in command buffer
It's much easier to understand what command the perf warning refers to
when it's visible in the command buffer using a graphics API debugger.
This change creates ANGLE_VK_PERF_WARNING which gives the warning both
to the application (through ANGLE_PERF_WARNING) and inserts it in the
command buffer.
Bug: angleproject:2472
Change-Id: Ie84feed53eca5cda93e1f2bc653fcbf9bcd57b56
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3275839
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Tim Van Patten <timvp@google.com>
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
|
bae19e06
|
2021-10-26T13:35:57
|
|
Vulkan: Avoid unnecessary wait if mapBufferRange indicates read only
When we call BufferVk::mapRangeImpl(), both from internal code paths for
data reads or due to glMapBufferRange call, we are not passing the
access bit to the call. This CL passes the proper access bits to the
call and only wait for GPU writes to finish if access is for read only.
This CL also adds access bitfield to the BufferVk::mapImpl() API and
have various callers pass in the proper access bits as well.
Bug: b/203582620
Change-Id: Ica8493c902dbd7b15996266c81ce0fd4dbfc2520
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3245487
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Tim Van Patten <timvp@google.com>
Commit-Queue: Charlie Lao <cclao@google.com>
|
|
|
6cb599f9
|
2021-10-06T13:07:27
|
|
Vulkan: Change dest to dst for consistency
Cleanup only, no functional change. dst aligns better with src.
Bug: angleproject:6502
Change-Id: I69821b1aae50a7ce647c7cc876468b6de309eec8
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3208514
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Cody Northrop <cnorthrop@google.com>
|
|
cf8c5678
|
2021-09-17T13:16:36
|
|
Vulkan: Don't sync VAOs after BufferSubData calls.
We still need to syncState after buffers that contain converted
attributes are updated. Includes a perf regression test.
Bug: angleproject:6371
Change-Id: I54227fc43e7b3fe79072da7783dab0177ccb0486
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3182706
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Tim Van Patten <timvp@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
|
cebca7c2
|
2021-09-24T07:55:38
|
|
Texture: Ignore buffer contents changed events.
Texture doesn't need to care when its attached buffer gets different
contents via a SubData call. This CL updates the BufferVk logic to
ensure that SubData calls trigger a storage changed notification when
there's a new storage, and otherwise Texture can ignore SubData calls.
Will make it easier to split "contents" changed notifications to their
own event, for optimizing Vertex Buffer updates.
Bug: angleproject:6371
Change-Id: I4f15ad3ad2da5d838bd51fb065184b7344b188d8
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3181562
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Tim Van Patten <timvp@google.com>
|
|
|
0a592aa4
|
2021-09-01T19:00:24
|
|
Vulkan: Add warning when a fallback texture format is used.
Emit a performance warning message when a fallback texture format is
been used for sampling. Emit a performance warning when we have to do
data copy during format fallback transition.
Bug: b/196456356
Change-Id: Ifbe66069e506597dbacfefda10e699a8e9f320d5
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3139239
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Tim Van Patten <timvp@google.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
|
|
|
856a0e03
|
2021-09-01T18:09:14
|
|
Vulkan: Make vk::Format from struct to class
With all the recent changes that there are two actualImageFormatIDs,
retrieve the actual format requires pass in a renderable boolean. And
the vertex format also has a similar requirement to the real format may
differ depends on if it is compressed or not. This struct no longer safe
to expose the underline data members directly. This CL turns it into a
class and expose the actual format via method that requires renderable
or compressed boolean.
Bug: b/196456356
Change-Id: Ie2f8308cc408bde1b0787e0b392e143187cc4425
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3139236
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Tim Van Patten <timvp@google.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
|
e354ff1a
|
2021-03-05T04:07:21
|
|
Vulkan: Allow DynamicBuffer suballocation in BufferVk
When allocations are made from DynamicBuffer, they suballocate from a
possibly larger BufferHelper. In BufferVk, the offset of the
suballocation was discarded, which limited the use of DynamicBuffer to a
pool of small buffers.
This change applies any such offset that may arise from suballocations
everywhere, and makes BufferVk use a larger buffer size when the
GL_DYNAMIC_* buffer usage hints are provided.
Bug: angleproject:5719
Change-Id: I3df3317f7acff1b1b06a5e3e2bb707616a7d0512
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2738650
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Tim Van Patten <timvp@google.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
|
|
ccd984ff
|
2021-04-08T12:06:57
|
|
Reland "Add a Vulkan feature to compress float32 vertex formats."
This reverts commit 8ace36f8c15877264fb58af7b54baad635899dca.
Original change's description:
> Revert "Add a Vulkan feature to compress float32 vertex formats."
>
> Bug: b/167404532
> Bug: b/161716126
> Change-Id: I95157a006d5c1fd2d3c0c2c2be37fa0403c07f93
> Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2510011
> Reviewed-by: Tim Van Patten <timvp@google.com>
> Reviewed-by: Jamie Madill <jmadill@chromium.org>
> Commit-Queue: Geoff Lang <geofflang@chromium.org>
Bug: b/167404532
Bug: b/161716126
Change-Id: Ic6811fe3a7124e6eb1efe7c72a1a03a120217753
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2815260
Commit-Queue: Geoff Lang <geofflang@chromium.org>
Reviewed-by: Tim Van Patten <timvp@google.com>
Reviewed-by: Charlie Lao <cclao@google.com>
|
|
|
3813c361
|
2021-04-12T11:45:46
|
|
Vulkan: More perf warning for vertex format conversion
The perf warning was previously only issued when a GPU buffer had to be
converted. This change adds a warning when client data needs to change
format too.
Bug: b/184355822
Change-Id: I3539e22f277593d60e5e1ce172baf7b8db0477fa
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2821751
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Ian Elliott <ianelliott@google.com>
|
|
fedb85b7
|
2021-04-07T16:25:54
|
|
Vulkan: Warn for unsupported vertex attrib format
Provide both a performance warning and a debug-util label (for AGI)
when loading a vertex attribute requires a format conversion.
Bug: b/184355822
Change-Id: Id8cbb34f70214327e1f5cc96559e4ea66dc17889
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2801154
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Tim Van Patten <timvp@google.com>
|
|
|
e366e2c3
|
2021-02-27T01:00:02
|
|
Vulkan: Keep dynamic buffer's free list trimmed
ContextVk's staging buffer never gets a chance to free its free buffer
list. During application load time, a large amount of memory may be
allocated from this buffer to stage texture updates and they would
remain throughout the life of the application.
This change ensures that the free buffer list doesn't grow unbounded. In
the Manhattan trace, this saves >1GB of memory on Linux.
There are now three policies for vk::DynamicBuffer:
- Always reuse buffers: This is useful for dynamic buffers that make
frequent small allocations, such as default uniforms, driver uniforms,
default vertex attributes and UBO updates.
- Never reuse buffers: This is for situations where the buffer is
unlikely to be used after some initial usage, such as texture data
upload or vertex format emulation (as the conversion result is cached,
so it's never redone).
- Limited reuse of buffers: For the staging buffer in the context which
is shared by all immutable texture data uploads, it's useful to keep a
limited number of buffers (1 in this change) to support future texture
streaming while allowing a large number of buffers allocated in a
burst to be discarded.
Bug: angleproject:5690
Change-Id: Ic39ce61e6beb3165dbce4b668e1d3984a2b35986
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2725499
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Tim Van Patten <timvp@google.com>
Reviewed-by: Charlie Lao <cclao@google.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
|
|
|
7ae8531b
|
2021-02-17T15:20:07
|
|
Vulkan: Fix crash with 0-sized element buffer
VertexArray::syncState syncs all dirty bits, including
DIRTY_BIT_ELEMENT_ARRAY_BUFFER even for draw calls that don't use this
buffer, such as glDrawArrays. If the element buffer is given 0 size,
this caused a crash in the Vulkan backend.
Bug: chromium:1172577
Change-Id: I02d78c9660c07b896f7403867b648901478251fe
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2701831
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
|
bb062070
|
2021-02-09T15:30:04
|
|
Vulkan: Fix updates to element buffer
If glBufferSubData results in a new vk::BufferHelper allocation,
VertexArrayVk::mCurrentElementArrayBuffer needs to be updated.
VertexArrayVk::syncState was working under the assumption that
DIRTY_BIT_ELEMENT_ARRAY_BUFFER_DATA cannot result in a vk::BufferHelper
pointer change.
This assumption was broken in
https://chromium-review.googlesource.com/c/angle/angle/+/2204655.
Bug: b/178231226
Change-Id: I969549c5ffec3456bdc08ac3e03a0fa0e7b4593f
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2685439
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
|
8ace36f8
|
2020-10-30T09:51:16
|
|
Revert "Add a Vulkan feature to compress float32 vertex formats."
Bug: b/167404532
Bug: b/161716126
Change-Id: I95157a006d5c1fd2d3c0c2c2be37fa0403c07f93
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2510011
Reviewed-by: Tim Van Patten <timvp@google.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Geoff Lang <geofflang@chromium.org>
|
|
|
f0b02054
|
2020-08-06T20:55:05
|
|
Add a Vulkan feature to compress float32 vertex formats.
Use the vertex conversion pipeline in VertexArrayVk to detect
static vertex data and convert float32 vertices to float16. This
feature is useful for determining if an allication is vertex
bandwidth bound and seeing what gains could be had by using smaller
attributes.
This feature could be implemented in ANGLE's frontend but new
infrastructure for converting and storing the converted attributes
would need to be added to gl::VertexArray. Our backends already
have the functionality needed to handle unsupported attribute formats
and this can be repurposed for compressing vertex formats.
Bug: b/167404532
Bug: b/161716126
Change-Id: I9a09656a72e8499faa4124adf876d7261c8341c9
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2342285
Commit-Queue: Geoff Lang <geofflang@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
|
|
|
234be194
|
2020-07-15T14:55:18
|
|
Vulkan: Move mEmptyBuffer from program to ContextVk
And merge that with TheNUllBuffer as well so that you only have one
dummy buffer per context.
Bug: b/161391337
Change-Id: I75fddb5c48393876e745ff237e11d9c5672ae10e
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2300707
Commit-Queue: Charlie Lao <cclao@google.com>
Reviewed-by: Tim Van Patten <timvp@google.com>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
|
|
|
959037e0
|
2020-05-25T15:40:38
|
|
Vulkan: Preserve RPs on XFB changes when possible.
Instead of unconditonally ending the RenderPass we keep a set of
active XFB buffers in the ContextVk. This lets us re-use RPs when
we don't write to the same buffer repeatedly.
Reduces the RenderPass count in our Manhattan capture from 29->23.
Bug: angleproject:4622
Change-Id: I28c2d4d3db1490e5d07be3c48d21fd2cc6ff85d6
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2196957
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Tim Van Patten <timvp@google.com>
Reviewed-by: Charlie Lao <cclao@google.com>
Commit-Queue: Jamie Madill <jmadill@chromium.org>
|
|
|
93577b20
|
2020-05-28T15:16:46
|
|
Vulkan: Move "null" buffer to RendererVk.
This will allow the TransformFeedback and other classes to share
the same buffer. Also should save a bit of memory.
Bug: chromium:1086532
Change-Id: I198170b4e09165a4770b68af6df9aa7b690e8d66
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2219138
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Tim Van Patten <timvp@google.com>
Reviewed-by: Courtney Goeltzenleuchter <courtneygo@google.com>
|
|
|
745e0712
|
2020-03-21T17:46:05
|
|
Vulkan: Enable CPU only buffers for PBOs
Add support for a CPU only buffer for PBOs that serve as
the destination for all host operations like MapBuffer*.
This removes the latency caused by waiting for the in-flight
GPU commands to be complete before handing over the buffer
to the app.
This change removes a ~6ms wait/sleep on the first call to
MapBuffer* in each frame of Manhattan
Bug: angleproject:4339
Tests: angle_end2end_tests --gtest_filter=BufferDataTest*Vulkan
Change-Id: I52016b160af8a670cc30f01c05e48f699521310f
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2116874
Commit-Queue: Mohan Maiya <m.maiya@samsung.com>
Reviewed-by: Tobin Ehlis <tobine@google.com>
|
|
|
7daf31d8
|
2020-03-18T09:19:52
|
|
Vulkan: Use device local memory for conversion buffers that can be converted with the GPU
When converting a vertex buffer by using GPU, the conversion buffer
doesn't need to be host mappable. Hence the conversion buffer can
be allocated on device local memory for faster GPU access times.
Bug: angleproject:3534
Change-Id: I2efabec20186992479920bddd3abd36f9c13babc
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2108706
Commit-Queue: Mohan Maiya <m.maiya@samsung.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
|
|
f8b28678
|
2020-02-26T19:12:39
|
|
Vulkan: Add support for VK_EXT_index_type_uint8
Enable VK_EXT_index_type_uint8 Vulkan extension if supported by
VkDevice.
Bug: angleproject:4405
Change-Id: I84d030497898c5944a36d9a88a31e7377ccd5e9e
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2082391
Commit-Queue: Xiaoxuan Liu <xiaoxuan.liu@arm.com>
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
|