This patch makes sure we have a gl_buffer_state present when using
direct-display protocol extensions (which forbids any GL imports, and
assumes a direct path with the display unit to perform a KMS import).
Without this patch we would basically have no gl_buffer_state at repaint
time because we never manged to create one, as direct-display code path
will return much early.
Partially fixes gitlab.freedesktop.org/wayland/weston/-/issues/621.
Suggested-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Added cLUT profile creation to validate linearization algorithm
for DToB3 tag (direction dev to PCS). The 3DLUT is
built by using raw matrix conversion from dev to XYZ and reverse
(XYZ to device).
The test uses floating point pipeline, known as unbounded mode of LCMS.
The details are described in ICCSpecRevision_02_11_06_Float.pdf
The purpose of these new test cases is to keep the GL-renderer 3D LUT
path tested even after color-lcms and GL-renderer start using
specialized matrix-shaper paths.
These also exercise build_eotf_from_clut_profile() in color-lcms, but do
not actually verify it. These cases only test that the recovered EOTF
and its inverse produce an identity mapping together.
BT.2020 is not used in these tests, because the RGB-XYZ conversion
matrix does not stay inside [0.0, 1.0] in either direction, which would
be a problem for the 3D LUT element in the multiProcessingElement
pipelines. Handling that would have been possible, but testing with
AdobeRGB color space should suffice while keeping the test code from
being even more complicated.
roundtrip_verification() tests that we succeed in creating cms
pipelines correctly in both directions so that the resulting ICC file is
better behaved. The Weston test itself only cares about the BToD
direction.
Credits to:
Vladimir Lachine <vladimir.lachine@amd.com>
Graeme Gill <graeme@argyllcms.com>
Co-authored-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Signed-off-by: Vitaly Prosyak <vitaly.prosyak@amd.com>
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
We will want to run the same color spaces with different types of ICC
profiles. To help with that:
1. Let struct lcms_pipeline define the test color space and
transformations and move the tolerance into a new per test case
structure.
2. Added profile type: PTYPE_MATRIX_SHAPER, PTYPE_CLUT.
PTYPE_MATRIX_SHAPER is the previously implemented type.
Co-authored-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Signed-off-by: Vitaly Prosyak <vitaly.prosyak@amd.com>
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This function sets some basic text tags to make an ICC file better
formed.
The code is taken from LittleCMS, https://github.com/mm2/Little-CMS.git
git revision
lcms2.13.1-28-g6ae2e99 (6ae2e99a3535417ca5c95b602eb61fdd29d294d0)
file src/cmsvirt.c.
Suggested-by: Vitaly Prosyak <vitaly.prosyak@amd.com>
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This adds a new test helper library that depends on LittleCMS 2.
For starters, the library implements conversion from enum transfer_fn to
ICC multiProcessingElements compatible LittleCMS curve object.
That conversion allows encoding transfer funtions in ICC files and
LittleCMS pipelines with full float32 precision instead of forcing a
conversion to a 1D LUT which for power-type curves is surprisingly
imprecise.
This also adds CI tests to make sure the conversion matches our
hand-coded transfer functions.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
These helpers allow collecting color difference statistics easily.
To be used in color-shaper-matrix-test.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
When defining a color space with a transfer function, this looks up the
inverse transfer function without needing to store that separately.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This will be useful to make a curve in a color pipeline pass-through
without needing to special-case skipping the curve.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Fix up whitespace and document what this array is for.
For the sake of slightly better readability.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Re-use color_float_apply_curve() instead of open-coding it.
Maybe makes reading the code a little easier.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Make process_pixel_using_pipeline() slightly easier to read by
extracting a meaningful function.
Pure refactoring, no behavioral changes.
Compared to previous, flip the scalar multiplication around, so that it
matches the mathematical order of matrix-vector multiplication.
Also document the layout conventions for lcmsVEC3 and lcmsMAT3. These
follow the convention used in LittleCMS for cmsVEC3 and cmsMAT3, and are
necessary to understand to review the matrix-vector multiplication for
correctness.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Make process_pixel_using_pipeline() slightly easier to read by
extracting a meaningful function.
Pure refactoring, no changes.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Looks like this was forgotten, and I managed to get compiler errors
about redeclaring all enums.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
It's bad form to set the same variable in multiple places, and not all
of them were even equivalent.
Move lcms2 finding to the root level build file only. It is still an
optional dependency like before, and the if-not-found checks are still
in place where actually needed.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
When color management is disabled, the fragment shader was still first
ensuring straight alpha and then immediately just going back to
pre-multiplied. This is near-impossible for a shader compiler to
optimize out, I guess because of the if-statement to handle division by
zero. Having view alpha applied in between certainly didn't make it
easier.
That causes extra fragment computations that are unnecessary. In the
issue report this was found to cause a notable performance regression.
Fix the performance regression by introducing special-case paths for
when straight alpha is not needed. This skips the unnecessary
computations.
Fixes: https://gitlab.freedesktop.org/wayland/weston/-/issues/623
Fixes: 9a6a4e7032
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Currently, the opaque is discarded for all transformations other than a simple
translation, because correctly transforming the opaque area is not possible in
general.
However, there is one simple case that is probably the most common one: A fully
opaque surface that is translated and scaled. In this case the opaque area is
simply the new bounding box. So set the transformed opaque area accordingly.
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
If surface->is_opaque is set then we can assume that the whole surface is
opaque. In the trivial case (no transformation or translation only) this means
that transform.boundingbox is exactly the view area and is fully opaque. So it
can be used for transform.opaque.
This is important because damage calculation uses transform.opaque. Without
this, anything underneath a surface without an explicit opaque region but a
pixel format without alpha channel is drawn unnecessarily.
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
The drm_device is initialized as a side effect of the (badly named)
drm_device_is_kms function. Explicitly pass the drm_device to be able to
initialize kms devices that are not the main drm device of the drm backend.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
If we have multiple drm devices, we cannot use the drm device from the backend,
because we would only get the primary device and not the device of the output.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
If Weston receives a hotplug event, it has to check if the hotplug device
actually belongs to the drm device before updating the heads of the device. The
hotplug event should only remove heads that belong to the device and must not
change heads of other devices.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
The compositor lists the heads from all devices, but we must only disable the
connectors that belong to the current device. Therefore, other heads must be
ignored.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
The outputs, heads, crtcs, and connectors are specific to a drm device and not
the backend in general.
Link them to the device that they belong to to be able to retrieve the
respective device.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
The commits happen per device instead of per backend. The pending state is
therefore per device as well. Allow to retrieve the device from the pending
state.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
The scanout format for the dma-buf feedback are specific to the kms device that
is used for scanout. Therefore, we have to pass the device of the output when
retrieving the scanout formats.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
The fbs are specific to the device on which they will be displayed. Therefore,
we have to tell which device shall be used when we are creating the fb.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
The atomic commit is device specific. If we have multiple kms devices, we need
to know which device was used for the atomic commit.
Pass the device instead of the backend through the atomic commit.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
Extract the kms device from the backend to allow a better separation of the
backend and the kms device. This will allow to handle multiple kms devices with
a single drm backend.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
Get the backend at the beginning of the function instead of retrieving it from
another object in the debug statement. This simplifies refactoring, as the debug
statement is not affected by changes how the backend is retrieved.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
The gbm_format is the same as the drm format used by the pixel format.
Print the format name using the pixel format in the error message to make the
error message easier to understand for humans.
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
The fb already contains a DRM fd for later use. So just use that one instead of
fetching it from the backend.
This is necessary if the fbs are allocated on different devices, since otherwise
the wrong device might be used to get the fd of the passed fb.
Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
Looks like we missed this one during the conversion to
weston_signal_emit_mutable.
Found by running weston under valgrind and running/killing
weston-simple-dmabuf-egl
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
This reverts commit 1618697dc3.
The original commit was a workaround for
https://gitlab.freedesktop.org/mesa/mesa/-/issues/2219 which was fixed
in Mesa:
- c7617d8908a970124321ce731b43d5996c3c5775 released as 20.1.0-rc1
- a0e6341fe4417e41cda0b19e4fa7f8bbe4e1dba1 released as 19.3.5
- f27e5d9df5bc9c85d45c2cb1f2a4997b453365fe released as 20.0.0
This workaround should not be necessary anymore, we don't use it in our
CI, and it was manual to begin with. Therefore remove it.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
The RDP spec says we can trust x, y position on all messages except
PTR_FLAGS_WHEEL and PTR_FLAGS_HWHEEL, so let's do that to ensure
proper sync with the RDP client.
Co-authored-by: Steve Pronovost <spronovo@microsoft.com>
Co-authored-by: Brenton DeGeer <brdegeer@microsoft.com>
Signed-off-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Steve Pronovost <spronovo@microsoft.com>
Signed-off-by: Brenton DeGeer <brdegeer@microsoft.com>
And use it to get a feedback event for when adding scanout tranche.
With this change, I get back a feedback event for dmabuf-feedback
on VC4:
���� tranche: target device /dev/dri/card0, scanout
� ���� format ABGR2101010, modifier LINEAR (0x0)
� ���� format XBGR2101010, modifier LINEAR (0x0)
� ���� format ARGB8888, modifier LINEAR (0x0)
� ���� format ABGR8888, modifier LINEAR (0x0)
� ���� format XRGB8888, modifier LINEAR (0x0)
� ���� format XBGR8888, modifier LINEAR (0x0)
� ���� format RGB565, modifier LINEAR (0x0)
� ���� format YUV420, modifier LINEAR (0x0)
� ���� format YUV422, modifier LINEAR (0x0)
� ���� format YVU420, modifier LINEAR (0x0)
� ���� format YVU422, modifier LINEAR (0x0)
� ���� format NV12, modifier LINEAR (0x0)
� ���� format NV12, modifier BROADCOM_SAND128 (0x700000000000004)
� ���� format NV16, modifier LINEAR (0x0)
� ���� end of tranche
Besides that, it can place a fullscreen state of simple-egl on the
primary plane, which without this change wasn't possible.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
I guess this reverts commit 73bdc0ce85
"xwm: Fix fd leak in weston_wm_send_data()"
That commit closes the send half of a pipe in weston_wm_send_data,
claiming that it's dup()licated later, and we'll leak the fd if
we don't close it.
That may have been true at the time? But currently that fd is only
duplicated by wl_event_loop_add_fd() in its normal operation, and
closing our original before that fd handler ever fires results
in an EBADF on write, and the data never reaching its intended
destination.
Worse, by the time that handler is called there might be another
use for that fd, and we could push data into it and close it.
To provoke the problem, launch an app like FireFox over Xwayland,
cut something to the clipboard, then close the app (this is the
path where the wm has stored the clipboard contents and the
app has gone away). relaunch it and paste the clipboard content
back in. clipboard_client_data() will EBADF on write, and the
data won't be pasted.
Reported-by: Hideyuki Nagase <hideyukn@microsoft.com>
Signed-off-by: Derek Foreman <derek.foreman@collabora.com>
This a new matrix inversion test written from scratch to be suitable for
running in CI: quick to run and automatically detects success/failure.
This all is a result of what I learnt while working on
https://gitlab.freedesktop.org/pq/fourbyfour
Computing the residual error with infinity norm comes straight from
fourbyfour documentation on how to evaluate matrix inversion error.
Most of the hard-coded test matrices have been generated with fourbyfour
project as well, as it contains the generator code. The matrices are
hard-coded here also to make testing faster, but primarily because the
generator code needs BLAS and LAPACK, and having those as Weston
dependencies would be far too much just for this.
Now, if someone wants to modify weston_matrix stuff, we should at least
detect matrix inversion and multiplication bugs.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This #define was used only by the matrix-test program, which was removed
in the previous commit.
Remove it as unused and fold away MATRIX_TEST_EXPORT.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
This test program was useful a decade ago when weston_matrix_invert()
was being developed. It was a manual test program that ran for a certain
number of seconds and required human interpretation of numbers to see if
results were acceptable or not. Hence it was foundamentally unsuitable
for CI.
The way it generated random matrices for inversion testing was also very
naive, and it used the determinant value to determine invertability
which is completely bogus. This made it also a bad test for correctness.
Much better speed and correctness testing is implemented in
https://gitlab.freedesktop.org/pq/fourbyfour
with documented testing procedures. It has a copy of the weston_matrix
implementation.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Buffer scale is common enough in the modern desktop space to
expect average GL clients to handle it. Thus lets include it into
our main example client.
While on it, also handle buffer transforms. It's essentially free
for GL clients in terms of computing power but may increase the
chance that Wayland compositors are able to hit scanout fast paths.
Thus having an example client for it is likely valueabel for client
and compositor developers.
Signed-off-by: Robert Mader <robert.mader@collabora.com>
Setting the opaque region correctly is common source of error for
clients that simply want to express that a whole surface is opaque.
This is especially true once buffer_scale and buffer_transform come
into play, as unlike for damage, where buffer_damage is the
encouraged and user friendly way today, opaque regions are always
in logical coordinates.
As faulty opaque regions don't have a visual impact in these cases
but only increase resource consumption, these errors often remain
for long times. See
1e2bc68171
for one of many examples.
Give an easy example how to set the opaque region in a conformant
and reliable way.
Signed-off-by: Robert Mader <robert.mader@collabora.com>
Obviously the first allocation is always leaked, there is a second
zalloc() right below. Fix the leak.
Found by code inspection.
Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com>
As we could have situations where dmabuf import failed when attempting
to figure it the framebuffer is scanout-capable, make sure we also have
a way to store that information. Otherwise, we could end up
NULL-dereferencing, as we don't provide a valid storage for it.
Further more, with this, we also print out the reason why it failed, to
aid in further debugging.
Observed on platforms where GBM_BO_HANDLE failed + in combination w/
direct-display proto extension.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Marius Vlad
Vitaly Prosyak
Pekka Paalanen
Michael Olbrich
Michael Tretter
Derek Foreman
Hideyuki Nagase
Robert Mader