-
Notifications
You must be signed in to change notification settings - Fork 438
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
vkCmdBlitImage on itself. Resource conflicts issue #1441
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 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 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.
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
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 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 |
If the CommandBuffer encountered an error in the lock_submit function, it would unlock all the previously locked resources and return the error. It would unlock all the images with a None value which would panic in an assertion in the unlock method of the images. Now it checks for the final_layout and unlocks the image this way. This change should help with the panics mentioned in vulkano-rs#1441 and vulkano-rs#1447. With this fix it should return the error that is occuring instead of panicking.
* Unlock images correctly on Err If the CommandBuffer encountered an error in the lock_submit function, it would unlock all the previously locked resources and return the error. It would unlock all the images with a None value which would panic in an assertion in the unlock method of the images. Now it checks for the final_layout and unlocks the image this way. This change should help with the panics mentioned in #1441 and #1447. With this fix it should return the error that is occuring instead of panicking. * Add fix to the change log to inform users.
I am going to try to continue the dev during the weekend! Thanks for you all |
Good news, I succeed to generate mipmaps ! |
This should be solved by the various changes made to |
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 #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 :
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 :)
The text was updated successfully, but these errors were encountered: