Skip to content

Fixed false-positive image reuploads #1557

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

Merged
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/video_core/buffer_cache/buffer_cache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ bool BufferCache::SynchronizeBufferFromImage(Buffer& buffer, VAddr device_addr,
"Texel buffer aliases image subresources {:x} : {:x}", device_addr,
image.info.guest_address);
boost::container::small_vector<vk::BufferImageCopy, 8> copies;
u32 offset = buffer.Offset(image.cpu_addr);
u32 offset = buffer.Offset(image.info.guest_address);
const u32 num_layers = image.info.resources.layers;
const u32 max_offset = offset + size;
for (u32 m = 0; m < image.info.resources.levels; m++) {
Expand Down
16 changes: 12 additions & 4 deletions src/video_core/page_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ struct PageManager::Impl {

// Notify rasterizer about the fault.
const VAddr addr = msg.arg.pagefault.address;
const VAddr addr_page = Common::AlignDown(addr, PAGESIZE);
rasterizer->InvalidateMemory(addr_page, PAGESIZE);
const VAddr addr_page = GetPageAddr(addr);
rasterizer->InvalidateMemory(addr, addr_page, PAGESIZE);
}
}

Expand Down Expand Up @@ -157,8 +157,8 @@ struct PageManager::Impl {
const auto addr = reinterpret_cast<VAddr>(fault_address);
const bool is_write = Common::IsWriteError(context);
if (is_write && owned_ranges.find(addr) != owned_ranges.end()) {
const VAddr addr_aligned = Common::AlignDown(addr, PAGESIZE);
rasterizer->InvalidateMemory(addr_aligned, PAGESIZE);
const VAddr addr_aligned = GetPageAddr(addr);
rasterizer->InvalidateMemory(addr, addr_aligned, PAGESIZE);
return true;
}
return false;
Expand All @@ -174,6 +174,14 @@ PageManager::PageManager(Vulkan::Rasterizer* rasterizer_)

PageManager::~PageManager() = default;

VAddr PageManager::GetPageAddr(VAddr addr) {
return Common::AlignDown(addr, PAGESIZE);
}

VAddr PageManager::GetNextPageAddr(VAddr addr) {
return Common::AlignUp(addr + 1, PAGESIZE);
}

void PageManager::OnGpuMap(VAddr address, size_t size) {
impl->OnMap(address, size);
}
Expand Down
3 changes: 3 additions & 0 deletions src/video_core/page_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ class PageManager {
/// Increase/decrease the number of surface in pages touching the specified region
void UpdatePagesCachedCount(VAddr addr, u64 size, s32 delta);

static VAddr GetPageAddr(VAddr addr);
static VAddr GetNextPageAddr(VAddr addr);

private:
struct Impl;
std::unique_ptr<Impl> impl;
Expand Down
6 changes: 3 additions & 3 deletions src/video_core/renderer_vulkan/vk_rasterizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -792,9 +792,9 @@ u32 Rasterizer::ReadDataFromGds(u32 gds_offset) {
return value;
}

void Rasterizer::InvalidateMemory(VAddr addr, u64 size) {
buffer_cache.InvalidateMemory(addr, size);
texture_cache.InvalidateMemory(addr, size);
void Rasterizer::InvalidateMemory(VAddr addr, VAddr addr_aligned, u64 size) {
buffer_cache.InvalidateMemory(addr_aligned, size);
texture_cache.InvalidateMemory(addr, addr_aligned, size);
}

void Rasterizer::MapMemory(VAddr addr, u64 size) {
Expand Down
2 changes: 1 addition & 1 deletion src/video_core/renderer_vulkan/vk_rasterizer.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class Rasterizer {

void InlineData(VAddr address, const void* value, u32 num_bytes, bool is_gds);
u32 ReadDataFromGds(u32 gsd_offset);
void InvalidateMemory(VAddr addr, u64 size);
void InvalidateMemory(VAddr addr, VAddr addr_aligned, u64 size);
void MapMemory(VAddr addr, u64 size);
void UnmapMemory(VAddr addr, u64 size);

Expand Down
3 changes: 1 addition & 2 deletions src/video_core/texture_cache/image.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,7 @@ void UniqueImage::Create(const vk::ImageCreateInfo& image_ci) {
Image::Image(const Vulkan::Instance& instance_, Vulkan::Scheduler& scheduler_,
const ImageInfo& info_)
: instance{&instance_}, scheduler{&scheduler_}, info{info_},
image{instance->GetDevice(), instance->GetAllocator()}, cpu_addr{info.guest_address},
cpu_addr_end{cpu_addr + info.guest_size_bytes} {
image{instance->GetDevice(), instance->GetAllocator()} {
mip_hashes.resize(info.resources.levels);
ASSERT(info.pixel_format != vk::Format::eUndefined);
// Here we force `eExtendedUsage` as don't know all image usage cases beforehand. In normal case
Expand Down
20 changes: 14 additions & 6 deletions src/video_core/texture_cache/image.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,12 @@ VK_DEFINE_HANDLE(VmaAllocator)
namespace VideoCore {

enum ImageFlagBits : u32 {
CpuDirty = 1 << 1, ///< Contents have been modified from the CPU
Empty = 0,
MaybeCpuDirty = 1 << 0, ///< The page this image is in was touched before the image address
CpuDirty = 1 << 1, ///< Contents have been modified from the CPU
GpuDirty = 1 << 2, ///< Contents have been modified from the GPU (valid data in buffer cache)
Dirty = CpuDirty | GpuDirty,
Dirty = MaybeCpuDirty | CpuDirty | GpuDirty,
GpuModified = 1 << 3, ///< Contents have been modified from the GPU
Tracked = 1 << 4, ///< Writes and reads are being hooked from the CPU
Registered = 1 << 6, ///< True when the image is registered
Picked = 1 << 7, ///< Temporary flag to mark the image as picked
MetaRegistered = 1 << 8, ///< True when metadata for this surface is known and registered
Expand Down Expand Up @@ -78,7 +79,9 @@ struct Image {

[[nodiscard]] bool Overlaps(VAddr overlap_cpu_addr, size_t overlap_size) const noexcept {
const VAddr overlap_end = overlap_cpu_addr + overlap_size;
return cpu_addr < overlap_end && overlap_cpu_addr < cpu_addr_end;
const auto image_addr = info.guest_address;
const auto image_end = info.guest_address + info.guest_size_bytes;
return image_addr < overlap_end && overlap_cpu_addr < image_end;
}

ImageViewId FindView(const ImageViewInfo& info) const {
Expand All @@ -99,14 +102,18 @@ struct Image {
void CopyImage(const Image& image);
void CopyMip(const Image& image, u32 mip);

bool IsTracked() {
return track_addr != 0 && track_addr_end != 0;
}

const Vulkan::Instance* instance;
Vulkan::Scheduler* scheduler;
ImageInfo info;
UniqueImage image;
vk::ImageAspectFlags aspect_mask = vk::ImageAspectFlagBits::eColor;
ImageFlagBits flags = ImageFlagBits::Dirty;
VAddr cpu_addr = 0;
VAddr cpu_addr_end = 0;
VAddr track_addr = 0;
VAddr track_addr_end = 0;
std::vector<ImageViewInfo> image_view_infos;
std::vector<ImageViewId> image_view_ids;

Expand All @@ -130,6 +137,7 @@ struct Image {
std::vector<State> subresource_states{};
boost::container::small_vector<u64, 14> mip_hashes{};
u64 tick_accessed_last{0};
u64 hash{0};

struct {
union {
Expand Down
Loading