[vlc-devel] [PATCH 1/2] OpenGL: put in a new file the geometry building functions to reduce a bit the size of vout_helper.c

[Adrien Maglo]

On 31/07/2017 17:44, Rémi Denis-Courmont wrote:
> Le maanantaina 31. heinäkuuta 2017, 17.27.07 EEST Adrien Maglo a écrit :
>> On 31/07/2017 17:13, Rémi Denis-Courmont wrote:
>>> Le maanantaina 31. heinäkuuta 2017, 16.30.19 EEST Adrien Maglo a écrit :
>>>> ---
>>>>
>>>>    modules/video_output/Makefile.am          |   3 +-
>>>>    modules/video_output/opengl/geometry.c    | 296
>>>>
>>>> ++++++++++++++++++++++++++++++ modules/video_output/opengl/geometry.h
>>>> |
>>>> 49 +++++
>>>>
>>>>    modules/video_output/opengl/vout_helper.c | 271
>>>>    +--------------------------
>>>>
>>>> 4 files changed, 348 insertions(+), 271 deletions(-)
>>>>
>>>>    create mode 100644 modules/video_output/opengl/geometry.c
>>>>    create mode 100644 modules/video_output/opengl/geometry.h
>>>>
>>>> diff --git a/modules/video_output/Makefile.am
>>>> b/modules/video_output/Makefile.am index 094a7f5025..274b79968c 100644
>>>> --- a/modules/video_output/Makefile.am
>>>> +++ b/modules/video_output/Makefile.am
>>>> @@ -8,7 +8,8 @@ OPENGL_COMMONLDFLAGS =
>>>>
>>>>    OPENGL_COMMONLIBS =
>>>>    OPENGL_COMMONSOURCES = video_output/opengl/vout_helper.c \
>>>>    
>>>>    	video_output/opengl/vout_helper.h video_output/opengl/internal.h \
>>>>
>>>> -	video_output/opengl/converters.c
>>>> +	video_output/opengl/converters.c video_output/opengl/geometry.h \
>>>> +	video_output/opengl/geometry.c
>>>>
>>>>    if HAVE_ANDROID
>>>>    OPENGL_COMMONSOURCES += video_output/opengl/converter_android.c
>>>>    endif
>>>>
>>>> diff --git a/modules/video_output/opengl/geometry.c
>>>> b/modules/video_output/opengl/geometry.c new file mode 100644
>>>> index 0000000000..09966a9ec9
>>>> --- /dev/null
>>>> +++ b/modules/video_output/opengl/geometry.c
>>>> @@ -0,0 +1,296 @@
>>>> +/***********************************************************************
>>>> *** *** + * geometry.c: OpenGL geometry builder
>>>> +
>>>> *************************************************************************
>>>> **
>>>> ** + * Copyright (C) 2016 VLC authors and VideoLAN
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or modify
>>>> it
>>>> + * under the terms of the GNU Lesser General Public License as published
>>>> by + * the Free Software Foundation; either version 2.1 of the License,
>>>> or + * (at your option) any later version.
>>>> + *
>>>> + * This program is distributed in the hope that it will be useful,
>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>>> + * GNU Lesser General Public License for more details.
>>>> + *
>>>> + * You should have received a copy of the GNU Lesser General Public
>>>> License + * along with this program; if not, write to the Free Software
>>>> Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston MA
>>>> 02110-1301, USA. +
>>>> ************************************************************************
>>>> *** **/ +#ifdef HAVE_CONFIG_H
>>>> +# include "config.h"
>>>> +#endif
>>>> +
>>>> +#include <math.h>
>>>> +
>>>> +#include "geometry.h"
>>>> +
>>>> +
>>>> +int BuildSphere(unsigned nbPlanes,
>>>> +                GLfloat **vertexCoord, GLfloat **textureCoord, unsigned
>>>> *nbVertices, +                GLushort **indices, unsigned *nbIndices,
>>>> +                const float *left, const float *top,
>>>> +                const float *right, const float *bottom)
>>>> +{
>>>
>>> Either use restrict, cache the values in local variables, or both.
>>
>> I understand the remark about the use of restrict but I am not sure
>> about the cache.
>> These geometry building functions are called only if the video format
>> resolution or offset change. This should not happen often.
> 
> On an architecture with a lot of registers, using automatic variables is the
> same as using restrict. On those with few registers, using restrict is better
> as automatic variables will be stacked.

Ok, thanks.

I agree all these functions could be optimized but this patch is just 
about moving an existing code. Maybe, the optimizations could be the aim 
of later patches.

Best regards,


-- 
MagSoft