[Mesa-dev] [PATCH 1/3] gallium: change pipe_sampler_view::first_element/last_element -> offset/size

[Marek Olšák]

On Tue, Aug 16, 2016 at 1:14 AM, Roland Scheidegger <sroland at vmware.com> wrote:
> Am 15.08.2016 um 19:48 schrieb Marek Olšák:
>> On Sun, Aug 14, 2016 at 11:38 AM, Marek Olšák <maraeo at gmail.com> wrote:
>>> On Aug 12, 2016 8:49 PM, "Roland Scheidegger" <sroland at vmware.com> wrote:
>>>>
>>>> I can't say I'm a big fan of this.
>>>> From an "api cleanness" point of view, defining things in elements makes
>>>> more sense imho.
>>>> This is due to the GL api though which uses generic buffer api to turn
>>>> buffers into textures which can be sampled (somewhat different to what
>>>> d3d10 does).
>>>> Can't you just say you require 16 bytes alignment and call it a day? I
>>>> don't think all hw can do 4 bytes anyway (actually though to satisfy
>>>> d3d10 requirements, your hw would still need to be able to even support
>>>> 1-byte alignment, if the format is r8).
>>>>
>>>> It also makes things asymmetric wrt pipe_surface (granted that's
>>>> something gl can't do IIRC).
>>>>
>>>> But I guess we could live with this if necessary... Maybe hw does work
>>>> more like that and not what d3d10 wants anyway...
>>>
>>> I'd like to expose what our hw can do and this is the only way.
>>
>> Just to clarify...
>>
>> The "offset" should be in bytes, because we can do 4-byte-aligned
>> access for texel sizes >= 4 and 1-byte-aligned access otherwise. The
>> hardware doesn't really have the "offset" field. The "offset" is just
>> added to the buffer address and the new address is used instead. The
>> fact that we have a bug report about it and a user demanding it is a
>> strong indication that it's a design bug that should be fixed.
> Hence why I suggested to return 16 as minimum alignment which would also
> fix this. Unless there's some extension saying you have to support dword
> alignment.
> I say it's a design bug in the GL api :-).
> (You're saying you can actually use 1 byte aligned accesses for 1 byte
> texel sizes, and you can't expose that with GL neither.)
>
>>
>> The "size" field can be in bytes or elements. I don't have a strong
>> opinion here. "size" is more consistent with other interfaces
>> (constant buffers, shader buffers).
> If offset is in bytes, I'd say size should be as well (though it
> probably should already be correctly rounded down). Mixing different
> units there is probably more confusing.
>
>>
>> pipe_surface buffer support (for DX10?) isn't really important or
>> relevant because no in-tree state tracker uses it, and most drivers
>> either don't implement it, or don't implement it correctly.
> That is true. llvmpipe might be the only one fully supporting it.
>
>> If we ever
>> see an in-tree DX10 state tracker, it should use "shader buffers"
>> instead.
> You can't quite easily emulate that, since the dx10 feature treats this
> like any ordinary rtv. Meaning you can do things like blending.
>
>> I don't see a reason to have pipe_surface buffer support in
>> the tree except for dx10->gallium->dx10 translation (I guess we can
>> both agree that such a use case is highly unlikely in open source).
>
> Don't forget even the sampler_view bits for supporting buffer textures
> predate support for texture buffers in mesa IIRC. It might have been
> possible gl supports writing to buffers in that way one day as well
> (there's definitely plenty of features in gallium which eventually got
> supported by GL even though they were initially not used by mesa state
> tracker).
>
> Anyway, I don't really have objections to this change, I'm just grumpy
> GL didn't get the API right :-). I agree it's a better match for GL. And
> it is still possible to support what d3d10 needs there.

OK, I'll assume this is ready to land if there are no other comments.

BTW, Vulkan is the same as GL here, so according to you, Vulkan didn't
get it right either. :)

Marek