Bug 88962 - [osmesa] Crash on postprocessing if z buffer is NULL
Summary: [osmesa] Crash on postprocessing if z buffer is NULL
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Other (show other bugs)
Version: git
Hardware: All All
: medium normal
Assignee: mesa-dev
QA Contact: mesa-dev
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-02-04 06:18 UTC by Park, Jeongmin
Modified: 2015-02-10 01:16 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
patch #1 (1.44 KB, patch)
2015-02-04 06:18 UTC, Park, Jeongmin
Details | Splinter Review
patch #2 (944 bytes, patch)
2015-02-04 06:19 UTC, Park, Jeongmin
Details | Splinter Review

Description Park, Jeongmin 2015-02-04 06:18:08 UTC
It seems that z buffer is not optional to pp_run looking at other codes
but it is not checked in osmesa_st_framebuffer_flush_front().

It crashes if zsbuf is NULL with pp_jimenezmlaa. With pp_nored it doesn't crash,
but if pipe_sampler_view.reference is not the first member in
struct pipe_sampler_view, it would crash in pipe_sampler_view_reference at
>   if (pipe_reference_described(&(*ptr)->reference, &view->reference,
>                                (debug_reference_descriptor)debug_describe_sampler_view))
Comment 1 Park, Jeongmin 2015-02-04 06:18:58 UTC
Created attachment 113147 [details] [review]
patch #1
Comment 2 Park, Jeongmin 2015-02-04 06:19:15 UTC
Created attachment 113148 [details] [review]
patch #2
Comment 3 Alex Deucher 2015-02-04 14:23:47 UTC
Please send the patches to the mesa-dev mailing list if you haven't already.
Comment 4 Brian Paul 2015-02-04 16:47:21 UTC
The first patch looks good.  I posted something similar to mesa-dev yesterday.

But the second patch doesn't seem right.  There could be post-process stages that don't care about the depth/stencil buffer but we'd still want them to run.  So instead, I think the pp_mlaa.c code should check for a missing/null depth/stencil buffer.
Comment 5 Park, Jeongmin 2015-02-04 23:29:21 UTC
Sorry about the first patch. I didn't see that.

In dri_sw.c, it checks for z/s buffer:
>      if (ctx->pp && drawable->textures[ST_ATTACHMENT_DEPTH_STENCIL])
>         pp_run(ctx->pp, ptex, ptex, drawable->textures[ST_ATTACHMENT_DEPTH_STENCIL]);
and in dri_drawable.c:
>   if (ctx->pp && src && zsbuf)
>      pp_run(ctx->pp, src, src, zsbuf);

Should I remove these checks and add the checks in pp_mlaa.cc instead?
Comment 6 Brian Paul 2015-02-06 15:43:32 UTC
(In reply to Park, Jeongmin from comment #5)
> Sorry about the first patch. I didn't see that.
> 
> In dri_sw.c, it checks for z/s buffer:
> >      if (ctx->pp && drawable->textures[ST_ATTACHMENT_DEPTH_STENCIL])
> >         pp_run(ctx->pp, ptex, ptex, drawable->textures[ST_ATTACHMENT_DEPTH_STENCIL]);
> and in dri_drawable.c:
> >   if (ctx->pp && src && zsbuf)
> >      pp_run(ctx->pp, src, src, zsbuf);
> 
> Should I remove these checks and add the checks in pp_mlaa.cc instead?

Yes, I think that would be better.
Comment 7 Park, Jeongmin 2015-02-10 01:16:13 UTC
Fixed by 2e6ba6af and 0467a52d.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.