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>
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>
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>
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>
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>
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>
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>
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>
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>
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
https://gitlab.freedesktop.org/wayland/weston/-/issues/476#note_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: b1e56143c5
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>
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 https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/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>
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 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>
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>
Reading the input texture is just one part of the future color pipeline,
so separate it into a function of its own. This makes it easier to add
more steps to the pipeline, and shows the green tint is separate as
well.
Making use of early returns, reducing the if-else ladder should help
with readability. Sharing the call to yuva2rgba() likewise.
Setting yuva.w = alpha is not shared though, in case support for AYUV
format might be added in the future.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Do not call texture2D() in the shader when we already have the result.
Simpler code, maybe even a little bit faster?
Suggested-by: Harish Krupo <harishkrupo@gmail.com>
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
These same magic constants were used in all cases, so move them into a
common place.
While we are touching all these lines, also change from the four floats
into a vec4. This allows further clean-up in the next patch.
This makes the code easier to read.
Behavior and results are unchanged.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Mathematically the result is the same, while multiplying RGB with alpha
is easier to understand as correct than the earlier form.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
A more unique name is easier to grep for. Using 'color' as a local
variable might be useful in the future.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This will hit the XYUV shader variant in GL-renderer that was not
covered in the test suite before.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This support is added so that the XYUV shader variant can be tested with
wl_shm from the test suite.
Libwayland version requirement is bumped to get WL_SHM_FORMAT_XYUV8888.
Libwayland is bumped to 1.18 too in the CI image. libwayland-dev package
is dropped, because we build wayland anyway.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This header is for sharing fallback definitions for drm_fourcc.h. A new
test in tests/yuv-buffer-test.c is going to be needing XYUV8888 format,
and more new formats will be expected with HDR supports.
Share these fallback definitions in one place instead of copying them
all over.
All users of drm_fourcc.h are converted to include weston-drm-fourcc.h
instead for consistency: have the same definitions available everywhere.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
MOD_INVALID came with libdrm 2.4.83 and MOD_LINEAR came with libdrm
2.4.82. libweston unconditionally depends on libdrm >= 2.4.95, so the
fallback is not necessary.
Since linux-dmabuf.h itself has no use for these and also forgets to
include drm_fourcc.h, .c files including drm_fourcc.h after this header
would trigger compiler warnings.
linux-dmabuf.c does need these, so add the proper include.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
These were introduced in libdrm 2.4.68, commit
268ae7cae5afd76462c3ef14ed9021a2d40c2e57. Weston unconditionally
requires libdrm >= 2.4.95, so these fallback definitions are
unnecessary now.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Extend the existing output-damage test to test
blit_shadow_to_output() specifically. This function had problems
originally, so make sure they can't reappear.
The added quirk is explained in the test.
An additional check of the quirk in gl_renderer_output_create() ensures
that the shadow framebuffer is really used. The test could false-pass if
the shadow is not used.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
The version we used does not advertise GL_EXT_color_buffer_half_float
with llvmpipe even though the functionality seems to work.
If the extension is not advertised, the future commit
"tests: extend output-damage to GL shadow framebuffer"
will result in a test failure.
Upgrade Mesa, this gets the extension advertised and the test is happy.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This adds an output section option use-renderer-shadow in weston.ini.
This option is only recognized with headless and DRM backends, because
it requires GL-renderer and does not support resizing outputs.
The option is called use-renderer-shadow because this is what it does
right now. In the future the same setting will be used to turn on more
complex image processing when operational pieces required for color
management land. Once color management is implemented, this option is
expected to be removed. This option allows developer testing of features
to be used to implement color management.
This is a rewrite of "weston.ini: introduce use-shadow-fbo in output
config" by Harish Krupo. The main.c code is structured differently, the
weston.ini option is renamed, and the man page paragraph is moved to
weston.ini.man with different content.
Cc: Harish Krupo <harishkrupo@gmail.com>
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Marius Vlad
Jonathan Marler
Pekka Paalanen
Simon Ser
Leandro Ribeiro
Vlad Zahorodnii