Re: [Mesa-dev] [PATCH 1/4] dri: Fix __DRIconfig reporting of __DRI_ATTRIB_SWAP_METHOD
On 10 August 2017 at 17:40, Thomas Hellstrom wrote: > It's checked in a later patch > > https://lists.freedesktop.org/archives/mesa-dev/2017-August/165848.html > > But I'm currently splitting it up into a series with some extra fixes in it. > Great, thanks. -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/4] dri: Fix __DRIconfig reporting of __DRI_ATTRIB_SWAP_METHOD
Hi, Emil, On 08/10/2017 06:20 PM, Emil Velikov wrote: Hi Thomas, Apologies for dropping in so late. On 9 August 2017 at 10:53, Thomas Hellstrom wrote: The attribMap had two entries for this attribute, and driGetConfigAttribIndex didn't return a proper value for this attribute. Fix this, and also make sure we return SWAP_UNDEFINED for single-buffer configs as required by the GLX_OML_swap_method spec. Finally bump the dri core extension version to 2, indicating that we correctly report __DRI_ATTRIB_SWAP_METHOD. Signed-off-by: Thomas Hellstrom --- include/GL/internal/dri_interface.h| 5 - src/mesa/drivers/dri/common/dri_util.c | 2 +- src/mesa/drivers/dri/common/utils.c| 8 ++-- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h index f676ac5..5e8fce7 100644 --- a/include/GL/internal/dri_interface.h +++ b/include/GL/internal/dri_interface.h @@ -726,9 +726,12 @@ struct __DRIuseInvalidateExtensionRec { /** * This extension defines the core DRI functionality. + * + * Version >= 2 indicates that getConfigAttrib with __DRI_ATTRIB_SWAP_METHOD + * returns a reliable value. */ #define __DRI_CORE "DRI_Core" -#define __DRI_CORE_VERSION 1 +#define __DRI_CORE_VERSION 2 The interface is bumped, yet nobody checks it. If I understand your series correctly, when old drivers are present we fallback to UNDEFINED and things should just work. Hence the version bump isn't really needed. Am I missing something? Thanks Emil It's checked in a later patch https://lists.freedesktop.org/archives/mesa-dev/2017-August/165848.html But I'm currently splitting it up into a series with some extra fixes in it. /Thomas ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/4] dri: Fix __DRIconfig reporting of __DRI_ATTRIB_SWAP_METHOD
Hi Thomas, Apologies for dropping in so late. On 9 August 2017 at 10:53, Thomas Hellstrom wrote: > The attribMap had two entries for this attribute, and > driGetConfigAttribIndex didn't return a proper value for this attribute. > Fix this, and also make sure we return SWAP_UNDEFINED for single-buffer > configs as required by the GLX_OML_swap_method spec. > > Finally bump the dri core extension version to 2, indicating that we > correctly report __DRI_ATTRIB_SWAP_METHOD. > > Signed-off-by: Thomas Hellstrom > --- > include/GL/internal/dri_interface.h| 5 - > src/mesa/drivers/dri/common/dri_util.c | 2 +- > src/mesa/drivers/dri/common/utils.c| 8 ++-- > 3 files changed, 7 insertions(+), 8 deletions(-) > > diff --git a/include/GL/internal/dri_interface.h > b/include/GL/internal/dri_interface.h > index f676ac5..5e8fce7 100644 > --- a/include/GL/internal/dri_interface.h > +++ b/include/GL/internal/dri_interface.h > @@ -726,9 +726,12 @@ struct __DRIuseInvalidateExtensionRec { > > /** > * This extension defines the core DRI functionality. > + * > + * Version >= 2 indicates that getConfigAttrib with __DRI_ATTRIB_SWAP_METHOD > + * returns a reliable value. > */ > #define __DRI_CORE "DRI_Core" > -#define __DRI_CORE_VERSION 1 > +#define __DRI_CORE_VERSION 2 > The interface is bumped, yet nobody checks it. If I understand your series correctly, when old drivers are present we fallback to UNDEFINED and things should just work. Hence the version bump isn't really needed. Am I missing something? Thanks Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/4] dri: Fix __DRIconfig reporting of __DRI_ATTRIB_SWAP_METHOD
On 10/08/17 02:25 PM, Thomas Hellstrom wrote: > On 08/10/2017 05:20 AM, Michel Dänzer wrote: >> On 09/08/17 06:53 PM, Thomas Hellstrom wrote: >>> The attribMap had two entries for this attribute, and >>> driGetConfigAttribIndex didn't return a proper value for this attribute. >>> Fix this, and also make sure we return SWAP_UNDEFINED for single-buffer >>> configs as required by the GLX_OML_swap_method spec. >>> >>> Finally bump the dri core extension version to 2, indicating that we >>> correctly report __DRI_ATTRIB_SWAP_METHOD. >>> >>> Signed-off-by: Thomas Hellstrom >> [...] >> >>> diff --git a/src/mesa/drivers/dri/common/utils.c >>> b/src/mesa/drivers/dri/common/utils.c >>> index c37d446..f3ea61e 100644 >>> --- a/src/mesa/drivers/dri/common/utils.c >>> +++ b/src/mesa/drivers/dri/common/utils.c >>> @@ -284,8 +284,9 @@ driCreateConfigs(mesa_format format, >>> modes->transparentIndex = GLX_DONT_CARE; >>> modes->rgbMode = GL_TRUE; >>> -if ( db_modes[i] == GLX_NONE ) { >>> +if ( db_modes[i] == GLX_NONE) { >>> modes->doubleBufferMode = GL_FALSE; >>> +modes->swapMethod = GLX_SWAP_UNDEFINED_OML; >> The indentation of the added line doesn't match that of the previous >> line. Patch 4 further changes this code (which is also what Brian's >> comment was about AFAICT) to be even less consistently indented compared >> to the surrounding code. > > Hmm, Actually this looks awkward in the patches only due to me using > spaces rather than tabs to indent in the new code (old code uses tabs). > Not sure what the preferred strategy is here, use mesa coding style > (spaces) or old dri coding style (tabs). I think first priority should be consistency with surrounding code whenever possible, which it is in this case. If the formatting is to be changed, it should be done in one go for the whole file or at least big portions at a time, without making any functional changes at the same time. > If the latter, we should probably at some point add subsystem editor > setup files that override the mesa default ones. I agree. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/4] dri: Fix __DRIconfig reporting of __DRI_ATTRIB_SWAP_METHOD
On 08/10/2017 05:20 AM, Michel Dänzer wrote: On 09/08/17 06:53 PM, Thomas Hellstrom wrote: The attribMap had two entries for this attribute, and driGetConfigAttribIndex didn't return a proper value for this attribute. Fix this, and also make sure we return SWAP_UNDEFINED for single-buffer configs as required by the GLX_OML_swap_method spec. Finally bump the dri core extension version to 2, indicating that we correctly report __DRI_ATTRIB_SWAP_METHOD. Signed-off-by: Thomas Hellstrom [...] diff --git a/src/mesa/drivers/dri/common/utils.c b/src/mesa/drivers/dri/common/utils.c index c37d446..f3ea61e 100644 --- a/src/mesa/drivers/dri/common/utils.c +++ b/src/mesa/drivers/dri/common/utils.c @@ -284,8 +284,9 @@ driCreateConfigs(mesa_format format, modes->transparentIndex = GLX_DONT_CARE; modes->rgbMode = GL_TRUE; - if ( db_modes[i] == GLX_NONE ) { + if ( db_modes[i] == GLX_NONE) { modes->doubleBufferMode = GL_FALSE; +modes->swapMethod = GLX_SWAP_UNDEFINED_OML; The indentation of the added line doesn't match that of the previous line. Patch 4 further changes this code (which is also what Brian's comment was about AFAICT) to be even less consistently indented compared to the surrounding code. Hmm, Actually this looks awkward in the patches only due to me using spaces rather than tabs to indent in the new code (old code uses tabs). Not sure what the preferred strategy is here, use mesa coding style (spaces) or old dri coding style (tabs). If the latter, we should probably at some point add subsystem editor setup files that override the mesa default ones. With that fixed in both patches, the series is Reviewed-by: Michel Dänzer Thanks for reviewing! Thomas ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/4] dri: Fix __DRIconfig reporting of __DRI_ATTRIB_SWAP_METHOD
On 09/08/17 06:53 PM, Thomas Hellstrom wrote: > The attribMap had two entries for this attribute, and > driGetConfigAttribIndex didn't return a proper value for this attribute. > Fix this, and also make sure we return SWAP_UNDEFINED for single-buffer > configs as required by the GLX_OML_swap_method spec. > > Finally bump the dri core extension version to 2, indicating that we > correctly report __DRI_ATTRIB_SWAP_METHOD. > > Signed-off-by: Thomas Hellstrom [...] > diff --git a/src/mesa/drivers/dri/common/utils.c > b/src/mesa/drivers/dri/common/utils.c > index c37d446..f3ea61e 100644 > --- a/src/mesa/drivers/dri/common/utils.c > +++ b/src/mesa/drivers/dri/common/utils.c > @@ -284,8 +284,9 @@ driCreateConfigs(mesa_format format, > modes->transparentIndex = GLX_DONT_CARE; > modes->rgbMode = GL_TRUE; > > - if ( db_modes[i] == GLX_NONE ) { > + if ( db_modes[i] == GLX_NONE) { > modes->doubleBufferMode = GL_FALSE; > +modes->swapMethod = GLX_SWAP_UNDEFINED_OML; The indentation of the added line doesn't match that of the previous line. Patch 4 further changes this code (which is also what Brian's comment was about AFAICT) to be even less consistently indented compared to the surrounding code. With that fixed in both patches, the series is Reviewed-by: Michel Dänzer -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/4] dri: Fix __DRIconfig reporting of __DRI_ATTRIB_SWAP_METHOD
The attribMap had two entries for this attribute, and driGetConfigAttribIndex didn't return a proper value for this attribute. Fix this, and also make sure we return SWAP_UNDEFINED for single-buffer configs as required by the GLX_OML_swap_method spec. Finally bump the dri core extension version to 2, indicating that we correctly report __DRI_ATTRIB_SWAP_METHOD. Signed-off-by: Thomas Hellstrom --- include/GL/internal/dri_interface.h| 5 - src/mesa/drivers/dri/common/dri_util.c | 2 +- src/mesa/drivers/dri/common/utils.c| 8 ++-- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h index f676ac5..5e8fce7 100644 --- a/include/GL/internal/dri_interface.h +++ b/include/GL/internal/dri_interface.h @@ -726,9 +726,12 @@ struct __DRIuseInvalidateExtensionRec { /** * This extension defines the core DRI functionality. + * + * Version >= 2 indicates that getConfigAttrib with __DRI_ATTRIB_SWAP_METHOD + * returns a reliable value. */ #define __DRI_CORE "DRI_Core" -#define __DRI_CORE_VERSION 1 +#define __DRI_CORE_VERSION 2 struct __DRIcoreExtensionRec { __DRIextension base; diff --git a/src/mesa/drivers/dri/common/dri_util.c b/src/mesa/drivers/dri/common/dri_util.c index 39ecaf0..31a3040 100644 --- a/src/mesa/drivers/dri/common/dri_util.c +++ b/src/mesa/drivers/dri/common/dri_util.c @@ -767,7 +767,7 @@ driSwapBuffers(__DRIdrawable *pdp) /** Core interface */ const __DRIcoreExtension driCoreExtension = { -.base = { __DRI_CORE, 1 }, +.base = { __DRI_CORE, 2 }, .createNewScreen= NULL, .destroyScreen = driDestroyScreen, diff --git a/src/mesa/drivers/dri/common/utils.c b/src/mesa/drivers/dri/common/utils.c index c37d446..f3ea61e 100644 --- a/src/mesa/drivers/dri/common/utils.c +++ b/src/mesa/drivers/dri/common/utils.c @@ -284,8 +284,9 @@ driCreateConfigs(mesa_format format, modes->transparentIndex = GLX_DONT_CARE; modes->rgbMode = GL_TRUE; - if ( db_modes[i] == GLX_NONE ) { + if ( db_modes[i] == GLX_NONE) { modes->doubleBufferMode = GL_FALSE; +modes->swapMethod = GLX_SWAP_UNDEFINED_OML; } else { modes->doubleBufferMode = GL_TRUE; @@ -403,7 +404,6 @@ static const struct { unsigned int attrib, offset; } attribMap[] = { * so the iterator includes them though.*/ __ATTRIB(__DRI_ATTRIB_RENDER_TYPE, level), __ATTRIB(__DRI_ATTRIB_CONFIG_CAVEAT, level), -__ATTRIB(__DRI_ATTRIB_SWAP_METHOD, level) }; @@ -428,10 +428,6 @@ driGetConfigAttribIndex(const __DRIconfig *config, else *value = 0; break; -case __DRI_ATTRIB_SWAP_METHOD: -/* XXX no return value??? */ - break; - default: /* any other int-sized field */ *value = *(unsigned int *) -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev