Vulkan-docs: Timeline semaphore documentation suggestions

[mbechard]

Quick one first, I notice the docs say:

• VUID-VkSemaphoreSignalInfo-value-03259
value must be less than the value of any pending semaphore signal operations

Shouldn't this say 'must be greater than'? The core_validation.cpp correctly has the message "value must be greater than value of pending signal operation" for this VUID.

Second suggestion: a more subtle issue I've found when reading about this feature is the use of 'monotonically increasing' to describe it. The first hit for the definition of this term is:

"(mathematics, of a function) always increasing or remaining constant".

The extension then uses this term in this sentence:

"A 64-bit unsigned integer that can only be set to monotonically increasing values by signal operations and is not changed by wait operations."

Which isn't correct as per the definition, since it's not legal to set it to the same value it currently is (i.e 'remaining constant'). I would contend that since it's illegal to re-set it to it's current value, it's not monotonically increasing and this term should not be used to describe it. Perhaps monotonically strictly increasing would be better, or probably even better to just be explicit and say 'increasing by between one and maxTimelineSemaphoreValueDifference'.

[sfricke-samsung]

The core_validation.cpp

// const VkSemaphoreSignalInfo *pSignalInfo
pSignalInfo->value >= signal_semaphore.payload

The validation layers message contradicts what it is checking actually (aka, that message needs to get fixed)

But if you look at the test example from the validation layers:

```c++
VkTimelineSemaphoreSubmitInfoKHR timeline_semaphore_submit_info{};
uint64_t waitValue = 10;
uint64_t signalValue = 20;
timeline_semaphore_submit_info.sType = VK_STRUCTURE_TYPE_TIMELINE_SEMAPHORE_SUBMIT_INFO_KHR;
timeline_semaphore_submit_info.waitSemaphoreValueCount = 1;
timeline_semaphore_submit_info.pWaitSemaphoreValues = &waitValue;
timeline_semaphore_submit_info.signalSemaphoreValueCount = 1;
timeline_semaphore_submit_info.pSignalSemaphoreValues = &signalValue;

VkPipelineStageFlags stageFlags = VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT;
VkSubmitInfo submit_info{};
submit_info.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO;
submit_info.pNext = &timeline_semaphore_submit_info;
submit_info.pWaitDstStageMask = &stageFlags;
submit_info.waitSemaphoreCount = 1;
submit_info.pWaitSemaphores = &(semaphore[1]);
submit_info.signalSemaphoreCount = 1;
submit_info.pSignalSemaphores = &(semaphore[0]);
ASSERT_VK_SUCCESS(vk::QueueSubmit(m_device->m_queue, 1, &submit_info, VK_NULL_HANDLE));

VkSemaphoreSignalInfo semaphore_signal_info{};
semaphore_signal_info.sType = VK_STRUCTURE_TYPE_SEMAPHORE_SIGNAL_INFO;
semaphore_signal_info.semaphore = semaphore[0];
semaphore_signal_info.value = 25;

m_errorMonitor->SetDesiredFailureMsg(kErrorBit, "VUID-VkSemaphoreSignalInfo-value-03259");
vkSignalSemaphoreKHR(m_device->device(), &semaphore_signal_info);
m_errorMonitor->VerifyFound();
```

In this case, if vkSignalSemaphoreKHR set it to 25 before QueueSubmit finishes then it would force the driver to set it BACK down to 20 which is why the VUID says "Must be less than"

[sfricke-samsung]

Which isn't correct as per the definition, since it's not legal to set it to the same value it currently is (i.e 'remaining constant').

I am not sure where you think it says it is legal to set to the same value?

is not changed by wait operations

Is referring to VkTimelineSemaphoreSubmitInfoKHR::pWaitSemaphoreValues are not allowed to write to the value

[mbechard]

Thanks for the clarification on issue 1, makes total sense. At least I fell backwards into an actual issue :)

For issue 2:

A 64-bit unsigned integer that can only be set to monotonically increasing values by signal operations

I wasn't 100% sure of the definition of monotonically increasing so I looked it up, saw that it allows for values to remain constant, and then read the statement as:

A 64-bit unsigned integer that can only be set to "always increasing or remaining constant" values by signal operations

I understand it's clarified elsewhere, but since "monotonically increasing" fails to properly describe the actual required behavior of the signal operations, IMO it should be described using another term.

[sfricke-samsung]

@mbechard so agreed in the working ground that it should be "strictly increasing" instead of "monotonically increasing" as we already discussed this and made the updates/changes only to the VUID and not the spec language itself. Going to make the internal spec fix. Thanks for rasing this to our attention!

[oddhack]

This should be fixed in the 1.2.171 spec update.