[Mesa-dev] [PATCH] mesa/st: NumLayers is only valid for array textures
Roland Scheidegger
sroland at vmware.com
Wed Sep 24 14:17:55 PDT 2014
I don't really qualified to review, IIRC I mentioned it was tricky to
see if it's right when you pushed it first, and this has not changed.
Some comment inline though...
Am 24.09.2014 20:30, schrieb Ilia Mirkin:
> Marek/Roland -- do either of those comments count as a R-b? I'd like
> to push this out tonight, pending a full piglit run.
>
> On Wed, Sep 24, 2014 at 1:35 PM, Roland Scheidegger <sroland at vmware.com> wrote:
>> Yes cubemaps should have array_size == 6 always in gallium. You just
>> have to be careful whenever translating things from mesa to gallium as
>> things like that won't be true in core mesa of course (similar to 1d
>> array textures having height and so on) due to OpenGL weirdness for
>> historical reasons.
>>
>> Roland
>>
>>
>> Am 24.09.2014 19:19, schrieb Marek Olšák:
>>> Interesting, I didn't know about that. Nevermind. st/mesa indeed sets it to 6.
>>>
>>> Marek
>>>
>>> On Wed, Sep 24, 2014 at 6:26 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>>> On Wed, Sep 24, 2014 at 12:20 PM, Marek Olšák <maraeo at gmail.com> wrote:
>>>>> Cubemaps have array_size == 1, but you can still set the target to 2D
>>>>
>>>> Are you *sure* about that? Everything I'm seeing indicates that
>>>> cubemaps have array_size == 6. For example this code in nv50_tex.c:
>>>>
>>>> depth = MAX2(mt->base.base.array_size, mt->base.base.depth0);
>>>> ...
>>>> case PIPE_TEXTURE_CUBE:
>>>> depth /= 6;
>>>> ...
>>>>
>>>>> and set first_layer <= last_layer <= 6 in the sample view. Instead of
>>>>> checking array_size, maybe NumLayers should be used instead. Just
>>>>> guessing.
>>>>>
>>>>> Marek
>>>>>
>>>>> On Wed, Sep 24, 2014 at 5:05 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>>>>> The disguise of cubemap's layeredness is insufficient to trip up this
>>>>>> code :) They still get their NumLayers set, and the resources still
>>>>>> have an array size. Perhaps there's a scenario I'm not considering?
>>>>>>
>>>>>> On Wed, Sep 24, 2014 at 5:23 AM, Marek Olšák <maraeo at gmail.com> wrote:
>>>>>>> Maybe something similar also needs to be done for cubemaps, because
>>>>>>> they are just layered textures in disguise?
>>>>>>>
>>>>>>> Marek
>>>>>>>
>>>>>>> On Wed, Sep 24, 2014 at 7:01 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>>>>>>> For 3d textures, NumLayers is set to 1, which is not what we want. This
>>>>>>>> fixes the newly added gl-layer-render-storage test (which constructs
>>>>>>>> immutable 3d textures). Fixes regression introduced in d82bd7eb060.
>>>>>>>>
>>>>>>>> Bugzilla: https://urldefense.proofpoint.com/v1/url?u=https://bugs.freedesktop.org/show_bug.cgi?id%3D84145&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=F4msKE2WxRzA%2BwN%2B25muztFm5TSPwE8HKJfWfR2NgfY%3D%0A&m=p4L3w5mFWeprslDAPtBcySi4H8PhKEdqz0pOU77lcjg%3D%0A&s=5599aec76e1694c8ec71dcfe6209603bf8b152884c942363fd84e9d56edaaa72
>>>>>>>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>>>>>>> ---
>>>>>>>> src/mesa/state_tracker/st_atom_texture.c | 2 +-
>>>>>>>> src/mesa/state_tracker/st_cb_fbo.c | 3 ++-
>>>>>>>> src/mesa/state_tracker/st_texture.c | 3 ++-
>>>>>>>> 3 files changed, 5 insertions(+), 3 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/src/mesa/state_tracker/st_atom_texture.c b/src/mesa/state_tracker/st_atom_texture.c
>>>>>>>> index ed9a444..19072ae 100644
>>>>>>>> --- a/src/mesa/state_tracker/st_atom_texture.c
>>>>>>>> +++ b/src/mesa/state_tracker/st_atom_texture.c
>>>>>>>> @@ -223,7 +223,7 @@ static unsigned last_level(struct st_texture_object *stObj)
>>>>>>>>
>>>>>>>> static unsigned last_layer(struct st_texture_object *stObj)
>>>>>>>> {
>>>>>>>> - if (stObj->base.Immutable)
>>>>>>>> + if (stObj->base.Immutable && stObj->pt->array_size > 1)
>>>>>>>> return MIN2(stObj->base.MinLayer + stObj->base.NumLayers - 1,
>>>>>>>> stObj->pt->array_size - 1);
>>>>>>>> return stObj->pt->array_size - 1;
>>>>>>>> diff --git a/src/mesa/state_tracker/st_cb_fbo.c b/src/mesa/state_tracker/st_cb_fbo.c
>>>>>>>> index 470ab27..7b6a444 100644
>>>>>>>> --- a/src/mesa/state_tracker/st_cb_fbo.c
>>>>>>>> +++ b/src/mesa/state_tracker/st_cb_fbo.c
>>>>>>>> @@ -451,7 +451,8 @@ st_update_renderbuffer_surface(struct st_context *st,
>>>>>>>> }
>>>>>>>>
>>>>>>>> /* Adjust for texture views */
>>>>>>>> - if (strb->is_rtt) {
>>>>>>>> + if (strb->is_rtt && resource->array_size > 1 &&
>>>>>>>> + strb->Base.TexImage->TexObject->Immutable) {
>>>>>>>> struct gl_texture_object *tex = strb->Base.TexImage->TexObject;
>>>>>>>> first_layer += tex->MinLayer;
>>>>>>>> if (!strb->rtt_layered)
>>>>>>>> diff --git a/src/mesa/state_tracker/st_texture.c b/src/mesa/state_tracker/st_texture.c
>>>>>>>> index c84aa45..2cd95ec 100644
>>>>>>>> --- a/src/mesa/state_tracker/st_texture.c
>>>>>>>> +++ b/src/mesa/state_tracker/st_texture.c
>>>>>>>> @@ -263,7 +263,8 @@ st_texture_image_map(struct st_context *st, struct st_texture_image *stImage,
>>>>>>>> if (stObj->base.Immutable) {
>>>>>>>> level += stObj->base.MinLevel;
>>>>>>>> z += stObj->base.MinLayer;
>>>>>>>> - d = MIN2(d, stObj->base.NumLayers);
>>>>>>>> + if (stObj->pt->array_size > 1)
>>>>>>>> + d = MIN2(d, stObj->base.NumLayers);
>>>>>>>> }
>>>>>>>>
>>>>>>>> z += stImage->base.Face;
This is pretty confusing, so for a cubemap you get the initial z
(presumably zero), add the MinLayer for the view, then also the Face
from the initial image? This isn't new but all the translation from mesa
seems really tricky here and elsewhere.
The fix though is probably correct but really I can't tell...
But as long as it fixes things go for it...
Roland
>>>>>>>> --
>>>>>>>> 1.8.5.5
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> mesa-dev mailing list
>>>>>>>> mesa-dev at lists.freedesktop.org
>>>>>>>> https://urldefense.proofpoint.com/v1/url?u=http://lists.freedesktop.org/mailman/listinfo/mesa-dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=F4msKE2WxRzA%2BwN%2B25muztFm5TSPwE8HKJfWfR2NgfY%3D%0A&m=p4L3w5mFWeprslDAPtBcySi4H8PhKEdqz0pOU77lcjg%3D%0A&s=da68def8d99a0b8cd601334ed2b794d24bcb8e278d84e5a8aa06cc21afc5967d
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://urldefense.proofpoint.com/v1/url?u=http://lists.freedesktop.org/mailman/listinfo/mesa-dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=F4msKE2WxRzA%2BwN%2B25muztFm5TSPwE8HKJfWfR2NgfY%3D%0A&m=p4L3w5mFWeprslDAPtBcySi4H8PhKEdqz0pOU77lcjg%3D%0A&s=da68def8d99a0b8cd601334ed2b794d24bcb8e278d84e5a8aa06cc21afc5967d
>>>
>>
More information about the mesa-dev
mailing list