VideoCommon: Cache native vertex formats #463

degasus · 2014-06-05T16:52:11Z

We are used to have a 1:1 mapping of GX vertex formats and the native (OGL + D3D) ones, but there are by far more GX ones.
This new cache maps them directly so that we don't flush on GX vertex format changes as long as the native one doesn't change.

The idea is stolen from galop1n.

Source/Core/VideoCommon/VertexLoaderManager.cpp

@@ -38,6 +38,7 @@ namespace VertexLoaderManager
 {

 static VertexLoaderMap g_VertexLoaderMap;
+std::map<PortableVertexDeclaration, NativeVertexFormat*> g_native_vertex_cache;


degasus · 2014-06-05T16:57:59Z

galop1n's comments are copied from: degasus@6b42af2

Source/Core/VideoCommon/VertexLoaderManager.cpp

@@ -55,6 +56,11 @@ void Shutdown()
 		delete p.second;
 	}
 	g_VertexLoaderMap.clear();
+	for (auto& v :g_native_vertex_cache)


degasus · 2014-06-06T11:38:37Z

updated to use unique_ptr

degasus · 2014-06-06T11:52:01Z

updated again to work around vs issue which doesn't know about std::map::emplace_hint

Source/Core/VideoCommon/VertexLoaderManager.h

@@ -20,6 +23,8 @@ namespace VertexLoaderManager

 	// For debugging
 	void AppendListToString(std::string *dest);
+
+	extern std::map<PortableVertexDeclaration, std::unique_ptr<NativeVertexFormat> > g_native_vertex_cache;


degasus · 2014-07-03T19:03:36Z

Updated with a Get* Function instead of a global

Sonicadvance1 · 2014-07-04T10:31:56Z

LGTM.
Now we just need to VertexLoaderManager changed over to a class so we get rid of ugly globals.
Out of the scope of this PR though ;)

Source/Core/VideoCommon/VertexLoaderManager.cpp

@@ -131,6 +135,18 @@ int GetVertexSize(int vtx_attr_group)
 	return RefreshLoader(vtx_attr_group)->GetVertexSize();
 }

+NativeVertexFormat *GetNativeVertexFormat(const PortableVertexDeclaration& format, u32 components) {


delroth · 2014-07-04T11:47:34Z

@JMC47 care for some performance testing?

JMC47 · 2014-07-04T11:54:27Z

I'm guessing higher polygon games would be more affected, right? I tried out both Galaxy games and was unable to determine a difference in speed. Maybe MaJoR could try out Last Story and see if that beast of a game is affected.

degasus · 2014-07-04T12:02:19Z

The amount of primitives doesn't affect this change. It is only affected if a game engine uses lots of different vertex formats alternatingly which only differs in vertex attribute type.

galop1n · 2014-07-04T12:24:39Z

The game i made the optim first was Donkey Kong ( won 30% of flush ), you
can look at the stat display for the amount of flush to compare the
influence between a build with an without the optim.

On Fri, Jul 4, 2014 at 2:02 PM, Markus Wick notifications@github.com
wrote:

The amount of primitives doesn't affect this change. It is only affected
if a game engine uses lots of different vertex formats alternatingly which
only differs in vertex attribute type.

—
Reply to this email directly or view it on GitHub
#463 (comment).

degasus · 2014-07-04T12:29:19Z

I haven't seen any win of flushes (I haven't tried Donkey Kong), but I saw a lot of redundant vertex formats, so imo it's still better for the driver through.

-30% of flushes is a nice improvment :)

Source/Core/VideoCommon/VertexLoaderManager.cpp


 namespace VertexLoaderManager
 {

 static VertexLoaderMap g_VertexLoaderMap;
+static NativeVertexLoaderMap g_native_vertex_map;


neobrain · 2014-07-04T12:35:32Z

Apart from the two coding style issues, LGTM.

We are used to have a 1:1 mapping of GX vertex formats and the native (OGL + D3D) ones, but there are by far more GX ones. This new cache maps them directly so that we don't flush on GX vertex format changes as long as the native one doesn't change. The idea is stolen from galop1n.

degasus · 2014-07-06T09:43:13Z

ready for merge

JMC47 · 2014-07-06T10:21:52Z

I didn't notice any breakage while testing, so it looks good to me. If someone could actually get some framerate comparisons from DKCR, that'd be great for the progress report.

VideoCommon: Cache native vertex formats

galop1n · 2014-07-06T10:27:13Z

it will change nothing, it was not the optim i was talking about (
comparing native vertex format to skip flushes ). That change is not a
performance optim, more a clean to not create duplicated object from the
d3d backend.

On Sun, Jul 6, 2014 at 12:21 PM, JMC47 notifications@github.com wrote:

I didn't notice any breakage while testing, so it looks good to me. If
someone could actually get some framerate comparisons from DKCR, that'd be
great for the progress report.

—
Reply to this email directly or view it on GitHub
#463 (comment).

JMC47 · 2014-07-06T10:28:39Z

Ok, thanks for explanation.

degasus · 2014-07-06T10:29:30Z

@galop1n It's both. So we likely hit this code less often: https://github.com/dolphin-emu/dolphin/blob/master/Source/Core/VideoCommon/VertexLoader.cpp#L822

degasus reviewed Jun 5, 2014
View reviewed changes

neobrain reviewed Jun 6, 2014
View reviewed changes

Source/Core/VideoCommon/VertexLoaderManager.cpp

@@ -55,6 +56,11 @@ void Shutdown()

delete p.second;

}

g_VertexLoaderMap.clear();

for (auto& v :g_native_vertex_cache)

This comment was marked as off-topic.

Sign in to view

neobrain reviewed Jun 6, 2014
View reviewed changes

delroth reviewed Jul 4, 2014
View reviewed changes

neobrain reviewed Jul 4, 2014
View reviewed changes

Source/Core/VideoCommon/VertexLoaderManager.cpp

namespace VertexLoaderManager

{

static VertexLoaderMap g_VertexLoaderMap;

static NativeVertexLoaderMap g_native_vertex_map;

This comment was marked as off-topic.

Sign in to view

Sonicadvance1 added a commit that referenced this pull request Jul 6, 2014

Merge pull request #463 from degasus/vertex_format_cache

4483b64

VideoCommon: Cache native vertex formats

Sonicadvance1 merged commit 4483b64 into dolphin-emu:master Jul 6, 2014

degasus deleted the vertex_format_cache branch July 12, 2014 10:28

VideoCommon: Cache native vertex formats #463

VideoCommon: Cache native vertex formats #463

degasus commented Jun 5, 2014

This comment was marked as off-topic.

This comment was marked as off-topic.

degasus commented Jun 5, 2014

This comment was marked as off-topic.

degasus commented Jun 6, 2014

degasus commented Jun 6, 2014

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

degasus commented Jul 3, 2014

Sonicadvance1 commented Jul 4, 2014

This comment was marked as off-topic.

delroth commented Jul 4, 2014

JMC47 commented Jul 4, 2014

degasus commented Jul 4, 2014

galop1n commented Jul 4, 2014

degasus commented Jul 4, 2014

This comment was marked as off-topic.

neobrain commented Jul 4, 2014

degasus commented Jul 6, 2014

JMC47 commented Jul 6, 2014

galop1n commented Jul 6, 2014

JMC47 commented Jul 6, 2014

degasus commented Jul 6, 2014

VideoCommon: Cache native vertex formats #463

VideoCommon: Cache native vertex formats #463

Conversation

degasus commented Jun 5, 2014

This comment was marked as off-topic.

This comment was marked as off-topic.

degasus commented Jun 5, 2014

This comment was marked as off-topic.

degasus commented Jun 6, 2014

degasus commented Jun 6, 2014

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

degasus commented Jul 3, 2014

Sonicadvance1 commented Jul 4, 2014

This comment was marked as off-topic.

delroth commented Jul 4, 2014

JMC47 commented Jul 4, 2014

degasus commented Jul 4, 2014

galop1n commented Jul 4, 2014

degasus commented Jul 4, 2014

This comment was marked as off-topic.

neobrain commented Jul 4, 2014

degasus commented Jul 6, 2014

JMC47 commented Jul 6, 2014

galop1n commented Jul 6, 2014

JMC47 commented Jul 6, 2014

degasus commented Jul 6, 2014