[Mesa-dev] [PATCH 00/45] Remove many of the FEATURE_* defines.

[Oliver McFadden]

On Tue, Sep 11, 2012 at 07:59:38AM -0600, Brian Paul wrote:
> On 09/11/2012 03:56 AM, Oliver McFadden wrote:
> > Hi,
> >
> > As requested here is the patch series which removes many of the FEATURE_*
> > defines, typically used to enable or disable some GL feature or extension.
> > I have ran this series through automatic git-bisect with a script to compile
> > Mesa.  The series is bisect-clean with the following configuration:
> >
> > $ ./autogen.sh --prefix=/usr --without-gallium-drivers --with-dri-drivers=i965,swrast --enable-gles1 --enable-gles2
> >
> > I expect it's also bisect-clean with all other configurations, but if you wish
> > for further testing with Gallium and more DRI drivers then I need to install and
> > setup some other packages.
> >
> > mfeatures.h still exists and we still have FEATURE_GL, FEATURE_ES, FEATURE_ES1,
> > FEATURE_ES2, and FEATURE_remap_table.  I did not touch the last feature because
> > there is some Python code involved there and that's not my forte.
> >
> > The features are removed in roughly the order they were specified in
> > mfeatures.h; I took some liberties where it would make things easier to do
> > out-of-order operations.
> >
> > We probably have quite a few areas that are guarded by `#if FEATURE_GL' which
> > don't necessarily need to be due to the code checking the API; the goal of this
> > patch set was to reduce the feature defines down to GL, ES1 and ES2.  In that
> > regard it's successful.
> >
> > A follow-up patch could look at removing redundant FEATURE_GL guards, but they
> > will not hurt anything.
> >
> > 119 files changed, 327 insertions(+), 683 deletions(-)
> >
> > Comments are welcome just hopefully not "rewrite the whole series" (although
> > that's usually how things turn out.)
> 
> The "remove" patches look fine but I'm wondering what the story is 
> with the "replace" patches.  Why not just do removal instead?

I wanted to avoid the possibility of breaking something by removing a
guard around some code that is not checking ctx->API, for example.

This might cause some unknown breakage when compiling for ES1 and ES2
APIs that would not be detectable at compile time.

This is why I was going to go through and review each '#if FEATURE_GL'
line manually, after this set is applied, and check that there is a
following 'if (ctx->API == API_OPENGL)', then it's safe to remove the
guard.

Would you prefer that I do that "inline" (i.e. rewrite the patches and
do the manual checking for each patch) or just submit another patchset
to cleanup the extra '#if FEATURE_GL' blocks?

-- 
Oliver McFadden.