Kenny Levinsen authored 4 years ago and Pekka Paalanen committed 3 years ago

get_vt is used to check if VTs are enabled, by verifying that a VT greater than
0 is returned.

libseat always implements switching, with switch to an active session
currently being a noop in all backends. libseat does not currently have
a get_vt implementation. Make get_vt errors more explicit, and allow VT
switching anyway if the error is ENOSYS.

Signed-off-by: Kenny Levinsen <kl@kl.wtf>

Kenny Levinsen authored 4 years ago and Pekka Paalanen committed 3 years ago

This adds support for libseat as a seat backend. libseat provides seatd,
(e)logind and direct seat backends as compile-time and runtime options.

The backend is currently disabled by default. It can be enabled through the
launcher-libseat option.

Signed-off-by: Kenny Levinsen <kl@kl.wtf>

Signed-off-by: Jonathan Marler <johnnymarler@gmail.com>

Marius Vlad authored 3 years ago and Daniel Stone committed 3 years ago

In some situations, like positioning a sub-surface that exceeds the
output's dimensions we would adjust the plane state dimensions to some
lower values to that of the buffer. That would ultimately trip the cursor
update function because the buffer itself actually exceeds the maximum
size/dimension of the cursor.

The plane state destination co-ordinates is the area of the view which
is visible on the output, which in some situations, would actually be
smaller than the original buffer dimensions (making it so that it will
pass the cropping/scaling check), but depending on of
how large is the surface buffer, it would tripping the assert wrt to
cursor width/height dimensions.

This hasn't been seen so far due to the fact that until recently we had
a cursor surface that always reached the cursor plane and that was
already being set-up by default (with desktop-shell, which is no longer
the case), and also because kiosk-shell, which doesn't set-up a cursor
surface, was not available.

This adds a check to skip placing the view in the cursor plane if the
buffer dimensions exceed the cursor permitted width/height.
(Suggested-by Daniel Stone).

Signed-off-by: Marius Vlad <marius.vlad@collabora.com>

Jonathan Marler authored 3 years ago and Daniel Stone committed 3 years ago

fixes issue #484 (race condition with message to/from weston launch)

The race condition occurs after weston sends the WESTON_LAUNCHER_OPEN
message to weston-launch.  The race is between when weston-launch replies
with the fd handle versus weston-launch sending an activation message.  If
weston-launch sends an activation message before sending the fd handle,
then weston will be in an invalid state.

To fix this, I modified the fd handle reply that weston-launch sends to
include a message id at the beginning, which I called
WESTON_LAUNCHER_OPEN_REPLY.  Along with this, weston now inspects the
first part of any reply to determine whether it is an activation message
or a reply to the OPEN message.  In the newly handled case that it's an
activation message, it tracks whether the latest result is a deactivate
message and stores it in a flag to be handled once the open function has
completed.

Signed-off-by: Jonathan Marler <johnnymarler@gmail.com>

Pekka Paalanen authored 3 years ago and Pekka Paalanen committed 3 years ago

Now that pieces of color management implementation start to land, the
fallback shader becomes even more special than before. It is the only
case where the compositor ignores color management.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

Pekka Paalanen authored 3 years ago and Pekka Paalanen committed 3 years ago

The texture target can be uniquely inferred from the shader variant, so
do not store texture target separately.

Shortens the code a bit.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

Pekka Paalanen authored 3 years ago and Pekka Paalanen committed 3 years ago

Replace the shader_requirements with just shader_variant. The variant is
the only thing gl_surface_state will actually carry. All the other
requirements fields are always unused.

Co-authored-by: Sebastian Wick <sebastian@sebastianwick.net>
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

Pekka Paalanen authored 3 years ago and Pekka Paalanen committed 3 years ago

This patch gathers all values to be loaded to shader uniforms into a new
struct gl_shader_config along with texture target and filter
information. Struct gl_shader becomes opaque outside of gl-shaders.c.
Everything that used or open-coded these are converted.

The aim is to make gl-renderer.c easier to read. Previously, uniform
values were loaded up in various places, texture units were set up in
one place, textures were bound into units in different places. Stuff was
all over the place.

Now, shader requirements and associated uniform data is stored in a
single struct. The data is loaded into a shader program in one function
only.

That makes it easy for things like maybe_censor_override() to replace
the whole config rather than poke only the shader requirements. This may
not look like much right now, but when color management adds more
uniforms and even hardcoded color need to go through the proper color
pipeline, doing things the old way would become intractable.

Similar simplification can be seen in draw_view(), where the RGBA->RGBX
override becomes more contained. There is no longer a need to "pre-load"
the shader used by triangle fan debug. Triangle fan debug no longer
needs to play tricks with saving and restoring the current shader.

The real benefit of this change will probably come when almost all
shader operations need to take color spaces into account. That means
filling in gl_shader_config parts based on a color transformation.

This is based on an idea Sebastian already used in his Weston color
management work.

Co-authored-by: Sebastian Wick <sebastian@sebastianwick.net>
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

Pekka Paalanen authored 3 years ago and Pekka Paalanen committed 3 years ago

This paves way for making struct gl_shader opaque outside of
gl-shaders.c.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

Pekka Paalanen authored 3 years ago and Pekka Paalanen committed 3 years ago

Avoid looking up 'gr' from view->compositor by passing it explicitly
into the functions needing it.

Also fixes the whitespace in repaint_region() signature.

Clarifies code by removing local variables, but also future changes will
need 'gr' more.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

Pekka Paalanen authored 3 years ago and Pekka Paalanen committed 3 years ago

A future change will call this function from draw_view(), so move it
upwards to avoid adding a function declaration.

No functional or even cosmetic change.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

Pekka Paalanen authored 3 years ago and Pekka Paalanen committed 3 years ago

These functions are related to shaders, so they are more at home in
gl-shaders.c. gl-renderer.c is too long already.

This allows making a couple functions static while the moved functions
become non-static. Future changes turn some of these functions into
static again, with the ultimate goal of making struct gl_shader opaque.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

desktop-shell's client is able to read-up from the config file, [shell]
section the background, but for kiosk-shell we don't actually have
client that does that, so instead allow the shell do that directly.
Seems to be a useful thing to have, as a default background color.

Signed-off-by: Marius Vlad <marius.vlad@collabora.com>

When using xwayland surfaces and multiple outputs we need to notify
xwayland surface that the surface position has changed, otherwise we're
going to end up with pop-ups being displayed on other outputs rather
than the one were the main surface resides.

Stolen from desktop-shell.

Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Suggested-by: Alexandros Frantzis <alexandros.frantzis@collabora.com>

Signed-off-by: Marius Vlad <marius.vlad@collabora.com>

Tearing down the drm-backend when there are no input devices, would call
for the gbm device destruction before compositor shutdown. The latter
would call into the renderer detroy function and assume that the
EGLDisplay, which was created using the before-mentioned gbm device, is
still available. This patch re-orders the gbm destruction after the
compositor shutdown when no one would make use of it.

Fixes: #314

Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Suggested-by: Daniel Stone <daniel.stone@collabora.com>

This allows to specify a custom DRM format. For instance, to test
XBGR2101010:

    weston-simple-dmabuf-egl -f 0x30334258

Signed-off-by: Simon Ser <contact@emersion.fr>

This used a cargo-culted form of the ACTION check. Kernel is allowed to
invent new ACTIONs and IIRC there are already actions like bind and
unbind.

Udev events before rule processing always start with a clean property
list. This means that if you only match ACTION==add to add some value to
the event, then that value will not be present for ACTION==bind. Udev
event consumers do not accumulate values, so inconsistent value setting
may confuse them.

Therefore one needs to match ACTION!=remove, not ACTION==add|change, to
keep the device properties consistent for every event. It doesn't hurt
to set them on remove either, but it's a habit to try to avoid
processing when not strictly needed.

This issue came up in
#476 (comment 841430)

For more information, see
https://lists.freedesktop.org/archives/systemd-devel/2020-November/045570.html


the part "KERNEL API INCOMPATIBILITY" near the beginning.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

Since commit "libweston: add required_capabilities test suite quirk"
a new function that depends on test_data is being called in wet_main().

We should check if test_data is NULL before calling it, otherwise
we have a segfault when running outside the test suite.

Signed-off-by: Leandro Ribeiro <leandro.ribeiro@collabora.com>

In CI we should never get a skip, so turn them into failures to make
sure we notice.

This is enabled only for the configuration where we build everything. If
anything is disabled, skips are expected.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

This will be useful in CI, where we do not want to see any skips. If
something starts to skip, that's a mistake somewhere, and want to catch
it.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

There is no case in pre-processor directives where would like to have
undefined identifiers be silently replaced with a zero. This warning can
discover typos and forgotten includes.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

Everywhere else uses #ifdef, this used just #if. When the next commit
adds -Wundef to the compiler options, this #if here will start failing
as ENABLE_EGL is undefined.

It would be much better to use Meson's set10() for ENABLE_EGL and change
all #ifdef into #if, but I opted for the smaller change for now.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

Code is using the form
  #if ENABLE_JUNIT_XML
which is fine until we start using -Wundef. I think the existing code
would fail or at least warn if you disabled test-junit-xml with -Wundef.

Make sure ENABLE_JUNIT_XML is always defined so that -Wundef can be
added to build flags.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

If the compositor does not have the shadow buffer capability (implied by
the color ops capability bit), then trying to run the shadow buffer test
is useless, it would just fail. Let it skip instead.

Fixes: b1e56143

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

This allows tests to skip when required capabilities are not present.
The output damage test for the shadow buffer case needs this.

required_capabilities is added to struct weston_testsuite_quirks which
is libweston public API just because there is no better place currently.
This is a little weird because the code to check it is in compositor,
not libweston.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

Gives a string for all existing capability bits. This is useful for the
next commit.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

The capability strings will be printed also in another occasion, where
the colon does not fit with the capability description.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

This should not be necessary, but my testing with fd.o Gitlab 13.9.1
shows this is needed. Otherwise the coverage markings will not appear in
a MR diff view.

Apparently Gitlab has some problem with 'filename' attribute containing
"../" in Cobertura XML files, even when that does result in a correct
path. Or maybe the problem is is with the <source> path referring to the
build dir which from Gitlab perspective does not exist in the project,
even though builddir/../ is a good path.

This sed hack removes the "../" part and the last element in the
<source> path correspondingly.

See !567


for my testing.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

This runs the coverage tools to produce HTML pages listing the code lines /
functions / branches hit/totalled by the test suite.

Nowadays Gitlab has some Cobertura support itself:
https://docs.gitlab.com/ee/user/project/merge_requests/test_coverage_visualization.html



lcov is needed for the HTML report, gcovr is needed for the Cobertura
report. 'ninja clean' must be removed, otherwise it deletes the coverage
files before they are analysed.

Seeing the test suite code coverage is really interesting. It can guide
designing tests. If Gitlab MRs show the coverage in diff view, it shows
if new code actually gets executed in CI.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

Vlad Zahorodnii authored 4 years ago and Simon Ser committed 4 years ago

The -listen <fd> option has been deprecated. Its replacement is -listenfd.

Signed-off-by: Vlad Zahorodnii <vlad.zahorodnii@kde.org>

Doing it when the surface is being added would cause clients that
wait for frame callbacks to wait indefinitely as the surface being
activated is not yet, committed.

Fixes: #473

Signed-off-by: Marius Vlad <marius.vlad@collabora.com>

This should help correlate which shaders were used in repainting outputs
recently.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

This adds a heuristic for freeing shader programs that have not been
needed for a while. The intention is to stop Weston accumulating shader
programs indefinitely, especially in the future when color management
will explode the number of possible different shader programs.

Shader programs that have not been used in the past minute are freed,
except always keep the ten most recently used shader programs anyway.
The former rule is to ensure we keep shader programs that are actively
used regardless of how many.  The latter rule is to prevent freeing too
many shader programs after Weston has been idle for a long time and then
repaints just a small area. Many of the shader programs could still be
relevant even though not needed in the first repaint after idle.

The numbers ten and one minute in the above are arbitrary and not based
on anything.

These heuristics are simpler to implement than e.g. views taking
references on shader programs. Expiry by time allows shader programs to
survive a while even after their last user is gone, with the hope of
being re-used soon. Tracking actual use instead of references also
adapts to what is actually visible rather than what merely exists.

Keeping the shader list in most recently used order might also make
gl_renderer_get_program() more efficient on average.

last_repaint_start time is used for shader timestamp to avoid calling
clock_gettime() more often. Adding that variable is an ABI break, but
libweston major has already been bumped to 10 since last release.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

This is useful for seeing that the shader program garbage collection
works in a future patch.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

One more thing is coming to need this, so add the compositor pointer and
migrate existing places to use it where it simplifies things.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

Fix some deviations from the coding style.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

I have verified that the conversion here follows ITU-R BT.601 except for
the offsets 16/256 and 128/256 which should be 16/255 and 128/255
respectively.

I used to following octave script to verify this:

rf = 0.299;
gf = 0.587;
bf = 0.114;

crdiv = 1.402;
cbdiv = 1.772;

M = [ rf, gf, bf ;
      -rf / cbdiv, -gf / cbdiv, (1 - bf) / cbdiv;
      (1 - rf) / crdiv, -gf / crdiv, -bf / crdiv ];

YCbCr = [ 'Y'; 'Cb'; 'Cr' ];
RGB = [ 'R'; 'G'; 'B' ];
eq = [ ' '; '='; ' ' ];
l = [ ' [ '; ' [ '; ' [ ' ];
r = [ ' ] '; ' ] '; ' ] ' ];

mat = [
	sprintf('%9f %9f %9f', M(1,:));
	sprintf('%9f %9f %9f', M(2,:));
	sprintf('%9f %9f %9f', M(3,:));
];

[ l YCbCr r eq l mat r l RGB r ]

R = inv(M);

mat = [
	sprintf('%9f %9f %9f', R(1,:));
	sprintf('%9f %9f %9f', R(2,:));
	sprintf('%9f %9f %9f', R(3,:));
];

[ l RGB r eq l mat r l YCbCr r ]

[ R(:,1), R(:,2:3) .* (255/224) ]

The final matrix printed is what the shader uses down to +/- one digit,
so at least 7 correct decimals.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>

Sampling input texture has nothing to do with view alpha. This clarifies
the code structure.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>