[waffle] [PATCH 2/4] wcore: don't parse/validate the exact major/minor version
Chad Versace
chad.versace at intel.com
Tue Nov 10 12:08:02 PST 2015
On Tue 01 Sep 2015, Emil Velikov wrote:
> Keeping track what is and isn't the correct version at any given time
> sounds unfeasable.
>
> On one hand currently we allow any random value as minor for ES2 and
> ES3, whist for GL we allow any values as long as they are >= 1.0
I agree, that inconsistency is ugly.
> If we are to keep track what versions are valid, this is going to cause
> problems as new versions of the specs get released.
In theory, I don't believe that would be a problem if Waffle validated
only the context version's *lower* bound. The lower bounds should never
change, not even when new specs are released. For OpenGL !ES, Waffle
validates only the lower bound.
For OpenGL ES, though, Waffle tries to be smarter, and therefore is
dumber. So I agree with you...
> Let's do the sane thing and remove the bookkeeping from waffle.
... let's do the sane thing and remove all the fine-grained validation.
> Note: this will break the tests. Should we update them prior, alongside
> or after this commit ?
Let's avoid regressing the unit tests. Update them in this commit.
> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
> ---
> src/waffle/core/wcore_config_attrs.c | 71 ------------------------------------
> 1 file changed, 71 deletions(-)
>
> diff --git a/src/waffle/core/wcore_config_attrs.c b/src/waffle/core/wcore_config_attrs.c
> index 4a2cb5d..3c6753a 100644
> --- a/src/waffle/core/wcore_config_attrs.c
> +++ b/src/waffle/core/wcore_config_attrs.c
> @@ -160,74 +160,6 @@ set_context_version_default(struct wcore_config_attrs *attrs)
> }
>
> static bool
> -parse_context_version(struct wcore_config_attrs *attrs,
> - const int32_t attrib_list[])
> -{
> - wcore_attrib_list32_get(attrib_list, WAFFLE_CONTEXT_MAJOR_VERSION,
> - &attrs->context_major_version);
> - wcore_attrib_list32_get(attrib_list, WAFFLE_CONTEXT_MINOR_VERSION,
> - &attrs->context_minor_version);
Ack! You can't remove the above two calls. Otherwise
attrs->context_major/minor_version never get set. That leads to weird
bugs like this:
On master, all commands fail when given bogus context version 4.7.
$ bin/wflinfo -p gbm -a gles2 --version 4.7
Waffle error: 0x8 WAFFLE_ERROR_BAD_ATTRIBUTE: for OpenGL ES2, the requested major context version must be 2
$ bin/wflinfo -p gbm -a gl --profile core --version 4.7
Waffle error: 0x2 WAFFLE_ERROR_UNKNOWN: eglCreateContext failed with error EGL_BAD_MATCH(0x3009)
$ bin/gl_basic --platform glx --api gl --profile core --version 4.7
gl_basic: error: WAFFLE_ERROR_UNKNOWN: glXCreateContextAttribsARB failed
The same commands, with this patch applied. wflinfo succeeds in
creating an OpenGL ES 4.7 context! And the other commands fail with
incorrect error messages.
$ bin/wflinfo -p gbm -a gles2 --version 4.7
Mesa warning: couldn't open libtxc_dxtn.so, software DXTn
compression/decompression unavailable
Waffle platform: gbm
Waffle api: gles2
OpenGL vendor string: Intel Open Source Technology Center
OpenGL renderer string: Mesa DRI Intel(R) HD Graphics 5500 (Broadwell GT2)
OpenGL version string: OpenGL ES 3.0 Mesa 11.0.4 (git-774dd01)
$ bin/wflinfo -p gbm -a gl --profile core --version 4.7
Waffle error: 0x8 WAFFLE_ERROR_BAD_ATTRIBUTE: for OpenGL < 3.2, WAFFLE_CONTEXT_PROFILE must be WAFFLE_NONE
$ bin/gl_basic --platform glx --api gl --profile core --version 4.7
gl_basic: error: WAFFLE_ERROR_BAD_ATTRIBUTE: for OpenGL < 3.2, WAFFLE_CONTEXT_PROFILE must be WAFFLE_NONE
> -
> - if (attrs->context_major_version < 1) {
> - wcore_errorf(WAFFLE_ERROR_BAD_ATTRIBUTE,
> - "WAFFLE_CONTEXT_MAJOR_VERSION must be >= 1");
> - return false;
> - }
> -
> - if (attrs->context_minor_version < 0) {
> - wcore_errorf(WAFFLE_ERROR_BAD_ATTRIBUTE,
> - "WAFFLE_CONTEXT_MINOR_VERSION must be >= 0");
> - return false;
> - }
I agree that all validation below this line should be killed.
However, I want to keep the above two sanity checks. Those checks do
catch completely incorrect input. For example, the checks would prevent
the user from providing WAFFLE_DONT_CARE as a context version, which
would eventually get passed to EGL/GLX/WGL as a *negative* context
version. Waffle should avoid passing negative version to GL drivers...
who knows what fun driver bugs we'll discover in requesting a -1.0
context?
> -
> - switch (attrs->context_api) {
> - case WAFFLE_CONTEXT_OPENGL:
> - if (wcore_config_attrs_version_lt(attrs, 10)) {
> - wcore_errorf(WAFFLE_ERROR_BAD_ATTRIBUTE,
> - "for OpenGL, the requested context version "
> - "must be >= 1.0");
> - return false;
> - }
> - break;
> -
> - case WAFFLE_CONTEXT_OPENGL_ES1:
> - if (!wcore_config_attrs_version_eq(attrs, 10) &&
> - !wcore_config_attrs_version_eq(attrs, 11)) {
> - wcore_errorf(WAFFLE_ERROR_BAD_ATTRIBUTE,
> - "for OpenGL ES1, the requested context version "
> - "must be 1.0 or 1.1");
> - return false;
> - }
> - break;
> -
> - case WAFFLE_CONTEXT_OPENGL_ES2:
> - if (attrs->context_major_version != 2) {
> - wcore_errorf(WAFFLE_ERROR_BAD_ATTRIBUTE,
> - "for OpenGL ES2, the requested major context "
> - "version must be 2");
> - return false;
> - }
> - break;
> -
> - case WAFFLE_CONTEXT_OPENGL_ES3:
> - if (attrs->context_major_version != 3) {
> - wcore_errorf(WAFFLE_ERROR_BAD_ATTRIBUTE,
> - "for OpenGL ES3, the requested major context "
> - "version must be 3");
> - return false;
> - }
> - break;
> -
> - default:
> - wcore_error_internal("attrs->context_api has bad value 0x%x",
> - attrs->context_api);
> - return false;
> - }
> -
> - return true;
> -}
> -
> -static bool
> set_context_profile_default(struct wcore_config_attrs *attrs)
> {
> switch (attrs->context_api) {
> @@ -478,9 +410,6 @@ wcore_config_attrs_parse(
> if (!set_context_version_default(attrs))
> return false;
>
> - if (!parse_context_version(attrs, waffle_attrib_list))
> - return false;
> -
> if (!set_context_profile_default(attrs))
> return false;
>
> --
> 2.5.0
>
> _______________________________________________
> waffle mailing list
> waffle at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/waffle
More information about the waffle
mailing list