Make code faster on platforms not supporting unaligned access by
default.
SPICE_UNALIGNED_CAST is just silencing possible alignment warning and,
if enabled, produces some logs, however in this case we know that the
pointer can be misaligned.
Use packed structures to tell compiler to generate best code possible.
For more details see comment on commit 74e50b57ae ("Make the
compiler work out better way to write unaligned memory").
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
This patch just changes some spaces fixing some possible misleading
indentation.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
The DUMP_JPEG compile time flag is used to dump all jpeg images
to files.
The code was saving garbage instead of proper data.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Reported-by: 谢 昆明 <KunMing.Xie@hotmail.com>
Tested-by: 谢 昆明 <KunMing.Xie@hotmail.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
This is reproducible using desktop icons on Windows XP.
These drawing are sent for the icons on the desktop.
To get an extends.x1 >= 32 you have to move an icon out of the
screen on the left side. Set the icon size to 72 as the icon has
to be out of the screen quite a lot.
Disable the grid alignment on the desktop and move an icon out of
the screen. Select and unselect the icon.
Using "/ 32" the icon will have a white background instead of a
transparent one.
Using a "/ 8" the icon is rendered correctly.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe de Dinechin <cdupontd@redhat.com>
Due to different warning setting some GCC reports:
In file included from ../spice-common/common/sw_canvas.c:27:0,
from client_sw_canvas.c:20:
../spice-common/common/canvas_base.c: In function ‘canvas_get_lz’:
../spice-common/common/canvas_base.c:768:13: error: ‘palette’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
free(palette);
^~~~~~~~~~~~~
../spice-common/common/canvas_base.c:764:9: error: variable ‘free_palette’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
int free_palette = FALSE;
^~~~~~~~~~~~
cc1: all warnings being treated as errors
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Snir Sheriber <ssheribe@redhat.com>
longjmp can happen in different places, even after the palette
is allocated so we need to free it if it got allocated.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
Instead of using spice_malloc+memcpy use spice_memdup which is
doing the same.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
Palette images have padding at the end of each line, so their stride
can't be inferred from their width as is currently done. This causes a
wrong calculation of stride_encoded value which causes a wrong stride
adjustment.
Before commit 5603961ff "fix 16 bpp LZ image decompression", the output
stride was always computed as "stride = (n_comp_pixels / height) * 4"
that is assuming 4 bytes for pixel which was wrong for some output
however computing starting from width was wrong for palette images.
This commit was added to spice-gtk in v0.32~58, which nicely matches the
"client regression when upgrading from spice-gtk v0.31 to spice-gtk
v0.33".
This fix bug https://bugzilla.redhat.com/show_bug.cgi?id=1508847.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Tested-by: Philip J. Turmel <philip@turmel.org>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
pixman_image_unref() does not ignore NULL pointers, it tries to
dereference it which causes a crash. When trying to decode invalid QUIC
data, we could end up in a situation where 'surface' would still be
NULL when reaching the setjmp block.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
memmove already deal with any alignment so there's no
reason to have row byte pointer cast to uint32_t.
This also remove the confusing "dest" terminology used. The image
is aligned in place so the image bits are used for both destination
and source.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
For example, something like this:
uint8_t *p8;
uint32_t *p32 = (uint32_t *) p8;
generates a warning like this:
spice-channel.c:1350:10: error: cast from 'uint8_t *' (aka 'unsigned char *') to
'uint32_t *' (aka 'unsigned int *') increases required alignment from 1 to
4 [-Werror,-Wcast-align]
The warning indicates that we end up with a pointer to data that
should be 4-byte aligned, but its value may be misaligned. On x86,
this does not make much of a difference, except a relatively minor
performance penalty. However, on platforms such as older ARM, misaligned
accesses are emulated by the kernel, and support for them is optional.
So we may end up with a fault.
The intent of the fix here is to make it easy to identify and rework
places where actual mis-alignment occurs. Wherever casts raise the warning,
they are replaced with a macro:
- SPICE_ALIGNED_CAST(type, value) casts value to type, and indicates that
we believe the resulting pointer is aligned. If it is not, a runtime
warning will be issued. This check is disabled unless
--enable-alignment-checks is passed at configure time
- SPICE_UNALIGNED_CAST(type, value) casts value to type, and indicates that
we believe the resulting pointer is not always aligned.
Any code using SPICE_UNALIGNED_CAST may need to be revisited in order
to improve performance, e.g. by using memcpy.
There are normally no warnings for SPICE_UNALIGNED_CAST, but it is possible
to emit debug messages for mis-alignment in SPICE_UNALIGNED_CAST
by configuring with CFLAGS=-DSPICE_DEBUG_ALIGNMENT.
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
Neither Spice nor spice-gtk are using this since the
following commit in Spice "server: remove OpenGL"
c5c176a5c7718177f23b07981556b5d460627498
Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
Moving out a big switch statement that sole purpose is to retrieve the
surface (pixma_image_t)
Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
It is used in canvas_get_lz4() which is guarded by USE_LZ4, and
in canvas_get_lz() which is guarded by SW_CANVAS_CACHE.
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
LZ image decompression was broken for 16 bpp:
- stride was computed not computed correctly (as width*4). This caused
also a buffer underflow;
- stride in pixman is always multiple of 4 bytes (so for 16 bpp is
ALIGN(width*2, 4)) so image decompressed by lz_decode as some missing
bytes to be fixed.
The alignment code is reused from LZ4 function.
This fix also https://bugzilla.redhat.com/show_bug.cgi?id=1285469.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
Every time it's used, it's in constructs similar to:
#ifdef SW_CANVAS_CACHE
, SpiceImageCache *bits_cache
, SpicePaletteCache *palette_cache
#elif defined(SW_CANVAS_IMAGE_CACHE)
, SpiceImageCache *bits_cache
#endif
This can be rewritten as:
, SpiceImageCache *bits_cache
#ifdef SW_CANVAS_CACHE
, SpicePaletteCache *palette_cache
#endif
allowing to get rid of SW_CANVAS_IMAGE_CACHE.
Fix the row alignment for 16/24 bpp images when it is not in a 32bit
boundary. This is needed for 16bpp images when the width is an odd
number, and for the future support of 24bpp images.
SW_CANVAS_CACHE is always defined when building spice-gtk,
SW_CANVAS_IMAGE_CACHE is always defined when building spice-server, and
they are the only 2 users of spice-common. Moreover, build when none of
these is defined is broken.
All canvas_get_{quic,jpeg,lz4,jpeg_alpha,lz} methods have an 'invers'
argument, but are always called with that argument being 0, so we can
drop it from the argument list, and remove the code triggerring when
it's true.
- Add a new LZ4 image type to spice.proto.
- Add canvas_get_lz4() to common_canvas_base, to get a pixmap from an
lz4 image.
- Add an enable-lz4 switch to the configure script, disabled by default.
Thos function shows up in some profiling results, it seems we can
trivially replace it with a precomputed array of 256bytes.
before:
5.66% 691 lt-spicy-stats
libspice-client-glib-2.0.so.8.4.0 [.] revers_bits
after:
0.53% 64 lt-spicy-stats
libspice-client-glib-2.0.so.8.4.0 [.] revers_bits
Fixes: fedora 875348, 826036
When an image is not rendered, we still need to check if it contains
a palette that needs to be cached.
This bug caused the client to crash due to not finding palettes
in the cache.
To allow the compile to detect incorrect printf formats, any
var-args function should have a format annotation
* common/macros.h: Helper to define ATTR_PRINTF for code
which can't depend on glib
* common/canvas_base.c, common/lz.h, common/macros.h: Annotate
some var-args methods
