[Mesa-dev] [PATCH] mesa/mtypes: repack gl_texture_object.

[Thomas Helland]

2017-09-03 13:39 GMT+02:00 Dave Airlie <airlied at gmail.com>:
> On 3 September 2017 at 21:22, Thomas Helland <thomashelland90 at gmail.com> wrote:
>> 2017-09-03 13:18 GMT+02:00 Dave Airlie <airlied at gmail.com>:
>>> From: Dave Airlie <airlied at redhat.com>
>>>
>>> reduces size from 1144 to 1128.
>>>
>>> Signed-off-by: Dave Airlie <airlied at redhat.com>
>>> ---
>>>  src/mesa/main/mtypes.h | 10 +++++-----
>>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
>>> index d44897b..3d68a6d 100644
>>> --- a/src/mesa/main/mtypes.h
>>> +++ b/src/mesa/main/mtypes.h
>>> @@ -1012,7 +1012,6 @@ struct gl_texture_object
>>>     struct gl_sampler_object Sampler;
>>>
>>>     GLenum DepthMode;           /**< GL_ARB_depth_texture */
>>> -   bool StencilSampling;       /**< Should we sample stencil instead of depth? */
>>>
>>>     GLfloat Priority;           /**< in [0,1] */
>>>     GLint BaseLevel;            /**< min mipmap level, OpenGL 1.2 */
>>> @@ -1033,12 +1032,17 @@ struct gl_texture_object
>>>     GLboolean Immutable;        /**< GL_ARB_texture_storage */
>>>     GLboolean _IsFloat;         /**< GL_OES_float_texture */
>>>     GLboolean _IsHalfFloat;     /**< GL_OES_half_float_texture */
>>> +   bool StencilSampling;       /**< Should we sample stencil instead of depth? */
>>> +   bool HandleAllocated;       /**< GL_ARB_bindless_texture */
>>>
>>
>> Maybe we could use "pragma pack" here instead?
>> I'm debating with myself whether or not moving this
>> bool away from the rest of the bindless_texture related
>> variables is worth saving the few bytes.
>
> You don't ever want pragma pack for this. As that will force a
> uint32_t after a bool to be misaligned,
> which would suck for everyone.
>
> Saving 7 bytes of pointless padding at the end of a struct that gets
> allocated quite a lot by GL applications,
> seems worth it for me. gl_texture_object is probably one of the most
> allocated application object structs.
>
> Dave.
>

I thought I remembered something about unaligned access
being not that painfull on post-haswell x86. But hey, we've got
other architectures to support that are not that fortunate.
I had initially dropped this patch from a similar patch series
I have sitting locally due to not being sure of the benefit.
But the change looks good and rationale is sound, so:

Reviewed-by: Thomas Helland<thomashelland90 at gmail.com>

>>
>>>     GLuint MinLevel;            /**< GL_ARB_texture_view */
>>>     GLuint MinLayer;            /**< GL_ARB_texture_view */
>>>     GLuint NumLevels;           /**< GL_ARB_texture_view */
>>>     GLuint NumLayers;           /**< GL_ARB_texture_view */
>>>
>>> +   /** GL_EXT_memory_object */
>>> +   GLenum TextureTiling;
>>> +
>>>     /** Actual texture images, indexed by [cube face] and [mipmap level] */
>>>     struct gl_texture_image *Image[MAX_FACES][MAX_TEXTURE_LEVELS];
>>>
>>> @@ -1057,13 +1061,9 @@ struct gl_texture_object
>>>     /** GL_ARB_shader_image_load_store */
>>>     GLenum ImageFormatCompatibilityType;
>>>
>>> -   /** GL_EXT_memory_object */
>>> -   GLenum TextureTiling;
>>> -
>>>     /** GL_ARB_bindless_texture */
>>>     struct util_dynarray SamplerHandles;
>>>     struct util_dynarray ImageHandles;
>>> -   bool HandleAllocated;
>>>  };
>>>
>>>
>>> --
>>> 2.9.5
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev