It turns out the "step" variable could up as 0 when p.y ~= 3.0 ||
p.y ~= r.y - 3.0
That was not enough to trigger it though because if "start" and "end"
were the same value, the "y <= end" check in the loop would immediately
terminate it.
However, if start + epsilon == end so that end != start but (end - start)
/ 7 == 0, then step would end up as 0 and the loop would never
terminate.
And if that happened, it would bring down GPUs.
So recode this whole machinery to make it impossible to infloop.
Fixes#6896
The fix in commit 5e7f227d broke shadows while trying to make them
faster.
So use a better way to make them faster.
With the normalized blur radius, we can now conclude that all the values
too far from p.y will cause the gauss() call to return close to 0, so we
can skip any y value that is too far from p.y.
And that allows us to put an upper limit on the loop iterations.
Tests included
Fixes#6888
Instead of doing complicated math, normalize the values to a sigma
of 1.0, and then use that.
This should also be beneficial for shader performance, because 1.0 is a
constant and constant-elimination can kick in on the inlined functions.
When the compositor sends us an image description, we currently happily
reuse it.
However, those image descriptions may contain optional properties that
we do not handle - example: reference white level. So if we were to
reuse that image description, we would set a wrong reference white
level.
To avoid issues like that, never use compositor-provided image
descriptions.
However, query those image descriptions and map them to the closest
GdkColorState, so that we can quickly look up *our* version of that
image description and use that one.
When finalizing a subsurface, we need to make sure it is removed
from the sibling lists in its parent, or bad things will happen.
This should crashes seen in Epiphany nightly.
Fixes: #6891
convert_func2 is a 'from' conversion function, ie it expects to
be passed the target color state. This was wrong both in
gdk_memory_convert and gdk_memory_convert_color_state.
Previously GTK required a C99 compiler, but as discussed on
GNOME/gtk!7510 there's at least one anonymous union in public API
(in `GskPathPoint`), and that's a C11 feature.
Signed-off-by: Simon McVittie <smcv@debian.org>
When we allocate a graphene_point_t on the stack, there's no guarantee
that it will be aligned at an 8-byte boundary, which is an assumption
made by gsk_pathop_encode() (which wants to use the lowest 3 bits to
encode the operation). In the places where it matters, force the
points on the stack and embedded in structs to be nicely aligned.
By using a distinct type for this (a union with a suitable size and
alignment), we ensure that the compiler will warn or error whenever we
can't prove that a particular point is, in fact, suitably aligned.
We can go from a `GskAlignedPoint *` to a `graphene_point_t *`
(which is always valid, because the `GskAlignedPoint` is aligned)
via &aligned_points[0].pt, but we cannot go back the other way
(which is not always valid, because the `graphene_point_t` is not
necessarily aligned nicely) without a cast.
In practice, it seems that a graphene_point_t on x86_64 *is* usually
placed at an 8-byte boundary, but this is not the case on 32-bit
architectures or on s390x.
In many cases we can avoid needing an explicit reference to the more
complicated type by making use of a transparent union. There's already
at least one transparent union in GSK's public API, so it's presumably
portable enough to match GTK's requirements.
Increasing the alignment of GskAlignedPoint also requires adjusting how
a GskStandardContour is allocated and initialized. This data structure
allocates extra memory to hold an array of GskAlignedPoint outside the
bounds of the struct itself, and that array now needs to be aligned
suitably. Previously the array started with at next byte after the
flexible array of gskpathop, but the alignment of a gskpathop is only
4 bytes on 32-bit architectures, so depending on the number of gskpathop
in the trailing flexible array, that pointer might be an unsuitable
location to allocate a GskAlignedPoint.
Resolves: https://gitlab.gnome.org/GNOME/gtk/-/issues/6395
Signed-off-by: Simon McVittie <smcv@debian.org>
This is widely assumed, but is not guaranteed by Standard C, and is
known to be false on CHERI architectures (which have 64-bit sizes and
128-bit tagged pointers). Add a static assertion to ensure that GTK
will not build on platforms where this assumption does not hold.
As discussed on GNOME/gtk!7510, if GTK switches from gsize to uintptr_t
as its representation of the underlying bits in a pointer, GTK maintainers
would prefer that to be done project-wide so that it's done consistently,
after which this static assertion could be removed.
At the time of writing, GLib makes the same assumption (GNOME/glib#2842),
but GLib contributors are gradually removing it (mostly by replacing gsize
with uintptr_t where a pointer-sized quantity is needed). Finishing
that work in GLib would be a prerequisite for being able to make GTK
work on the affected platforms.
Signed-off-by: Simon McVittie <smcv@debian.org>
Similar to the previous commit, to avoid undefined behaviour we need
to avoid evaluating out-of-bounds shifts, even if their result is going
to ignored by being multiplied by 0 later.
Detected by running a subset of the test suite with
-Dsanitize=address,undefined on x86_64.
Signed-off-by: Simon McVittie <smcv@debian.org>
If, for example, e == 0, it is undefined behaviour to compute an
expression involving an out-of-range shift by (125 - e), even if the
result is in fact irrelevant because it's going to be multiplied by 0.
This was already fixed for the memorytexture test in
commit 5d1b839 "testsuite: Fix another ubsan warning", so use the
implementation from that test everywhere. It's in the header as an
inline function to keep the linking of the relevant tests simple:
its only caller in production code is fp16.c, so there will be no
duplication outside the test suite.
Detected by running a subset of the test suite with
-Dsanitize=address,undefined on x86_64.
Signed-off-by: Simon McVittie <smcv@debian.org>
Left-shifting a signed 32-bit integer by 31 bits (such that the value
overflows into the sign bit) is undefined behaviour. Use an unsigned
integer instead.
Detected by running a subset of the test suite with
-Dsanitize=address,undefined on x86_64.
Signed-off-by: Simon McVittie <smcv@debian.org>
Some callers of these functions ask to copy 0 items from a NULL source,
which would be valid if they were copied in a loop (because NULL would
never be dereferenced), but is declared to be undefined behaviour for
Standard C memcpy. Guard the call to memcpy so that we only call it
if we have more than 0 items, and therefore should have a non-NULL
source pointer.
Detected by running a subset of the test suite with
-Dsanitize=address,undefined on x86_64.
Signed-off-by: Simon McVittie <smcv@debian.org>
Shifting a 32-bit type by 32 bits is formally undefined behaviour,
even if it happens in code that is unreachable at runtime. Use a
compile-time check against GLib's GLIB_SIZEOF_SIZE_T, instead of hoping
a runtime check will be optimized away.
Signed-off-by: Simon McVittie <smcv@debian.org>
Unfortunately the format string for a size_t, `%zu`, is not portable
to all Windows compilers, and the appropriate format string for the
fundamental type that implements size_t varies between platforms
(typically `%u` on 32-bit platforms, `%lu` on 64-bit Linux or
`%llu` on 64-bit Windows).
In gtk-demo, cast the number of search results to long, to avoid
breaking up a translatable string.
Elsewhere, use GLib's abstraction for this.
Signed-off-by: Simon McVittie <smcv@debian.org>
