search

adrien-ben / imgui-rs-vulkan-renderer

A Vulkan renderer for imgui-rs using Ash
MIT License
63 stars 19 forks source link

Fix font texture building for Apple M1 devices #22

Closed 0x7c13 closed 2 years ago

0x7c13 commented 2 years ago

Apple's Metal API (MTLTextureDescriptor) only supports a maximum size of 16384 (128x128) for texture width/height on M1 devices. We need to set the desired width to be the max image dimension size get from device properties here to be safe.

Reference: https://github.com/imgui-rs/imgui-rs/blob/b798342157fad17488d2fc5978ecf2a5f5d76c42/imgui/src/fonts/atlas.rs API Doc: https://germangb.github.io/imgui-ext/imgui/struct.FontAtlas.html

adrien-ben commented 2 years ago

Hi!

I think this is something we'd rather keep in the user's hands. Forcing it in the renderer would prevent the user to configure it. Or at least only set it if its current value is 0.

0x7c13 commented 2 years ago

The context here is the font atlas class: imgui.fonts() not imgui itself which we are getting inside of fonts_texture load. There is no way to modified unless we provide a flag/parameter to the from_allocator function which means we also need to add the same to the with_vk_mem_allocator constructor as well.

adrien-ben commented 2 years ago

Yes but imgui.fonts() is accessible from outside the renderer, you don't need the renderer to set it for you. You just need to set tex_desired_width before creating the renderer. I just tried it here : e04f636 and it works well.

The log output without changing tex_desired_width

DEBUG [imgui_rs_vulkan_renderer::renderer] Texture atlas. Width: 1024 - Height: 1024

And with setting its value to 256

DEBUG [imgui_rs_vulkan_renderer::renderer] Texture atlas. Width: 256 - Height: 8192
0x7c13 commented 2 years ago

Yes but imgui.fonts() is accessible from outside the renderer, you don't need the renderer to set it for you. You just need to set tex_desired_width before creating the renderer. I just tried it here : e04f636 and it works well.

The log output without changing tex_desired_width

DEBUG [imgui_rs_vulkan_renderer::renderer] Texture atlas. Width: 1024 - Height: 1024

And with setting its value to 256

DEBUG [imgui_rs_vulkan_renderer::renderer] Texture atlas. Width: 256 - Height: 8192

Looks good to me, closing the PR now.