[Mesa-dev] [PATCH] i965: Fix calculation of layers array length for isl_view

[Lionel Landwerlin]

On 07/09/2018 16:32, Ilia Mirkin wrote:
> On Fri, Sep 7, 2018 at 11:09 AM, Danylo Piliaiev
> <danylo.piliaiev at gmail.com> wrote:
>> On 9/7/18 5:48 PM, Ilia Mirkin wrote:
>>> On Fri, Sep 7, 2018 at 10:41 AM, Danylo Piliaiev
>>> <danylo.piliaiev at gmail.com> wrote:
>>>> Comment for array_len field states:
>>>>    "Indicates the number of array elements starting at
>>>>      Base Array Layer."
>>>>
>>>> And most usages of array_len expect it to be equal or less than
>>>>    total layers - base layer
>>>>
>>>> Fixes: 5a8c8903
>>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107856
>>>>
>>>> Signed-off-by: Danylo Piliaiev <danylo.piliaiev at globallogic.com>
>>>> ---
>>>>    src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 6 ++++--
>>>>    1 file changed, 4 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>>>> b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>>>> index 42af41aca3..6adf4a5836 100644
>>>> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>>>> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>>>> @@ -1539,6 +1539,8 @@ update_image_surface(struct brw_context *brw,
>>>>          } else {
>>>>             struct intel_texture_object *intel_obj =
>>>> intel_texture_object(obj);
>>>>             struct intel_mipmap_tree *mt = intel_obj->mt;
>>>> +
>>>> +         const unsigned base_layer = obj->MinLayer + u->_Layer;
>>>>             const unsigned num_layers = u->Layered ?
>>>>                get_image_num_layers(mt, obj->Target, u->Level) : 1;
>>>>
>>>> @@ -1546,8 +1548,8 @@ update_image_surface(struct brw_context *brw,
>>>>                .format = format,
>>>>                .base_level = obj->MinLevel + u->Level,
>>>>                .levels = 1,
>>>> -            .base_array_layer = obj->MinLayer + u->_Layer,
>>>> -            .array_len = num_layers,
>>>> +            .base_array_layer = base_layer,
>>>> +            .array_len = num_layers - base_layer,
>>> See above - num_layers can be 1 if the image isn't bound as a layered
>>> image. But base layer can be whatever -- so this will end up as
>>> negative. I think the adjustment needs to be done only for the
>>> u->Layered case.
>> Oh, I see it now, thanks! Unless Lionel's patch for this issue is better
>> I'll send v2 of my patch.
> I believe yours is much closer to right. Lionel's was conceptually
> wrong. (Or I'm the one who's confused - reminder - I'm not an Intel
> driver developer, and ultimately won't approve or reject your patches.
> But I will point out things that I think are off.)
>
>    -ilia
> _


Yep, looks like I don't really understand the relationship between LOD, 
depth and layers...