Skip to content

Commit

Permalink
Fix some non-deterministic behavior found by Valgrind. Paranoid print…
Browse files Browse the repository at this point in the history
…ing.
  • Loading branch information
mcourteaux committed Jun 13, 2024
1 parent b067dc7 commit 52e3814
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 2 deletions.
16 changes: 16 additions & 0 deletions src/bgfx_p.h
Original file line number Diff line number Diff line change
Expand Up @@ -1700,6 +1700,9 @@ namespace bgfx
bind.m_idx = kInvalidHandle;
bind.m_type = 0;
bind.m_samplerFlags = 0;
bind.m_format = 0;
bind.m_access = 0;
bind.m_mip = 0;
}
}
};
Expand Down Expand Up @@ -2168,6 +2171,9 @@ namespace bgfx
bx::memSet(m_occlusion, 0xff, sizeof(m_occlusion) );

m_perfStats.viewStats = m_viewStats;

bx::printf(" -- Clearing m_renderItemBind -- \n");
bx::memSet(&m_renderItemBind[0], 0, sizeof(m_renderItemBind));
}

~Frame()
Expand Down Expand Up @@ -2445,6 +2451,13 @@ namespace bgfx
{
EncoderImpl()
{
// Although it will be cleared by the discard(), the fact that the
// struct is padded to have a size equal to the cache line size,
// will leaves bytes uninitialized. This will influence the hashing
// as it reads those bytes too. To make this deterministic, we will
// clear all bytes (inclusively the padding) before we start.
bx::memSet(&m_bind, 0, sizeof(m_bind));

discard(BGFX_DISCARD_ALL);
}

Expand Down Expand Up @@ -2725,6 +2738,9 @@ namespace bgfx
? BGFX_SAMPLER_INTERNAL_DEFAULT
: _flags
;
bind.m_format = 0;
bind.m_access = 0;
bind.m_mip = 0;

if (isValid(_sampler) )
{
Expand Down
26 changes: 24 additions & 2 deletions src/renderer_vk.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4287,6 +4287,10 @@ VK_IMPORT_DEVICE

if (0 != depthAspectMask)
{
attachments[mrt].colorAttachment = VK_ATTACHMENT_UNUSED;
// The above is meaningless and not required by the spec, but Khronos
// Validation Layer has a conditional jump depending on this, even
// without VK_IMAGE_ASPECT_COLOR_BIT set. Valgrind found this.
attachments[mrt].aspectMask = depthAspectMask;
attachments[mrt].clearValue.depthStencil.stencil = _clear.m_stencil;
attachments[mrt].clearValue.depthStencil.depth = _clear.m_depth;
Expand Down Expand Up @@ -4425,11 +4429,14 @@ VK_IMPORT_DEVICE
StagingBufferVK allocFromScratchStagingBuffer(uint32_t _size, uint32_t _align, const void *_data = NULL)
{
BGFX_PROFILER_SCOPE("allocFromScratchStagingBuffer", kColorResource);
bx::printf(" -- allocFromScratchStagingBuffer(%u, %u, %p) --\n", _size, _align, _data);
StagingBufferVK result;
ScratchBufferVK &scratch = m_scratchStagingBuffer[m_cmd.m_currentFrameInFlight];
if (_size <= BGFX_CONFIG_MAX_STAGING_SIZE_FOR_SCRACH_BUFFER) {
if (_size <= BGFX_CONFIG_MAX_STAGING_SIZE_FOR_SCRACH_BUFFER)
{
uint32_t scratchOffset = scratch.alloc(_size, _align);
if (scratchOffset != UINT32_MAX) {
if (scratchOffset != UINT32_MAX)
{
result.m_isFromScratch = true;
result.m_size = _size;
result.m_offset = scratchOffset;
Expand Down Expand Up @@ -4703,6 +4710,7 @@ VK_DESTROY

uint32_t ScratchBufferVK::alloc(uint32_t _size, uint32_t _minAlign)
{
bx::printf(" -- ScratchBufferVK::alloc(%u, %u) -- \n", _size, _minAlign);
const uint32_t align = bx::uint32_lcm(m_align, _minAlign);
const uint32_t dstOffset = bx::strideAlign(m_pos, align);
if (dstOffset + _size <= m_size)
Expand Down Expand Up @@ -6199,6 +6207,8 @@ VK_DESTROY
bx::memCopy(mappedMemory, imageInfos[ii].data, imageInfos[ii].size);
mappedMemory += imageInfos[ii].size;
bufferCopyInfo[ii].bufferOffset += stagingBuffer.m_offset;
BX_ASSERT(bufferCopyInfo[ii].bufferOffset % blockInfo.blockSize == 0,
"Alignment for subimage %u is not aligned correctly (%u).", ii, bufferCopyInfo[ii].bufferOffset, blockInfo.blockSize);
}

if (!stagingBuffer.m_isFromScratch)
Expand Down Expand Up @@ -6303,6 +6313,9 @@ VK_DESTROY

StagingBufferVK stagingBuffer = s_renderVK->allocFromScratchStagingBuffer(size, align, data);
region.bufferOffset += stagingBuffer.m_offset;
BX_ASSERT(region.bufferOffset % align == 0,
"Alignment for image (mip %u, z %s) is not aligned correctly (%u).",
_mip, _z, region.bufferOffset, align);

if (VK_IMAGE_VIEW_TYPE_3D == m_type)
{
Expand Down Expand Up @@ -6480,6 +6493,14 @@ VK_DESTROY

setImageMemoryBarrier(_commandBuffer, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL);

const bimg::ImageBlockInfo &bi = bimg::getBlockInfo(bimg::TextureFormat::Enum(m_textureFormat));
for (uint32_t ii = 0; ii < _bufferImageCopyCount; ++ii)
{
bx::printf(" -- BufferImageCopy[%u].bufferOffset = %u (align = %u, texture format=%s)\n",
ii, _bufferImageCopy[ii].bufferOffset, bi.blockSize, bimg::getName(bimg::TextureFormat::Enum(m_textureFormat)));
BX_ASSERT(_bufferImageCopy[ii].bufferOffset % bi.blockSize == 0, "Buffer image copy offset (%u) is not aligned correctly (%u)",
_bufferImageCopy[ii].bufferOffset, bi.blockSize);
}
vkCmdCopyBufferToImage(
_commandBuffer
, _stagingBuffer
Expand Down Expand Up @@ -7887,6 +7908,7 @@ VK_DESTROY
m_queue = _queue;
m_numFramesInFlight = bx::clamp<uint32_t>(_numFramesInFlight, 1, BGFX_CONFIG_MAX_FRAME_LATENCY);
m_activeCommandBuffer = VK_NULL_HANDLE;
m_consumeIndex = 0;

return reset();
}
Expand Down

0 comments on commit 52e3814

Please sign in to comment.