Re: [Mesa-dev] [PATCH] i965/tex: ignore the diff between GL_TEXTURE_2D and GL_TEXTURE_RECTANGLE
Hi, I opened MR to fix this issue and started a discussion about the proper solution: https://gitlab.freedesktop.org/mesa/mesa/merge_requests/1258 Thanks, Andrii. On Wed, Jul 24, 2019 at 10:32 AM Olivier Fourdan wrote: > Hi, > > On Wed, May 22, 2019 at 4:01 AM Ian Romanick wrote: > > > On Thu, Jul 19, 2018 at 12:08 PM andrey simiklit > > > > > > Jason, can we reconsider Andrii's patch? It still applies cleanly > > > (https://patchwork.freedesktop.org/patch/237490/) > > > > Looking at the patch and the "Simple reproducer" in the bug, I think > > this just papers over the issue. It seems like the problem is somewhere > > down inside the driver's handling of glXBindTexImageEXT. My best guess > > is that the texture is GL_TEXTURE_2D but the miptree backing it is > > GL_TEXTURE_RECTANGLE. It seems that the glXBindTexImageEXT handling > > should mark the miptree as GL_TEXTURE_2D when binding the image to a > > texture that is GL_TEXTURE_2D. Or is that not possible for some > > non-obvious reason? > > Sorry to be a pain, but I still get bug reports in xfce for this, for > everyone using a Mesa build with debug enabled (or a pre-release for > that matter) on Intel, the `assert()` are enabled and this bug occurs > with the xfce compositor. > > Maybe Andrii's patch is just hiding the issue, but that's already the > case without the `assert()` enabled (i.e. all stable releases of > Mesa), so I guess it's not such a big deal anyway. > > Can we agree on a fix on this? > > Cheers, > Olivier > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/tex: ignore the diff between GL_TEXTURE_2D and GL_TEXTURE_RECTANGLE
Hi, On Wed, May 22, 2019 at 4:01 AM Ian Romanick wrote: > > On Thu, Jul 19, 2018 at 12:08 PM andrey simiklit > > > > Jason, can we reconsider Andrii's patch? It still applies cleanly > > (https://patchwork.freedesktop.org/patch/237490/) > > Looking at the patch and the "Simple reproducer" in the bug, I think > this just papers over the issue. It seems like the problem is somewhere > down inside the driver's handling of glXBindTexImageEXT. My best guess > is that the texture is GL_TEXTURE_2D but the miptree backing it is > GL_TEXTURE_RECTANGLE. It seems that the glXBindTexImageEXT handling > should mark the miptree as GL_TEXTURE_2D when binding the image to a > texture that is GL_TEXTURE_2D. Or is that not possible for some > non-obvious reason? Sorry to be a pain, but I still get bug reports in xfce for this, for everyone using a Mesa build with debug enabled (or a pre-release for that matter) on Intel, the `assert()` are enabled and this bug occurs with the xfce compositor. Maybe Andrii's patch is just hiding the issue, but that's already the case without the `assert()` enabled (i.e. all stable releases of Mesa), so I guess it's not such a big deal anyway. Can we agree on a fix on this? Cheers, Olivier ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/tex: ignore the diff between GL_TEXTURE_2D and GL_TEXTURE_RECTANGLE
On 5/21/19 4:36 AM, Olivier Fourdan wrote: > Hi all, > > On Thu, Jul 19, 2018 at 12:08 PM andrey simiklit > wrote: >>> Ugh... not so good. According to Oliver on the bug, this just make the >>> assert go away and doesn't actually fix anything. Likely this is needed >>> but not sufficient. >> >> So as far as I understand Oliver found the bad commit in xorg glamor: >> https://bugs.freedesktop.org/show_bug.cgi?id=107287 >> >> So at the moment we should fix just this "assertion" issue for Intel because >> "rendering" issue came from xorg/glamor and there is no "rendering" issue in >> Intel part. >> Please correct me if I incorrect. > > Reviving an old thread/patch here. > > Andrey, I reckon your patch here is still much needed as it fixes the > assert() issue: > > intel_mipmap_tree.c:1301: intel_miptree_match_image: Assertion > `image->TexObject->Target == mt->target' failed. > > Which is still occurring even with current master. > > My patch was to fix the rendering issue (landed a while ago before > 18.1 iirc), but yours was never merged and is still needed, I can > reproduce the assert() at will with the reproducer from > https://bugs.freedesktop.org/show_bug.cgi?id=107117 > > Jason, can we reconsider Andrii's patch? It still applies cleanly > (https://patchwork.freedesktop.org/patch/237490/) Looking at the patch and the "Simple reproducer" in the bug, I think this just papers over the issue. It seems like the problem is somewhere down inside the driver's handling of glXBindTexImageEXT. My best guess is that the texture is GL_TEXTURE_2D but the miptree backing it is GL_TEXTURE_RECTANGLE. It seems that the glXBindTexImageEXT handling should mark the miptree as GL_TEXTURE_2D when binding the image to a texture that is GL_TEXTURE_2D. Or is that not possible for some non-obvious reason? > Cheers, > Olivier > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/tex: ignore the diff between GL_TEXTURE_2D and GL_TEXTURE_RECTANGLE
> > Hi all, > > On Thu, Jul 19, 2018 at 12:08 PM andrey simiklit > wrote: > > > Ugh... not so good. According to Oliver on the bug, this just make > the assert go away and doesn't actually fix anything. Likely this is > needed but not sufficient. > > > > So as far as I understand Oliver found the bad commit in xorg glamor: > > https://bugs.freedesktop.org/show_bug.cgi?id=107287 > > > > So at the moment we should fix just this "assertion" issue for Intel > because "rendering" issue came from xorg/glamor and there is no "rendering" > issue in Intel part. > > Please correct me if I incorrect. > > Reviving an old thread/patch here. > > Andrey, I reckon your patch here is still much needed as it fixes the > assert() issue: > > intel_mipmap_tree.c:1301: intel_miptree_match_image: Assertion > `image->TexObject->Target == mt->target' failed. > > Which is still occurring even with current master. > > My patch was to fix the rendering issue (landed a while ago before > 18.1 iirc), but yours was never merged and is still needed, I can > reproduce the assert() at will with the reproducer from > https://bugs.freedesktop.org/show_bug.cgi?id=107117 > > I could re-create this changes as MR instead of patch unless nobody against it. I fixed a bit the style of this patch to avoid new local variables: https://gitlab.freedesktop.org/asimiklit/mesa/commit/c11aae1cd403dde240c3877974f6a4b467cc83f5 > Jason, can we reconsider Andrii's patch? It still applies cleanly > (https://patchwork.freedesktop.org/patch/237490/) > > Cheers, > Olivier > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/tex: ignore the diff between GL_TEXTURE_2D and GL_TEXTURE_RECTANGLE
Hi all, On Thu, Jul 19, 2018 at 12:08 PM andrey simiklit wrote: > > Ugh... not so good. According to Oliver on the bug, this just make the > > assert go away and doesn't actually fix anything. Likely this is needed > > but not sufficient. > > So as far as I understand Oliver found the bad commit in xorg glamor: > https://bugs.freedesktop.org/show_bug.cgi?id=107287 > > So at the moment we should fix just this "assertion" issue for Intel because > "rendering" issue came from xorg/glamor and there is no "rendering" issue in > Intel part. > Please correct me if I incorrect. Reviving an old thread/patch here. Andrey, I reckon your patch here is still much needed as it fixes the assert() issue: intel_mipmap_tree.c:1301: intel_miptree_match_image: Assertion `image->TexObject->Target == mt->target' failed. Which is still occurring even with current master. My patch was to fix the rendering issue (landed a while ago before 18.1 iirc), but yours was never merged and is still needed, I can reproduce the assert() at will with the reproducer from https://bugs.freedesktop.org/show_bug.cgi?id=107117 Jason, can we reconsider Andrii's patch? It still applies cleanly (https://patchwork.freedesktop.org/patch/237490/) Cheers, Olivier ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/tex: ignore the diff between GL_TEXTURE_2D and GL_TEXTURE_RECTANGLE
Hi, > Ugh... not so good. According to Oliver on the bug, this just make the assert go away and doesn't actually fix anything. Likely this is needed but not sufficient. So as far as I understand Oliver found the bad commit in xorg glamor: https://bugs.freedesktop.org/show_bug.cgi?id=107287 So at the moment we should fix just this "assertion" issue for Intel because "rendering" issue came from xorg/glamor and there is no "rendering" issue in Intel part. Please correct me if I incorrect. Regards, Andrii. On Tue, Jul 10, 2018 at 8:04 PM, Olivier Fourdan wrote: > Hi, > > On Tue, 10 Jul 2018 at 18:56, Jason Ekstrand wrote: > > > > Ugh... not so good. According to Oliver on the bug, this just make the > assert go away and doesn't actually fix anything. Likely this is needed > but not sufficient. > > Well, maybe not even needed, at least in my case I don't hit that > assert() with the current Mesa code (from either master and 18.1), > I've just seen that happening with past commits while doing the > bisection in git... So adding this now wouldn't help with bisection > (again, in my case, not sure about others). > > All I meant pointing at this assert() failure is that it breaks the > bisection in git as I am unable to tell if the rendering is correct or > not as it makes the test program abort. > > Cheers, > Olivier > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/tex: ignore the diff between GL_TEXTURE_2D and GL_TEXTURE_RECTANGLE
Hi, On Tue, 10 Jul 2018 at 18:56, Jason Ekstrand wrote: > > Ugh... not so good. According to Oliver on the bug, this just make the > assert go away and doesn't actually fix anything. Likely this is needed but > not sufficient. Well, maybe not even needed, at least in my case I don't hit that assert() with the current Mesa code (from either master and 18.1), I've just seen that happening with past commits while doing the bisection in git... So adding this now wouldn't help with bisection (again, in my case, not sure about others). All I meant pointing at this assert() failure is that it breaks the bisection in git as I am unable to tell if the rendering is correct or not as it makes the test program abort. Cheers, Olivier ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/tex: ignore the diff between GL_TEXTURE_2D and GL_TEXTURE_RECTANGLE
Ugh... not so good. According to Oliver on the bug, this just make the
assert go away and doesn't actually fix anything. Likely this is needed
but not sufficient.
--Jason
On Tue, Jul 10, 2018 at 8:17 AM Jason Ekstrand wrote:
> On Tue, Jul 10, 2018 at 4:13 AM Andrii Simiklit
> wrote:
>
>> the difference between GL_TEXTURE_2D and GL_TEXTURE_RECTANGLE
>> doesn't matter as far as the miptree is concerned;
>> genX(update_sampler_state) only looks at the
>> gl_texture_object and not the miptree when determining whether or
>> not to use normalized coordinates.
>>
>> Signed-off-by: Andrii Simiklit
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107117
>>
>> ---
>> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 10 +-
>> 1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> index 7d1fa96..dc45a06 100644
>> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> @@ -58,6 +58,12 @@ static void *intel_miptree_map_raw(struct brw_context
>> *brw,
>>
>> static void intel_miptree_unmap_raw(struct intel_mipmap_tree *mt);
>>
>> +static GLenum
>> +tex_rect_to_tex2d(GLenum val)
>> +{
>> +return (GL_TEXTURE_RECTANGLE == val) ? GL_TEXTURE_2D : val;
>> +}
>> +
>> static bool
>> intel_miptree_supports_mcs(struct brw_context *brw,
>> const struct intel_mipmap_tree *mt)
>> @@ -1320,13 +1326,15 @@ intel_miptree_match_image(struct
>> intel_mipmap_tree *mt,
>> {
>> struct intel_texture_image *intelImage = intel_texture_image(image);
>> GLuint level = intelImage->base.Base.Level;
>> + GLenum texObjTarget = tex_rect_to_tex2d(mt->target);
>> + GLenum mipmapTreeTarget = tex_rect_to_tex2d(image->TexObject->Target);
>> int width, height, depth;
>>
>> /* glTexImage* choose the texture object based on the target passed
>> in, and
>> * objects can't change targets over their lifetimes, so this should
>> be
>> * true.
>> */
>> - assert(image->TexObject->Target == mt->target);
>> + assert(texObjTarget == mipmapTreeTarget);
>>
>
> If the only use of that helper is in an assert, just put the calls to the
> helper inside the assert. Otherwise, good job debugging. :-)
>
> --Jason
>
>
>> mesa_format mt_format = mt->format;
>> if (mt->format == MESA_FORMAT_Z24_UNORM_X8_UINT && mt->stencil_mt)
>> --
>> 2.7.4
>>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965/tex: ignore the diff between GL_TEXTURE_2D and GL_TEXTURE_RECTANGLE
On Tue, Jul 10, 2018 at 4:13 AM Andrii Simiklit
wrote:
> the difference between GL_TEXTURE_2D and GL_TEXTURE_RECTANGLE
> doesn't matter as far as the miptree is concerned;
> genX(update_sampler_state) only looks at the
> gl_texture_object and not the miptree when determining whether or
> not to use normalized coordinates.
>
> Signed-off-by: Andrii Simiklit
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107117
>
> ---
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 7d1fa96..dc45a06 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -58,6 +58,12 @@ static void *intel_miptree_map_raw(struct brw_context
> *brw,
>
> static void intel_miptree_unmap_raw(struct intel_mipmap_tree *mt);
>
> +static GLenum
> +tex_rect_to_tex2d(GLenum val)
> +{
> +return (GL_TEXTURE_RECTANGLE == val) ? GL_TEXTURE_2D : val;
> +}
> +
> static bool
> intel_miptree_supports_mcs(struct brw_context *brw,
> const struct intel_mipmap_tree *mt)
> @@ -1320,13 +1326,15 @@ intel_miptree_match_image(struct intel_mipmap_tree
> *mt,
> {
> struct intel_texture_image *intelImage = intel_texture_image(image);
> GLuint level = intelImage->base.Base.Level;
> + GLenum texObjTarget = tex_rect_to_tex2d(mt->target);
> + GLenum mipmapTreeTarget = tex_rect_to_tex2d(image->TexObject->Target);
> int width, height, depth;
>
> /* glTexImage* choose the texture object based on the target passed
> in, and
> * objects can't change targets over their lifetimes, so this should be
> * true.
> */
> - assert(image->TexObject->Target == mt->target);
> + assert(texObjTarget == mipmapTreeTarget);
>
If the only use of that helper is in an assert, just put the calls to the
helper inside the assert. Otherwise, good job debugging. :-)
--Jason
> mesa_format mt_format = mt->format;
> if (mt->format == MESA_FORMAT_Z24_UNORM_X8_UINT && mt->stencil_mt)
> --
> 2.7.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
