From ba3c84168a0850a72721f1eb91461a585476bcb0 Mon Sep 17 00:00:00 2001 From: GPUCode Date: Mon, 3 Oct 2022 23:48:17 +0300 Subject: [PATCH] video_core: Small code improvements --- .../rasterizer_cache/rasterizer_cache.h | 110 ++++++++++-------- .../renderer_vulkan/renderer_vulkan.cpp | 12 +- .../renderer_vulkan/vk_rasterizer.cpp | 4 +- .../renderer_vulkan/vk_renderpass_cache.h | 10 +- 4 files changed, 76 insertions(+), 60 deletions(-) diff --git a/src/video_core/rasterizer_cache/rasterizer_cache.h b/src/video_core/rasterizer_cache/rasterizer_cache.h index ed592a839..71d78806f 100644 --- a/src/video_core/rasterizer_cache/rasterizer_cache.h +++ b/src/video_core/rasterizer_cache/rasterizer_cache.h @@ -191,13 +191,14 @@ auto RasterizerCache::FindMatch(const SurfaceCache& surface_cache, const Surf ? (params.res_scale == surface->res_scale) : (params.res_scale <= surface->res_scale); // validity will be checked in GetCopyableInterval - bool is_valid = + const bool is_valid = True(find_flags & MatchFlags::Copy) ? true : surface->IsRegionValid(validate_interval.value_or(params.GetInterval())); - if (False(find_flags & MatchFlags::Invalid) && !is_valid) + if (False(find_flags & MatchFlags::Invalid) && !is_valid) { continue; + } auto IsMatch_Helper = [&](auto check_type, auto match_fn) { if (False(find_flags & check_type)) @@ -270,14 +271,19 @@ bool RasterizerCache::BlitSurfaces(const Surface& src_surface, Common::Rectan const Surface& dst_surface, Common::Rectangle dst_rect) { MICROPROFILE_SCOPE(RasterizerCache_BlitSurface); - if (!CheckFormatsBlittable(src_surface->pixel_format, dst_surface->pixel_format)) [[unlikely]] { + if (!CheckFormatsBlittable(src_surface->pixel_format, + dst_surface->pixel_format)) [[unlikely]] { return false; } dst_surface->InvalidateAllWatcher(); - // Prefer texture copy over blit if possible - if (src_rect.GetWidth() == dst_rect.GetWidth() && src_rect.bottom < src_rect.top) { + // Prefer texture copy over blit if possible. This is possible when the following is true: + // 1. No scaling (the dimentions of src and dest rect are the same + // 2. No flipping (if the bottom value is bigger than the top this indicates texture flip + if (src_rect.GetWidth() == dst_rect.GetWidth() && + src_rect.GetHeight() == dst_rect.GetHeight() && + src_rect.bottom < src_rect.top) { const TextureCopy texture_copy = { .src_level = 0, .dst_level = 0, @@ -354,7 +360,7 @@ void RasterizerCache::CopySurface(const Surface& src_surface, const Surface& template auto RasterizerCache::GetSurface(const SurfaceParams& params, ScaleMatch match_res_scale, bool load_if_create) -> Surface { - if (params.addr == 0 || params.height * params.width == 0) { + if (params.addr == 0 || params.height * params.width == 0) [[unlikely]] { return nullptr; } @@ -366,7 +372,7 @@ auto RasterizerCache::GetSurface(const SurfaceParams& params, ScaleMatch matc Surface surface = FindMatch(surface_cache, params, match_res_scale); - if (surface == nullptr) { + if (!surface) { u16 target_res_scale = params.res_scale; if (match_res_scale != ScaleMatch::Exact) { // This surface may have a subrect of another surface with a higher res_scale, find @@ -374,15 +380,16 @@ auto RasterizerCache::GetSurface(const SurfaceParams& params, ScaleMatch matc SurfaceParams find_params = params; Surface expandable = FindMatch( surface_cache, find_params, match_res_scale); - if (expandable != nullptr && expandable->res_scale > target_res_scale) { + if (expandable && expandable->res_scale > target_res_scale) { target_res_scale = expandable->res_scale; } + // Keep res_scale when reinterpreting d24s8 -> rgba8 if (params.pixel_format == PixelFormat::RGBA8) { find_params.pixel_format = PixelFormat::D24S8; expandable = FindMatch( surface_cache, find_params, match_res_scale); - if (expandable != nullptr && expandable->res_scale > target_res_scale) { + if (expandable && expandable->res_scale > target_res_scale) { target_res_scale = expandable->res_scale; } } @@ -404,7 +411,7 @@ auto RasterizerCache::GetSurface(const SurfaceParams& params, ScaleMatch matc template auto RasterizerCache::GetSurfaceSubRect(const SurfaceParams& params, ScaleMatch match_res_scale, bool load_if_create) -> SurfaceRect_Tuple { - if (params.addr == 0 || params.height * params.width == 0) { + if (params.addr == 0 || params.height * params.width == 0) [[unlikely]] { return std::make_tuple(nullptr, Common::Rectangle{}); } @@ -416,10 +423,10 @@ auto RasterizerCache::GetSurfaceSubRect(const SurfaceParams& params, ScaleMat // If that's the case create a new surface with // the dimensions of the lower res_scale surface // to suggest it should not be used again - if (surface == nullptr && match_res_scale != ScaleMatch::Ignore) { + if (!surface && match_res_scale != ScaleMatch::Ignore) { surface = FindMatch(surface_cache, params, ScaleMatch::Ignore); - if (surface != nullptr) { + if (surface) { SurfaceParams new_params = *surface; new_params.res_scale = params.res_scale; @@ -437,10 +444,10 @@ auto RasterizerCache::GetSurfaceSubRect(const SurfaceParams& params, ScaleMat } // Check for a surface we can expand before creating a new one - if (surface == nullptr) { + if (!surface) { surface = FindMatch(surface_cache, aligned_params, match_res_scale); - if (surface != nullptr) { + if (surface) { aligned_params.width = aligned_params.stride; aligned_params.UpdateParams(); @@ -466,7 +473,7 @@ auto RasterizerCache::GetSurfaceSubRect(const SurfaceParams& params, ScaleMat } // No subrect found - create and return a new surface - if (surface == nullptr) { + if (!surface) { SurfaceParams new_params = aligned_params; // Can't have gaps in a surface new_params.width = aligned_params.stride; @@ -488,7 +495,7 @@ auto RasterizerCache::GetTextureSurface(const Pica::TexturingRegs::FullTextur template auto RasterizerCache::GetTextureSurface(const Pica::Texture::TextureInfo& info, u32 max_level) -> Surface { - if (info.physical_address == 0) { + if (info.physical_address == 0) [[unlikely]] { return nullptr; } @@ -501,34 +508,34 @@ auto RasterizerCache::GetTextureSurface(const Pica::Texture::TextureInfo& inf params.res_scale = /*texture_filterer->IsNull() ?*/ 1 /*: resolution_scale_factor*/; params.UpdateParams(); - u32 min_width = info.width >> max_level; - u32 min_height = info.height >> max_level; + const u32 min_width = info.width >> max_level; + const u32 min_height = info.height >> max_level; if (min_width % 8 != 0 || min_height % 8 != 0) { - LOG_CRITICAL(HW_GPU, "Texture size ({}x{}) is not multiple of 8", min_width, - min_height); + LOG_CRITICAL(HW_GPU, "Texture size ({}x{}) is not multiple of 8", min_width, min_height); return nullptr; } + if (info.width != (min_width << max_level) || info.height != (min_height << max_level)) { - LOG_CRITICAL(HW_GPU, - "Texture size ({}x{}) does not support required mipmap level ({})", + LOG_CRITICAL(HW_GPU, "Texture size ({}x{}) does not support required mipmap level ({})", params.width, params.height, max_level); return nullptr; } auto surface = GetSurface(params, ScaleMatch::Ignore, true); - if (!surface) + if (!surface) { return nullptr; + } // Update mipmap if necessary if (max_level != 0) { if (max_level >= 8) { - // since PICA only supports texture size between 8 and 1024, there are at most eight + // Since PICA only supports texture size between 8 and 1024, there are at most eight // possible mipmap levels including the base. LOG_CRITICAL(HW_GPU, "Unsupported mipmap level {}", max_level); return nullptr; } - // Allocate more mipmap level if necessary + // Allocate more mipmap levels if necessary if (surface->max_level < max_level) { /*if (!texture_filterer->IsNull()) { // TODO: proper mipmap support for custom textures @@ -625,12 +632,9 @@ auto RasterizerCache::GetTextureCube(const TextureCubeConfig& config) -> cons if (surface) { watcher = surface->CreateWatcher(); } else { - /** - * Can occur when texture address is invalid. We mark the watcher with nullptr - * in this case and the content of the face wouldn't get updated. These are - * usually leftover setup in the texture unit and games are not supposed to draw - * using them. - */ + // Can occur when texture address is invalid. We mark the watcher with nullptr + // in this case and the content of the face wouldn't get updated. These are usually + // leftover setup in the texture unit and games are not supposed to draw using them. watcher = nullptr; } } @@ -678,11 +682,13 @@ auto RasterizerCache::GetFramebufferSurfaces(bool using_color_fb, bool using_ texture_filterer->Reset(Settings::values.texture_filter_name, VideoCore::GetResolutionScaleFactor())*/false; - if (resolution_scale_changed || texture_filter_changed) { + if (resolution_scale_changed || texture_filter_changed) [[unlikely]] { resolution_scale_factor = VideoCore::GetResolutionScaleFactor(); FlushAll(); - while (!surface_cache.empty()) + while (!surface_cache.empty()) { UnregisterSurface(*surface_cache.begin()->second.begin()); + } + texture_cube_cache.clear(); } @@ -793,7 +799,7 @@ auto RasterizerCache::GetTexCopySurface(const SurfaceParams& params) -> Surfa Surface match_surface = FindMatch( surface_cache, params, ScaleMatch::Ignore); - if (match_surface != nullptr) { + if (match_surface) { ValidateSurface(match_surface, params.addr, params.size); SurfaceParams match_subrect; @@ -830,6 +836,7 @@ void RasterizerCache::DuplicateSurface(const Surface& src_surface, const Surf regions += pair.first; } } + for (const auto& interval : regions) { dirty_regions.set({interval, dest_surface}); } @@ -849,7 +856,8 @@ void RasterizerCache::ValidateSurface(const Surface& surface, PAddr addr, u32 } auto validate_regions = surface->invalid_regions & validate_interval; - auto NotifyValidated = [&](SurfaceInterval interval) { + + const auto NotifyValidated = [&](SurfaceInterval interval) { surface->invalid_regions.erase(interval); validate_regions.erase(interval); }; @@ -860,8 +868,8 @@ void RasterizerCache::ValidateSurface(const Surface& surface, PAddr addr, u32 break; } - const auto interval = *it & validate_interval; // Look for a valid surface to copy from + const auto interval = *it & validate_interval; SurfaceParams params = surface->FromInterval(interval); Surface copy_surface = @@ -954,13 +962,14 @@ void RasterizerCache::DownloadSurface(const Surface& surface, SurfaceInterval surface->Download(download, staging); - MemoryRef dest_ptr = VideoCore::g_memory->GetPhysicalRef(flush_start); - if (!dest_ptr) [[unlikely]] { - return; - } + download_queue.push_back([this, surface, flush_start, flush_end, flush_info, mapped = staging.mapped]() { + MemoryRef dest_ptr = VideoCore::g_memory->GetPhysicalRef(flush_start); + if (!dest_ptr) [[unlikely]] { + return; + } + + const auto download_dest = dest_ptr.GetWriteBytes(flush_end - flush_start); - const auto download_dest = dest_ptr.GetWriteBytes(flush_end - flush_start); - download_queue.push_back([this, surface, download_dest, flush_start, flush_end, flush_info, mapped = staging.mapped]() { if (surface->is_tiled) { std::vector temp_data(flush_info.width * flush_info.height * GetBytesPerPixel(flush_info.pixel_format)); runtime.FormatConvert(*surface, false, mapped, temp_data); @@ -1017,11 +1026,11 @@ bool RasterizerCache::NoUnimplementedReinterpretations(const Surface& surface for (PixelFormat format : all_formats) { if (GetFormatBpp(format) == surface->GetFormatBpp()) { params.pixel_format = format; - // This could potentially be expensive, - // although experimentally it hasn't been too bad + // This could potentially be expensive, although experimentally it hasn't been too bad Surface test_surface = FindMatch(surface_cache, params, ScaleMatch::Ignore, interval); - if (test_surface != nullptr) { + + if (test_surface) { LOG_WARNING(HW_GPU, "Missing pixel_format reinterpreter: {} -> {}", PixelFormatAsString(format), PixelFormatAsString(surface->pixel_format)); @@ -1029,6 +1038,7 @@ bool RasterizerCache::NoUnimplementedReinterpretations(const Surface& surface } } } + return implemented; } @@ -1149,7 +1159,7 @@ void RasterizerCache::FlushRegion(PAddr addr, u32 size, Surface flush_surface // before we issue the CPU to GPU flush and reduces scheduler slot switches in Vulkan if (!download_queue.empty()) { runtime.Finish(); - for (auto& download_func : download_queue) { + for (const auto& download_func : download_queue) { download_func(); } @@ -1184,12 +1194,13 @@ void RasterizerCache::InvalidateRegion(PAddr addr, u32 size, const Surface& r for (const auto& pair : RangeFromInterval(surface_cache, invalid_interval)) { for (const auto& cached_surface : pair.second) { - if (cached_surface == region_owner) + if (cached_surface == region_owner) { continue; + } // If cpu is invalidating this region we want to remove it // to (likely) mark the memory pages as uncached - if (region_owner == nullptr && size <= 8) { + if (!region_owner && size <= 8) { FlushRegion(cached_surface->addr, cached_surface->size, cached_surface); remove_surfaces.emplace(cached_surface); continue; @@ -1234,7 +1245,6 @@ template auto RasterizerCache::CreateSurface(SurfaceParams& params) -> Surface { Surface surface = std::make_shared(params, runtime); surface->invalid_regions.insert(surface->GetInterval()); - return surface; } @@ -1245,6 +1255,7 @@ void RasterizerCache::RegisterSurface(const Surface& surface) { if (surface->registered) { return; } + surface->registered = true; surface_cache.add({surface->GetInterval(), SurfaceSet{surface}}); rasterizer.UpdatePagesCachedCount(surface->addr, surface->size, 1); @@ -1257,6 +1268,7 @@ void RasterizerCache::UnregisterSurface(const Surface& surface) { if (!surface->registered) { return; } + surface->registered = false; rasterizer.UpdatePagesCachedCount(surface->addr, surface->size, -1); surface_cache.subtract({surface->GetInterval(), SurfaceSet{surface}}); diff --git a/src/video_core/renderer_vulkan/renderer_vulkan.cpp b/src/video_core/renderer_vulkan/renderer_vulkan.cpp index 18b2a2e26..4c9b44b9e 100644 --- a/src/video_core/renderer_vulkan/renderer_vulkan.cpp +++ b/src/video_core/renderer_vulkan/renderer_vulkan.cpp @@ -341,19 +341,18 @@ void RendererVulkan::BeginRendering() { .color = clear_color }; - const auto& layout = render_window.GetFramebufferLayout(); const vk::RenderPassBeginInfo begin_info = { .renderPass = renderpass_cache.GetPresentRenderpass(), .framebuffer = swapchain.GetFramebuffer(), .renderArea = vk::Rect2D{ .offset = {0, 0}, - .extent = {layout.width, layout.height} + .extent = swapchain.GetExtent() }, .clearValueCount = 1, .pClearValues = &clear_value, }; - command_buffer.beginRenderPass(begin_info, vk::SubpassContents::eInline); + renderpass_cache.EnterRenderpass(begin_info); } void RendererVulkan::LoadFBToScreenInfo(const GPU::Regs::FramebufferConfig& framebuffer, @@ -756,7 +755,6 @@ void RendererVulkan::DrawSingleScreen(u32 screen_id, float x, float y, float w, command_buffer.bindVertexBuffers(0, vertex_buffer.GetHandle(), {0}); command_buffer.draw(4, 1, offset / sizeof(ScreenRectVertex), 0); - command_buffer.endRenderPass(); } void RendererVulkan::DrawSingleScreenStereoRotated(u32 screen_id_l, u32 screen_id_r, @@ -993,8 +991,7 @@ void RendererVulkan::DrawScreens(const Layout::FramebufferLayout& layout, bool f } } - vk::CommandBuffer command_buffer = scheduler.GetRenderCommandBuffer(); - command_buffer.endRenderPass(); + renderpass_cache.ExitRenderpass(); } void RendererVulkan::SwapBuffers() { @@ -1029,6 +1026,8 @@ void RendererVulkan::SwapBuffers() { command_buffer.setViewport(0, viewport); command_buffer.setScissor(0, scissor); + renderpass_cache.ExitRenderpass(); + for (auto& info : screen_infos) { auto alloc = info.display_texture ? info.display_texture : &info.texture.alloc; runtime.Transition(command_buffer, *alloc, vk::ImageLayout::eShaderReadOnlyOptimal, 0, alloc->levels); @@ -1053,6 +1052,7 @@ void RendererVulkan::OnSlotSwitch() { } runtime.OnSlotSwitch(scheduler.GetCurrentSlotIndex()); + renderpass_cache.OnSlotSwitch(); rasterizer->pipeline_cache.MarkDirty(); } diff --git a/src/video_core/renderer_vulkan/vk_rasterizer.cpp b/src/video_core/renderer_vulkan/vk_rasterizer.cpp index 9a3acf8fd..5e63503cd 100644 --- a/src/video_core/renderer_vulkan/vk_rasterizer.cpp +++ b/src/video_core/renderer_vulkan/vk_rasterizer.cpp @@ -680,6 +680,8 @@ bool RasterizerVulkan::Draw(bool accelerate, bool is_indexed) { } }; + renderpass_cache.ExitRenderpass(); + // Sync and bind the texture surfaces const auto pica_textures = regs.texturing.GetTextures(); for (unsigned texture_index = 0; texture_index < pica_textures.size(); ++texture_index) { @@ -868,8 +870,6 @@ bool RasterizerVulkan::Draw(bool accelerate, bool is_indexed) { } } - renderpass_cache.ExitRenderpass(); - vertex_batch.clear(); // Mark framebuffer surfaces as dirty diff --git a/src/video_core/renderer_vulkan/vk_renderpass_cache.h b/src/video_core/renderer_vulkan/vk_renderpass_cache.h index 1db3aa94a..37aaa83d1 100644 --- a/src/video_core/renderer_vulkan/vk_renderpass_cache.h +++ b/src/video_core/renderer_vulkan/vk_renderpass_cache.h @@ -26,17 +26,21 @@ public: /// Exits from any currently active renderpass instance void ExitRenderpass(); + /// Creates the renderpass used when rendering to the swapchain + void CreatePresentRenderpass(vk::Format format); + /// Returns the renderpass associated with the color-depth format pair [[nodiscard]] vk::RenderPass GetRenderpass(VideoCore::PixelFormat color, VideoCore::PixelFormat depth, bool is_clear) const; - /// Returns the swapchain clear renderpass [[nodiscard]] vk::RenderPass GetPresentRenderpass() const { return present_renderpass; } - /// Creates the renderpass used when rendering to the swapchain - void CreatePresentRenderpass(vk::Format format); + /// Invalidates the currently active renderpass + void OnSlotSwitch() { + active_renderpass = VK_NULL_HANDLE; + } private: /// Creates a renderpass configured appropriately and stores it in cached_renderpasses