<div dir="ltr">De-duplicating and then trimming down works for me.<div class="gmail_extra"> <div class="gmail_quote">On Thu, Oct 19, 2017 at 3:31 AM, Emil Velikov <<a href="mailto:emil.l.velikov@gmail.com" target="_blank">emil.l.velikov@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 18 October 2017 at 23:36, Gurchetan Singh <<a href="mailto:gurchetansingh@chromium.org">gurchetansingh@chromium.org</a>> wrote: >> Then again, I'd suggest keeping that as separate series. These patches >> started as a way to minimise the duplication we have in drivers/dri2. > > I'm fine with dri2_$action_$object. We can modify the existing functions > later, but I recommend adopting more concise conventions in this patchset, > i.e: > > dri2_egl_surface_record_buffers_and_update_back_buffer --> > dri2_set_back_buffer_surface > dri2_egl_surface_free_outdated_buffers_and_update_size --> > dri2_fixup_surface > dri2_egl_surface_update_buffer_age --> dri2_update_age_surface > dri2_egl_surface_get_image_front --> dri2_get_front_image_surface > Sure thing, let's use consistent names with this series. >> goal the series is to a) remove a handful of the ifdef spaghetti and > > I agree, struct dri2_egl_surface can be refactored. I would advocate a > solution where the surface (a) has everything a platform needs but nothing > else (b) has a minimal amount of duplication. I would like to look at the > struct and see if it defines buffers[5], it must mean the platform > implements get_buffers_with_format for example. If a platform doesn't > define color_buffers, it means EXT_buffer_age is not used for whatever > reason. Everything has dri_image_front -- then everything must use the > image extension. I think this type of self-consistency is useful, from a > code is documentation point of view. Here's pseudo-code of what I would > want: > I agreed with the goals but I would swap the order/priority - de-duplicate, then trim down. By de-duplicating/refactoring one could add support for X/Y fairly easily. Once all that is done, we can polish ;-) I fear that otherwise there will be a lot of unnecessary churn. Thanks Emil </blockquote></div> </div></div>