On 20/08/14 17:14, Roland Scheidegger wrote:
Am 20.08.2014 17:55, schrieb Ilia Mirkin:
On Wed, Aug 20, 2014 at 11:47 AM, Jose Fonseca <jfons...@vmware.com> wrote:
On 20/08/14 16:31, Ilia Mirkin wrote:

Hm, it's not tested. And you're right, that would (most likely) mess
up, since it would only have the pipe_resource's target. Any
suggestions on how to fix it? Should the target be added to
pipe_sampler_view?

On Wed, Aug 20, 2014 at 11:25 AM, Roland Scheidegger <srol...@vmware.com>
wrote:

Didn't look at it that closely, but I'm pretty surprised this really
works. One things ARB_texture_view can do is cast cube maps (and cube
map arrays) to 2d arrays and vice versa (also 1d/2d to the respective
array type), and we cannot express that in sampler views (yet) (we can't
express it in surfaces neither but there it should not matter). Which
means the type used in the shader for sampling will not match the
sampler view, which sounds quite broken to me.

Roland


Probably the only sane thing to do eliminate the disctinction between
PIPE_TEXTURE_FOO and PIPE_TEXTURE_FOO_ARRAY like  in
https://urldefense.proofpoint.com/v1/url?u=http://msdn.microsoft.com/en-us/library/windows/desktop/ff476202.aspx&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=F4msKE2WxRzA%2BwN%2B25muztFm5TSPwE8HKJfWfR2NgfY%3D%0A&m=x03OmuVWAQgfFbsFB2SLMLwSYavxkU8Zsypu9lEIkpg%3D%0A&s=4b0ca75d91e6d53d92658d9e334cf2a73a01efe5667464c969f5c085409052ff
 ,
e.g.,:

enum pipe_texture_target {
    PIPE_BUFFER           = 0,
    PIPE_TEXTURE_1D       = 1,
    PIPE_TEXTURE_2D       = 2,
    PIPE_TEXTURE_3D       = 3,
    PIPE_TEXTURE_CUBE     = 4, // Must have same layout as PIPE_TEXTURE_2D
    PIPE_TEXTURE_RECT     = 5,
    PIPE_TEXTURE_1D_ARRAY = PIPE_TEXTURE_1D,
    PIPE_TEXTURE_2D_ARRAY = PIPE_TEXTURE_2D,
    PIPE_TEXTURE_CUBE_ARRAY = PIPE_TEXTURE_CUBE,
    PIPE_MAX_TEXTURE_TYPES
};


We could also remove PIPE_TEXTURE_CUBE and have cube-maps be PIPE_TEXTURE_2D
with a flag, but that's probably a lot of work. Instead, drivers that want
to be able to support ARB_texture_view will need to ensure
PIPE_TEXTURE_CUBE/PIPE_TEXTURE_2D layout match.

Another quick + cheap alternative (at least looking at nv50/nvc0 code)
would be to pass a separate target parameter to
->create_sampler_view(). That would be enough for nouveau, but perhaps
not more generally? Take a look at nv50_tex.c:nv50_create_texture_view
-- it also needs to work out the depth of the texture (presumably to
deal with out-of-bounds accesses) and that is written to the texture
info structure.
Well that should be enough, but I don't think it fits out design.


> We've
encapsulated other "override" information like the format in the view
already, and I see no reason why the target "cast" should be treated any
different.

In other words, you're arguing for:

diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h
index a82686b..c87ac4e 100644
--- a/src/gallium/include/pipe/p_state.h
+++ b/src/gallium/include/pipe/p_state.h
@@ -333,6 +333,7 @@ struct pipe_surface
    struct pipe_reference reference;
struct pipe_resource *texture; /**< resource into which this is a view */
    struct pipe_context *context; /**< context this surface belongs to */
+   enum pipe_texture target;
    enum pipe_format format;

    /* XXX width/height should be removed */


It's a fair point.  And I don't object that solution.

Of course, for this to work, drivers will need to treat the _ARRAY and non _ARRAY targets the same when determining the texture layout for this to work.


I just felt this would be a good oportunity to slim down pipe_texture_target too. I'm not sure the _ARRAY distinction still matters at this level, but I suppose it doesn't hurt.


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

Reply via email to