Skip to content
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

Proper lifetime handling of those "->as_something()" (and similar) objects (should be temporary lifetime!) #3

Open
5 tasks
johannesugb opened this issue Jul 28, 2020 · 0 comments
Labels
C++ C++-centric task enhancement New feature or request good first issue Good for newcomers

Comments

@johannesugb
Copy link
Member

johannesugb commented Jul 28, 2020

When using image_view_t::as_storage_image or similar (intended to be temporary) objects of the pattern "as_something", the following situation can occur:

An image_view_as_storage_image stores the vk::ImageView handle internally, and its lifetime might extend the lifetime of the image_view it has stored the handle from. That's not good. Optimally, the framework would prevent such usages.

One option would be that the image_view_as_storage_image co-owns the image_view which would imply (for most cases, probably) that enable_shared_ownership() is being applied to an image_view. That is a potentially huge overhead for an operation which produces an object that shall only be used as a temporary object anyways. So, what to do?

Best option would probably be to modify image_view_as_storage_image in a way so that it can not be stored somewhere but can only be used as a temporary object. Maybe all functions/methods that take a parameter of type image_view_as_storage_image shall take it as rvalue only (i.e. image_view_as_storage_image&&). If, furthermore, its move constructor and move assignment operator would be disabled, that might prevent the user from std::move-ing it into those functions/methods that take an image_view_as_storage_image&& parameter.

This shall be implemented for the following types:

  • class image_view_as_input_attachment
  • class image_view_as_storage_image
  • class buffer_descriptor
  • class buffer_view_descriptor
  • shader_binding_table_ref

Pattern for disabling all kinds of stuff for type T:

		T() = delete;
		T(T&&) noexcept = delete;
		T(const T&) = delete;
		T& operator=(T&&) noexcept = delete;
		T& operator=(const T&) = delete;
		~T() = delete;

Update:
With the introduction of the functions as_uniform_buffers, as_uniform_texel_buffer_views, as_storage_buffers, as_storage_texel_buffer_views, as_storage_images, and as_input_attachments in bindings.hpp, the request to make those helper classes non-storable has become a bit more challenging, because they are stored in those functions.

Idea: Can constructors be declared as being friend of something, so that only those functions may use them, but not "ordinary" users of the framework? I'm not a big fan of restricted usage either, but we need to ensure correctness, and if those classes do not own the resources they are referencing, we can not ensure correctness.

@johannesugb johannesugb changed the title Proper lifetime handling of those "->as_something()" objects (should be temporary lifetime!) Proper lifetime handling of those "->as_something()" (and similar) objects (should be temporary lifetime!) Jul 28, 2020
@johannesugb johannesugb transferred this issue from cg-tuwien/Auto-Vk-Toolkit Jul 30, 2020
@johannesugb johannesugb added C++ C++-centric task enhancement New feature or request good first issue Good for newcomers labels Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C++ C++-centric task enhancement New feature or request good first issue Good for newcomers
Development

No branches or pull requests

1 participant