NervLand: Support to resize the window

In our previous post, we implemented the support to use VertexBuffer objects to provide the vertex data we want to draw. This time we are going to do something completely different and focus instead on the support for resizing the main window of our application.

• As shown in the Little Vulkan Engine video tutorial series, I just expect the application to crash currently if I try to resize the window: let's confirm that 😊.
• Actually, first, to be able to resize the window I must make it resizable in SDL, so I'm now preparing the Window traits in a dedicated SDLApp method that I will override in the VulkanApp:

  -- Prepare the window traits for the main window creation.
  function Class:createWindowTraits()
      local wt = nv.Window.Traits("main_window", 800, 600)
      wt.title = "NervSeed main window"
      wt.fullscreen = false
      wt.hidden = false
      wt.resizable = false
  
      return wt
  end
  

• And then our override is:

  -- Prepare the window traits for the main window creation.
  ---@return nv.Window.Traits
  function Class:createWindowTraits()
      local wt = Class.super:createWindowTraits()
      logDEBUG("Setting window traits as resizable.")
      wt.resizable = true
      return wt
  end

• ⇒ Running the app with this code and then trying to resize the window we indeed get a crash, starting with those outputs:

  2022-11-10 09:14:51.643215 [DEBUG] Resizing swapchain image specific cmd buffers to 3
  2022-11-10 09:14:51.646637 [DEBUG] Done recording 3 command buffers.
  2022-11-10 09:17:32.339473 [FATAL] Cannot present image from swapchain
  2022-11-10 09:17:32.339685 [FATAL] Error in lua app:
  C++ exception

• So, we need to rebuild our Swapchain here obviously, but then we also have to rebuild our renderpass, and also the framebuffers lol. So let's try to see how we can structure those changes properly.

• First thing first: I need to be notified when the window size is changed… Or do I ? 🤔 ⇒ I mean, in the Renderer::start_next_frame() method we should already get an error ? but anyway, let's check how I could handle resize events.

• Starting to think about it: I believe I could in fact make the VulkanRenderer responsible for handling the change in window size… This means that I should connect the Window to the Renderer somehow. In fact, I should probably provide that in the constructor of the renderer: then maybe the renderer could handle the swapchains internally ? Hmmm.

• Note that we have to keep in mind here that eventually we could also want to create a Renderer with no target window (or swapchain) for offscreen renderer: in that case we could simply provide a null pointer for the window.

• ⇒ So let's get started on this. … And, I can already tell this is going to be a bit tricky 😅:
• First I have now moved the VulkanSurface ownership into the renderer itself: so we build the surface when we assing a target window to the renderer:

  void VulkanRenderer::set_target_window(nv::Window* window) {
      // For now we assume the window is not Null:
      CHECK(window != nullptr, "Invalid target window.");
  
      // Assign the window:
      _window = window;
  
      // create the window surface:
      auto* eng = VulkanEngine::instance();
  
      // Note that we use the default VulkanInstance below:
      auto* inst = eng->get_instance(0);
  
      _surface = inst->create_presentation_surface(_window->get_handle());
  
      // Reset the swapchain pointer as we don't want to use it as previous
      // swapchain:
      _swapchain = nullptr;
  
      // We should also connect a resize handler on the window here:
      _window->set_resize_handler([this](int width, int height) {
          logDEBUG("Window resized to {}x{}", width, height);
          rebuild_swap_chain();
      });
  
      // Assign the default swapchain:
      rebuild_swap_chain();
  }

• Then I have that method call on init or each to we get a resize event to rebuild our swapchain:

  void VulkanRenderer::rebuild_swap_chain() {
      // Should first ensure that we have a window pointer here:
      CHECK(_window != nullptr, "Invalid window pointer.");
      CHECK(_surface != nullptr, "Invalid window surface.");
  
      // get the window extent:
      int width = 0;
      int height = 0;
      _window->get_drawable_size(width, height);
  
      logDEBUG("Creating swapchain with drawable size {}x{}", width, height);
      CHECK(width > 0 && height > 0, "Invalid window dimensions.");
  
      auto extent = VkExtent2D{(U32)width, (U32)height};
  
      // create the swapchain:
      _swapchain =
          _device->create_swap_chain(_surface.get(), extent, _presentFormat);
      U32 numImages = _swapchain->get_num_images();
      logDEBUG("Done creating swapchain with {} images", numImages);
  }
  

• But when we rebuild the swapchain we also have to rebuild the renderpass itself. Currently this is done only once in lua with this method:

  -- Create our render pass:
  function Class:createRenderPass()
      local adesc = nvk.VulkanAttachmentDescription()
  
      -- color attachment:
      local schain = self.vkeng:get_swap_chain(self.swapchain)
      adesc:setFormat(schain:get_image_format())
      adesc:setSamples(vk.SampleCountFlagBits['1_BIT'])
      adesc:setLoadOp(vk.AttachmentLoadOp.CLEAR)
      adesc:setStoreOp(vk.AttachmentStoreOp.STORE)
      adesc:setStencilLoadOp(vk.AttachmentLoadOp.DONT_CARE)
      adesc:setStencilStoreOp(vk.AttachmentStoreOp.DONT_CARE)
      adesc:setInitialLayout(vk.ImageLayout.UNDEFINED)
      adesc:setFinalLayout(vk.ImageLayout.PRESENT_SRC_KHR)
  
      -- depth attachment:
      -- adesc:addItem(vk.Format.D24_UNORM_S8_UINT)
      adesc:addItem(vk.Format.D32_SFLOAT)
      adesc:setSamples(vk.SampleCountFlagBits['1_BIT'])
      adesc:setLoadOp(vk.AttachmentLoadOp.CLEAR)
      adesc:setStoreOp(vk.AttachmentStoreOp.DONT_CARE)
      adesc:setStencilLoadOp(vk.AttachmentLoadOp.DONT_CARE)
      adesc:setStencilStoreOp(vk.AttachmentStoreOp.DONT_CARE)
      adesc:setInitialLayout(vk.ImageLayout.UNDEFINED)
      adesc:setFinalLayout(vk.ImageLayout.DEPTH_STENCIL_ATTACHMENT_OPTIMAL)
  
      -- Setup a first subpass here:
      local sdesc = nvk.VulkanSubpassDescription()
      sdesc:addColorAttachment(0, vk.ImageLayout.COLOR_ATTACHMENT_OPTIMAL)
      sdesc:setDepthStencilAttachment(1, vk.ImageLayout.DEPTH_STENCIL_ATTACHMENT_OPTIMAL)
  
      local ddesc = nvk.VulkanSubpassDependency()
      ddesc:setSrcStageMask(vk.PipelineStageFlagBits.COLOR_ATTACHMENT_OUTPUT_BIT +
          vk.PipelineStageFlagBits.EARLY_FRAGMENT_TESTS_BIT)
      ddesc:setDstStageMask(vk.PipelineStageFlagBits.COLOR_ATTACHMENT_OUTPUT_BIT +
          vk.PipelineStageFlagBits.EARLY_FRAGMENT_TESTS_BIT)
      ddesc:setSrcAccessMask(0)
      ddesc:setDstAccessMask(vk.AccessFlagBits.COLOR_ATTACHMENT_WRITE_BIT +
          vk.AccessFlagBits.DEPTH_STENCIL_ATTACHMENT_WRITE_BIT)
  
      local rpass = self.vkeng:create_render_pass(adesc, sdesc, ddesc)
      logDEBUG("Done creating render pass.")
      return rpass
  end

• ⇒ So what I'm thinking is that I could instead keep track of the VulkanAttachmentDescription, VulkanSubpassDescription and VulkanSubpassDependency directly inside the renderer. But… wait a minute: if we look more carefully, all we really need from the swapchain here is the image format, ie, what we call the presentFormat when building our renderer, so we already have access to that and it will not change with the size of the window! So no swapchain needed here in the end 🤣.

• What else do we need to change ? Next major thing we need when the swapchain dimensions are changed is to update the framebuffers we have for each of the swapchain images. Those framebuffers will need access to the swapchain images (with a view) but may also require additional images for instance fot he depth buffer.

• ⇒ In fact all the images we may need are defined in the renderpass attachment description ⇒ So we should keep a copy of that in our RenderPass class.

• Important note: one thing I should note here is that, now that I'm starting to keep track of the attachment, etc in the renderpass I should ensure I'm not modifying those configs externally afterwards. Maybe I should really just make a copy of that data internally instead 🤔 ⇒ yeah, I think that's way better!

• Now the renderpass should also be provided to the renderer: so that we ca use it to rebuild the frambuffers as needed. And this also means we should rather assign the window and the render pass at the same time, so that we can prepare the image when building the swapchain. ⇒ So let's not pass the window in the constructor then (or pass the rendepass too ? )

• Feeww.. So now we have some pretty complex mechanism to rebuild the framebuffers when the swapchain is reconstructed:

  void VulkanRenderer::rebuild_swap_chain() {
      // Should first ensure that we have a window pointer here:
      CHECK(_window != nullptr, "Invalid window pointer.");
      CHECK(_surface != nullptr, "Invalid window surface.");
      CHECK(_renderpass != nullptr, "Invalid renderpass");
  
      // Clear the framebuffers:
      _framebuffers.clear();
  
      // Clear the previous list of images and views:
      _fbViews.clear();
  
      // get the window extent:
      int width = 0;
      int height = 0;
      _window->get_drawable_size(width, height);
  
      logDEBUG("Creating swapchain with drawable size {}x{}", width, height);
      CHECK(width > 0 && height > 0, "Invalid window dimensions.");
  
      auto extent = VkExtent2D{(U32)width, (U32)height};
  
      // create the swapchain:
      _swapchain =
          _device->create_swap_chain(_surface.get(), extent, _presentFormat);
      U32 numImages = _swapchain->get_num_images();
      logDEBUG("Done creating swapchain with {} images", numImages);
  
      // Once the swapchain is reconstructed, we should also reconstruct the
      // framebuffers:
      _framebuffers.resize(numImages);
  
      // Get the attachment descriptions:
      const auto& adesc = _renderpass->get_attachment_descs();
      U32 numAttachments = adesc.size();
  
      // Index in the attachment list for the swapchain image itself:
      U32 swapchainImageIndex = 0;
  
      VulkanImageViewList views;
  
      for (U32 i = 0; i < numImages; ++i) {
          // Iterate on the list of attachment, preparing our views in the
          // process:
          views.clear();
          for (U32 j = 0; j < numAttachments; ++j) {
              nv::RefPtr<VulkanImageView> view;
  
              if (j == swapchainImageIndex) {
                  // This is where we should place the view on the swapchain
                  // image:
                  view = _swapchain->get_image(i)->create_view();
              } else {
                  // We must create the corresponding image first:
                  const auto& att = adesc[j];
                  // Get the default usage for that attachment:
                  auto usage = get_attachment_usage(att.format);
  
                  auto img = _device->create_image(
                      VK_IMAGE_TYPE_2D, att.format, {(U32)width, (U32)height, 1},
                      1, 1, VK_SAMPLE_COUNT_1_BIT, usage);
  
                  // Create the view for that image:
                  view = img->create_view();
              }
  
              // Store that view:
              views.push_back(view);
              _fbViews.push_back(view);
          }
  
          // Once we have all the views we can create the frambuffer:
          _framebuffers[i] = _device->create_framebuffer(
              _renderpass.get(), views, (U32)width, (U32)height, 1);
      }
  }

• Next we should tackle rebuilding the command buffers for our new framebuffers. How should we do that 🤔? The VulkanRenderer should be able to trigger the rebuild of the command buffer targetting a given image index when the swapchain is changed.

• So far we had a function to assign a command buffer for a given image:

      /** Add a command buffer specific to a given image index in the swapchain**/
      void add_swapchain_img_cmd_buffer(VulkanCommandBuffer* cbuf,
                                        U32 imageIndex);

• ⇒ I think we should now replace that with some kind of “commnd buffer generator” instead. That generator would get the renderer itself and the swapchain image index are argument, and build the command buffer from that. But how could I construct this in a generic way, hmmm 🤔, tricky…

• So instead, I could maybe just start with introducing a lua callback here (?) Let's see… Arrgggh… that's also tricky: I need to introduce some kind of callback, but I need to hide the details about the fact that this callback will be executed in lua, and I was hoping to avoid inheritance here… but I don't quite see how to achieve that, so let's add inheritance into this now.

• So introducing the base Callback class:

  struct Callback {
      Callback(const Callback& rhs) = default;
      Callback(Callback&& rhs) = default;
      auto operator=(const Callback&) -> Callback& = default;
      auto operator=(Callback&&) -> Callback& = default;
  
      virtual ~Callback() = default;
      virtual void operator()() const = 0;
  };
  

• And providing a Lua function specific version:

  class NVCORE_EXPORT LuaCallback : public Callback {
      NV_DECLARE_NO_COPY(LuaCallback)
      NV_DECLARE_NO_MOVE(LuaCallback)
  
      LuaCallback(lua_State* L, I32 idx);
      ~LuaCallback() override;
      void operator()() const override;
  
      luna::LuaFunction* func;
  };
  

• ⇒ I don't really like the solution above, because I need to dynamically allocate the memory for a LuaFunction object inside the LuaCallback class… ⇒ I think for now I could just as well just pass the state/index values.

• Arrrgghh… then I got some trouble setting up a LuaFunction correctly when adding a lua extension on the vulkanRenderer, but here we are now:

  inline void _lunaext_on_swapchain_updated(nvk::VulkanRenderer& obj,
                                            lua_State* L,
                                            luna::LuaFunction& func) {
      nv::RefPtr<nv::LuaCallback> cb =
          nv::create_ref_object<nv::LuaCallback>(func.state, func.index);
  
      obj.on_swapchain_updated(cb.get());
  };

• Okay, so now, we should connect this callback in lua to rebuild the command buffers:

  self.renderer:on_swapchain_updated(function() self:recordCommandBuffers(rpass, self.pipeline, vbuf) end)

• And in fact I'm now creating new command buffers directly in this recordCommandBuffers() method, so no need for the method createCommandBuffers anymore. Yet we still need the part were we create the CommandPool, so we keep that.

• Finally, we still have the GraphicsPipeline itself, where we use the swapchain dimensions to setup the viewport: right now we are not rebuilding the pipeline, so the viewport will not change. Results will be non-impressive, but at least we should not crash anymore, so let's see what we've got 😇

• ⇒ Hmmm, unfortunately I get an error when re-creating the swapchain here. But reading the documentation further I see that we actually must provide the reference to the previous swapchain when using the same surface! So let's fix that. Done:

      // create the swapchain:
      // Note: we must provide the previous swapchain if the surface is not
      // changed:
      nv::RefPtr<VulkanSwapChain> prevSwapChain = _currentSwapChain;
  
      _currentSwapChain = _device->create_swap_chain(
          _surface.get(), extent, _presentFormat, prevSwapChain.get());
      U32 numImages = _currentSwapChain->get_num_images();
      logDEBUG("Done creating swapchain with {} images", numImages);
  

• And this is indeed working better now, but we get another crash after that point.

• The next problem seems to be related to how we create our Lua callback.
• And in fact, I started to recollect some memories on lua, and this got me thinking about “references” 😅: when I create a lua callback, currently I just take the index of the lua function on the stack and store that. Except that just after exiting the C function assigning the lua callback that stack will change, and the index will then contain something different! So calling a function at that location is an error!

• Instead we have to use lua references, as described here: https://www.lua.org/pil/27.3.2.html

• So here are the updated methods in the LuaCallback class:

  LuaCallback::LuaCallback(lua_State* L, I32 idx) : state(L), ref(LUA_NOREF) {
      CHECK(lua_isfunction(state, idx), "Invalid lua function.");
      // Duplicate the function at the given index:
      lua_pushvalue(L, idx);
  
      // Store the reference on the function and keep its ref index:
      // NOLINTNEXTLINE
      ref = luaL_ref(L, LUA_REGISTRYINDEX);
      CHECK(ref != LUA_NOREF, "Invalid ref.");
  }
  
  LuaCallback::~LuaCallback() {
      // destroy the ref:
      luaL_unref(state, LUA_REGISTRYINDEX, ref);
  };
  
  void LuaCallback::operator()() const {
      // func->execute();
      // Push the function:
      lua_rawgeti(state, LUA_REGISTRYINDEX, ref);
  
      // call with no args and no results:
      lua_call(state, 0, 0);
  }

• ⇒ Cool! That'w working much better now (ie. the lua callback is executed properly).

• But there is still an issue on the reference counting of the previous swapchains… I need to clarify that.

• Hmmm, okay: so I had a bug in the move assignment of the RefPtr template 😳! That was pretty nasty! 😨 But this should be fixed now with this version:

      auto operator=(RefPtr&& ref) noexcept -> RefPtr& {
          T* tmp_ptr = _ptr;
          // NOLINTNEXTLINE
          _ptr = ref._ptr;
          // we do not change the ptr count for the ref (since we add 1 and sub 1)
          // Just reset that ref ptr:
          ref._ptr = nullptr;
          // And unref our prev ptr:
          if (tmp_ptr) {
              tmp_ptr->unref();
          }
          return *this;
      }

• Next I got another issue when destroying the renderer now: since we have a lua callback attached there, and we try to unregister the ref on destruction. For now I fixed that by adding binding support for the nv::Callback class, and then explicitly setting the callback to nil on uninit in lua:

      logDEBUG("Uninitializing VulkanApp")
  
      -- Wait untill all current operations are completed:
      self.vkeng:wait_device_idle()
      self.renderer:on_swapchain_updated(nil)

• Note: ⇒ Eventually would be good to just destroy the LuaCallback without unregister if the state is not valid anymore.

• And now, yet another memory issue, this time with the framebuffers, hmmm… there is something not quite right here. ⇒ OK that was due to the fact that I was calling release() on RefPtr object in lua, and using reset() instead seems to be fixing the problem without any side effect so far (?):

  template <> struct LunaProvider<nv::RefObject> {
      using container_t = nv::RefPtr<nv::RefObject>;
  
      static auto get(const container_t& cont) -> nv::RefObject* {
          return cont.get();
      };
  
      static void set(container_t& cont, nv::RefObject* ptr) {
          logTRACE("Acquiring RefObject at {}", (const void*)ptr);
          cont = ptr;
      };
  
      static void release(container_t& cont) {
          logTRACE("Releasing RefObject at {}", (const void*)cont.get());
          // cont.release();
          cont.reset();
      };
  
      static void destruct(lua_State* L, container_t& cont) {
          logTRACE("Resetting RefObject at {}", (const void*)cont.get());
          cont.reset();
      };
  };

• After fixing all the issues and crashes mentioned above, here is the result we now have (spoiler alert: that's pretty unimpressive for sure 🤣):

• Now that the initial support for resizing the swapchainis ready, let's push this one step further and also update the graphics pipeline viewport to get the triangle to stretch to fit the window.

• First I added a couple of functions to get the swapchain width/height from the renderer (since the swapchain is mostly hidden inside the renderer now):

      /** Get the swapchain width **/
      auto get_swapchain_width() const -> U32;
  
      /** Get the swapchain height **/
      auto get_swapchain_height() const -> U32;

• And with that we can introduce the update of the viewport and reconstruction of the graphics pipeline just before using it:

      -- Retrieve he width/height of the swapchain:
      local width = self.renderer:get_swapchain_width()
      local height = self.renderer:get_swapchain_height()
  
      -- Now we should update the viewport dimensions in our graphics pipeline config:
      local vp = cfg:getCurrentViewportState()
      vp:setViewport(width, height)
  
      -- Remove the previous pipeline:
      if self.pipeline ~= nil then
          self.vkeng:remove_pipeline(self.pipeline)
      end
  
      -- And we rebuild the pipeline with that config:
      self.pipeline = self.vkeng:create_graphics_pipeline(cfg, self.pipelineCache)

• And now we can finally observe the stretching of the triangle as expected:

• Lastly, when connecting the event handler on SDL side I noticed there was also a SIZE_CHANGED event, which I think might be called while we are dragging the mouse: so let's try to use that one instead to get progressive updates on the swapchain dimensions… arrff, no, actually that's not it (cf. documentation at https://wiki.libsdl.org/SDL_WindowEventID).

• But anyway, let's just use the SIZE_CHANGED event for now which seems more generic.

• ⇒ I think that a good point to stop this session now as we have working support to resize the swapchain/window 👍!