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

renderer_opengl: Rewrite stream buffer. #3666

Merged
merged 2 commits into from Apr 25, 2018

Conversation

Projects
None yet
5 participants
@degasus
Contributor

degasus commented Apr 17, 2018

This is a followup to #3504 .

The idea of this rewrite is to orphan the buffer when it is full instead of syncing. This allows the driver to allocate many buffer objects and so to increase/decrease the size of the ring buffer. This however needs a glMapBufferRange call on orphaning, which likely synchronize the drivers worker thread once a frame.

On rewriting the implementation, I've also updated the API to flush the used buffer space on unmapping. This allows to generate the vertex data in-place and to flush the generated data afterwards. The new API also returns if the old allocations are still valid. This might allow us to use this kind of buffer for uniforms and texture buffers. In this way, we just need to reupload all of them on orphaning.


This change is Reviewable

@cluezbot

This comment has been minimized.

cluezbot commented Apr 17, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-04-17T19:54:58Z: 2d372ba...degasus:272c9efbf6cb684dabd4ff704ae3c9cae70e4da5

std::deque<Fence> head;
std::deque<Fence> tail;
OGLStreamBuffer::OGLStreamBuffer(GLenum target, GLsizeiptr size, bool coherent) {
gl_target = target;

This comment has been minimized.

@lioncash

lioncash Apr 17, 2018

Member

These can be assigned in the constructor initializer list.

if (!coherent) {
map_flags |= GL_MAP_FLUSH_EXPLICIT_BIT;
}
mapped_ptr = reinterpret_cast<u8*>(glMapBufferRange(gl_target, 0, buffer_size, map_flags));

This comment has been minimized.

@lioncash

lioncash Apr 17, 2018

Member

static_cast

glClientWaitSync(sync.handle, GL_SYNC_FLUSH_COMMANDS_BIT, GL_TIMEOUT_IGNORED);
sync.Release();
if (flags & GL_MAP_INVALIDATE_BUFFER_BIT || !(flags & GL_MAP_PERSISTENT_BIT)) {
mapped_ptr = reinterpret_cast<u8*>(

This comment has been minimized.

@lioncash

lioncash Apr 17, 2018

Member

static_cast

@degasus degasus force-pushed the degasus:vertex_streaming branch from 272c9ef to 0108e9e Apr 17, 2018

@cluezbot

This comment has been minimized.

cluezbot commented Apr 17, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-04-17T20:27:06Z: 2d372ba...degasus:0108e9e4d5295e7b75f22eb98fcdfd8fd4e300d6

@cluezbot

This comment has been minimized.

cluezbot commented Apr 18, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-04-18T04:53:36Z: 2d372ba...degasus:f7409681cc20feda612f3d134040c5bedfd2eab3

@degasus degasus force-pushed the degasus:vertex_streaming branch 2 times, most recently from f740968 to 9084565 Apr 18, 2018

@cluezbot

This comment has been minimized.

cluezbot commented Apr 21, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-04-21T09:33:36Z: c8d4ca8...degasus:90845658e1ee4ec2aed12592ae2db1a8a5ad59e0

@cluezbot

This comment has been minimized.

cluezbot commented Apr 21, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-04-21T12:43:28Z: c8d4ca8...degasus:22eccb693a947ba986526883a602b750f76c9aa8

glDrawArrays(GL_TRIANGLES, map.second / sizeof(HardwareVertex), (GLsizei)vertices);
u8* vbo;
GLintptr offset;
std::tie(vbo, offset, std::ignore) = vertex_buffer.Map(vertex_size);

This comment has been minimized.

@wwylele

wwylele Apr 21, 2018

Member

Technically this map needs an alignment of sizeof(HardwareVertex) since you do offset / sizeof(HardwareVertex) below. This becomes more important when we mix other data into the buffer in glvtx. Sorry that I didn't catch this in the first version!

@degasus degasus force-pushed the degasus:vertex_streaming branch from 22eccb6 to c4010e3 Apr 21, 2018

@cluezbot

This comment has been minimized.

cluezbot commented Apr 21, 2018

Hi, this is neobot, using neobrain's account. I'm keeping an archive of versions of this PR:

2018-04-21T14:12:48Z: c8d4ca8...degasus:c4010e3f935d3b84e57755a5fb738c83511ffd8b

@bunnei

bunnei approved these changes Apr 25, 2018

@bunnei bunnei merged commit b003e13 into citra-emu:master Apr 25, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

degasus added a commit to degasus/yuzu that referenced this pull request Aug 10, 2018

degasus added a commit to degasus/yuzu that referenced this pull request Aug 10, 2018

degasus added a commit to degasus/yuzu that referenced this pull request Aug 10, 2018

degasus added a commit to degasus/yuzu that referenced this pull request Aug 12, 2018

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