| Message ID | 1421665245-5994-3-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 01/19/2015 03:00 AM, Chris Wilson wrote: > Since the introduction of DRI2GetBuffersWithFormat, the old > DRI2GetBuffers interface would always recreate all buffers all the time > as it was no longer agnostic to the format value being set by the DDXes. > This causes an issue with clients intermixing the two requests, > rendering any sharing or caching of buffers (e.g. for triple buffering) > void. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > hw/xfree86/dri2/dri2.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c > index 43a1899..f9f594d 100644 > --- a/hw/xfree86/dri2/dri2.c > +++ b/hw/xfree86/dri2/dri2.c > @@ -464,14 +464,16 @@ find_attachment(DRI2DrawablePtr pPriv, unsigned attachment) > static Bool > allocate_or_reuse_buffer(DrawablePtr pDraw, DRI2ScreenPtr ds, > DRI2DrawablePtr pPriv, > - unsigned int attachment, unsigned int format, > + unsigned int attachment, > + int has_format, unsigned int format, > int dimensions_match, DRI2BufferPtr * buffer) > { > int old_buf = find_attachment(pPriv, attachment); > > if ((old_buf < 0) > || attachment == DRI2BufferFrontLeft > - || !dimensions_match || (pPriv->buffers[old_buf]->format != format)) { > + || !dimensions_match > + || (has_format && pPriv->buffers[old_buf]->format != format)) { > *buffer = create_buffer(ds, pDraw, attachment, format); Shouldn't the create_buffer change if !has_format? If !has_format and, say, !dimensions_match, create_buffer will get format = 0 when it should get format = pPriv->buffers[old_buf]->format. Right? Another alternative would be to have the caller always pass a format: either the format supplied in the protocol or the format of the old buffer. That might be more messy. Dunno. > return TRUE; > > @@ -549,7 +551,8 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height, > const unsigned format = (has_format) ? *(attachments++) : 0; > > if (allocate_or_reuse_buffer(pDraw, ds, pPriv, attachment, > - format, dimensions_match, &buffers[i])) > + has_format, format, dimensions_match, > + &buffers[i])) > buffers_changed = 1; > > if (buffers[i] == NULL) > @@ -584,7 +587,7 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height, > > if (need_real_front > 0) { > if (allocate_or_reuse_buffer(pDraw, ds, pPriv, DRI2BufferFrontLeft, > - front_format, dimensions_match, > + has_format, front_format, dimensions_match, > &buffers[i])) > buffers_changed = 1; > > @@ -595,7 +598,7 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height, > > if (need_fake_front > 0) { > if (allocate_or_reuse_buffer(pDraw, ds, pPriv, DRI2BufferFakeFrontLeft, > - front_format, dimensions_match, > + has_format, front_format, dimensions_match, > &buffers[i])) > buffers_changed = 1; >
On 20/01/15 22:49, Ian Romanick wrote: > On 01/19/2015 03:00 AM, Chris Wilson wrote: >> Since the introduction of DRI2GetBuffersWithFormat, the old >> DRI2GetBuffers interface would always recreate all buffers all the time >> as it was no longer agnostic to the format value being set by the DDXes. >> This causes an issue with clients intermixing the two requests, >> rendering any sharing or caching of buffers (e.g. for triple buffering) >> void. >> >> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >> --- >> hw/xfree86/dri2/dri2.c | 13 ++++++++----- >> 1 file changed, 8 insertions(+), 5 deletions(-) >> >> diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c >> index 43a1899..f9f594d 100644 >> --- a/hw/xfree86/dri2/dri2.c >> +++ b/hw/xfree86/dri2/dri2.c >> @@ -464,14 +464,16 @@ find_attachment(DRI2DrawablePtr pPriv, unsigned attachment) >> static Bool >> allocate_or_reuse_buffer(DrawablePtr pDraw, DRI2ScreenPtr ds, >> DRI2DrawablePtr pPriv, >> - unsigned int attachment, unsigned int format, >> + unsigned int attachment, >> + int has_format, unsigned int format, >> int dimensions_match, DRI2BufferPtr * buffer) >> { >> int old_buf = find_attachment(pPriv, attachment); >> >> if ((old_buf < 0) >> || attachment == DRI2BufferFrontLeft >> - || !dimensions_match || (pPriv->buffers[old_buf]->format != format)) { >> + || !dimensions_match >> + || (has_format && pPriv->buffers[old_buf]->format != format)) { >> *buffer = create_buffer(ds, pDraw, attachment, format); > Shouldn't the create_buffer change if !has_format? If !has_format and, > say, !dimensions_match, create_buffer will get format = 0 when it should > get format = pPriv->buffers[old_buf]->format. Right? This is still a problem in the current patchset that I have. Since the client did not specifically ask for a certain format, why not increase the likeliness of us being able to reuse the buffer later on by using a format that the application already asked before? > > Another alternative would be to have the caller always pass a format: > either the format supplied in the protocol or the format of the old > buffer. That might be more messy. Dunno. > >> return TRUE; >> >> @@ -549,7 +551,8 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height, >> const unsigned format = (has_format) ? *(attachments++) : 0; >> >> if (allocate_or_reuse_buffer(pDraw, ds, pPriv, attachment, >> - format, dimensions_match, &buffers[i])) >> + has_format, format, dimensions_match, >> + &buffers[i])) >> buffers_changed = 1; >> >> if (buffers[i] == NULL) >> @@ -584,7 +587,7 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height, >> >> if (need_real_front > 0) { >> if (allocate_or_reuse_buffer(pDraw, ds, pPriv, DRI2BufferFrontLeft, >> - front_format, dimensions_match, >> + has_format, front_format, dimensions_match, >> &buffers[i])) >> buffers_changed = 1; >> >> @@ -595,7 +598,7 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height, >> >> if (need_fake_front > 0) { >> if (allocate_or_reuse_buffer(pDraw, ds, pPriv, DRI2BufferFakeFrontLeft, >> - front_format, dimensions_match, >> + has_format, front_format, dimensions_match, >> &buffers[i])) >> buffers_changed = 1; >> > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 43a1899..f9f594d 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -464,14 +464,16 @@ find_attachment(DRI2DrawablePtr pPriv, unsigned attachment) static Bool allocate_or_reuse_buffer(DrawablePtr pDraw, DRI2ScreenPtr ds, DRI2DrawablePtr pPriv, - unsigned int attachment, unsigned int format, + unsigned int attachment, + int has_format, unsigned int format, int dimensions_match, DRI2BufferPtr * buffer) { int old_buf = find_attachment(pPriv, attachment); if ((old_buf < 0) || attachment == DRI2BufferFrontLeft - || !dimensions_match || (pPriv->buffers[old_buf]->format != format)) { + || !dimensions_match + || (has_format && pPriv->buffers[old_buf]->format != format)) { *buffer = create_buffer(ds, pDraw, attachment, format); return TRUE; @@ -549,7 +551,8 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height, const unsigned format = (has_format) ? *(attachments++) : 0; if (allocate_or_reuse_buffer(pDraw, ds, pPriv, attachment, - format, dimensions_match, &buffers[i])) + has_format, format, dimensions_match, + &buffers[i])) buffers_changed = 1; if (buffers[i] == NULL) @@ -584,7 +587,7 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height, if (need_real_front > 0) { if (allocate_or_reuse_buffer(pDraw, ds, pPriv, DRI2BufferFrontLeft, - front_format, dimensions_match, + has_format, front_format, dimensions_match, &buffers[i])) buffers_changed = 1; @@ -595,7 +598,7 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height, if (need_fake_front > 0) { if (allocate_or_reuse_buffer(pDraw, ds, pPriv, DRI2BufferFakeFrontLeft, - front_format, dimensions_match, + has_format, front_format, dimensions_match, &buffers[i])) buffers_changed = 1;
Since the introduction of DRI2GetBuffersWithFormat, the old DRI2GetBuffers interface would always recreate all buffers all the time as it was no longer agnostic to the format value being set by the DDXes. This causes an issue with clients intermixing the two requests, rendering any sharing or caching of buffers (e.g. for triple buffering) void. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- hw/xfree86/dri2/dri2.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)