From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keith Packard Subject: Re: [PATCH 7/8] dri: add __DRIimageLoaderExtension and __DRIimageDriverExtension Date: Tue, 05 Nov 2013 15:47:02 -0800 Message-ID: <86ppqe5s3t.fsf@miki.keithp.com> References: <1383618208-21310-1-git-send-email-keithp@keithp.com> <1383618208-21310-8-git-send-email-keithp@keithp.com> <87zjpifwc3.fsf@eliezer.anholt.net> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1752295858==" Return-path: Received: from keithp.com (home.keithp.com [63.227.221.253]) by gabe.freedesktop.org (Postfix) with ESMTP id 723A0FB8F4 for ; Tue, 5 Nov 2013 15:47:07 -0800 (PST) In-Reply-To: <87zjpifwc3.fsf@eliezer.anholt.net> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org To: Eric Anholt , mesa3d-dev@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============1752295858== Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" --=-=-= Content-Transfer-Encoding: quoted-printable Eric Anholt writes: > Most of my review was going to be whining about yet another (broken) > copy of dri2CreateNewScreen2. Sounds like you've fixed that. Yup, figured that out all on my own after I re-read the code -- the only difference was that I need to look for the DRIimageLoader hooks, which I now just do in the shared function. >> + >> +#define __DRI_DRIVER_EXTENSIONS "__driDriverExtensions" > > This looks like rebase fail Removed. >> +//struct gl_context; >> +//struct dd_function_table; > > Looks like development leftovers. Removed. > Maybe append "Func" to the typedefs so they don't look like just another > struct in the declarations? And since they're supposed to be the same > function pointers as in the __DRIswrastExtensionRec and > __DRIdri2ExtensionRec, change them to this typedef, too? I've moved the typedefs, renamed them and stuck that part of the patch in the first patch of the series that renames the functions. > It looks like getBuffers could just be two getBuffer calls, except for > the updating of width and height. Have you looked into doing things > that way at all? Yes, that was the way I started doing it. There are two reasons for doing it in a single call: 1) Pixmaps always need a front buffer, and the driver doesn't know which drawables are pixmaps. 2) Deleting buffers when changing modes. I'd need to add another function to let the driver delete unused buffers. =20=20 Overall, I liked moving more buffer management logic to the loader and out of the driver, so I followed the DRI2 API here. > Style nit: we try and put braces around multi-line things like this, > even if they are a single statement. Fixed. >> -bool >> +GLboolean >> brwCreateContext(gl_api api, >> const struct gl_config *mesaVis, >> __DRIcontext *driContextPriv, ... >> __DRIdrawable *drawable); >>=20=20 >> -bool brwCreateContext(gl_api api, >> - const struct gl_config *mesaVis, >> - __DRIcontext *driContextPriv, >> - unsigned major_version, >> - unsigned minor_version, >> - uint32_t flags, >> - unsigned *error, >> - void *sharedContextPrivate); >> +GLboolean brwCreateContext(gl_api api, >> + const struct gl_config *mesaVis, >> + __DRIcontext *driContextPriv, >> + unsigned major_version, >> + unsigned minor_version, >> + uint32_t flags, >> + unsigned *error, >> + void *sharedContextPrivate); ... > > Unrelated change? Pulled out to a separate patch -- the return type for createContext is GLboolean, not bool, and my compiler was whinging. I've pushed these changes, along with a bunch of new comments in dri3_glx.c to my dri3 branch. I can send the new series to the list if that would be helpful. =2D-=20 keith.packard@intel.com --=-=-= Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.15 (GNU/Linux) iQIVAwUBUnmDdtsiGmkAAAARAQi63w//VWLl5N3Oqov0gBNRcTSrETYDxFRFkFUT s25XU9KB9Q0RDF/GWx6fDEThvolNkMv/J7T17Q43sz/RCbA1dhkCRFfn/0nsi8PY DjsJh8U6iXe5PfqXEPxcBdASdGatmSrq7emPEmuUbK9RpyauaP89/LdC4Pmsvoue Eas08LN+Z8dUvJc5r8chhz4NvFhhLN+IQDOP2EuBEg2ZbdC4Yv1sh+0wLHW4E043 sNkhjjlsaqKtl5cyMjsqgTSrk+CXLtxWRJ6MLtz+1b4s20Kqga3dFW2S/x1RqeE6 EqKhrjoxbnwCHqx3lt0MumshQBW+r4fi3cjQfbPV9KRNpKGCQMxrJUyK/GMHXdDK XzzEWiEVSeNf2eZveu3Sao3r48YEC0gUpBikJ86r17gMU6X9CLHdsMCFQ22G2F0f DDKJUxcLxRZVe3Py0FHBypZY0NLmCaS4UfC9lFjiicIvz+0eQsFc0T2kGoAKAOhZ XXq8dRi7w0Dot3u9kGSrU+mZPhFekUVvLZf5erwFioVQ4aXUGHn0Qa1JgY9YpOBr u/bFRFSX/5C+LE8+38PrKnWSC3LEsiDEIodP8eu2Gk1kUIATTpSn12auQaNO6mBy /MiIMihdFqZ0dyUq5m0pDG7bLQOzyiegl6qQLgQdJY7oMOIhQyr+SVkvmLqCNSPU NAmYaizx9rY= =pcfQ -----END PGP SIGNATURE----- --=-=-=-- --===============1752295858== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel --===============1752295858==--