Move trasnfer_framebuffer to a member of RasterCache. Address review comments

src/core/memory.cpp

@@ -404,61 +404,69 @@ void RasterizerMarkRegionCached(PAddr start, u32 size, bool cached) {
 }
 
 void RasterizerFlushRegion(PAddr start, u32 size) {
-    if (VideoCore::g_renderer != nullptr) {
+    if (VideoCore::g_renderer == nullptr) {
-        VideoCore::g_renderer->Rasterizer()->FlushRegion(start, size);
+        return;
     }
+
+    VideoCore::g_renderer->Rasterizer()->FlushRegion(start, size);
 }
 
 void RasterizerInvalidateRegion(PAddr start, u32 size) {
-    if (VideoCore::g_renderer != nullptr) {
+    if (VideoCore::g_renderer == nullptr) {
-        VideoCore::g_renderer->Rasterizer()->InvalidateRegion(start, size);
+        return;
     }
+
+    VideoCore::g_renderer->Rasterizer()->InvalidateRegion(start, size);
 }
 
 void RasterizerFlushAndInvalidateRegion(PAddr start, u32 size) {
     // Since pages are unmapped on shutdown after video core is shutdown, the renderer may be
     // null here
-    if (VideoCore::g_renderer != nullptr) {
+    if (VideoCore::g_renderer == nullptr) {
-        VideoCore::g_renderer->Rasterizer()->FlushAndInvalidateRegion(start, size);
+        return;
     }
+
+    VideoCore::g_renderer->Rasterizer()->FlushAndInvalidateRegion(start, size);
 }
 
 void RasterizerFlushVirtualRegion(VAddr start, u32 size, FlushMode mode) {
     // Since pages are unmapped on shutdown after video core is shutdown, the renderer may be
     // null here
-    if (VideoCore::g_renderer != nullptr) {
+    if (VideoCore::g_renderer == nullptr) {
-        VAddr end = start + size;
+        return;
-
-        auto CheckRegion = [&](VAddr region_start, VAddr region_end) {
-            if (start >= region_end || end <= region_start) {
-                // No overlap with region
-                return;
-            }
-
-            VAddr overlap_start = std::max(start, region_start);
-            VAddr overlap_end = std::min(end, region_end);
-
-            PAddr physical_start = TryVirtualToPhysicalAddress(overlap_start).value();
-            u32 overlap_size = overlap_end - overlap_start;
-
-            auto* rasterizer = VideoCore::g_renderer->Rasterizer();
-            switch (mode) {
-            case FlushMode::Flush:
-                rasterizer->FlushRegion(physical_start, overlap_size);
-                break;
-            case FlushMode::Invalidate:
-                rasterizer->InvalidateRegion(physical_start, overlap_size);
-                break;
-            case FlushMode::FlushAndInvalidate:
-                rasterizer->FlushAndInvalidateRegion(physical_start, overlap_size);
-                break;
-            }
-        };
-
-        CheckRegion(LINEAR_HEAP_VADDR, LINEAR_HEAP_VADDR_END);
-        CheckRegion(NEW_LINEAR_HEAP_VADDR, NEW_LINEAR_HEAP_VADDR_END);
-        CheckRegion(VRAM_VADDR, VRAM_VADDR_END);
     }
+
+    VAddr end = start + size;
+
+    auto CheckRegion = [&](VAddr region_start, VAddr region_end) {
+        if (start >= region_end || end <= region_start) {
+            // No overlap with region
+            return;
+        }
+
+        VAddr overlap_start = std::max(start, region_start);
+        VAddr overlap_end = std::min(end, region_end);
+
+        PAddr physical_start = TryVirtualToPhysicalAddress(overlap_start).value();
+        u32 overlap_size = overlap_end - overlap_start;
+
+        auto* rasterizer = VideoCore::g_renderer->Rasterizer();
+        switch (mode) {
+        case FlushMode::Flush:
+            rasterizer->FlushRegion(physical_start, overlap_size);
+            break;
+        case FlushMode::Invalidate:
+            rasterizer->InvalidateRegion(physical_start, overlap_size);
+            break;
+        case FlushMode::FlushAndInvalidate:
+            rasterizer->FlushAndInvalidateRegion(physical_start, overlap_size);
+            break;
+        }
+    };
+
+    CheckRegion(LINEAR_HEAP_VADDR, LINEAR_HEAP_VADDR_END);
+    CheckRegion(NEW_LINEAR_HEAP_VADDR, NEW_LINEAR_HEAP_VADDR_END);
+    CheckRegion(VRAM_VADDR, VRAM_VADDR_END);
 }
 
 u8 Read8(const VAddr addr) {

src/video_core/renderer_opengl/gl_rasterizer.cpp

@@ -1150,8 +1150,8 @@ void RasterizerOpenGL::SamplerInfo::Create() {
     wrap_s = wrap_t = TextureConfig::Repeat;
     border_color = 0;
 
-    glSamplerParameteri(sampler.handle, GL_TEXTURE_MIN_FILTER,
+    // default is GL_LINEAR_MIPMAP_LINEAR
-                        GL_LINEAR); // default is GL_LINEAR_MIPMAP_LINEAR
+    glSamplerParameteri(sampler.handle, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
     // Other attributes have correct defaults
 }
 

src/video_core/renderer_opengl/gl_rasterizer_cache.cpp

@@ -34,8 +34,6 @@
 using SurfaceType = SurfaceParams::SurfaceType;
 using PixelFormat = SurfaceParams::PixelFormat;
 
-static std::array<OGLFramebuffer, 2> transfer_framebuffers;
-
 struct FormatTuple {
     GLint internal_format;
     GLenum format;
@@ -153,7 +151,7 @@ static void MortonCopy(u32 stride, u32 height, u8* gl_buffer, PAddr base, PAddr
         glbuf_next_tile();
     }
 
-    u8* const buffer_end = tile_buffer + aligned_end - aligned_start;
+    const u8* const buffer_end = tile_buffer + aligned_end - aligned_start;
     while (tile_buffer < buffer_end) {
         MortonCopyTile<morton_to_gl, format>(stride, tile_buffer, gl_buffer);
         tile_buffer += tile_size;
@@ -234,7 +232,8 @@ static void AllocateSurfaceTexture(GLuint texture, const FormatTuple& format_tup
 }
 
 static bool BlitTextures(GLuint src_tex, const MathUtil::Rectangle<u32>& src_rect, GLuint dst_tex,
-                         const MathUtil::Rectangle<u32>& dst_rect, SurfaceType type) {
+                         const MathUtil::Rectangle<u32>& dst_rect, SurfaceType type,
+                         GLuint read_handle, GLuint draw_handle) {
     OpenGLState state = OpenGLState::GetCurState();
 
     OpenGLState prev_state = state;
@@ -246,8 +245,8 @@ static bool BlitTextures(GLuint src_tex, const MathUtil::Rectangle<u32>& src_rec
     state.ResetTexture(dst_tex);
 
     // Keep track of previous framebuffer bindings
-    state.draw.read_framebuffer = transfer_framebuffers[0].handle;
+    state.draw.read_framebuffer = read_handle;
-    state.draw.draw_framebuffer = transfer_framebuffers[1].handle;
+    state.draw.draw_framebuffer = draw_handle;
     state.Apply();
 
     u32 buffers = 0;
@@ -294,7 +293,7 @@ static bool BlitTextures(GLuint src_tex, const MathUtil::Rectangle<u32>& src_rec
 }
 
 static bool FillSurface(const Surface& surface, const u8* fill_data,
-                        const MathUtil::Rectangle<u32>& fill_rect) {
+                        const MathUtil::Rectangle<u32>& fill_rect, GLuint draw_handle) {
     OpenGLState state = OpenGLState::GetCurState();
 
     OpenGLState prev_state = state;
@@ -308,7 +307,7 @@ static bool FillSurface(const Surface& surface, const u8* fill_data,
     state.scissor.width = static_cast<GLsizei>(fill_rect.GetWidth());
     state.scissor.height = static_cast<GLsizei>(fill_rect.GetHeight());
 
-    state.draw.draw_framebuffer = transfer_framebuffers[1].handle;
+    state.draw.draw_framebuffer = draw_handle;
     state.Apply();
 
     if (surface->type == SurfaceType::Color || surface->type == SurfaceType::Texture) {
@@ -610,13 +609,14 @@ void RasterizerCacheOpenGL::CopySurface(const Surface& src_surface, const Surfac
         for (int i : {0, 1, 2, 3})
             fill_buffer[i] = src_surface->fill_data[fill_buff_pos++ % src_surface->fill_size];
 
-        FillSurface(dst_surface, &fill_buffer[0], dst_surface->GetScaledSubRect(subrect_params));
+        FillSurface(dst_surface, &fill_buffer[0], dst_surface->GetScaledSubRect(subrect_params),
+                    draw_framebuffer.handle);
         return;
     }
     if (src_surface->CanSubRect(subrect_params)) {
         BlitTextures(src_surface->texture.handle, src_surface->GetScaledSubRect(subrect_params),
                      dst_surface->texture.handle, dst_surface->GetScaledSubRect(subrect_params),
-                     src_surface->type);
+                     src_surface->type, read_framebuffer.handle, draw_framebuffer.handle);
         return;
     }
     UNREACHABLE();
@@ -777,7 +777,7 @@ void CachedSurface::UploadGLTexture(const MathUtil::Rectangle<u32>& rect) {
         scaled_rect.bottom *= res_scale;
 
         BlitTextures(unscaled_tex.handle, {0, rect.GetHeight(), rect.GetWidth(), 0}, texture.handle,
-                     scaled_rect, type);
+                     scaled_rect, type, read_framebuffer_handle, draw_framebuffer_handle);
     }
 }
 
@@ -814,7 +814,8 @@ void CachedSurface::DownloadGLTexture(const MathUtil::Rectangle<u32>& rect) {
 
         MathUtil::Rectangle<u32> unscaled_tex_rect{0, rect.GetHeight(), rect.GetWidth(), 0};
         AllocateSurfaceTexture(unscaled_tex.handle, tuple, rect.GetWidth(), rect.GetHeight());
-        BlitTextures(texture.handle, scaled_rect, unscaled_tex.handle, unscaled_tex_rect, type);
+        BlitTextures(texture.handle, scaled_rect, unscaled_tex.handle, unscaled_tex_rect, type,
+                     read_framebuffer_handle, draw_framebuffer_handle);
 
         state.texture_units[0].texture_2d = unscaled_tex.handle;
         state.Apply();
@@ -823,7 +824,7 @@ void CachedSurface::DownloadGLTexture(const MathUtil::Rectangle<u32>& rect) {
         glGetTexImage(GL_TEXTURE_2D, 0, tuple.format, tuple.type, &gl_buffer[buffer_offset]);
     } else {
         state.ResetTexture(texture.handle);
-        state.draw.read_framebuffer = transfer_framebuffers[0].handle;
+        state.draw.read_framebuffer = read_framebuffer_handle;
         state.Apply();
 
         if (type == SurfaceType::Color || type == SurfaceType::Texture) {
@@ -952,16 +953,16 @@ Surface FindMatch(const SurfaceCache& surface_cache, const SurfaceParams& params
 }
 
 RasterizerCacheOpenGL::RasterizerCacheOpenGL() {
-    transfer_framebuffers[0].Create();
+    read_framebuffer.Create();
-    transfer_framebuffers[1].Create();
+    draw_framebuffer.Create();
 }
 
 RasterizerCacheOpenGL::~RasterizerCacheOpenGL() {
     FlushAll();
     while (!surface_cache.empty())
         UnregisterSurface(*surface_cache.begin()->second.begin());
-    transfer_framebuffers[0].Release();
+    read_framebuffer.Release();
-    transfer_framebuffers[1].Release();
+    draw_framebuffer.Release();
 }
 
 bool RasterizerCacheOpenGL::BlitSurfaces(const Surface& src_surface,
@@ -972,7 +973,8 @@ bool RasterizerCacheOpenGL::BlitSurfaces(const Surface& src_surface,
         return false;
 
     return BlitTextures(src_surface->texture.handle, src_rect, dst_surface->texture.handle,
-                        dst_rect, src_surface->type);
+                        dst_rect, src_surface->type, read_framebuffer.handle,
+                        draw_framebuffer.handle);
 }
 
 Surface RasterizerCacheOpenGL::GetSurface(const SurfaceParams& params, ScaleMatch match_res_scale,
@@ -1203,6 +1205,9 @@ Surface RasterizerCacheOpenGL::GetFillSurface(const GPU::Regs::MemoryFillConfig&
     new_surface->size = new_surface->end - new_surface->addr;
     new_surface->type = SurfaceType::Fill;
     new_surface->res_scale = std::numeric_limits<u16>::max();
+    new_surface->read_framebuffer_handle = read_framebuffer.handle;
+    new_surface->draw_framebuffer_handle = draw_framebuffer.handle;
+
     std::memcpy(&new_surface->fill_data[0], &config.value_32bit, 4);
     if (config.fill_32bit) {
         new_surface->fill_size = 4;
@@ -1274,7 +1279,7 @@ void RasterizerCacheOpenGL::ValidateSurface(const Surface& surface, PAddr addr,
         return;
     }
 
-    for (;;) {
+    while (true) {
         const auto it = surface->invalid_regions.find(validate_interval);
         if (it == surface->invalid_regions.end())
             break;
@@ -1309,6 +1314,8 @@ void RasterizerCacheOpenGL::FlushRegion(PAddr addr, u32 size, Surface flush_surf
 
     for (auto& pair : RangeFromInterval(dirty_regions, flush_interval)) {
         // small sizes imply that this most likely comes from the cpu, flush the entire region
+        // the point is to avoid thousands of small writes every frame if the cpu decides to access
+        // that region, anything higher than 8 you're guaranteed it comes from a service
         const auto interval = size <= 8 ? pair.first : pair.first & flush_interval;
         auto& surface = pair.second;
 
@@ -1397,6 +1404,8 @@ void RasterizerCacheOpenGL::InvalidateRegion(PAddr addr, u32 size, const Surface
 Surface RasterizerCacheOpenGL::CreateSurface(const SurfaceParams& params) {
     Surface surface = std::make_shared<CachedSurface>();
     static_cast<SurfaceParams&>(*surface) = params;
+    surface->read_framebuffer_handle = read_framebuffer.handle;
+    surface->draw_framebuffer_handle = draw_framebuffer.handle;
 
     surface->texture.Create();
 

src/video_core/renderer_opengl/gl_rasterizer_cache.h

@@ -224,11 +224,11 @@ struct SurfaceParams {
     }
 
     u32 PixelsInBytes(u32 size) const {
-        return size * 8 / GetFormatBpp(pixel_format);
+        return size * CHAR_BIT / GetFormatBpp(pixel_format);
     }
 
     u32 BytesInPixels(u32 pixels) const {
-        return pixels * GetFormatBpp(pixel_format) / 8;
+        return pixels * GetFormatBpp(pixel_format) / CHAR_BIT;
     }
 
     bool ExactMatch(const SurfaceParams& other_surface) const;
@@ -284,6 +284,9 @@ struct CachedSurface : SurfaceParams {
     std::unique_ptr<u8[]> gl_buffer;
     size_t gl_buffer_size = 0;
 
+    GLuint read_framebuffer_handle;
+    GLuint draw_framebuffer_handle;
+
     // Read/Write data in 3DS memory to/from gl_buffer
     void LoadGLBuffer(PAddr load_start, PAddr load_end);
     void FlushGLBuffer(PAddr flush_start, PAddr flush_end);
@@ -359,4 +362,6 @@ private:
     SurfaceMap dirty_regions;
     PageMap cached_pages;
     SurfaceSet remove_surfaces;
+    OGLFramebuffer read_framebuffer;
+    OGLFramebuffer draw_framebuffer;
 };