Add proper offscreen frame boundary handling#2507
Add proper offscreen frame boundary handling#2507marius-pelegrin-arm wants to merge 4 commits intoLunarG:devfrom
Conversation
|
CI gfxreconstruct build queued with queue ID 576325. |
|
CI gfxreconstruct build # 8164 running. |
|
CI gfxreconstruct build # 8164 passed. |
`VulkanSwapchain::CreateSurface` doesn't need that many arguments, as most of them are just forwarding of replay options. In particular some options are badly forwarded (window size), some default value for options are set as constants in the Vulkan swapchain header for no reason etc. This commit tries to clean that a bit to make it more readable and maintainable. Change-Id: Iee579f79b35cdb8f3aa32210c7650bc487b6e8bb
Definition: I call "offscreen frame boundary" all Vulkan calls or
structures that define a new frame and its attached resources but
that are NOT a call to `vkQueuePresentKHR`.
This concept could be extended to non-Vulkan calls like DX12
calls or to future GFXReconstruct meta-commands, but this is NOT
handled by this commit.
Currently handled offscreen frame boundaries are:
- Calls to `vkFrameBoundaryANDROID`
- Calls to `vkQueueSubmit{2}` when the `pNext` chain of
`VkSubmitInfo{2}` contains a `VkFrameBoundaryEXT` structure
- Submission of command buffers containing a recorded call to
`vkCmdInsertDebugUtilsLabelEXT` or `vkCmdDebugMarkerInsertEXT` in
which the label or marker name contains
`graphics::kVulkanVrFrameDelimiterString`
Offscreen frame boundaries may or may not contain an image.
The goal of this commit is to provide a uniform and user-friendly
way to specify what GFXReconstruct does when encountering frame
boundaries (offscreen or onscreen).
I propose, in this commit, to add a new behavior for "presenting"
offscreen frame boundary images when they exist.
The idea is to consider offscreen frame boundaries as belonging to
some "additional swapchain and surface" and to allow to present
them or not using the existing `--swapchain` option and a new
`--use-ext-frame-boundary` to force the conversion of frame
boundaries to `VkFrameBoundaryEXT`.
This is necessary because some platforms use offscreen frame
boundaries as a valid way to actually present to the screen. Making
them not that "offscreen" anymore. A good example of that is Android
using `vkFrameBoundaryANDROID` calls in its UI applications to
present `AHardwareBuffer`-based images to the screen. At capture
time, the user sees a normal application, but replaying the same
calls will lead to a black screen, even on the exact same device.
Here is the new behavior I implement:
- The `--swapchain` option remains, and no new flag is extended,
but its definition is extended, as it now also defines the
behavior of offscreen frame boundaries
- The `--use-ext-frame-boundary` option is added and allows
to specify the difference between "letting default offscreen
frame boundaries" and "using `VkFrameBoundaryEXT` for everything".
This option automatically triggers `--swapchain offscreen`.
- The `--offscreen-swapchain-frame-boundary` is deprecated and is
now and alias to `--use-ext-frame-boundary`
Here is the new behavior of `--swapchain`:
- `captured`: All the calls are forwarded "as-is".
- `offscreen`: No `VkSwapchainKHR` object is created at all, and
by default offscreen frame boundaries are just "forwarded as-is".
If `--use-ext-frame-boundary` is set, everything is converted to
submissions of `VkFrameBoundaryEXT`.
- `virtual`: The behavior is the same for normal Vulkan swapchains.
When encountering the first offscreen frame boundary, a new window,
surface and swapchain are created, and this and future offscreen
frame boundaries are converted to `vkQueuePresentKHR` calls on the
new swapchain.
Change-Id: I7cdce638c6372d51e35875d771d9ce63d72b93f0
fabian-lunarg
force-pushed
the
add-offscreen-swapchain-boundary-handling
branch
from
4dfcef9 to
66fce6c
Compare
|
CI gfxreconstruct build queued with queue ID 591460. |
|
CI gfxreconstruct build # 8292 running. |
|
CI gfxreconstruct build # 8292 passed. |
|
CI gfxreconstruct build queued with queue ID 591488. |
|
CI gfxreconstruct build # 8293 running. |
| uint32_t windowed_width{ 0 }; | ||
| uint32_t windowed_height{ 0 }; | ||
| uint32_t windowed_width{ 320 }; | ||
| uint32_t windowed_height{ 240 }; |
There was a problem hiding this comment.
what does that mean? where/how did we populate windowed_width / windowed_height
before?
don't think we should change here.
this should rather define an (invalid) default-state for the struct.
and another place would be better suited to set default values windowed_width/height
There was a problem hiding this comment.
This is the default place for initialization of all replay options. These magic numbers come from "magic constants" I erased at the top of vulkan_swapchain.h
The PR is cut into two commits and the first commit is just cleaning these default values and how the values are (not) transferred to the WSI
fabian-lunarg
force-pushed
the
add-offscreen-swapchain-boundary-handling
branch
from
b959abc to
b6e9814
Compare
|
CI gfxreconstruct build queued with queue ID 591507. |
|
CI gfxreconstruct build # 8294 running. |
|
CI gfxreconstruct build # 8294 failed. |
|
CI gfxreconstruct build queued with queue ID 591692. |
|
CI gfxreconstruct build # 8295 running. |
|
CI gfxreconstruct build # 8295 passed. |
MarkY-LunarG
left a comment
MarkY-LunarG
left a comment
There was a problem hiding this comment.
Overall I understand the direction but it does still feel messy (see my comments). One thing is I think the whole "forcing ext frame boundary" change needs to be separated from the "virtual swapchain offscreen rendering behavior." It seems they serve slightly different purposes and it feels like putting them in the same change is a little complex.
| decode::BeginInjectedCommands(); | ||
| device_table->GetDeviceQueue(device_info->handle, 0, 0, &queue); | ||
| device_table->QueueSubmit(queue, 1, &submitInfo, VK_NULL_HANDLE); | ||
| decode::EndInjectedCommands(); |
There was a problem hiding this comment.
I think we should be tracking what Queue is submitted to last in the same thread and then using that same queue to insert end frame in the "force" scenario. Otherwise, we could end up using a Queue (in this case Queue index 0) which is scheduled differently on the hardware than we would like resulting in the "end of frame" marker occurring before/after it should during the actual replay.
| if args.offscreen_swapchain_frame_boundary: | ||
| arg_list.append('--offscreen-swapchain-frame-boundary') | ||
|
|
||
| if args.use_ext_frame_boundary: |
There was a problem hiding this comment.
I wonder if "force_ext_frame_boundary" might be a better name for all of these.
Change-Id: I82f8809e1c6402ec5155e58bc0b9f6a5d3d9d977
|
CI gfxreconstruct build queued with queue ID 605545. |
|
CI gfxreconstruct build # 8400 running. |
|
CI gfxreconstruct build # 8400 passed. |
4 participants
Definition: I call "offscreen frame boundary" all Vulkan calls or structures that define a new frame and its attached resources but that are NOT a call to
vkQueuePresentKHR. This concept could be extended to non-Vulkan calls like DX12 calls or to future GFXReconstruct meta-commands, but this is NOT handled by this commit.Currently handled offscreen frame boundaries are:
vkFrameBoundaryANDROIDvkQueueSubmit{2}when thepNextchain ofVkSubmitInfo{2}contains aVkFrameBoundaryEXTstructurevkCmdInsertDebugUtilsLabelEXTorvkCmdDebugMarkerInsertEXTin which the label or marker name containsgraphics::kVulkanVrFrameDelimiterStringOffscreen frame boundaries may or may not contain an image.
The goal of this commit is to provide a uniform and user-friendly way to specify what GFXReconstruct does when encountering frame boundaries (offscreen or onscreen).
I propose, in this commit, to add a new behavior for "presenting" offscreen frame boundary images when they exist. The idea is to consider offscreen frame boundaries as belonging to some "additional swapchain and surface" and to allow to present them or not using the existing
--swapchainoption and a new--use-ext-frame-boundaryto force the conversion of frame boundaries toVkFrameBoundaryEXT.This is necessary because some platforms use offscreen frame boundaries as a valid way to actually present to the screen. Making them not that "offscreen" anymore. A good example of that is Android using
vkFrameBoundaryANDROIDcalls in its UI applications to presentAHardwareBuffer-based images to the screen. At capture time, the user sees a normal application, but replaying the same calls will lead to a black screen, even on the exact same device.Here is the new behavior I implement:
--swapchainoption remains, and no new flag is extended, but its definition is extended, as it now also defines the behavior of offscreen frame boundaries--use-ext-frame-boundaryoption is added and allows to specify the difference between "letting default offscreen frame boundaries" and "usingVkFrameBoundaryEXTfor everything". This option automatically triggers--swapchain offscreen.--offscreen-swapchain-frame-boundaryis deprecated and is now and alias to--use-ext-frame-boundaryHere is the new behavior of
--swapchain:captured: All the calls are forwarded "as-is".offscreen: NoVkSwapchainKHRobject is created at all, and by default offscreen frame boundaries are just "forwarded as-is". If--use-ext-frame-boundaryis set, everything is converted to submissions ofVkFrameBoundaryEXT.virtual: The behavior is the same for normal Vulkan swapchains. When encountering the first offscreen frame boundary, a new window, surface and swapchain are created, and this and future offscreen frame boundaries are converted tovkQueuePresentKHRcalls on the new swapchain.I think this commit would be ready as a first step of improvement in the handling of offscreen frame boundaries but there are still holes in the implementation that could be addressed later:
VkFrameBoundaryEXT)