mesa3d-dev

[Mesa3d-dev] Revert ST_SURFACE_DEPTH = BUFFER_DEPTH in master too?

[Luca B. <lu...@lu...>]

In mesa_7_7_branch, 52d83efdbc4735d721e6fc9b44f29bdd432d4d73 reverts
commit 9d17ad2891b58de9e33e943ff918a678c6a3c2bd.

How about cherry-picking that commit into master, until a fix for the
bugs the revert commit introduces are found?

The reverted commit currently breaks the Warsow main menu for me,
making it show garbage.

Re: [Mesa3d-dev] Revert ST_SURFACE_DEPTH = BUFFER_DEPTH in master too?

[Marek O. <ma...@gm...>]

I second that. The commit breaks 6 glean tests (api2, clipFlat, fragProg1,
occluQry, pointAtten, texCombine4) with r300g.

-Marek

On Wed, Mar 10, 2010 at 10:50 PM, Luca Barbieri <lu...@lu...>wrote:

> In mesa_7_7_branch, 52d83efdbc4735d721e6fc9b44f29bdd432d4d73 reverts
> commit 9d17ad2891b58de9e33e943ff918a678c6a3c2bd.
>
> How about cherry-picking that commit into master, until a fix for the
> bugs the revert commit introduces are found?
>
> The reverted commit currently breaks the Warsow main menu for me,
> making it show garbage.
>
>
> ------------------------------------------------------------------------------
> Download Intel&#174; Parallel Studio Eval
> Try the new software tools for yourself. Speed compiling, find bugs
> proactively, and fine-tune applications for parallel performance.
> See why Intel Parallel Studio got high marks during beta.
> http://p.sf.net/sfu/intel-sw-dev
> _______________________________________________
> Mesa3d-dev mailing list
> Mes...@li...
> https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
>

Re: [Mesa3d-dev] Revert ST_SURFACE_DEPTH = BUFFER_DEPTH in master too?

[Brian P. <bri...@gm...>]

Mesa/master is correct as is.  Changing ST_SURFACE_DEPTH to be !=
BUFFER_DEPTH is just hiding another problem elsewhere.

If ST_SURFACE_DEPTH==8 then calling st_set_framebuffer_surface(fb,
ST_SURFACE_DEPTH, surf) is effectively setting the fb's COLOR0
attachment to be a Z/stencil buffer (and leaves the fb's DEPTH
attachment undefined (or set to a default surface)).  I'm surprised
that doesn't cause tons of problems elsewhere.

To debug this, I'd start by looking for calls to
st_set_framebuffer_surface() with surfIndex==ST_SURFACE_DEPTH, then
no-op those calls.  That's roughly what would be happening if
ST_SURFACE_DEPTH==8.

-Brian


On Wed, Mar 10, 2010 at 6:33 PM, Marek Olšák <ma...@gm...> wrote:
> I second that. The commit breaks 6 glean tests (api2, clipFlat, fragProg1,
> occluQry, pointAtten, texCombine4) with r300g.
>
> -Marek
>
> On Wed, Mar 10, 2010 at 10:50 PM, Luca Barbieri <lu...@lu...>
> wrote:
>>
>> In mesa_7_7_branch, 52d83efdbc4735d721e6fc9b44f29bdd432d4d73 reverts
>> commit 9d17ad2891b58de9e33e943ff918a678c6a3c2bd.
>>
>> How about cherry-picking that commit into master, until a fix for the
>> bugs the revert commit introduces are found?
>>
>> The reverted commit currently breaks the Warsow main menu for me,
>> making it show garbage.
>>
>>
>> ------------------------------------------------------------------------------
>> Download Intel&#174; Parallel Studio Eval
>> Try the new software tools for yourself. Speed compiling, find bugs
>> proactively, and fine-tune applications for parallel performance.
>> See why Intel Parallel Studio got high marks during beta.
>> http://p.sf.net/sfu/intel-sw-dev
>> _______________________________________________
>> Mesa3d-dev mailing list
>> Mes...@li...
>> https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
>
>
> ------------------------------------------------------------------------------
> Download Intel&#174; Parallel Studio Eval
> Try the new software tools for yourself. Speed compiling, find bugs
> proactively, and fine-tune applications for parallel performance.
> See why Intel Parallel Studio got high marks during beta.
> http://p.sf.net/sfu/intel-sw-dev
> _______________________________________________
> Mesa3d-dev mailing list
> Mes...@li...
> https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
>
>

Re: [Mesa3d-dev] Revert ST_SURFACE_DEPTH = BUFFER_DEPTH in master too?

[Brian P. <br...@vm...>]

Luca Barbieri wrote:
> I've looked into the issue, and found a workaround by looking at what
> st_renderbuffer_alloc_storage (which is called to create the depth
> buffer with ST_SURFACE_DEPTH != BUFFER_DEPTH) does.
> 
> Adding:
> if(ctx) ctx->NewState |= _NEW_BUFFERS;
> 
> at the end of st_set_framebuffer_surface seems to solve the warsow
> problem with no other regressions.
> 
> Brian, is this the right fix?

That's probably the right direction, but I think there's several other 
things to be fixed in st_set_framebuffer_surface().  I'll have a patch 
soon...

-Brian

Re: [Mesa3d-dev] Revert ST_SURFACE_DEPTH = BUFFER_DEPTH in master too?

[Luca B. <lu...@lu...>]

I've looked into the issue, and found a workaround by looking at what
st_renderbuffer_alloc_storage (which is called to create the depth
buffer with ST_SURFACE_DEPTH != BUFFER_DEPTH) does.

Adding:
if(ctx) ctx->NewState |= _NEW_BUFFERS;

at the end of st_set_framebuffer_surface seems to solve the warsow
problem with no other regressions.

Brian, is this the right fix?
Marek, does it fix your r300g problems too?

Re: [Mesa3d-dev] Revert ST_SURFACE_DEPTH = BUFFER_DEPTH in master too?

[Marek O. <ma...@gm...>]

On Thu, Mar 11, 2010 at 4:41 PM, Luca Barbieri <lu...@lu...>wrote:

> I've looked into the issue, and found a workaround by looking at what
> st_renderbuffer_alloc_storage (which is called to create the depth
> buffer with ST_SURFACE_DEPTH != BUFFER_DEPTH) does.
>
> Adding:
> if(ctx) ctx->NewState |= _NEW_BUFFERS;
>
> at the end of st_set_framebuffer_surface seems to solve the warsow
> problem with no other regressions.
>
> Brian, is this the right fix?
> Marek, does it fix your r300g problems too?
>

Mesa master merged with 7.8 fixes all the glean regressions. Thanks.

Re: [Mesa3d-dev] Revert ST_SURFACE_DEPTH = BUFFER_DEPTH in master too?

[Brian P. <br...@vm...>]

Brian Paul wrote:
> Luca Barbieri wrote:
>> I've looked into the issue, and found a workaround by looking at what
>> st_renderbuffer_alloc_storage (which is called to create the depth
>> buffer with ST_SURFACE_DEPTH != BUFFER_DEPTH) does.
>>
>> Adding:
>> if(ctx) ctx->NewState |= _NEW_BUFFERS;
>>
>> at the end of st_set_framebuffer_surface seems to solve the warsow
>> problem with no other regressions.
>>
>> Brian, is this the right fix?
> 
> That's probably the right direction, but I think there's several other 
> things to be fixed in st_set_framebuffer_surface().  I'll have a patch 
> soon...

OK, here's a patch which sets that flag, and:

1. always allocates the renderbuffer if it's not already present.

2. removes the framebuffer size update code.  Core Mesa will do this 
during state validation if _NEW_BUFFERS is set.

Please test.  Thanks.

-Brian

Re: [Mesa3d-dev] Revert ST_SURFACE_DEPTH = BUFFER_DEPTH in master too?

[Luca B. <lu...@lu...>]

Solves the Warsow issue and seems to work.
Thanks!

Re: [Mesa3d-dev] Revert ST_SURFACE_DEPTH = BUFFER_DEPTH in master too?

[Brian P. <br...@vm...>]

Luca Barbieri wrote:
> Solves the Warsow issue and seems to work.

OK, I think you can commit the patch to 7.8 then.

-Brian

Re: [Mesa3d-dev] Revert ST_SURFACE_DEPTH = BUFFER_DEPTH in master too?

[Michel D. <mi...@da...>]

On Thu, 2010-03-11 at 11:28 -0700, Brian Paul wrote: 
> Luca Barbieri wrote:
> > Solves the Warsow issue and seems to work.
> 
> OK, I think you can commit the patch to 7.8 then.

Can this also be backported to mesa_7_7_branch?


-- 
Earthling Michel Dänzer           |                http://www.vmware.com
Libre software enthusiast         |          Debian, X and DRI developer

Re: [Mesa3d-dev] Revert ST_SURFACE_DEPTH = BUFFER_DEPTH in master too?

[Brian P. <br...@vm...>]

Michel Dänzer wrote:
> On Thu, 2010-03-11 at 11:28 -0700, Brian Paul wrote: 
>> Luca Barbieri wrote:
>>> Solves the Warsow issue and seems to work.
>> OK, I think you can commit the patch to 7.8 then.
> 
> Can this also be backported to mesa_7_7_branch?

Sure, if you can test it.  We might want to wait a few days and make 
sure nothing regresses with it on the 7.8 branch.  I'll commit it there.

-Brian

Re: [Mesa3d-dev] Revert ST_SURFACE_DEPTH = BUFFER_DEPTH in master too?

[Luca B. <lu...@lu...>]

Shouldn't

_mesa_add_renderbuffer(&stfb->Base, BUFFER_FRONT_LEFT, rb);

be

_mesa_add_renderbuffer(&stfb->Base, surfIndex, rb);

instead, since you seem to make the on-demand creation mechanism
generic and no longer limited to the front buffer?

Re: [Mesa3d-dev] Revert ST_SURFACE_DEPTH = BUFFER_DEPTH in master too?

[Brian P. <br...@vm...>]

Luca Barbieri wrote:
> Shouldn't
> 
> _mesa_add_renderbuffer(&stfb->Base, BUFFER_FRONT_LEFT, rb);
> 
> be
> 
> _mesa_add_renderbuffer(&stfb->Base, surfIndex, rb);
> 
> instead, since you seem to make the on-demand creation mechanism
> generic and no longer limited to the front buffer?

Yes. Fixed now.

-Brian