Am 20.08.2014 18:12, schrieb Jose Fonseca: > On 20/08/14 17:02, Roland Scheidegger wrote: >> Am 20.08.2014 17:47, schrieb Jose Fonseca: >>> 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 >>> http://msdn.microsoft.com/en-us/library/windows/desktop/ff476202.aspx , >>> 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, >> I think you meant PIPE_TXTURE_2D there? > > No, I expclitely left PIPE_TEXTURE_CUBE due to the reasons I explained > below. > >> >>> 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. >> >> Yes, but that's not what I'm talking about. >> Even d3d10, which does not have any distinct cube/array texture layouts > > Precisely. D3D10 uses D3D10_RESOURCE_DIMENSION_TEXTURE2D for cubes plus > the D3D10_RESOURCE_MISC_TEXTURECUBE misc flag. Which is precisely what > I was talking about when I said " We could also remove PIPE_TEXTURE_CUBE > and have cube-maps be PIPE_TEXTURE_2D with a flag". > >> (actually 10 still had a separate one for cubes, because there was hw >> which really required a different layout afaik, but it got abandoned in >> 10.1), > > No D3D10 doesn't have D3D10_RESOURCE_DIMENSION_TEXTURECUBE. D3D 10, > 10.1, and 11, they all use RESOURCE_DIMENSION_TEXTURE2D + > RESOURCE_MISC_TEXTURECUBE for cubemaps or cubemaps arrays. You are right that d3d10 did not rally have a cube layout - they required the misc_texturecube flag as you said (this is really the same thing to me if you have a 2d + flag or explicit cube). But this flag is more or less dead and buried with newer d3d versions, it is there for compatibility reasons (with api level 10_0 you still need it for cube maps, since it didn't allow the target casting from cube to 2d array or vice versa later in the resource view). It was never used for cube map arrays.
> >> still requires shader resource views to have them (and they must >> match what's declared in the shader): >> http://msdn.microsoft.com/en-us/library/windows/desktop/ff476211%28v=vs.85%29.aspx >> > > Right, there are different enums for resource types and view types. > >> So, my guess is we should do the same - just have that type in the >> sampler view (and drivers wishing to support the extension must take the >> type from the view, and not the underlying resource - or they could get >> it from the shader itself, presumably, if they really wanted, this is >> actually what we do for texture size queries in llvmpipe, but it's more >> of a necessary hack). >> >> You are right though we would not really require distinct types at the >> resource level, but they don't really get in the way neither. > > Yes, we could do the same. But I do think that in that case we should > have a separate enum for views different from pipe_texture_target. And > pipe_texture_target would be slim down. Yes that would make sense. I'm not sure it's worth the trouble of changing the code though. > > > But if you remove PIPE_TEXTURE_CUBE from pipe_texture_target you'll need > to pass that info in a new flag (like D3D10_RESOURCE_MISC_TEXTURECUBE). > I don't feel strongly, but I'm not sure this is much more elegant than > keeping PIPE_TEXTURE_CUBE around. Ok understood PIPE_TEXTURE_CUBE would have to stay (as there's hw which needs to know the distinction to an array). So I guess we could do: 1) slim the pipe_texture_target enum down and use a different pipe_view_target (or whatever) in the sampler view which has all values, and allow all required casts (those involving cubes are probably the only ones which really need to be restricted to drivers supporting ARB_texture_view). It requires changing all drivers, state trackers to the new scheme though. 2) just keep the pipe_texture_target enum the same and use it for both resource and sampler views. The advantage here is that you don't need to rewrite any driver code for now, unless you want to expose the new ARB_texture_view capabilities (and for old state tracker code to still work it should be enough to fill in a default view_target just as is done for format). 3) keep the pipe_texture_target enum for now but use a new pipe_view_target, with the intention of slimming pipe_texture_target down later (when just using it at the resource level, it is now definitely misnamed). Roland _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
