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

Speed up paletized texture #1791

Merged
merged 14 commits into from Dec 2, 2019

Conversation

@PatrickvL
Copy link
Member

PatrickvL commented Nov 21, 2019

Include a palette hash in the texture lookup key, instead of patching Palette_Lock functions and freeing textures when setting palette's.

To avoid overflowing the resource cache, PruneResourceCache now applies cache eviction as well.
Thanks to that change, two Palette_Lock patches could be removed as well.

This speeds up titles that use paletized textures a lot, here Yu-Gi-Oh! The Dawn Of Destiny (YuGiOh DOD) running in-game at 30fps :
image

@PatrickvL PatrickvL force-pushed the PatrickvL:hash_palete_in_texture_key branch 2 times, most recently from b290e70 to 5322862 Nov 21, 2019
@JusicP

This comment has been minimized.

Copy link

JusicP commented Nov 21, 2019

Half-Life 2 Crashes on startup.

[0x32F8] MAIN: Received Exception (Code := 0xC0000005)

 EIP := 0x04108AA0
 EFL := 0x00010246
 EAX := 0x00000100 EBX := 0x00000400 ECX := 0x00000000 EDX := 0xFFFFFFFF
 ESI := 0x00000000 EDI := 0x00000000 ESP := 0x099FF9D0 EBP := 0x099FF9DC
 CR2 := 0x00000000
@PatrickvL

This comment has been minimized.

Copy link
Member Author

PatrickvL commented Nov 21, 2019

Try the latest build (I force-pushed a safe-guard against that crash)

@JusicP

This comment has been minimized.

Copy link

JusicP commented Nov 21, 2019

Now it works

@LukeUsher LukeUsher self-requested a review Nov 21, 2019
Copy link
Member

LukeUsher left a comment

I'm happy with this, let's leave it overnight for our testers to play with, and if nothing major comes up, we'll merge tomorrow!

Great job.

@literalmente-game

This comment has been minimized.

Copy link
Contributor

literalmente-game commented Nov 21, 2019

These have new rendering issues:

  • Kill Switch (wrong textures being loaded for some models)
  • RalliSport Challenge (black floor)
  • Bloodrayne (black textures for many models, also black floor)
  • Bloodrayne 2 (black textures for most of the models, black floor)
@NZJenkins

This comment has been minimized.

Copy link
Contributor

NZJenkins commented Nov 22, 2019

These don't go in-game with the PR:

  • MechAssault
  • Metal Slug 5
  • Crash N Burn

These have new rendering issues:

  • Kill Switch (wrong textures being loaded for some models)
  • RalliSport Challenge (black floor)
  • Bloodrayne (black textures for many models, also black floor)
  • Bloodrayne 2 (black textures for most of the models, black floor)

Mechassault appears broken since 6b08ee8 so may need to double check with the previous build

@literalmente-game

This comment has been minimized.

Copy link
Contributor

literalmente-game commented Nov 22, 2019

These don't go in-game with the PR:

  • MechAssault
  • Metal Slug 5
  • Crash N Burn

These have new rendering issues:

  • Kill Switch (wrong textures being loaded for some models)
  • RalliSport Challenge (black floor)
  • Bloodrayne (black textures for many models, also black floor)
  • Bloodrayne 2 (black textures for most of the models, black floor)

Mechassault appears broken since 6b08ee8 so may need to double check with the previous build

Updated the comment, you're right, this list of games was affected by the mentioned change, not this PR:

  • MechAssault
  • Metal Slug 5
  • Crash N Burn
@AzurikRiseOfPerathia

This comment has been minimized.

Copy link

AzurikRiseOfPerathia commented Nov 23, 2019

Azurik : Rise Of Perathia : Black texture, and some texture poorly modeled (Image has the support)

2019-11-23_005825

@hyperspeedgx

This comment has been minimized.

Copy link

hyperspeedgx commented Nov 23, 2019

Does we need to set some kinda of settings for Yu-Gi-Oh TDoD work? My only shows a black screen, I saw that pal ver. works but I don't have it.

@PatrickvL

This comment has been minimized.

Copy link
Member Author

PatrickvL commented Nov 23, 2019

@AzurikRiseOfPerathia the exploding vertices are not related to this pull request, since it doesn't touch vertices at all.

It's probably caused by the recent MOVA fix, @NZJenkins is already looking into it.

As for the black textures, I'm investigating the root cause, but it's eluding me for now...

PatrickvL added 8 commits Nov 21, 2019
… Palette_Lock functions and freeing textures when setting palette's.

To avoid overflowing the resource cache, PruneResourceCache now applies cache eviction as well.
…eFactor as a multiplier (never as a divisor).
Consequences of this change :
GetHostBaseTexture won't be called on X_D3DCOMMON_TYPE_SURFACE anymore.
GetHostTexture became obsolete because of this,

Also corected a few typo's & made bits of code simpler.
…ces instead of in EmuVerifyResourceIsRegistered), this might prevents black textures.
@PatrickvL PatrickvL force-pushed the PatrickvL:hash_palete_in_texture_key branch from 79ec3b7 to e281d1e Nov 25, 2019
@PatrickvL

This comment has been minimized.

Copy link
Member Author

PatrickvL commented Nov 25, 2019

@literalmente-game commit db45305 may fix black textures, could you do a quick test?

…hance of hash collisions.

This can solve part of the reported regressions (but there's more, like not-pruneing render targets).
@PatrickvL

This comment has been minimized.

Copy link
Member Author

PatrickvL commented Nov 26, 2019

Although the latest commit might improve/fix a few regressions, the bigger problem this PR still has, is that when the resource cache is pruned (which right now means it's cleared entirely), the association between Xbox render targets and the host counterparts is lost.

This results in the creation of new host-side render-targets, but those won't contain the same data as before anymore.

Also, some resources (like the backbuffer and other globals) should not be removed at all, but are also cleared right now. For this, we have to come up with two solutions :

  1. a real cache eviction algorithm that's fast, efficient and easy to implement and maintain
  2. a way to avoid removing entries that must be kept alive at all times
@RinMaru

This comment has been minimized.

Copy link

RinMaru commented Nov 27, 2019

Vexx No longer gets ingame

…s pruned when it overflows),

one for all other resources (which cache doesn't prune, like it did not do before, too).

This way, only paletized textures get pruned from their specialized cache, while other resources,
including render-targets (which can't be paletized) will stay resident in the general cache (which won't overflow).

Doing it this way, probably fixes the reported black texture regressions,
but it might also lead to less of a performance increase than before.
@PatrickvL PatrickvL force-pushed the PatrickvL:hash_palete_in_texture_key branch from 10ff792 to de1b815 Nov 28, 2019
@PatrickvL

This comment has been minimized.

Copy link
Member Author

PatrickvL commented Nov 28, 2019

With the most recent commit, I've implemented an easier solution for now;

Instead of implementing an actual cache eviction policy, I've split up the cache in two parts.

This way, only the paletized textures get pruned (still with the poor-mans cache eviction policy : clean all), and all other resource types stay in the main cache, which implicitly prevents render target mappings getting lost.

PatrickvL added 2 commits Nov 28, 2019
…oring GetHostBaseTexture to it's previous behaviour (if this works, I'll create a separate issue with all relevant detail so that we can follow-up on this later)
@RinMaru

This comment has been minimized.

Copy link

RinMaru commented Nov 28, 2019

Annotation 2019-11-28 183826
getting this error in vexx due to this PR

@PatrickvL

This comment has been minimized.

Copy link
Member Author

PatrickvL commented Nov 29, 2019

I think I'll have to redo this entire pr, in smaller increments, making sure each step doesn't regress

PatrickvL added 2 commits Dec 1, 2019
…Y flag at all, so we can't identify resources with it either!
…reported cases); Turned out to be an oversight (GetHostBaseTexture 2nd argument is not the TextureStage, but D3DUsage, dug!). I had to re-do each change line by line, testing as I went, to find this... Ah well, at least it didn't have anything to do with the new cache!
@EternalShackles

This comment has been minimized.

Copy link

EternalShackles commented Dec 1, 2019

Well, latest build fixed the black textures in all games that were broken with this pr earlier
GW OG
GW FIXED

src/core/hle/D3D8/XbD3D8Types.h Show resolved Hide resolved
@LukeUsher LukeUsher merged commit 63bc65e into Cxbx-Reloaded:develop Dec 2, 2019
3 checks passed
3 checks passed
Cxbx-Reloaded.Cxbx-Reloaded #20191201.2 succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@PatrickvL PatrickvL deleted the PatrickvL:hash_palete_in_texture_key branch Dec 2, 2019
@PatrickvL

This comment has been minimized.

Copy link
Member Author

PatrickvL commented Dec 5, 2019

@LastBreathToday reported :

That latest PR on Appvoyer breaks Crash TWOC the main rooms glass floors don't render correctly when walking around plus the gates don't render anymore
Might just be an AMD problem
Not sure if the crystals rendering black is a AMD problem either
Ugh AMD issue it is
For me the floor will flicker black then normal but the elemental gates don't render and this all started from the recent PR that was causing regressions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants
You can’t perform that action at this time.