Minor refactor for vulkan renderer(The second)#179
Conversation
- Change tbm_format& to const tbm_format& in header and implementation - Improves API correctness and allows passing const arguments Co-Authored-By: Cline SR
- Log a warning when an unsupported TBM format is encountered - Helps with debugging by recording the unknown format value - Follows existing logging patterns using FT_LOG(Warn) Co-Authored-By: Cline SR
Change the return type of GetMemoryFdPropertiesKHR from VkResult to bool to maintain consistency with other private helper methods in the class. - Updated header file declaration - Updated implementation to return bool (true on success, false on failure) - Fixed usage site to use boolean check instead of VkResult comparison Co-Authored-By: Cline SR
Change the output parameter of GetFdMemoryTypeIndex from non-const reference to pointer for better C++ best practices. This makes it explicit at the call site that the argument may be modified. - Updated header file declaration to use uint32_t* instead of uint32_t& - Added null pointer check in implementation - Changed assignment to use pointer dereference (*index_out) - Fixed usage site to pass address using & Co-Authored-By: Cline SR
- Changed VkDeviceMemory& to VkDeviceMemory* in AllocateMemory function - Updated call sites to pass addresses (&image_memory_, &staging_buffer_memory_) - Aligns with Vulkan API convention for output parameters Co-Authored-By: Cline SR
Add ReleaseBuffer() and ReleaseImage() calls when CopyBufferToImage fails to properly clean up allocated Vulkan resources (staging buffer and image). This prevents resource leaks when vkMapMemory or other operations in CopyBufferToImage fail during texture population. Co-Authored-By: Cline SR
…kan.cc - Correct spelling error in TODO comment on line 29 Co-Authored-By: Cline SR
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors Vulkan external texture handling in the Tizen embedder, including resource cleanup on copy failures, pointer-based memory allocation, and improved format conversion logging. The review feedback suggests resetting dimensions to zero upon copy failure to maintain object consistency, adding a null check for the newly introduced memory pointer, passing the primitive tbm_format by value instead of const reference, and wrapping a line that exceeds the 80-character limit.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Code Review
This pull request modifies Vulkan external texture handling by updating several function signatures to use pointers instead of references, adding nullptr checks, and improving resource cleanup on failure. Additionally, GetMemoryFdPropertiesKHR is refactored to return a boolean indicating success. The reviewer suggests simplifying GetMemoryFdPropertiesKHR by removing the redundant device parameter, as GetDevice() is already available from the base class.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
The gemini-code-assist code review suggested that passing tbm_format by const reference is unnecessary and less efficient than passing by value, since tbm_format is an enum type (primitive type). Changes: - external_texture_surface_vulkan_buffer.h: Change ConvertFormat parameter from 'const tbm_format&' to 'tbm_format' - external_texture_surface_vulkan_buffer.cc: Update implementation to match header declaration This change improves performance by avoiding unnecessary indirection and allows better compiler optimization for this small enum type. Co-Authored-By: Cline SR
Add defensive null pointer check for the memory parameter to prevent undefined behavior when passing to vkAllocateMemory. Co-Authored-By: Cline SR
When CopyBufferToImage fails in PopulateVulkanTexture(), the allocated buffer and image are released, but width_ and height_ were not reset to 0. This left the object in an inconsistent state where the dimensions were non-zero but the Vulkan resources were destroyed. Resetting them to 0 ensures the object state remains consistent after failure cleanup. Co-Authored-By: Cline SR
2 participants
Main changes: