Re: [Mesa-dev] [PATCH v2] st/mesa: fix mipmap generation for immutable textures with incomplete pyramids

2015-10-29 Thread Marek Olšák 
On Wed, Oct 28, 2015 at 1:00 PM, Nicolai Hähnle  wrote:
> Without the clamping by NumLevels, the state tracker would reallocate the
> texture storage (incorrect) and even fail to copy the base level image
> after reallocation, leading to the graphical glitch of
> https://bugs.freedesktop.org/show_bug.cgi?id=91993 .
>
> A piglit test has been submitted for review as well (subtest of
> arb_texture_storage-texture-storage).
>
> v2: also bypass all calls to st_finalize_texture (suggested by Marek Olšák)
>
> Cc: mesa-sta...@lists.freedesktop.org
> Reviewed-by: Marek Olšák 

This looks good.

(a minor nit: an updated patch should not contain any reviewed-by
tags, because the updated version hadn't been seen by anybody at the
time of sending it to the list; it's okay to keep the tag now that
I've reviewed it)

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2] st/mesa: fix mipmap generation for immutable textures with incomplete pyramids

2015-10-29 Thread Nicolai Hähnle 

On 29.10.2015 14:13, Marek Olšák wrote:

On Wed, Oct 28, 2015 at 1:00 PM, Nicolai Hähnle  wrote:

Without the clamping by NumLevels, the state tracker would reallocate the
texture storage (incorrect) and even fail to copy the base level image
after reallocation, leading to the graphical glitch of
https://bugs.freedesktop.org/show_bug.cgi?id=91993 .

A piglit test has been submitted for review as well (subtest of
arb_texture_storage-texture-storage).

v2: also bypass all calls to st_finalize_texture (suggested by Marek Olšák)

Cc: mesa-sta...@lists.freedesktop.org
Reviewed-by: Marek Olšák 


This looks good.

(a minor nit: an updated patch should not contain any reviewed-by
tags, because the updated version hadn't been seen by anybody at the
time of sending it to the list; it's okay to keep the tag now that
I've reviewed it)


Sorry about that, I'll be more careful about that in the future.

Nicolai



Marek



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2] st/mesa: fix mipmap generation for immutable textures with incomplete pyramids

2015-10-29 Thread Emil Velikov 
On 29 October 2015 at 22:56, Nicolai Hähnle  wrote:
> On 29.10.2015 14:13, Marek Olšák wrote:
>>
>> On Wed, Oct 28, 2015 at 1:00 PM, Nicolai Hähnle 
>> wrote:
>>>
>>> Without the clamping by NumLevels, the state tracker would reallocate the
>>> texture storage (incorrect) and even fail to copy the base level image
>>> after reallocation, leading to the graphical glitch of
>>> https://bugs.freedesktop.org/show_bug.cgi?id=91993 .
>>>
>>> A piglit test has been submitted for review as well (subtest of
>>> arb_texture_storage-texture-storage).
>>>
>>> v2: also bypass all calls to st_finalize_texture (suggested by Marek
>>> Olšák)
>>>
>>> Cc: mesa-sta...@lists.freedesktop.org
>>> Reviewed-by: Marek Olšák 
>>
>>
>> This looks good.
>>
>> (a minor nit: an updated patch should not contain any reviewed-by
>> tags, because the updated version hadn't been seen by anybody at the
>> time of sending it to the list; it's okay to keep the tag now that
>> I've reviewed it)
>
>
> Sorry about that, I'll be more careful about that in the future.
>
Fwiw some of us tend to add the revision based on which the r-b was
given, when the revision history is present in the commit message.
Something like:

v2: Changed foo and bar.

Reviewed-by: Slim Shady  (v1)

Then again, you might want to follow your colleague's suggestion.
-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v2] st/mesa: fix mipmap generation for immutable textures with incomplete pyramids

2015-10-28 Thread Nicolai Hähnle 
Without the clamping by NumLevels, the state tracker would reallocate the
texture storage (incorrect) and even fail to copy the base level image
after reallocation, leading to the graphical glitch of
https://bugs.freedesktop.org/show_bug.cgi?id=91993 .

A piglit test has been submitted for review as well (subtest of
arb_texture_storage-texture-storage).

v2: also bypass all calls to st_finalize_texture (suggested by Marek Olšák)

Cc: mesa-sta...@lists.freedesktop.org
Reviewed-by: Marek Olšák 
---
 src/mesa/state_tracker/st_gen_mipmap.c | 68 ++
 1 file changed, 36 insertions(+), 32 deletions(-)

diff --git a/src/mesa/state_tracker/st_gen_mipmap.c 
b/src/mesa/state_tracker/st_gen_mipmap.c
index 26e1c21..b370040 100644
--- a/src/mesa/state_tracker/st_gen_mipmap.c
+++ b/src/mesa/state_tracker/st_gen_mipmap.c
@@ -61,6 +61,8 @@ compute_num_levels(struct gl_context *ctx,
 
numLevels = texObj->BaseLevel + baseImage->MaxNumLevels;
numLevels = MIN2(numLevels, (GLuint) texObj->MaxLevel + 1);
+   if (texObj->Immutable)
+  numLevels = MIN2(numLevels, texObj->NumLevels);
assert(numLevels >= 1);
 
return numLevels;
@@ -99,38 +101,40 @@ st_generate_mipmap(struct gl_context *ctx, GLenum target,
 */
stObj->lastLevel = lastLevel;
 
-   if (pt->last_level < lastLevel) {
-  /* The current gallium texture doesn't have space for all the
-   * mipmap levels we need to generate.  So allocate a new texture.
-   */
-  struct pipe_resource *oldTex = stObj->pt;
-
-  /* create new texture with space for more levels */
-  stObj->pt = st_texture_create(st,
-oldTex->target,
-oldTex->format,
-lastLevel,
-oldTex->width0,
-oldTex->height0,
-oldTex->depth0,
-oldTex->array_size,
-0,
-oldTex->bind);
-
-  /* This will copy the old texture's base image into the new texture
-   * which we just allocated.
-   */
-  st_finalize_texture(ctx, st->pipe, texObj);
-
-  /* release the old tex (will likely be freed too) */
-  pipe_resource_reference(, NULL);
-  st_texture_release_all_sampler_views(st, stObj);
-   }
-   else {
-  /* Make sure that the base texture image data is present in the
-   * texture buffer.
-   */
-  st_finalize_texture(ctx, st->pipe, texObj);
+   if (!texObj->Immutable) {
+  if (pt->last_level < lastLevel) {
+ /* The current gallium texture doesn't have space for all the
+ * mipmap levels we need to generate.  So allocate a new texture.
+ */
+ struct pipe_resource *oldTex = stObj->pt;
+
+ /* create new texture with space for more levels */
+ stObj->pt = st_texture_create(st,
+   oldTex->target,
+   oldTex->format,
+   lastLevel,
+   oldTex->width0,
+   oldTex->height0,
+   oldTex->depth0,
+   oldTex->array_size,
+   0,
+   oldTex->bind);
+
+ /* This will copy the old texture's base image into the new texture
+ * which we just allocated.
+ */
+ st_finalize_texture(ctx, st->pipe, texObj);
+
+ /* release the old tex (will likely be freed too) */
+ pipe_resource_reference(, NULL);
+ st_texture_release_all_sampler_views(st, stObj);
+  }
+  else {
+ /* Make sure that the base texture image data is present in the
+ * texture buffer.
+ */
+ st_finalize_texture(ctx, st->pipe, texObj);
+  }
}
 
pt = stObj->pt;
-- 
2.5.0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev