[PATCH weston 1/5] gl-renderer: Rename gl_renderer_create to gl_renderer_display_create
Derek Foreman
derekf at osg.samsung.com
Thu May 12 13:54:26 UTC 2016
On 11/05/16 10:53 AM, Miguel Angel Vico wrote:
> Thanks, Yong.
>
> Inline.
>
> On Wed, 11 May 2016 09:45:15 -0500
> Yong Bakos <junk at humanoriented.com> wrote:
>
>> On May 11, 2016, at 7:48 AM, Miguel A. Vico <mvicomoya at nvidia.com>
>> wrote:
>>>
>>> No functional change. This patch only renames gl_renderer_create()
>>> to gl_renderer_display_create(), which is something more
>>> descriptive of what the function does.
>>>
>>> Signed-off-by: Miguel A Vico Moya <mvicomoya at nvidia.com>
>>> Reviewed-by: James Jones <jajones at nvidia.com>
>>
>> Hi Miguel,
>> When renaming, I suggest adjusting the indentation of subsequent
>> arguments as well, to preserve alignment. Example noted inline below.
>> I'm noticing this issue throughout this whole patch series.
>
> The thing is arguments aren't correctly aligned throughout all weston
> code. I think the code-style rule of using hard-tabs instead of spaces
> for indentation made things to be a bit messy, since arguments are
> usually aligned using hard-tabs as well, but I think spaces should be
> used instead for those cases in order to keep a correct indentation
> regardless tab length settings of the editor.
The style of this weston source code is to use hard tabs, 8 chars wide.
It won't look right in an editor configured with any other tab width.
I'm with Yong on this one - if you're going to rename a function, it's
preferable to fix up the indenting at the same time for those call sites
(even if they didn't match the style guidelines before). This fix-up
should be done with tabs for alignment.
Otherwise trivial refactoring patches would rapidly make the code look
terrible.
For what it's worth, I personally prefer tabs for indenting and spaces
for alignment - but that's not what this project uses, so it's not what
I use on this project. :)
> I wanted to be as conservative as possible with my patches, and don't
> add many of those tab-by-space-replacement changes.
>
> I can update my patches, if that's fine, or prepare a separate set
> of changes to only fix those indentation issues.
>
> Please, let me know what's preferred.
I general, fixing up the patches would be best, and we appreciate the
extra work - however, I'm not sure about this particular patch.
The function sets up a bunch of function pointers, and does more than
just create a display. It also has a symmetrical gl_renderer_destroy()
which hasn't been renamed.
I don't think adding the word "display" here really makes anything more
clear.
Thanks,
Derek
> Thanks,
> Miguel.
>
>>
>> Respectfully,
>> yong
>>
>>
>>> ---
>>> src/compositor-drm.c | 2 +-
>>> src/compositor-fbdev.c | 2 +-
>>> src/compositor-wayland.c | 2 +-
>>> src/compositor-x11.c | 5 +++--
>>> src/gl-renderer.c | 4 ++--
>>> src/gl-renderer.h | 2 +-
>>> 6 files changed, 9 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
>>> index a47b453..9c58710 100644
>>> --- a/src/compositor-drm.c
>>> +++ b/src/compositor-drm.c
>>> @@ -1585,7 +1585,7 @@ drm_backend_create_gl_renderer(struct
>>> drm_backend *b)
>>>
>>> if (format[1])
>>> n_formats = 3;
>>> - if (gl_renderer->create(b->compositor,
>>> + if (gl_renderer->display_create(b->compositor,
>>> EGL_PLATFORM_GBM_KHR,
>>> (void *)b->gbm,
>>> gl_renderer->opaque_attribs,
>>
>> The subsequent args should be aligned with the first (the 'b' in
>> 'b->compositor').
>>
>>
>>
>>> diff --git a/src/compositor-fbdev.c b/src/compositor-fbdev.c
>>> index ee762e3..c6ffcd7 100644
>>> --- a/src/compositor-fbdev.c
>>> +++ b/src/compositor-fbdev.c
>>> @@ -785,7 +785,7 @@ fbdev_backend_create(struct weston_compositor
>>> *compositor, int *argc, char *argv goto out_launcher;
>>> }
>>>
>>> - if (gl_renderer->create(compositor,
>>> NO_EGL_PLATFORM,
>>> + if (gl_renderer->display_create(compositor,
>>> NO_EGL_PLATFORM, EGL_DEFAULT_DISPLAY,
>>> gl_renderer->opaque_attribs,
>>> NULL, 0) < 0) {
>>> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
>>> index d0d1082..e4595c7 100644
>>> --- a/src/compositor-wayland.c
>>> +++ b/src/compositor-wayland.c
>>> @@ -2261,7 +2261,7 @@ wayland_backend_create(struct
>>> weston_compositor *compositor, }
>>>
>>> if (!b->use_pixman) {
>>> - if (gl_renderer->create(compositor,
>>> + if (gl_renderer->display_create(compositor,
>>> EGL_PLATFORM_WAYLAND_KHR,
>>> b->parent.wl_display,
>>> gl_renderer->alpha_attribs,
>>> diff --git a/src/compositor-x11.c b/src/compositor-x11.c
>>> index 629b5f3..78b9c62 100644
>>> --- a/src/compositor-x11.c
>>> +++ b/src/compositor-x11.c
>>> @@ -1557,8 +1557,9 @@ init_gl_renderer(struct x11_backend *b)
>>> if (!gl_renderer)
>>> return -1;
>>>
>>> - ret = gl_renderer->create(b->compositor,
>>> EGL_PLATFORM_X11_KHR, (void *) b->dpy,
>>> - gl_renderer->opaque_attribs,
>>> NULL, 0);
>>> + ret = gl_renderer->display_create(b->compositor,
>>> EGL_PLATFORM_X11_KHR,
>>> + (void *) b->dpy,
>>> +
>>> gl_renderer->opaque_attribs, NULL, 0);
>>>
>>> return ret;
>>> }
>>> diff --git a/src/gl-renderer.c b/src/gl-renderer.c
>>> index 23c0cd7..197e5c2 100644
>>> --- a/src/gl-renderer.c
>>> +++ b/src/gl-renderer.c
>>> @@ -2873,7 +2873,7 @@ platform_to_extension(EGLenum platform)
>>> }
>>>
>>> static int
>>> -gl_renderer_create(struct weston_compositor *ec, EGLenum platform,
>>> +gl_renderer_display_create(struct weston_compositor *ec, EGLenum
>>> platform, void *native_window, const EGLint *attribs,
>>> const EGLint *visual_id, int n_ids)
>>> {
>>> @@ -3147,7 +3147,7 @@ WL_EXPORT struct gl_renderer_interface
>>> gl_renderer_interface = { .opaque_attribs =
>>> gl_renderer_opaque_attribs, .alpha_attribs =
>>> gl_renderer_alpha_attribs,
>>>
>>> - .create = gl_renderer_create,
>>> + .display_create = gl_renderer_display_create,
>>> .display = gl_renderer_display,
>>> .output_create = gl_renderer_output_create,
>>> .output_destroy = gl_renderer_output_destroy,
>>> diff --git a/src/gl-renderer.h b/src/gl-renderer.h
>>> index 71f6b46..cd67a89 100644
>>> --- a/src/gl-renderer.h
>>> +++ b/src/gl-renderer.h
>>> @@ -75,7 +75,7 @@ struct gl_renderer_interface {
>>> const EGLint *opaque_attribs;
>>> const EGLint *alpha_attribs;
>>>
>>> - int (*create)(struct weston_compositor *ec,
>>> + int (*display_create)(struct weston_compositor *ec,
>>> EGLenum platform,
>>> void *native_window,
>>> const EGLint *attribs,
>>> --
>>> 2.8.0
>>>
>>> _______________________________________________
>>> wayland-devel mailing list
>>> wayland-devel at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>>
>
>
More information about the wayland-devel
mailing list