Refactor the specification of OpenGL context attributes
ClosedPublic

Authored by graesslin on Jun 26 2017, 3:52 PM.

Details

Summary

The current way to specify the OpenGL context attributes does no longer
scale as can be seen in D6344. There are too many different context
attribute sets and with every addition we grow lots of copied code. The
chances to introduce errors in that code which is difficult to debug are
very high. As can be seen in the glx backend which defines major 1,
minor 2, but it should be major 2, minor 1.

This change reworks this code by creating a builder class which contains
only an abstract definition of what needs to be in the attributes.
E.g. the version, whether it's robust and so on.

Now we can just have a list of possible attributes in a well described
way:

auto builder;
builder.setVersion(3, 1);
builder.setRobust(true);

All possible builders are added to a list and operated on in a for loop
which tries to creat a context. Once it succeeded it breaks the list.
In addition a debug statement is added which prints out the set of
options which went into the context.

So far this is only done for EGL, GLX can follow once D6344 is merged.

Test Plan

New unit test added, kwin_wayland OpenGL tests run and verified

Diff Detail

Repository
R108 KWin
Branch
context-attribute-builder
Lint
No Linters Available
Unit
No Unit Test Coverage
graesslin created this revision.Jun 26 2017, 3:52 PM
Restricted Application added a project: KWin. · View Herald TranscriptJun 26 2017, 3:52 PM
Restricted Application added subscribers: kwin, plasma-devel. · View Herald Transcript
davidedmundson requested changes to this revision.Jun 26 2017, 4:16 PM
davidedmundson added a subscriber: davidedmundson.
davidedmundson added inline comments.
abstract_egl_backend.cpp
235

this is different from line 252 on the original, which additionally checks hasCreateContext

This revision now requires changes to proceed.Jun 26 2017, 4:16 PM
graesslin updated this revision to Diff 15894.Jun 26 2017, 8:14 PM
graesslin edited edge metadata.

Fix missing haveCreateContext - good spot Sir!

Restricted Application edited projects, added Plasma; removed KWin. · View Herald TranscriptJun 26 2017, 8:14 PM
graesslin marked an inline comment as done.Jun 26 2017, 8:18 PM
davidedmundson accepted this revision.Jun 26 2017, 8:43 PM
davidedmundson added inline comments.
egl_context_attribute_builder.cpp
51

Do you have some sort of checklist of C++11 features to use?

For a single integer, emplace_back is going to do the exact same thing as the standard push_back.

This revision is now accepted and ready to land.Jun 26 2017, 8:43 PM
graesslin added inline comments.Jun 27 2017, 4:22 AM
egl_context_attribute_builder.cpp
51

If one always uses emplace one doesn't forget to use it when it matters

This revision was automatically updated to reflect the committed changes.