On Fri, Jun 15, 2012 at 8:40 PM, Brian Paul <brian.e.p...@gmail.com> wrote:
> On Fri, Jun 15, 2012 at 12:29 PM, Marek Olšák <mar...@gmail.com> wrote:
>> Hi Jerome,
>>
>> Your patch somehow fixes all regressions, but it's not correct. Please
>> see below.
>>
>> On Fri, Jun 15, 2012 at 5:25 PM, <j.gli...@gmail.com> wrote:
>>> From: Jerome Glisse <jgli...@redhat.com>
>>>
>>> z or stencil texture should not be created with the z/stencil
>>> flags for surface creation as they are intended to be bound
>>> as texture.
>>>
>>> Signed-off-by: Jerome Glisse <jgli...@redhat.com>
>>> ---
>>> src/gallium/drivers/r600/r600_texture.c | 34
>>> +++++++++++++++++-------------
>>> 1 files changed, 19 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/src/gallium/drivers/r600/r600_texture.c
>>> b/src/gallium/drivers/r600/r600_texture.c
>>> index 5b15990..517f273 100644
>>> --- a/src/gallium/drivers/r600/r600_texture.c
>>> +++ b/src/gallium/drivers/r600/r600_texture.c
>>> @@ -237,7 +237,8 @@ static void r600_texture_set_array_mode(struct
>>> pipe_screen *screen,
>>>
>>> static int r600_init_surface(struct radeon_surface *surface,
>>> const struct pipe_resource *ptex,
>>> - unsigned array_mode)
>>> + unsigned array_mode, unsigned is_transfer,
>>> + unsigned is_texture)
>>
>> We use the "boolean" type in gallium for bools. You also seem to need
>> only one flag, not two, because you don't make any distinction between
>> is_transfer and is_texture further in the code. The "is_texture" flag
>> is not very useful anyway, see below.
>>
>>> {
>>> surface->npix_x = ptex->width0;
>>> surface->npix_y = ptex->height0;
>>> @@ -298,7 +299,7 @@ static int r600_init_surface(struct radeon_surface
>>> *surface,
>>> if (ptex->bind & PIPE_BIND_SCANOUT) {
>>> surface->flags |= RADEON_SURF_SCANOUT;
>>> }
>>> - if (util_format_is_depth_and_stencil(ptex->format)) {
>>> + if (util_format_is_depth_and_stencil(ptex->format) && !is_transfer
>>> && !is_texture) {
>>> surface->flags |= RADEON_SURF_ZBUFFER;
>>> surface->flags |= RADEON_SURF_SBUFFER;
>>> }
>>> @@ -316,11 +317,6 @@ static int r600_setup_surface(struct pipe_screen
>>> *screen,
>>> unsigned i;
>>> int r;
>>>
>>> - if (util_format_is_depth_or_stencil(rtex->real_format)) {
>>> - rtex->surface.flags |= RADEON_SURF_ZBUFFER;
>>> - rtex->surface.flags |= RADEON_SURF_SBUFFER;
>>> - }
>>> -
>>> r = rscreen->ws->surface_init(rscreen->ws, &rtex->surface);
>>> if (r) {
>>> return r;
>>> @@ -572,7 +568,8 @@ r600_texture_create_object(struct pipe_screen *screen,
>>> r600_setup_miptree(screen, rtex, array_mode);
>>> if (rscreen->use_surface_alloc) {
>>> rtex->surface = *surface;
>>> - r = r600_setup_surface(screen, rtex, array_mode,
>>> pitch_in_bytes_override);
>>> + r = r600_setup_surface(screen, rtex, array_mode,
>>> + pitch_in_bytes_override);
>>> if (r) {
>>> FREE(rtex);
>>> return NULL;
>>> @@ -642,7 +639,9 @@ struct pipe_resource *r600_texture_create(struct
>>> pipe_screen *screen,
>>> }
>>> }
>>>
>>> - r = r600_init_surface(&surface, templ, array_mode);
>>> + r = r600_init_surface(&surface, templ, array_mode,
>>> + templ->flags & R600_RESOURCE_FLAG_TRANSFER,
>>> + templ->usage & PIPE_BIND_SAMPLER_VIEW);
>>
>> There are two reasons why this is wrong.
>>
>> 1) templ->usage may only contain one PIPE_USAGE_* flag.
>>
>> 2) Even if you had used (templ->bind & PIPE_BIND_SAMPLER_VIEW), it
>> wouldn't be any useful for OpenGL, because all textures and renderbuffers in
>> OpenGL can be used as samplers. Yeah, that's right, everything. The
>> typical use cases are glBlitFramebuffer and glCopyTex(Sub)Image.
>>
>> Also it would be safer not to trust the state tracker that it set the
>> bind flags correctly, and behave as if PIPE_BIND_SAMPLER_VIEW were set
>> always.
>>
>>> if (r) {
>>> return NULL;
>>> }
>>> @@ -723,7 +722,7 @@ struct pipe_resource *r600_texture_from_handle(struct
>>> pipe_screen *screen,
>>> else
>>> array_mode = 0;
>>>
>>> - r = r600_init_surface(&surface, templ, array_mode);
>>> + r = r600_init_surface(&surface, templ, array_mode, 0, 0);
>>
>> This is wrong for the same reason as above. The window framebuffer
>> allocated by the DDX can be used as a sampler in OpenGL.
>
> Just to be clear, you don't mean that at the GL API level, do you?
That's correct.
> OpenGL doesn't support using the window's color buffer (nor plain
> renderbuffers) to be used as a texture. A compositor might use the
> window image as a texture though.
I mean it from the perspective of an OpenGL driver which implements
glCopyTexImage by drawing a full-screen textured quad into the target texture.
Marek
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
