Commit Graph

687 Commits

Author SHA1 Message Date
Frediano Ziglio
75d9842e7d lz: Move ENCODE_PIXEL for RGB24 and RGB32 to a common place
The macro for both depth is the same, reuse the definition.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2018-05-10 19:50:50 +01:00
Frediano Ziglio
b98f19b168 protocol: Use a typedef to specify stream_id type
This change does not affect generated code but make source more
readable. Also document in a single location the range of this
type.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2018-05-10 19:49:03 +01:00
Jonathon Jongsma
fc46379b37 miLineArc(): initialize edge1, edge2
When compiling spice-common with meson/ninja under "release" mode, I get
several compiler warnings about possibly-uninitialized members. For
example:

    ../subprojects/spice-common/common/lines.c: In function ‘miLineArc’:
    ../subprojects/spice-common/common/lines.c:2167:17: error: ‘edge2.dx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
             edge->e += edge->dx; \
                     ^~
    ../subprojects/spice-common/common/lines.c:2426:24: note: ‘edge2.dx’ was declared here
         PolyEdgeRec edge1, edge2;
                            ^~~~~

Initializing these structures to zero silences the warnings.

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2018-05-10 10:05:41 -05:00
Frediano Ziglio
754cd54e1a codegen: Removed unused get_type methods
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Lukáš Hrázký <lhrazky@redhat.com>
2018-05-09 11:59:33 +01:00
Frediano Ziglio
46fa9d5efb codegen: Add some comments
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Lukáš Hrázký <lhrazky@redhat.com>
2018-05-09 11:59:33 +01:00
Victor Toso
5729664523 messages: document limitation of id in StreamCreate
Note that the ID limitation always existed but now we have the
limitation in the protocol itself with SPICE_MAX_NUM_STREAMS

Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2018-05-05 13:00:56 +01:00
Jonathon Jongsma
885b1a6bb9 Remove extra self parameter from member function
When testing out some experimental protocol changes, I managed to
trigger the following error:

  GEN      generated_client_demarshallers.c
Traceback (most recent call last):
  File "../../../spice-common/spice_codegen.py", line 267, in <module>
    demarshal.write_protocol_parser(writer, proto, True)
  File "/home/jjongsma/work/spice/spice-common/python_modules/demarshal.py", line 1270, in write_protocol_parser
    parsers[channel.value] = (channel.channel_type, write_channel_parser(writer, channel.channel_type, is_server))
  File "/home/jjongsma/work/spice/spice-common/python_modules/demarshal.py", line 1163, in write_channel_parser
    func = write_msg_parser(helpers, ids[i].message_type)
  File "/home/jjongsma/work/spice/spice-common/python_modules/demarshal.py", line 1061, in write_msg_parser
    num_pointers = message.get_num_pointers()
  File "/home/jjongsma/work/spice/spice-common/python_modules/ptypes.py", line 855, in get_num_pointers
    count = count + m.get_num_pointers()
  File "/home/jjongsma/work/spice/spice-common/python_modules/ptypes.py", line 662, in get_num_pointers
    return self.member_type.get_num_pointers()
  File "/home/jjongsma/work/spice/spice-common/python_modules/ptypes.py", line 507, in get_num_pointers
    if self.is_constant_length(self):
TypeError: is_constant_length() takes exactly 1 argument (2 given)

Calling a member function will implicitly pass 'self' as the first
argument, but we were also explicitly passing it as an argument
(self.is_constant_length(self)). This resulted in the above error.

Acked-by: Lukáš Hrázký <lhrazky@redhat.com>
2018-04-17 14:14:22 +01:00
Frediano Ziglio
4c2d0e9772 Add --enable-extra-checks option
Allow to enable code to do additional or expensive checks.
The option should be used by higher level libraries.
By default the option is disabled.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
2018-03-19 14:47:21 +00:00
Frediano Ziglio
2eaef9c16d protocol: Add some documentation for inval_all_pixmaps message
This message is not straight forward to grasp.
Not clear by the name why we need to wait other channels messages
before resetting the image cache.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2018-03-18 09:14:29 +00:00
Frediano Ziglio
8096b1206b canvas: Use SPICE_UNALIGNED_CAST to avoid -Wcast-align warnings
This solves https://bugs.freedesktop.org/show_bug.cgi?id=104521.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2018-01-31 15:10:35 +00:00
Frediano Ziglio
fd0aba2750 canvas: Fix some semi transparent drawing
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>
2018-01-25 22:27:34 +00:00
Frediano Ziglio
f3478aa4b6 canvas: Prevent some error compiling spice-gtk
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>
2018-01-24 10:41:02 +00:00
Paweł Pękala
122be3d1f7 Fix build with LibreSSL
Some FreeBSD configurations can use LibreSSL instead of OpenSSL.
The two libraries are really similar but need some minimal adjustment.

Signed-off-by: Paweł Pękala <pawelbsd@gmail.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2018-01-23 10:23:43 +00:00
Frediano Ziglio
41c7ea6611 canvas: Remove unused include header
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-01-18 14:34:26 +00:00
Frediano Ziglio
0dda6fbf55 canvas: Unify __surface_create_stride and surface_create_stride
They are now just the same function with same parameters,
just one calls the other.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-01-18 14:34:24 +00:00
Frediano Ziglio
3bf6e7b899 canvas: Remove dc fields from CanvasBase and LzDecodeUsrData
Now always NULL.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-01-18 14:34:22 +00:00
Frediano Ziglio
b0f1d31dd1 canvas: Remove dc parameter from SwCanvas::put_image
Not used anymore.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-01-18 14:34:19 +00:00
Frediano Ziglio
6ce1a61577 canvas: Remove unused dc parameter from surface_create
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-01-18 14:34:16 +00:00
Frediano Ziglio
00d1cda4ce canvas: Remove Windows bitmap allocation
There's no reason to use a DIB section if we are going to use just the
memory in it assigned to a pixman surface, use normal path and memory.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-01-18 14:34:14 +00:00
Frediano Ziglio
cd88d0294c canvas: Remove mutex field from PixmanData
The field is only assigned but never used.
This was used in the GDI canvas which has now been removed.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-01-18 14:34:12 +00:00
Frediano Ziglio
fea48544a8 canvas: Move PixmanData to C file
This structure is used only in canvas_utils.c

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-01-18 14:33:44 +00:00
Frediano Ziglio
27e32fa8a6 lz: Remove unused encode_level and only assigned io_start
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
2018-01-18 10:08:46 +00:00
Frediano Ziglio
5fda05740f lz: Simplify code
Remove some conditional code always defining CAST_PLT_DISTANCE
macro.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
2018-01-18 10:08:43 +00:00
Frediano Ziglio
552e842a16 lz: Avoid temporary variable
Use a break to exit the loop instead of using a variable.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
2018-01-18 10:08:41 +00:00
Frediano Ziglio
29eff61cf8 canvas: Remove possible leak on LZ decompression failure
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>
2018-01-18 10:08:39 +00:00
Frediano Ziglio
d11df6b66b canvas: Simplify code using spice_memdup
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>
2018-01-18 10:08:26 +00:00
Pavel Grunt
a3a2bb9ea7 Remove GDI canvas
Only spicec was using it - removed by spice server commit:
1876971442ef808b5dcdaa5dc12df617f2179cb5

Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2018-01-17 09:31:32 +00:00
Frediano Ziglio
637621a9b9 canvas-base: Fix width computation for palette images
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>
2017-12-22 18:19:54 +00:00
Frediano Ziglio
53f6d1269a protocol: Allow to specify a surface will be streamed
This flag will allow the client to perform some optimisations
on output and buffering processing.
Old clients will ignore this additional flag.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
2017-12-01 15:48:56 +00:00
Frediano Ziglio
94898df6b4 proto: Add some documentation to stream_report message
Most of the documentation is extracted from notes in spice-server
code and comments.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-11-29 11:50:54 +00:00
Frediano Ziglio
45e2844845 canvas_base: Allow to specify constant operations
There's no need for the canvas operations to be changed.
This allows without casts to have the operation structures
constants in the code.
This potentially allows to reduce attack surface having some
more data constant instead or read/write.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe de Dinechin <dinechin@redhat.com>
2017-11-07 13:11:19 +00:00
Frediano Ziglio
72ae9b2971 ring: Remove short living temporary variable
Just a style change, the variable does not help readability.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe de Dinechin <dinechin@redhat.com>
2017-11-07 13:11:19 +00:00
Frediano Ziglio
c208ca85cd ring: Remove __ring_remove function
Is just used by ring_remove, no reason to have it.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe de Dinechin <dinechin@redhat.com>
2017-11-07 13:11:19 +00:00
Frediano Ziglio
c2f2096d90 test-marshallers: Test demarshalling
Generate demarshallers code and check it too.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-09-25 16:13:27 +01:00
Frediano Ziglio
c25c75b552 test-marshallers: Use unaligned structure
Allows to test for bad performance on some systems.
For instance on ARMv6/ARMv7 which does not support by default
64 bit unaligned read/write this can be checked on Linux
using /proc/cpu/alignment file.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-09-25 16:13:18 +01:00
Frediano Ziglio
74e50b57ae Make the compiler work out better way to write unaligned memory
Instead of assuming that the system can safely do unaligned access
to memory use packed structures to allow the compiler generate
best code possible.
A packed structure tells the compiler to not leave padding inside it
and that the structure can be unaligned so any field can be unaligned
having to generate proper access code based on architecture.
For instance ARM7 can use unaligned access but not for 64 bit
numbers (currently these accesses are emulated by Linux kernel
with obvious performance consequences).

This changes the current methods from:

#ifdef WORDS_BIGENDIAN
#define read_uint32(ptr) ((uint32_t)SPICE_BYTESWAP32(*((uint32_t *)(ptr))))
#define write_uint32(ptr, val) *(uint32_t *)(ptr) = SPICE_BYTESWAP32((uint32_t)val)
#else
#define read_uint32(ptr) (*((uint32_t *)(ptr)))
#define write_uint32(ptr, val) (*((uint32_t *)(ptr))) = val
#endif

to:

#include <spice/start-packed.h>
typedef struct SPICE_ATTR_PACKED {
    uint32_t v;
} uint32_unaligned_t;
#include <spice/end-packed.h>

#ifdef WORDS_BIGENDIAN
#define read_uint32(ptr) ((uint32_t)SPICE_BYTESWAP32(((uint32_unaligned_t *)(ptr))->v))
#define write_uint32(ptr, val) ((uint32_unaligned_t *)(ptr))->v = SPICE_BYTESWAP32((uint32_t)val)
#else
#define read_uint32(ptr) (((uint32_unaligned_t *)(ptr))->v)
#define write_uint32(ptr, val) (((uint32_unaligned_t *)(ptr))->v) = val
#endif

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-09-25 16:12:45 +01:00
Frediano Ziglio
70d4739ce2 quic: avoid crash on specific images
encodes_ones is called to encode a long sequence of 1 bits.
In some conditions (I manage to reproduce with a 85000x4 pixel
image fill with a single color) encodes_ones is called with a
"n" value >= 32.
This cause encode to be called with a "len" value of 32 which
trigger this assert:

   spice_assert(len > 0 && len < 32);

causing a crash. Instead of calling encode with a constant
"len" as 32 call encode_32 which is supposed to encode
exactly 32 bit.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2017-08-23 22:47:08 +01:00
Frediano Ziglio
429ad96537 quic: turn back some commented out checks as compile time
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2017-08-09 07:07:36 +01:00
Frediano Ziglio
a6d6e8435a quic: use 32 bit for bppmask
In most occurrences bppmask is converted to 32 bit anyway.
In the left one a possible more bigger precision is not needed.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2017-08-09 07:07:34 +01:00
Frediano Ziglio
6a882282f1 quic: remove Channel::encoder
This field is easily accessible from Encoder structure.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2017-08-09 07:07:32 +01:00
Frediano Ziglio
3306492247 quic: remove only assigned CommonState::encoder
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2017-08-09 07:07:29 +01:00
Frediano Ziglio
65ba949fb0 quic: remove only assigned num_channels field
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2017-08-09 07:07:13 +01:00
Frediano Ziglio
9f6a671294 quic: fix typo golomb_deoding -> golomb_decoding
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-08-03 13:33:00 +01:00
Frediano Ziglio
b64ee40a60 quic: fix typo corelate -> correlate
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-08-03 13:32:45 +01:00
Christophe Fergeau
c39f2da17e quic: Use i == 0 rather than !i
This helps to make quic_rgb_tmpl.c and quic_tmpl.c more consistent.

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
2017-08-01 15:53:59 +02:00
Christophe Fergeau
8dd7374a1d quic: Fix "corelate" typo
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
2017-08-01 15:53:47 +02:00
Christophe Fergeau
9574246403 quic: Use SPICE_VERIFY for static check
DEFevol is known at compile-time, so we can use SPICE_VERIFY to check
its value.

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
2017-08-01 15:53:44 +02:00
Christophe Fergeau
5684ed10be quic: Use DEFwmimax constant rather than wmimax variable
We never change its value, so we basically have 2 constants for the
same thing, DEFwmimax and wmimax. This commit removes the latter.

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
2017-08-01 15:53:42 +02:00
Christophe Fergeau
f122d3dadc quic: Use DEFwminext constant rather than wminext variable
We never change its value, so we basically have 2 constants for the
same thing, DEFwminext and wminext. This commit removes the latter

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
2017-08-01 15:53:38 +02:00
Christophe Fergeau
db5bff3728 quic: Use DEFevol constant rather than evol variable
We never change 'evol' value, and this is currently causing issues with
gcc 7.1.1: quic.c is checking at compile-time that 'evol' is 1, 3 or 5.
This is a constant, so a static check should be good, but the compiler (gcc
7.1.1) is unable to know 'evol' value at compile-time. Since the removal
of spice_static_assert in favour of SPICE_VERIFY, this causes a
compile-time failure.

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
2017-08-01 15:53:34 +02:00