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

VideoBackends: Share GX vertex/index/uniform buffers with utility draws #7592

Merged
merged 7 commits into from Dec 4, 2018

Conversation

3 participants
@stenzek
Contributor

stenzek commented Nov 27, 2018

This PR is a prerequisite for our imgui integration. It makes VertexManager a little more generic, moving some of the logic into common, and enabling buffer sharing with utility shader draws (used for imgui).

In the future, we can also use this for drawing EFB copies, and post-processing, rather than having all these different buffers sitting around the place (and duplicated across the backends). This is mainly the case in Vulkan, not so much in the others.

@stenzek stenzek force-pushed the stenzek:imgui-prereq branch from d1c44a0 to 573b16e Nov 28, 2018

@delroth

This comment has been minimized.

Member

delroth commented Nov 29, 2018

LGTM, had a quick look through each commit and couldn't find much else to say other than "amazing refactoring" :)

Given this is a decently large change, I'm tempted to wait 2 days and merge it after beta is tagged. Your thoughts?

@stenzek

This comment has been minimized.

Contributor

stenzek commented Nov 29, 2018

Sounds fine to me. There is one path I haven't tested (drivers which don't support base_vertex). While I don't see any reason it should regress, I probably should check it just in case.

@stenzek stenzek force-pushed the stenzek:imgui-prereq branch from 573b16e to aebf0bc Nov 29, 2018

@delroth

This comment has been minimized.

Member

delroth commented Dec 4, 2018

Could you fix the merge conflict? Thanks!

stenzek added some commits Sep 9, 2017

RenderBase: Force a pipeline flush when drawing the XFB to the host
Since we use the common pipelines here and draw vertices if a batch is
currently being built by the vertex loader, we end up trampling over its
pointer, as we share the buffer with the loader, and it has not been
unmapped yet. Force a pipeline flush to avoid this.

@stenzek stenzek force-pushed the stenzek:imgui-prereq branch from aebf0bc to 7afd5cc Dec 4, 2018

@delroth delroth merged commit 59de8ea into dolphin-emu:master Dec 4, 2018

10 checks passed

default Very basic checks passed, handed off to Buildbot.
Details
lint Build succeeded on builder lint
Details
pr-android Build succeeded on builder pr-android
Details
pr-deb-dbg-x64 Build succeeded on builder pr-deb-dbg-x64
Details
pr-deb-x64 Build succeeded on builder pr-deb-x64
Details
pr-freebsd-x64 Build succeeded on builder pr-freebsd-x64
Details
pr-osx-x64 Build succeeded on builder pr-osx-x64
Details
pr-ubu-x64 Build succeeded on builder pr-ubu-x64
Details
pr-win-dbg-x64 Build succeeded on builder pr-win-dbg-x64
Details
pr-win-x64 Build succeeded on builder pr-win-x64
Details
g_renderer->SetPipeline(m_current_pipeline_object);
Draw(stride);
static_cast<Renderer*>(g_renderer.get())->SetPipeline(m_current_pipeline_object);
static_cast<Renderer*>(g_renderer.get())->DrawIndexed(base_index, num_indices, base_vertex);

This comment has been minimized.

@weihuoya

weihuoya Dec 4, 2018

Contributor
if (m_current_pipeline_object)
{
    static_cast<Renderer*>(g_renderer.get())->SetPipeline(m_current_pipeline_object);
    static_cast<Renderer*>(g_renderer.get())->DrawIndexed(base_index, num_indices, base_vertex);
}

can this simple to

g_renderer->DrawIndexed(base_index, num_indices, base_vertex);

This comment has been minimized.

@stenzek

stenzek Dec 4, 2018

Contributor

The cast is to avoid the virtual call for DrawIndexed (since it'll always be a GL renderer), but you are correct about the pipeline binding being redundant.

@stenzek stenzek deleted the stenzek:imgui-prereq branch Dec 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment