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

VideoCommon:VertexLoaderManager: Reduce CPConfig checks #11285

Merged
merged 1 commit into from Nov 21, 2022

Conversation

K0bin
Copy link
Contributor

@K0bin K0bin commented Nov 16, 2022

A bit of a micro optimization:
With this PR CheckCPConfiguration runs 350 times instead of 35k times. (Mario Galaxy)

@K0bin K0bin force-pushed the cp-checks branch 2 times, most recently from 0cfcd34 to a7effe5 Compare November 16, 2022 13:47
A bit of a micro optimization:
CheckCPConfiguration is called 350 times instead of 35k times.
Copy link
Contributor

@Pokechu22 Pokechu22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Untested, looks good to me though

return 0;
ASSERT(count > 0);

VertexLoaderBase* loader = RefreshLoader<IsPreprocess>(vtx_attr_group);

int size = count * loader->m_vertex_size;
if ((int)src.size() < size)
if ((int)src.size() < size) [[unlikely]]
return -1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check can probably be removed entirely, actually, as it's redundant with this:

const u32 vertex_size = callback.GetVertexSize(vat);
const u16 num_vertices = Common::swap16(&data[1]);
if (available < 3 + num_vertices * vertex_size)
return 0;
callback.OnPrimitiveCommand(primitive, vat, vertex_size, num_vertices, &data[3]);

and in fact this return value isn't properly handled anymore:

// HACK
DataReader src{const_cast<u8*>(vertex_data), const_cast<u8*>(vertex_data) + size};
const u32 bytes =
VertexLoaderManager::RunVertices<is_preprocess>(vat, primitive, num_vertices, src);
ASSERT(bytes == size);

It doesn't need to be removed in this PR, but this is probably something that can be refactored later (along with the DataReader itself since it isn't really doing much anymore to my understanding).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'll leave that to you since you understand that code muuuuch better than me. ;)

@Pokechu22 Pokechu22 merged commit 8f5a58f into dolphin-emu:master Nov 21, 2022
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants