[PATCH weston v2] ivi-layout: Initialize surface source rectange to 1x1

[Pekka Paalanen]

On Wed, 18 May 2016 12:22:06 +0100
mateuszx.potrola at intel.com wrote:

> From: Mateusz Polrola <mateuszx.potrola at intel.com>
> 
> If surface will be set to visible before its source rectangle will
> be defined it will be displayed in its orginal size.

Hi,

how does it happen that you set visibility but not the source rectangle?

Are you hitting the warning added in
bcff632727f9d65e4aa1daa724b8097db8321d1c?

If you are, then how is that not an error in using the ivi-layout API?

Even more so, because as far as I can see, to actually see the badly
sized surface you have to do this in ivi-layout API:
1. add the surface to a layer
2. set visibility true
3. commit_changes
4. set source rectangle
5. commit_changes

At step 3, the surface will be shown. It looks to me that you are very
specifically asking the surface to be shown with an unset source
rectangle. Is there any other way it can happen?

> This is because initial setting of destination rectangle to 1x1 is
> not causing surface resize and because source rectangle is 0x0
> appropiate transformation matrix is not calculated

Ok, but why is that wrong?

If you do not set source and destination rectangles, what do you expect
to get?

I thought they were not optional to set, but the documentation does not
really say if, or what is expected.


> V2 changes:
> Removed blank line
> 
> Signed-off-by: Mateusz Polrola <mateuszx.potrola at intel.com>
> ---
>  ivi-shell/ivi-layout.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c
> index 1601787..99d9db8 100644
> --- a/ivi-shell/ivi-layout.c
> +++ b/ivi-shell/ivi-layout.c
> @@ -250,6 +250,8 @@ init_surface_properties(struct ivi_layout_surface_properties *prop)
>  	 */
>  	prop->dest_width = 1;
>  	prop->dest_height = 1;
> +	prop->source_width = 1;
> +	prop->source_height = 1;
>  }
>  
>  /**

Is this a regression, btw?

The patch context is missing this:
static void
init_surface_properties(struct ivi_layout_surface_properties *prop)
{
	memset(prop, 0, sizeof *prop);
	prop->opacity = wl_fixed_from_double(1.0);
	/*
	 * FIXME: this shall be finxed by ivi-layout-transition.
	 */
	prop->dest_width = 1;
	prop->dest_height = 1;
}

The comment and assignments were added in
e259a7a7d243ba143ab53a56285c4cef2ba1ed68 which clearly indicates that
initializing to non-zero is just a workaround for a bug elsewhere. I
would not like to add even more duct tape.

Now, if you do initialize source rectangle to 1x1 at 0,0, it won't
actually fix anything. At step 3 in the sequence described above, you
will be showing the top-left pixel of the surface. Then at step 5
(assuming also destination rectangle is set), it will change to the
intended layout. In other words, that is a glitch: you temporarily show
something you did not intend to. It's just that it is much easier to
see a whole window flicker than a single pixel (which might even be
transparent as it is on the edge of a surface).

Is there something I missed here?


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160520/bb19d984/attachment-0001.sig>