Updated version to vulkano 0.30 #10
Conversation
… to conflicts being checked in newer version)
| let instance = Instance::new(InstanceCreateInfo { | ||
| enabled_extensions: required_extensions, | ||
| enabled_layers: vec!["VK_LAYER_KHRONOS_validation".into()], |
There was a problem hiding this comment.
I am not opposed to enabling this, but please check if it's available. I don't have it, so this just crashes with error LayerNotPresent for me.
There was a problem hiding this comment.
that can be removed, i forgot to remove it when pushing.
| .unwrap(); | ||
|
|
||
| }).unwrap(); | ||
| let _callback = unsafe { |
There was a problem hiding this comment.
This looks confusing, because it just looks like an unused variable. Apparently this is supposed to work as long as the variable remains in scope? If so, maybe add a comment explaining this?
There was a problem hiding this comment.
this is my debug implementation i forgot to remove. it doesn't work properly anyway.
| FenceSignalFuture(FenceSignalFuture<F>), | ||
| BoxedFuture(Box<dyn GpuFuture>), | ||
| } | ||
| pub struct FrameEndFuture(Box<dyn GpuFuture>); |
There was a problem hiding this comment.
It seems this is not used any more and could be removed?
| &mut self, | ||
| textures_delta: TexturesDelta, | ||
| builder: &mut AutoCommandBufferBuilder<PrimaryAutoCommandBuffer<P::Alloc>, P>, |
There was a problem hiding this comment.
Can you explain a bit why you did this, I'm not sure I understand it 100% yet. I guess you want to use your own command buffer so you can return a future that you can join later? Does this have implications for performance? https://developer.nvidia.com/blog/vulkan-dos-donts/ seems to suggest that there is some cost associated with creating a command buffer.
There was a problem hiding this comment.
0.30 crashes when you try to use the old way to change the textures as it added a runtime check for non-readonly textures. this was necessary to get it to work. However with further testing it is bugged, to reproduce use the font atlas widget and change the size rapidly to a big one. I could not find exactly where the issue is yet.
There was a problem hiding this comment.
Interestingly, that seems to work without problems for me. Maybe because I don't have the validation layer enabled?
There was a problem hiding this comment.
You need to create a lag spike during texture generation. It happens without the validation layers as well.
There was a problem hiding this comment.
Oh, I thought by "bugged" you meant a crash or something.
There was a problem hiding this comment.
it panics if there is a lag spike
There was a problem hiding this comment.
Oh, interesting. What's the error message?
There was a problem hiding this comment.
i can't reproduce as i changed to a more performant computer, but it was due to a readonly texture being written to at the same time as it is read. this happens if the nect frame starts processing when the first frame hasn't finished uploading the texture.
i suppose it can be simulated with a 1 second sleep before uploading the texture.
| ) -> Result<UpdateTexturesResult, UpdateTexturesError> | ||
| where | ||
| P: CommandPoolBuilderAlloc, | ||
| ) -> Result<impl GpuFuture, UpdateTexturesError> |
There was a problem hiding this comment.
How about returning Result<Option<impl GpuFuture>, UpdateTexturesError> and not creating any command buffer at all if there are no texture changes? Right now it seems we are submitting an empty command buffer in that case.
There was a problem hiding this comment.
we can do that, the advantage to this version is that it is easier to sync in the queue. it is probably related to the crash during texture regeneration. (0.30 vulkano got a lot stricter with the runtime checks)
| /// Create a Vulkano image for the given egui texture | ||
| fn create_image( | ||
| queue: Arc<Queue>, | ||
| fn create_immutable_image_full( |
There was a problem hiding this comment.
There seems to be a lot of code duplication between this and create_immutable_image_part. Maybe refactor the shared parts into a separate function?
a few important changes happened due to storage images having conflicts if not readonly, thus it was necessary to transition to immutable ones.
A performance issue that seems to have arisen is with the hovers over textboxes that create a lag spike. but it could have been there before.