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

rsx: Fix a few texture data leaks #7314

Merged
merged 6 commits into from
Jan 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
101 changes: 69 additions & 32 deletions rpcs3/Emu/RSX/Common/texture_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -213,16 +213,17 @@ namespace rsx
}
}

bool atlas_covers_target_area() const
// Returns true if at least threshold% is covered in pixels
bool atlas_covers_target_area(u32 threshold) const
{
if (external_subresource_desc.op != deferred_request_command::atlas_gather)
return true;

u16 min_x = external_subresource_desc.width, min_y = external_subresource_desc.height,
max_x = 0, max_y = 0;

// Require at least 50% coverage
const u32 target_area = (min_x * min_y) / 2;
// Require at least 90% coverage
const u32 target_area = ((min_x * min_y) * threshold) / 100u;

for (const auto &section : external_subresource_desc.sections_to_copy)
{
Expand Down Expand Up @@ -1553,6 +1554,14 @@ namespace rsx
{
if (cached_texture->matches(attr.address, attr.gcm_format, attr.width, attr.height, attr.depth, 0))
{
#ifdef TEXTURE_CACHE_DEBUG
if (!memory_range.inside(cached_texture->get_confirmed_range()))
{
// TODO. This is easily possible for blit_dst textures if the blit is incomplete in Y
// The possibility that a texture will be split into parts on the CPU like this is very rare
continue;
}
#endif
return{ cached_texture->get_view(encoded_remap, remap), cached_texture->get_context(), cached_texture->get_format_type(), scale, cached_texture->get_image_type() };
}
}
Expand Down Expand Up @@ -1646,32 +1655,37 @@ namespace rsx
return {};
}

if (!result.external_subresource_desc.sections_to_copy.empty() &&
(_pool == 0 || result.atlas_covers_target_area()))
if (const auto section_count = result.external_subresource_desc.sections_to_copy.size();
section_count > 0)
{
// TODO: Overlapped section persistance is required for framebuffer resources to work with this!
// Yellow filter in SCV is because of a 384x384 surface being reused as 160x90 (and likely not getting written to)
// Its then sampled again here as 384x384 and this does not work! (obviously)
// TODO: Some games may render a small region (e.g 1024x256x2) and sample a huge texture (e.g 1024x1024).
// Seen in APF2k8 - this causes missing bits to be reuploaded from CPU which can cause WCB requirement.
// Properly fix this by introducing partial data upload into the surface cache in such cases and making RCB/RDB
// enabled by default. Blit engine already handles this correctly.

// Optionally disallow caching if resource is being written to as it is being read from
for (const auto& section : overlapping_fbos)
if (_pool == 0 || /* Hack to avoid WCB requirement for some games with wrongly declared sampler dimensions */
result.atlas_covers_target_area(section_count == 1? 99 : 90))
{
if (m_rtts.address_is_bound(section.base_address))
// Optionally disallow caching if resource is being written to as it is being read from
for (const auto& section : overlapping_fbos)
{
if (result.external_subresource_desc.op == deferred_request_command::copy_image_static)
if (m_rtts.address_is_bound(section.base_address))
{
result.external_subresource_desc.op = deferred_request_command::copy_image_dynamic;
if (result.external_subresource_desc.op == deferred_request_command::copy_image_static)
{
result.external_subresource_desc.op = deferred_request_command::copy_image_dynamic;
}
else
{
result.external_subresource_desc.do_not_cache = true;
}

break;
}
else
{
result.external_subresource_desc.do_not_cache = true;
}

break;
}
}

return result;
return result;
}
}
}
}
Expand Down Expand Up @@ -1913,6 +1927,7 @@ namespace rsx

const bool is_copy_op = (fcmp(scale_x, 1.f) && fcmp(scale_y, 1.f));
const bool is_format_convert = (dst_is_argb8 != src_is_argb8);
bool skip_if_collision_exists = false;

// Offset in x and y for src is 0 (it is already accounted for when getting pixels_src)
// Reproject final clip onto source...
Expand Down Expand Up @@ -2067,6 +2082,9 @@ namespace rsx
return false;
}
}

// If a matching section exists with a different use-case, fall back to CPU memcpy
skip_if_collision_exists = true;
}

if (!g_cfg.video.use_gpu_texture_scaling)
Expand Down Expand Up @@ -2168,9 +2186,20 @@ namespace rsx
if (!dst_is_render_target)
{
// Check for any available region that will fit this one
const auto required_type = (use_null_region) ? texture_upload_context::dma : texture_upload_context::blit_engine_dst;
u32 required_type_mask;
if (use_null_region)
{
required_type_mask = texture_upload_context::dma;
}
else
{
required_type_mask = texture_upload_context::blit_engine_dst;
if (skip_if_collision_exists) required_type_mask |= texture_upload_context::shader_read;
}

const auto dst_range = address_range::start_length(dst_address, dst_payload_length);
auto overlapping_surfaces = find_texture_from_range(dst_range, dst.pitch, required_type);
auto overlapping_surfaces = find_texture_from_range(dst_range, dst.pitch, required_type_mask);

for (const auto &surface : overlapping_surfaces)
{
if (!surface->is_locked())
Expand All @@ -2186,23 +2215,30 @@ namespace rsx
continue;
}

if (!dst_range.inside(surface->get_section_range()))
{
// Hit test failed
continue;
}

if (use_null_region)
{
if (dst_range.inside(surface->get_section_range()))
{
// Attach to existing region
cached_dest = surface;
}

// Attach to existing region
cached_dest = surface;

// Technically it is totally possible to just extend a pre-existing section
// Will leave this as a TODO
continue;
}

const auto this_address = surface->get_section_base();
if (this_address > dst_address)
if (UNLIKELY(skip_if_collision_exists))
{
continue;
if (surface->get_context() != texture_upload_context::blit_engine_dst)
{
// This section is likely to be 'flushed' to CPU for reupload soon anyway
return false;
}
}

switch (surface->get_gcm_format())
Expand All @@ -2219,7 +2255,8 @@ namespace rsx
continue;
}

if (const u32 address_offset = dst_address - this_address)
if (const auto this_address = surface->get_section_base();
const u32 address_offset = dst_address - this_address)
{
const u16 offset_y = address_offset / dst.pitch;
const u16 offset_x = address_offset % dst.pitch;
Expand Down
8 changes: 8 additions & 0 deletions rpcs3/Emu/RSX/VK/VKGSRender.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1592,6 +1592,10 @@ void VKGSRender::end()
verify(HERE), sampler_state->upload_context == rsx::texture_upload_context::blit_engine_dst;
raw->change_layout(*m_current_command_buffer, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL);
break;
case VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL:
verify(HERE), sampler_state->upload_context == rsx::texture_upload_context::blit_engine_src;
raw->change_layout(*m_current_command_buffer, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL);
break;
case VK_IMAGE_LAYOUT_GENERAL:
verify(HERE), sampler_state->upload_context == rsx::texture_upload_context::framebuffer_storage;
if (!sampler_state->is_cyclic_reference)
Expand Down Expand Up @@ -1724,6 +1728,10 @@ void VKGSRender::end()
verify(HERE), sampler_state->upload_context == rsx::texture_upload_context::blit_engine_dst;
raw->change_layout(*m_current_command_buffer, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL);
break;
case VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL:
verify(HERE), sampler_state->upload_context == rsx::texture_upload_context::blit_engine_src;
raw->change_layout(*m_current_command_buffer, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL);
break;
case VK_IMAGE_LAYOUT_GENERAL:
verify(HERE), sampler_state->upload_context == rsx::texture_upload_context::framebuffer_storage;
if (!sampler_state->is_cyclic_reference)
Expand Down
37 changes: 36 additions & 1 deletion rpcs3/Emu/RSX/VK/VKTextureCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,14 @@ namespace vk
}
else
{
if (context != rsx::texture_upload_context::dma)
{
// Partial load for the bits outside the existing image
// NOTE: A true DMA section would have been prepped beforehand
// TODO: Parial range load/flush
vk::load_dma(valid_range.start, section_length);
}

std::vector<VkBufferCopy> copy;
copy.reserve(transfer_height);

Expand Down Expand Up @@ -1269,7 +1277,34 @@ namespace vk

vk::leave_uninterruptible();

change_image_layout(cmd, image, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL, subres_range);
// Insert appropriate barrier depending on use
VkImageLayout preferred_layout;
switch (context)
{
default:
preferred_layout = VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL;
break;
case rsx::texture_upload_context::blit_engine_dst:
preferred_layout = VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL;
break;
case rsx::texture_upload_context::blit_engine_src:
preferred_layout = VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL;
break;
}

if (preferred_layout != image->current_layout)
{
change_image_layout(cmd, image, preferred_layout, subres_range);
}
else
{
// Insert ordering barrier
verify(HERE), preferred_layout == VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL;
insert_image_memory_barrier(cmd, image->value, image->current_layout, preferred_layout,
VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT,
VK_ACCESS_TRANSFER_WRITE_BIT, VK_ACCESS_TRANSFER_WRITE_BIT,
subres_range);
}

section->last_write_tag = rsx::get_shared_tag();
return section;
Expand Down