docs: add RFC for opaque generational handles#8
docs: add RFC for opaque generational handles#8gituser12981u2 wants to merge 5 commits intoalex_stufffrom
Conversation
gituser12981u2
added
the
documentation
|
I do wonder if the singular handle type view maybe a blocker for more eccentric design choices down the road? One thing we may want to do is use (or create a simple one ourselves?) for a bump allocator, it's piss easy if it's thread local to self write but obviously not suitable. Probably best to investigate this? (The bump allocator would have to be non-homogenous, pretty simple though.) *Maybe this comes into mind later down the road, when we're handling game entity objects. Other than that random discourse, looks really slick. Also love the concept use haha. |
|
Okay, I add allocator input (optional) |
| @@ -1,3 +1,6 @@ | |||
| #include "quark/utils/error_types.hpp" | |||
There was a problem hiding this comment.
quark/utils/diagnostic.hpp already includes error_types.hpp and result.hpp.
We should only need diagnostic_prelude. So remove diagnostic.hpp and error_types.hpp and result.hpp and keep only diagnostic_prelude.hpp
| @@ -1,11 +1,18 @@ | |||
| #include "quark/utils/details/diagnostic_details.hpp" | |||
There was a problem hiding this comment.
I think all of these are included in diagnostic_prelude.
Make sure to delete extra imports before PRing or else I will tell Benjamin Netanyahu of all your transgressions and then you will be barred from Isreal.
| #include <string_view> | ||
| #include <vector> | ||
| #include <vulkan/vulkan.h> | ||
| #include <vulkan/vk_platform.h> |
There was a problem hiding this comment.
I like this.
Should cut down on compile times since vulkan.h is bigger than vk_platform + vulkan_core.
There was a problem hiding this comment.
I should look into a header that forward declares all the vk functionality much like .
|
|
||
| bool enable_portability = false; | ||
| #if defined(__APPLE__) | ||
| #ifdef __APPLE__ |
There was a problem hiding this comment.
Meaningless change?
There was a problem hiding this comment.
why write long words when few word do trick yes
| @@ -1,13 +1,18 @@ | |||
| #include <cstddef> | |||
| #include "quark/utils/result.hpp" | |||
There was a problem hiding this comment.
Result is included in diagnostic.
| #include <quark/vk/instance/details/instance_handle.hpp> | ||
| #include <quark/vk/instance/details/instance_registry.hpp> | ||
| #include <quark/vk/instance/instance_bundle.hpp> | ||
| #include <vulkan/vulkan_core.h> |
There was a problem hiding this comment.
Is this really needed? I don't see any Vulkan specifics here.
| // Destroy views | ||
| if (device_ != VK_NULL_HANDLE) { | ||
| for (VkImageView v : image_views_) { | ||
| for (VkImageView const v : image_views_) { |
There was a problem hiding this comment.
It was wrong, it just creates a const ptr, not a const object.
|
|
||
| const void *GlfwWindow::query_interface(InterfaceId id) const noexcept { | ||
| if (const void *cached = iface_.find(id)) { | ||
| if (const void *const cached = iface_.find(id)) { |
There was a problem hiding this comment.
I like the additions of const where needed. Netanyahu will be happy.
| value: true | ||
| - key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreUnnamedParams | ||
| value: true | ||
| - key: readability-convert-member-functions-to-static |
There was a problem hiding this comment.
Definitely keep this true
Good idea. I think this should be at the registry level and not the handle level. |
2 participants
Added RFC for opaque handles.
Please put all comments about this RFC here.