Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't execute command on swapchain image #977

Open
realitix opened this issue Jun 21, 2018 · 7 comments
Open

Can't execute command on swapchain image #977

realitix opened this issue Jun 21, 2018 · 7 comments

Comments

@realitix
Copy link

Hello,

When I try to execute a command directly on a swapchain image, I got a layout error:

thread '<unnamed>' panicked at 'Can't execute clear command buffer: AccessError { error: ImageNotInitialized { requested: PresentSrc }, command_name: "vkCmdCopyImageToBuffer", command_param: "source", command_offset: 0 }', libcore/result.rs:945:5

With a command like this one:

let cb = AutoCommandBufferBuilder::primary_one_time_submit(device.clone(), queue.family(),).unwrap()
            .copy_image_to_buffer(
                swapchain_images[image_num].clone(),
                buffer.clone()
            )
            .unwrap()
            .build()
            .unwrap();

I got the same error with the clear_color_image command.
I searched in the API but I can't find a way to manually update the layout. Why the AutoCommandBufferBuilder doesn't expose the vkCmdPipelineBarrier function ?

Thanks,
JS

@tomaka
Copy link
Member

tomaka commented Jul 8, 2018

Why the AutoCommandBufferBuilder doesn't expose the vkCmdPipelineBarrier function ?

Per design, the AutoCommandBufferBuilder is supposed to automatically handle pipeline barriers for you.

@dvc94ch
Copy link

dvc94ch commented Oct 27, 2018

Any updates on this?

@dvc94ch
Copy link

dvc94ch commented Oct 27, 2018

Or put differently: How can I initialize a swapchain image? I'd like to render a stream from my webcam after processing it with a compute shader straight to a window. Missing the straight to a window part, but I guess I can copy it back to ram and then push it through a graphics pipeline with the framebuffer/renderpass stuff

@mitchmindtree
Copy link
Contributor

Just want to add that I'm running into this exact issue - specifically, using an AutoCommandBufferBuilder with only the clear_color_image command on a swapchain image.

I can verify that something to do with .clear_color_image is definitely causing the issue as the AccessError no longer occurs when I remove the call.

#974 seems very similar, though runs into the issue with a StorageImage rather than a SwapchainImage. That said, it's definitely a helpful read for digging into this.

Both the initial_layout_requirement and final_layout_requirement are expected to be PresentSrc, however vkCmdClearColorImage requires either TransferDstOptimal or General. Seeing as it seems it is the role of SyncCommandBufferBuilder::prev_cmd_resource to handle these layout transitions, I'm going to look into this here first.

@realitix did you ever work out a solution? I noticed you mentioned you were going to try the solution mentioned in #974, any luck?

@mitchmindtree
Copy link
Contributor

Memory barrier image layouts seem OK

Memory barriers are inserted via the add_image_memory_barrier method. In the simple clear_color_image command buffer I describe in my previous comment, add_image_memory_barrier gets called twice:

  1. When calling the clear_color_image builder.
  2. When calling build.

The first call is a barrier with layout transition from PresentSrc -> TransferDstOptimal so that the image may be cleared. The second is a transition from TransferDstOptimal -> PresentSrc so that it is ready for present. If I print out the current_layout and new_layout within the add_image_memory_barrier it seems to be called correctly with both layout transitions, so it seems my incorrect-image-layout-theory is a dead-end.

Actual Err result trace

The exact place from which the error is returned is via the following trace:

  • GpuFuture::then_execute
  • CommandBuffer::execute_after
  • SyncCommandBuffer::lock_submit (via CommandBuffer::lock_submit impl)
  • SwapchainAcquireFuture::check_image_access (via Future::check_image_access)

The SwapchainAcquireFuture::check_image_access impl looks like this:

#[inline]
fn check_image_access(&self, image: &ImageAccess, layout: ImageLayout, _: bool, _: &Queue)
                      -> Result<Option<(PipelineStages, AccessFlagBits)>, AccessCheckError> {
    let swapchain_image = self.swapchain.raw_image(self.image_id).unwrap();
    if swapchain_image.image.internal_object() != image.inner().image.internal_object() {
        return Err(AccessCheckError::Unknown);
    }

    if self.swapchain.images[self.image_id]
        .undefined_layout
        .load(Ordering::Relaxed) && layout != ImageLayout::Undefined
    {
        println!("err a");
        return Err(AccessCheckError::Denied(AccessError::ImageNotInitialized {
                                                requested: layout,
                                            }));
    }

    if layout != ImageLayout::Undefined && layout != ImageLayout::PresentSrc {
        println!("err b");
        return Err(AccessCheckError::Denied(AccessError::UnexpectedImageLayout {
                                                allowed: ImageLayout::PresentSrc,
                                                requested: layout,
                                            }));
    }

    Ok(None)
}

Specifically, it's the first condition that is met and that is that branch which returns the error.

The thing I find strange about the first condition is that:

self.swapchain.images[self.image_id]
        .undefined_layout
        .load(Ordering::Relaxed)

is returning true in order to get into this branch, however from my understanding the swapchain image does have a defined layout (PresentSrc) so I would have expected this to return false. If I comment out that first error condition, my program seems to run correctly downstream (the window clears with the correct colour) which supports the possibility that undefined_layout might be incorrect.

I will see if I can investigate further into this undefined_layout field.

@mitchmindtree
Copy link
Contributor

It looks like undefined_layout is initialised to true within Swapchain::new_inner, but a quick

grep -nr undefined_layout ./vulkano/src/

shows that it can never be changed from this. The only other time the field seems to get read is within the SwapchainAcquireImage::check_image_access which I mentioned is the source of the Err in my previous comment.

This leads me to believe that the source of the problem is either:

  1. Swapchain images do initially start with ImageLayout::Undefined but are eventually changed to ImageLayout::PresentSrc and undefined_layout should be changed to false when this happens but currently is not or

  2. Swapchain images always initialise with ImageLayout::PresentSrc and this field is unnecessary, meaning that the error branch I'm currently hitting is unnecessary.

Will do some investigating into this.

mitchmindtree added a commit to mitchmindtree/vulkano that referenced this issue Nov 20, 2018
Upon `Swapchain::new_inner`, `ImageEntry`s are created for each image
with their `undefined_layout` field set to `true`.

Once set, this field is never altered within the vulkano codebase. The
only time it is used following initialisation is when it is read during
calls to `SwapchainAcquireFuture::check_image_access`. For reference,
the case in which I am hitting this method is when attempting to execute
a command buffer that clears a swapchain image using
`clear_color_image`.

If the `undefined_layout` field is `true` and the swapchain image layout
is not `Undefined`, the `check_image_access` function returns an error.
[See here](https://docs.rs/vulkano/0.11.1/src/vulkano/swapchain/swapchain.rs.html#828-835).
This is a problem as the implementation of
`ImageAccess::initial_layout_requirement` and
`ImageAccess::final_layout_requirement` both return
`ImageLayout::PresentSrc` which implies to me that by the time the
command buffer is executed the swapchain image layout *must* be
`PresentSrc` (the call to `add_image_memory_barrier` within
`SyncCommandBufferBuilder::build` [ensures
this](https://docs.rs/vulkano/0.11.1/src/vulkano/command_buffer/synced/base.rs.html#720))
and in turn, this error condition will always be true.

In this PR I remove the unused `undefined_layout` field as well as the
error condition that does not seem to make sense and as a result my
downstream project seems to work fine. However, it is more than likely
that there is some special case that @tomaka intended to cover here and
that there is a better, proper solution that involves checking that
swapchain image layouts are actually initially `PresentSrc` (and not
`Undefined`) and only then setting the `undefined_layout` field to
`false`.

@tomaka could you share your thoughts on this briefly? I'm happy to dive
in and do a proper fix if there is indeed some case you had in mind
here that I'm overlooking.

Eventually, this should close vulkano-rs#977.
mitchmindtree added a commit to mitchmindtree/vulkano that referenced this issue Nov 21, 2018
As mentioned by @tomaka on vulkano-rs#1117, swapchain images have an undefined
image layout upon creation. This change aims to reflect this, in turn
ensuring that the `add_image_memory_barrier` inserts the correct layout
transitions.

I believe this closes vulkano-rs#977.
@realitix
Copy link
Author

@mitchmindtree Thanks for your investigation.
I use Ash now, that let me more liberty.

mitchmindtree added a commit to mitchmindtree/vulkano that referenced this issue Dec 14, 2018
As mentioned by @tomaka on vulkano-rs#1117, swapchain images have an undefined
image layout upon creation. This change aims to reflect this, in turn
ensuring that the `add_image_memory_barrier` inserts the correct layout
transitions.

I believe this closes vulkano-rs#977.
freesig pushed a commit to freesig/vulkano that referenced this issue Dec 27, 2018
As mentioned by @tomaka on vulkano-rs#1117, swapchain images have an undefined
image layout upon creation. This change aims to reflect this, in turn
ensuring that the `add_image_memory_barrier` inserts the correct layout
transitions.

I believe this closes vulkano-rs#977.
mitchmindtree added a commit to mitchmindtree/vulkano that referenced this issue Jan 11, 2019
As mentioned by @tomaka on vulkano-rs#1117, swapchain images have an undefined
image layout upon creation. This change aims to reflect this, in turn
ensuring that the `add_image_memory_barrier` inserts the correct layout
transitions.

I believe this closes vulkano-rs#977.
mitchmindtree added a commit to mitchmindtree/vulkano that referenced this issue Feb 7, 2019
As mentioned by @tomaka on vulkano-rs#1117, swapchain images have an undefined
image layout upon creation. This change aims to reflect this, in turn
ensuring that the `add_image_memory_barrier` inserts the correct layout
transitions.

I believe this closes vulkano-rs#977.
mitchmindtree added a commit to mitchmindtree/vulkano that referenced this issue Feb 7, 2019
As mentioned by @tomaka on vulkano-rs#1117, swapchain images have an undefined
image layout upon creation. This change aims to reflect this, in turn
ensuring that the `add_image_memory_barrier` inserts the correct layout
transitions.

I believe this closes vulkano-rs#977.
mitchmindtree added a commit to mitchmindtree/vulkano that referenced this issue Feb 16, 2019
As mentioned by @tomaka on vulkano-rs#1117, swapchain images have an undefined
image layout upon creation. This change aims to reflect this, in turn
ensuring that the `add_image_memory_barrier` inserts the correct layout
transitions.

I believe this closes vulkano-rs#977.
mitchmindtree added a commit to mitchmindtree/vulkano that referenced this issue Mar 30, 2019
As mentioned by @tomaka on vulkano-rs#1117, swapchain images have an undefined
image layout upon creation. This change aims to reflect this, in turn
ensuring that the `add_image_memory_barrier` inserts the correct layout
transitions.

I believe this closes vulkano-rs#977.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants