Vulkano: vkCmdBlitImage on itself. Resource conflicts issue

Created on 24 Nov 2020  路  8Comments  路  Source: vulkano-rs/vulkano

Template

I am beginning in Rust, but I have some experiences with Vulkan in Cpp.
I am trying to implement the mipmap generation into ImmutableImage. (As I see it is in Todo list)

I was inspired from https://github.com/vulkano-rs/vulkano/issues/989, siince it seems to work (even if there was some layout transition issues that was apparently solved now)

So what I tried is that :

pub fn from_buffer<B, P>(
        source: B,
        dimensions: Dimensions,
        mipmaps: MipmapsCount,
        format: F,
        queue: Arc<Queue>,
    ) -> Result<
        (
            Arc<Self>,
            CommandBufferExecFuture<NowFuture, AutoCommandBuffer>,
        ),
        ImageCreationError,
    >
    where
        B: BufferAccess + TypedBufferAccess<Content = [P]> + 'static + Clone + Send + Sync,
        P: Send + Sync + Clone + 'static,
        F: FormatDesc + AcceptsPixels<P> + 'static + Send + Sync,
        Format: AcceptsPixels<P>,
    {
        let usage = ImageUsage {
            transfer_destination: true,
            transfer_source: true,
            sampled: true,
            ..ImageUsage::none()
        };
        let layout = ImageLayout::ShaderReadOnlyOptimal;

        let (buffer, init) = ImmutableImage::uninitialized(
            source.device().clone(),
            dimensions,
            format,
            mipmaps,
            usage,
            layout,
            source.device().active_queue_families(),
        )?;

        let mut cbb = AutoCommandBufferBuilder::new(source.device().clone(), queue.family())?;
        cbb.copy_buffer_to_image_dimensions(
            source,
            init,
            [0, 0, 0],
            dimensions.width_height_depth(),
            0,
            dimensions.array_layers_with_cube(),
            0,
        )
        .unwrap();

        let img_dim = buffer.dimensions.to_image_dimensions();
        for level in 1..buffer.mipmap_levels() {
            let [xs, ys, ds] = img_dim.mipmap_dimensions(level - 1).unwrap().width_height_depth();
            let [xd, yd, dd] = img_dim.mipmap_dimensions(level).unwrap().width_height_depth();

            cbb.blit_image(
                buffer.clone(), //source
                [0, 0, 0], //source_top_left
                [xs as i32, ys as i32, ds as i32], //source_bottom_right
                0, //source_base_array_layer
                level - 1, //source_mip_level
                buffer.clone(), //destination
                [0, 0, 0], //destination_top_left
                [xd as i32, yd as i32, dd as i32], //destination_bottom_right
                0, //destination_base_array_layer
                level, //destination_mip_level
                1, //layer_count
                Filter::Linear //filter
            ).expect("failed to blit a mip map to image!");
        }

        let cb = cbb.build().unwrap();

        let future = match cb.execute(queue) {
            Ok(f) => f,
            Err(_) => unreachable!(),
        };

        Ok((buffer, future))
    }

I am just blitting the image successively, but I get this runtime-error

thread 'main' panicked at 'failed to blit a mip map to image!: SyncCommandBufferBuilderError(Conflict { command1_name: "vkCmdBlitImage", command1_param: "source", command1_offset: 1, command2_name: "vkCmdBlitImage", command2_param: "destination", command2_offset: 1 })

As I understand the error, it seems that the VkCmdBlitImage does not understand that we don't use the same part of the image.
I am using MipmapCounts::Log2
It is a non power of 2 image : 1920x1080p.

Maybe I miss something :)

needs investigation bug

Most helpful comment

Good news,

I succeed to generate mipmaps !
I have to clean the code and I hope I can submit a pull request later this night or tomorrow

All 8 comments

@qnope Your code looks fine to my understanding. Bliting between different levels of the same image is safe Vulkan operation and is an efficient way to generate mipmaps. So it seems to be a bug in Vulkano of false-positive checking of resource occupation between different commands in command buffer builder, and is probably happening somewhere here: https://github.com/vulkano-rs/vulkano/blob/master/vulkano/src/command_buffer/synced/base.rs#L514

I don't have enough time to investigate it deeper at this moment, but if you want to invest some time in debugging it I will be very appreciated! In my project I generate mipmaps using two independent image resources to bypasses this issue, but it is memory/performance lesser efficient.

@Eliah-Lakhin
I am going to try to debug it this week. However, since I am a rust beginner, I don't know if it will be easy ahaha

Thanks

I begin to try to fix the issue.

I notice that the conflict* functions does not take care about the mip levels or the layer level of a resource. (It was the case in an old version, like 0.6, but there was a change for another version).

I also notice that a ImageAccesstrait does not have any information about the current access about the mip level or the layer level.

So I added two functions in the trait :

    /// Returns the current mip level that is accessed by the gpu
    fn current_miplevel_access(&self) -> u32 {
        0
    }

    /// Returns the current layer level that is accessed by the gpu
    fn current_layer_level_access(&self) -> u32 {
        0        
    }

The default behaviour will be changed lated, but it is just for the test purpose...

I created the following structure that represents an image accessed by one command and which mip level are used by the command

pub struct ImageResourceAccess<Img> {
    image: Arc<Img>,
    mip_level: u32,
    layer_level: u32
}

It implements the ImageAccess trait and forward the call to the image object

unsafe impl<Img> ImageAccess for ImageResourceAccess<Img> where
    Img : ImageAccess {
        fn inner(&self) -> ImageInner {
            self.image.inner()
        }

        #[inline]
        fn initial_layout_requirement(&self) -> ImageLayout {
            self.image.initial_layout_requirement()
        }
// ......

         fn conflicts_image(&self, other: &dyn ImageAccess) -> bool {
            self.conflict_key() == other.conflict_key() &&
            self.current_miplevel_access() == other.current_miplevel_access() &&
            self.current_layer_level_access() == other.current_layer_level_access()
        }

        fn current_miplevel_access(&self) -> u32 {
            self.mip_level
        }
        fn current_layer_level_access(&self) -> u32 {
            self.layer_level
        }

        #[inline]
        fn conflict_key(&self) -> u64 {
            self.image.conflict_key() + self.mip_level as u64 + self.layer_level as u64
        }
}

For the conflict_key, I will do a better version later with a proper combine_hash

Here is the way I use it to transfer from one level to another.

let [xs, ys, ds] = img_dim.mipmap_dimensions(level - 1).unwrap().width_height_depth();
            let [xd, yd, dd] = img_dim.mipmap_dimensions(level).unwrap().width_height_depth();

            println!("{}, {}, {}", xs, xd, level);

            let src = Arc::new(ImageResourceAccess {
                image: buffer.clone(),
                mip_level: level - 1,
                layer_level: 0
            });

            let dst = Arc::new(ImageResourceAccess {
                image: buffer.clone(),
                mip_level: level,
                layer_level: 0
            });

            cbb.blit_image(
                src, //source
                [0, 0, 0], //source_top_left
                [xs as i32, ys as i32, ds as i32], //source_bottom_right
                0, //source_base_array_layer
                level - 1, //source_mip_level
                dst, //destination
                [0, 0, 0], //destination_top_left
                [xd as i32, yd as i32, dd as i32], //destination_bottom_right
                0, //destination_base_array_layer
                level, //destination_mip_level
                1, //layer_count
                Filter::Linear //filter
            ).expect("failed to blit a mip map to image!");

When I build all the mip levels, I finish on unreachable part.

thread 'main' panicked at 'internal error: entered unreachable code', A:\Programmation\rust\vulkano\vulkano\src\image\immutable.rs:349:23

       let future = match cb.execute(queue) {
            Ok(f) => f,
            Err(_) => unreachable!(),
        };

When I build only the level 1 (transfer from 0 to 1), I got this error

  left: `None`,
 right: `Some(ShaderReadOnlyOptimal)`', A:\Programmation\rust\vulkano\vulkano\src\image\immutable.rs:643:9

here

unsafe impl<F, A> ImageAccess for ImmutableImageInitialization<F, A>
where
    F: 'static + Send + Sync,
{
    unsafe fn increase_gpu_lock(&self) {
        debug_assert!(self.used.load(Ordering::Relaxed));
    }

    #[inline]
    unsafe fn unlock(&self, new_layout: Option<ImageLayout>) {
        assert_eq!(new_layout, Some(self.image.layout));
        self.image.initialized.store(true, Ordering::Relaxed);
    }
}

Do you have any ideas?

@qnope Resource access control is probably one of the most difficult part of Vulkano. And I don't have a mindset on it's internal implementation architecture, unfortunately. So, the only way to fix or implement something inside it is to make such mindset by digging inside the internals.

For the first error message I would log what came inside Err(_) variant. Maybe there is useful metadata that could shed light on what has actually failed and on which stage. Otherwise you can try to follow cb.execute control flow from inside.

For the second error it seems like there is a mechanism on the lower level of architecture that implicitly prevents(or more likely not designed for) the desired behavior. Maybe it will require certain modifications to achieve this goal.

My apology for such generic answer :) I really don't have any expertise about it, but that's are steps I would do next if I work on this feature.

The internal panic here is the same as in #1447.

If the CommandBuffer encounters an error in the lock_submit function, it will unlock all the previously locked resources. It then tries to unlock every image with a None optional. The above assertion in the unlock method of the image fails.

@qnope The core issue with cross-layers bliting is probably fixed in #1449 thanks to @Gosuo .

Can you confirm please if the bliting between layers works fine on your end, and you can continue working on this feature?

I am going to try to continue the dev during the weekend!

Thanks for you all

Good news,

I succeed to generate mipmaps !
I have to clean the code and I hope I can submit a pull request later this night or tomorrow

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Craig-Macomber picture Craig-Macomber  路  11Comments

norru picture norru  路  11Comments

Vengarioth picture Vengarioth  路  7Comments

Eliah-Lakhin picture Eliah-Lakhin  路  6Comments

tomaka picture tomaka  路  8Comments