subject:"\[PATCH 9\/9\] glamor\: Use buffer

Re: [PATCH 9/9] glamor: Use buffer_storage

2014-03-10 Thread Markus Wick


Am 2014-03-10 21:09, schrieb Eric Anholt:

On release builds, glGetError won't be called and so this won't clear
the gl error log. So we'll fall back to the mbr code because of any gl
error somewhere in glamor.


Yeah, I think that's fine -- you shouldn't have any errors, anyway.


But then, we should explicit delete & alloc this buffer again. Atm we 
would fall back to mbr but buffer_storage was maybe successful. So non 
of the next glBufferData will success ...

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 9/9] glamor: Use buffer_storage

2014-03-10 Thread Eric Anholt

Markus Wick  writes:

> Am 2014-03-09 05:07, schrieb Eric Anholt:

>> diff --git a/glamor/glamor_vbo.c b/glamor/glamor_vbo.c
>> index be2c2af..f736cbe 100644
>> --- a/glamor/glamor_vbo.c
>> +++ b/glamor/glamor_vbo.c
>> @@ -52,7 +52,49 @@ glamor_get_vbo_space(ScreenPtr screen, unsigned
>> size, char **vbo_offset)
>> 
>>  glBindBuffer(GL_ARRAY_BUFFER, glamor_priv->vbo);
>> 
>> -if (glamor_priv->gl_flavor == GLAMOR_GL_DESKTOP) {
>> +if (glamor_priv->has_buffer_storage) {
>> +if (glamor_priv->vbo_size < glamor_priv->vbo_offset + size) {
>> +if (glamor_priv->vbo_size)
>> +glUnmapBuffer(GL_ARRAY_BUFFER);
>> +
>> +if (size > glamor_priv->vbo_size) {
>> +glamor_priv->vbo_size = MAX(GLAMOR_VBO_SIZE, size);
>> +
>> +/* We aren't allowed to resize glBufferStorage()
>> + * buffers, so we need to gen a new one.
>> + */
>> +glDeleteBuffers(1, &glamor_priv->vbo);
>> +glGenBuffers(1, &glamor_priv->vbo);
>> +glBindBuffer(GL_ARRAY_BUFFER, glamor_priv->vbo);
>> +
>> +assert(glGetError() == GL_NO_ERROR);
>
> On release builds, glGetError won't be called and so this won't clear 
> the gl error log. So we'll fall back to the mbr code because of any gl 
> error somewhere in glamor.

Yeah, I think that's fine -- you shouldn't have any errors, anyway.

>> +glBufferStorage(GL_ARRAY_BUFFER, 
>> glamor_priv->vbo_size, NULL,
>> +GL_MAP_WRITE_BIT |
>> +GL_MAP_PERSISTENT_BIT |
>> +GL_MAP_COHERENT_BIT);
>> +
>> +if (glGetError() != GL_NO_ERROR) {
>> +/* If the driver failed our coherent mapping, fall
>> + * back to the ARB_mbr path.
>> + */
>> +glamor_priv->has_buffer_storage = false;
>> +glamor_priv->vbo_size = 0;
>
> glamor_put_context(glamor_priv);

Fixed.

> Which kind of error do you expect for a driver which isn't able to map 
> coherent? I've only found a GL_OUT_OF_MEMORY, but this will invalidate 
> the complete gl state. So I guess there is no legal way to support 
> arb_buffer_storage without coherent mapping at all.

I wonder if the caller is supposed to expect GL_OOM from this path and
assume that the context isn't totally trashed like the general GL_OOM
case.

So far, it looks like we're going to just always support COHERENT in
Mesa.


pgp0yIZm78Y0J.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 9/9] glamor: Use buffer_storage

2014-03-10 Thread Markus Wick


Am 2014-03-09 05:07, schrieb Eric Anholt:

v2:
  - Make the default buffer size a #define. (by Markus Wick)
  - Fix the return offset for mapping with buffer_storage.  (oops!)
v3:
  - Avoid GL error at first rendering from unmapping no buffer.
  - Rebase on the glBindBuffer(GL_ARRAY_BUFFER, 0) change.
v4: Rebase on Markus's vbo init changes.

Signed-off-by: Eric Anholt 
---
 glamor/glamor.c  |  2 ++
 glamor/glamor_priv.h |  1 +
 glamor/glamor_vbo.c  | 51 
+--

 3 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/glamor/glamor.c b/glamor/glamor.c
index dc69c72..e856179 100644
--- a/glamor/glamor.c
+++ b/glamor/glamor.c
@@ -353,6 +353,8 @@ glamor_init(ScreenPtr screen, unsigned int flags)
 glamor_gl_has_extension("GL_MESA_pack_invert");
 glamor_priv->has_fbo_blit =
 glamor_gl_has_extension("GL_EXT_framebuffer_blit");
+glamor_priv->has_buffer_storage =
+glamor_gl_has_extension("GL_ARB_buffer_storage");
 glGetIntegerv(GL_MAX_RENDERBUFFER_SIZE, 
&glamor_priv->max_fbo_size);

 #ifdef MAX_FBO_SIZE
 glamor_priv->max_fbo_size = MAX_FBO_SIZE;
diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h
index 986e729..d15eabd 100644
--- a/glamor/glamor_priv.h
+++ b/glamor/glamor_priv.h
@@ -208,6 +208,7 @@ typedef struct glamor_screen_private {
 enum glamor_gl_flavor gl_flavor;
 int has_pack_invert;
 int has_fbo_blit;
+int has_buffer_storage;
 int max_fbo_size;

 struct xorg_list
diff --git a/glamor/glamor_vbo.c b/glamor/glamor_vbo.c
index be2c2af..f736cbe 100644
--- a/glamor/glamor_vbo.c
+++ b/glamor/glamor_vbo.c
@@ -52,7 +52,49 @@ glamor_get_vbo_space(ScreenPtr screen, unsigned
size, char **vbo_offset)

 glBindBuffer(GL_ARRAY_BUFFER, glamor_priv->vbo);

-if (glamor_priv->gl_flavor == GLAMOR_GL_DESKTOP) {
+if (glamor_priv->has_buffer_storage) {
+if (glamor_priv->vbo_size < glamor_priv->vbo_offset + size) {
+if (glamor_priv->vbo_size)
+glUnmapBuffer(GL_ARRAY_BUFFER);
+
+if (size > glamor_priv->vbo_size) {
+glamor_priv->vbo_size = MAX(GLAMOR_VBO_SIZE, size);
+
+/* We aren't allowed to resize glBufferStorage()
+ * buffers, so we need to gen a new one.
+ */
+glDeleteBuffers(1, &glamor_priv->vbo);
+glGenBuffers(1, &glamor_priv->vbo);
+glBindBuffer(GL_ARRAY_BUFFER, glamor_priv->vbo);
+
+assert(glGetError() == GL_NO_ERROR);


On release builds, glGetError won't be called and so this won't clear 
the gl error log. So we'll fall back to the mbr code because of any gl 
error somewhere in glamor.


+glBufferStorage(GL_ARRAY_BUFFER, 
glamor_priv->vbo_size, NULL,

+GL_MAP_WRITE_BIT |
+GL_MAP_PERSISTENT_BIT |
+GL_MAP_COHERENT_BIT);
+
+if (glGetError() != GL_NO_ERROR) {
+/* If the driver failed our coherent mapping, fall
+ * back to the ARB_mbr path.
+ */
+glamor_priv->has_buffer_storage = false;
+glamor_priv->vbo_size = 0;


glamor_put_context(glamor_priv);

+return glamor_get_vbo_space(screen, size, 
vbo_offset);

+}
+}
+
+glamor_priv->vbo_offset = 0;
+glamor_priv->vb = glMapBufferRange(GL_ARRAY_BUFFER,
+   0, 
glamor_priv->vbo_size,

+   GL_MAP_WRITE_BIT |
+   
GL_MAP_INVALIDATE_BUFFER_BIT |

+   GL_MAP_PERSISTENT_BIT |
+   GL_MAP_COHERENT_BIT);
+}
+*vbo_offset = (void *)(uintptr_t)glamor_priv->vbo_offset;
+data = glamor_priv->vb + glamor_priv->vbo_offset;
+glamor_priv->vbo_offset += size;
+} else if (glamor_priv->gl_flavor == GLAMOR_GL_DESKTOP) {
 if (glamor_priv->vbo_size < glamor_priv->vbo_offset + size) {
 glamor_priv->vbo_size = MAX(GLAMOR_VBO_SIZE, size);
 glamor_priv->vbo_offset = 0;
@@ -99,7 +141,12 @@ glamor_put_vbo_space(ScreenPtr screen)

 glamor_get_context(glamor_priv);

-if (glamor_priv->gl_flavor == GLAMOR_GL_DESKTOP) {
+if (glamor_priv->has_buffer_storage) {
+/* If we're in the ARB_buffer_storage path, we have a
+ * persistent mapping, so we can leave it around until we
+ * reach the end of the buffer.
+ */
+} else if (glamor_priv->gl_flavor == GLAMOR_GL_DESKTOP) {
 glUnmapBuffer(GL_ARRAY_BUFFER);
 } else {
 glBufferData(GL_ARRAY_BUFFER, glamor_priv->vbo_offset,


Which kind of error do you expect for a driver which isn't able to map 
coherent? I've only found a

[PATCH 9/9] glamor: Use buffer_storage

2014-03-08 Thread Eric Anholt

v2:
  - Make the default buffer size a #define. (by Markus Wick)
  - Fix the return offset for mapping with buffer_storage.  (oops!)
v3:
  - Avoid GL error at first rendering from unmapping no buffer.
  - Rebase on the glBindBuffer(GL_ARRAY_BUFFER, 0) change.
v4: Rebase on Markus's vbo init changes.

Signed-off-by: Eric Anholt 
---
 glamor/glamor.c  |  2 ++
 glamor/glamor_priv.h |  1 +
 glamor/glamor_vbo.c  | 51 +--
 3 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/glamor/glamor.c b/glamor/glamor.c
index dc69c72..e856179 100644
--- a/glamor/glamor.c
+++ b/glamor/glamor.c
@@ -353,6 +353,8 @@ glamor_init(ScreenPtr screen, unsigned int flags)
 glamor_gl_has_extension("GL_MESA_pack_invert");
 glamor_priv->has_fbo_blit =
 glamor_gl_has_extension("GL_EXT_framebuffer_blit");
+glamor_priv->has_buffer_storage =
+glamor_gl_has_extension("GL_ARB_buffer_storage");
 glGetIntegerv(GL_MAX_RENDERBUFFER_SIZE, &glamor_priv->max_fbo_size);
 #ifdef MAX_FBO_SIZE
 glamor_priv->max_fbo_size = MAX_FBO_SIZE;
diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h
index 986e729..d15eabd 100644
--- a/glamor/glamor_priv.h
+++ b/glamor/glamor_priv.h
@@ -208,6 +208,7 @@ typedef struct glamor_screen_private {
 enum glamor_gl_flavor gl_flavor;
 int has_pack_invert;
 int has_fbo_blit;
+int has_buffer_storage;
 int max_fbo_size;
 
 struct xorg_list
diff --git a/glamor/glamor_vbo.c b/glamor/glamor_vbo.c
index be2c2af..f736cbe 100644
--- a/glamor/glamor_vbo.c
+++ b/glamor/glamor_vbo.c
@@ -52,7 +52,49 @@ glamor_get_vbo_space(ScreenPtr screen, unsigned size, char 
**vbo_offset)
 
 glBindBuffer(GL_ARRAY_BUFFER, glamor_priv->vbo);
 
-if (glamor_priv->gl_flavor == GLAMOR_GL_DESKTOP) {
+if (glamor_priv->has_buffer_storage) {
+if (glamor_priv->vbo_size < glamor_priv->vbo_offset + size) {
+if (glamor_priv->vbo_size)
+glUnmapBuffer(GL_ARRAY_BUFFER);
+
+if (size > glamor_priv->vbo_size) {
+glamor_priv->vbo_size = MAX(GLAMOR_VBO_SIZE, size);
+
+/* We aren't allowed to resize glBufferStorage()
+ * buffers, so we need to gen a new one.
+ */
+glDeleteBuffers(1, &glamor_priv->vbo);
+glGenBuffers(1, &glamor_priv->vbo);
+glBindBuffer(GL_ARRAY_BUFFER, glamor_priv->vbo);
+
+assert(glGetError() == GL_NO_ERROR);
+glBufferStorage(GL_ARRAY_BUFFER, glamor_priv->vbo_size, NULL,
+GL_MAP_WRITE_BIT |
+GL_MAP_PERSISTENT_BIT |
+GL_MAP_COHERENT_BIT);
+
+if (glGetError() != GL_NO_ERROR) {
+/* If the driver failed our coherent mapping, fall
+ * back to the ARB_mbr path.
+ */
+glamor_priv->has_buffer_storage = false;
+glamor_priv->vbo_size = 0;
+return glamor_get_vbo_space(screen, size, vbo_offset);
+}
+}
+
+glamor_priv->vbo_offset = 0;
+glamor_priv->vb = glMapBufferRange(GL_ARRAY_BUFFER,
+   0, glamor_priv->vbo_size,
+   GL_MAP_WRITE_BIT |
+   GL_MAP_INVALIDATE_BUFFER_BIT |
+   GL_MAP_PERSISTENT_BIT |
+   GL_MAP_COHERENT_BIT);
+}
+*vbo_offset = (void *)(uintptr_t)glamor_priv->vbo_offset;
+data = glamor_priv->vb + glamor_priv->vbo_offset;
+glamor_priv->vbo_offset += size;
+} else if (glamor_priv->gl_flavor == GLAMOR_GL_DESKTOP) {
 if (glamor_priv->vbo_size < glamor_priv->vbo_offset + size) {
 glamor_priv->vbo_size = MAX(GLAMOR_VBO_SIZE, size);
 glamor_priv->vbo_offset = 0;
@@ -99,7 +141,12 @@ glamor_put_vbo_space(ScreenPtr screen)
 
 glamor_get_context(glamor_priv);
 
-if (glamor_priv->gl_flavor == GLAMOR_GL_DESKTOP) {
+if (glamor_priv->has_buffer_storage) {
+/* If we're in the ARB_buffer_storage path, we have a
+ * persistent mapping, so we can leave it around until we
+ * reach the end of the buffer.
+ */
+} else if (glamor_priv->gl_flavor == GLAMOR_GL_DESKTOP) {
 glUnmapBuffer(GL_ARRAY_BUFFER);
 } else {
 glBufferData(GL_ARRAY_BUFFER, glamor_priv->vbo_offset,
-- 
1.9.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 9/9] glamor: Use buffer_storage

Re: [PATCH 9/9] glamor: Use buffer_storage

Re: [PATCH 9/9] glamor: Use buffer_storage

[PATCH 9/9] glamor: Use buffer_storage

4 matches

Site Navigation

Mail list logo

Footer information