Re: Plan for libweston backend configuration?
Hello Bryce, Thanks for the summarization, I have few comment that I share here. Le 26/02/2016 22:37, Bryce Harrington a écrit : > To followup Pekka's recent libweston thread, here's the next actions it > looks like we should take? > >a. Revert 5ffbfffa >b. Land https://patchwork.freedesktop.org/patch/67547/, which covers >the drm-backend. (Is this patch proposal good as is, or would it >benefit from any additional review?) >c. Defer the two alternative options for now > https://patchwork.freedesktop.org/patch/73206/ >https://patchwork.freedesktop.org/patch/[73035,73036,73037,73038,73039] >d. Review/update wayland-backend and x11-backend to comply > https://patchwork.freedesktop.org/patch/74553/ > https://patchwork.freedesktop.org/patch/74504/ > > This establishes Giulio's "Well Defined Structs" approach for > configuring libweston backends. This uses versioned structs for > communicating parameters with the backends. > > If no one raises an objection to this plan, I can tackle (a), (b) and > (c) myself directly. For (d), offhand it appears they at least need to > add the structure versioning support, but might be suitable to consider > landing after that? (a), (b), (c) and (d) are fine for me. I would add an action to sort weston API function (i.e. function that are exposed to user(developer)) from functions that must be kept hidden. For this action I think we should start a libweston.h (or what ever name that look good) and the related libweston.a . Currently implementing a compositor from weston look durty, some low level actions require to use libwayland directly and "link" them to weston. Another issue is that user(developer) can use any weston internal API and some of those functions may be unsafe. I do not know well where the border are but we should start to draft one in the same manner we started with backends. I waiting for your review for (d) and I expect to provide patch for others back-end soon (i.e. maybe once per week until all testable back-end are done). Also note that I didn't added versionning header for two main reasons: 1. I do not know which best versionning method I should use, is one incremental number enough ? 2. I do not know how the user(developer) is expected to fill it properly. Once I get answer to this both questions I will be happy to updates previous patch :) > > > --- > A couple of more sophisticated solutions were proposed and evaluated, > but it feels like our requirements here are modest, and so simpler is > probably better. Things aren't cast in stone here and we can always > move to something more advanced later if conditions warrant it. > > In mind of this, here are the assumptions that are leading us to this > choice. In the future if these assumptions fail to hold adequately, > then this design should be re-evaluated. > > 1. libweston has no stability promises, and won't for a long time, and > it is and will remain parallel installable. We are freely able to > redo tomorrow any bad decisions we make today. > > 2. New options will be appended to the the struct, which will avoid ABI > breaks. > > 3. The struct is versioned, so if we do need to break ABI for some > reason, we can and backends can thereby check and verify their > version. > > 4. Configuration needs by the backends are on the tame side, mostly > just involving basic data types (strings, ints, etc.) This assumption (4) is already wrong, back-end at the moment needs what I consider complex setup, in particular to define outputs list or the output_setup callback in drm-backend. > > 5. Backend configuration is internal to the backend. End users won't > be exposed to it, and only backend developers will need to tinker > with these things. > > 6. Additions of or changes to configuration parameter definitions are > going to be done by developers who are either part of the Wayland > development community, or will be sending all of their changes to > the community. > > 7. All libweston backends will be living in the weston repository. We > do not provide support for third-party backends. > > It's probably non-fatal if we periodically violate one or two of these > assumptions, but if we get beyond that it should be a cue to revisit our > approach. > > > So far, two other alternatives were considered; here are the key design > differences: > > A. Opaque Structs > https://patchwork.freedesktop.org/patch/73206/ > > This still uses structs for sharing configuration parameters but the > structs are hidden as internal details. The backend instead uses > function calls to fill in parameters. > > While this does decouple things a bit and avoid ABI breakge on > structure definitions, this requires coding and maintaining an array > of backend-specific functions, which bring their own API breakage > potent
Plan for libweston backend configuration?
To followup Pekka's recent libweston thread, here's the next actions it looks like we should take? a. Revert 5ffbfffa b. Land https://patchwork.freedesktop.org/patch/67547/, which covers the drm-backend. (Is this patch proposal good as is, or would it benefit from any additional review?) c. Defer the two alternative options for now https://patchwork.freedesktop.org/patch/73206/ https://patchwork.freedesktop.org/patch/[73035,73036,73037,73038,73039] d. Review/update wayland-backend and x11-backend to comply https://patchwork.freedesktop.org/patch/74553/ https://patchwork.freedesktop.org/patch/74504/ This establishes Giulio's "Well Defined Structs" approach for configuring libweston backends. This uses versioned structs for communicating parameters with the backends. If no one raises an objection to this plan, I can tackle (a), (b) and (c) myself directly. For (d), offhand it appears they at least need to add the structure versioning support, but might be suitable to consider landing after that? --- A couple of more sophisticated solutions were proposed and evaluated, but it feels like our requirements here are modest, and so simpler is probably better. Things aren't cast in stone here and we can always move to something more advanced later if conditions warrant it. In mind of this, here are the assumptions that are leading us to this choice. In the future if these assumptions fail to hold adequately, then this design should be re-evaluated. 1. libweston has no stability promises, and won't for a long time, and it is and will remain parallel installable. We are freely able to redo tomorrow any bad decisions we make today. 2. New options will be appended to the the struct, which will avoid ABI breaks. 3. The struct is versioned, so if we do need to break ABI for some reason, we can and backends can thereby check and verify their version. 4. Configuration needs by the backends are on the tame side, mostly just involving basic data types (strings, ints, etc.) 5. Backend configuration is internal to the backend. End users won't be exposed to it, and only backend developers will need to tinker with these things. 6. Additions of or changes to configuration parameter definitions are going to be done by developers who are either part of the Wayland development community, or will be sending all of their changes to the community. 7. All libweston backends will be living in the weston repository. We do not provide support for third-party backends. It's probably non-fatal if we periodically violate one or two of these assumptions, but if we get beyond that it should be a cue to revisit our approach. So far, two other alternatives were considered; here are the key design differences: A. Opaque Structs https://patchwork.freedesktop.org/patch/73206/ This still uses structs for sharing configuration parameters but the structs are hidden as internal details. The backend instead uses function calls to fill in parameters. While this does decouple things a bit and avoid ABI breakge on structure definitions, this requires coding and maintaining an array of backend-specific functions, which bring their own API breakage potentials. This is essentially an incremental enhancement of the "Well Defined Structs" scheme, so would be straightforward to upgrade to later. B. Getter/Setters https://patchwork.freedesktop.org/patch/[73035,73036,73037,73038,73039] Establishes a key/value backend config API, with separate getter/setter calls for different config item data types. Backends then call these getters to retrieve configuration variables. This system includes provision for sections and default values. This is certainly a much more flexible system, but many of the same problems will exist when configuration parameters change. Just that instead of erroring at the API/ABI layer it breaks at the parser/configuration layer. Shifting to this lower level means errors may get detected later on, or may be missed entirely, where a broken struct will be more highly visible. Alternative A might be worth considering if we start seeing proliferation of backend options, or if the configuration settings start going beyond basic types. Alternative B could become more convenient if we need to expose options to end-users or if we start seeing third-party backends. Bryce ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 00/14] IVI Layout API Cleanup
On Fri, Feb 26, 2016 at 03:57:56PM +, Ucan, Emre (ADITG/SW1) wrote: > I removed the get APIs, because the same information can be get from > ivi_layout_get_properties_of_surface/layer APIs. Therefore, these APIs > are redundant. Looks like a good cleanup, but do we have any concerns about API stability in dropping these getter/setters? Bryce > Furthermore, I removed *_set_position/dimension APIs, because position > and dimension can be set > by ivi_layout_surface/layer_set_destination_rectangle APIs. > > I adjusted ivi-layout-transition.c, ivi shell test code > and hmi-controller.c for these changes. > > Emre Ucan (14): > ivi-shell: remove ivi_layout_surface_get_visibility API > ivi-shell: remove ivi_layout_layer_get_visibility API > ivi-shell: remove ivi_layout_surface_get_opacity API > ivi-shell: remove ivi_layout_layer_get_opacity API > ivi-shell: remove ivi_layout_surface_get_position API > ivi-shell: remove ivi_layout_layer_get_position API > ivi-shell: remove ivi_layout_surface_get_dimension API > ivi-shell: remove ivi_layout_layer_get_dimension API > ivi-shell: remove ivi_layout_surface_get_orientation API > ivi-shell: remove ivi_layout_layer_get_orientation API > ivi-shell: remove ivi_layout_surface_set_position API > ivi-shell: remove ivi_layout_layer_set_position API > ivi-shell: remove ivi_layout_surface_set_dimension API > ivi-shell: remove ivi_layout_layer_set_dimension API > > ivi-shell/hmi-controller.c| 17 ++- > ivi-shell/ivi-layout-export.h | 127 > ivi-shell/ivi-layout-private.h| 17 +-- > ivi-shell/ivi-layout-transition.c | 19 ++- > ivi-shell/ivi-layout.c| 237 > + > ivi-shell/ivi-shell.c |7 +- > tests/ivi_layout-internal-test.c | 220 +- > tests/ivi_layout-test-plugin.c| 126 +--- > 8 files changed, 85 insertions(+), 685 deletions(-) > > -- > 1.7.9.5 > ___ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 07/14] ivi-shell: remove ivi_layout_surface_get_dimension API
Signed-off-by: Emre Ucan
---
ivi-shell/ivi-layout-export.h|9 -
ivi-shell/ivi-layout-private.h |3 ---
ivi-shell/ivi-layout.c | 16
ivi-shell/ivi-shell.c|7 ++-
tests/ivi_layout-internal-test.c |7 ---
tests/ivi_layout-test-plugin.c | 28 ++--
6 files changed, 8 insertions(+), 62 deletions(-)
diff --git a/ivi-shell/ivi-layout-export.h b/ivi-shell/ivi-layout-export.h
index 6428ef0..b5c6627 100644
--- a/ivi-shell/ivi-layout-export.h
+++ b/ivi-shell/ivi-layout-export.h
@@ -330,15 +330,6 @@ struct ivi_layout_interface {
int32_t dest_width, int32_t
dest_height);
/**
-* \brief Get the horizontal and vertical dimension of the surface.
-*
-* \return IVI_SUCCEEDED if the method call was successful
-* \return IVI_FAILED if the method call was failed
-*/
- int32_t (*surface_get_dimension)(struct ivi_layout_surface *ivisurf,
-int32_t *dest_width, int32_t
*dest_height);
-
- /**
* \brief Sets the orientation of a ivi_surface.
*
* \return IVI_SUCCEEDED if the method call was successful
diff --git a/ivi-shell/ivi-layout-private.h b/ivi-shell/ivi-layout-private.h
index 913bb0c..f0dba91 100644
--- a/ivi-shell/ivi-layout-private.h
+++ b/ivi-shell/ivi-layout-private.h
@@ -171,9 +171,6 @@ ivi_layout_surface_create(struct weston_surface *wl_surface,
uint32_t id_surface);
void
ivi_layout_init_with_compositor(struct weston_compositor *ec);
-int32_t
-ivi_layout_surface_get_dimension(struct ivi_layout_surface *ivisurf,
-int32_t *dest_width, int32_t *dest_height);
void
ivi_layout_surface_add_configured_listener(struct ivi_layout_surface* ivisurf,
struct wl_listener* listener);
diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c
index 8119af4..5552f5d 100644
--- a/ivi-shell/ivi-layout.c
+++ b/ivi-shell/ivi-layout.c
@@ -2102,21 +2102,6 @@ ivi_layout_surface_set_dimension(struct
ivi_layout_surface *ivisurf,
return IVI_SUCCEEDED;
}
-int32_t
-ivi_layout_surface_get_dimension(struct ivi_layout_surface *ivisurf,
-int32_t *dest_width, int32_t *dest_height)
-{
- if (ivisurf == NULL || dest_width == NULL || dest_height == NULL) {
- weston_log("ivi_layout_surface_get_dimension: invalid
argument\n");
- return IVI_FAILED;
- }
-
- *dest_width = ivisurf->prop.dest_width;
- *dest_height = ivisurf->prop.dest_height;
-
- return IVI_SUCCEEDED;
-}
-
static int32_t
ivi_layout_surface_set_position(struct ivi_layout_surface *ivisurf,
int32_t dest_x, int32_t dest_y)
@@ -2678,7 +2663,6 @@ static struct ivi_layout_interface ivi_layout_interface =
{
.surface_set_destination_rectangle =
ivi_layout_surface_set_destination_rectangle,
.surface_set_position =
ivi_layout_surface_set_position,
.surface_set_dimension =
ivi_layout_surface_set_dimension,
- .surface_get_dimension =
ivi_layout_surface_get_dimension,
.surface_set_orientation=
ivi_layout_surface_set_orientation,
.surface_get_orientation=
ivi_layout_surface_get_orientation,
.surface_set_content_observer =
ivi_layout_surface_set_content_observer,
diff --git a/ivi-shell/ivi-shell.c b/ivi-shell/ivi-shell.c
index a767ccf..19140ce 100644
--- a/ivi-shell/ivi-shell.c
+++ b/ivi-shell/ivi-shell.c
@@ -88,11 +88,8 @@ surface_configure_notify(struct wl_listener *listener, void
*data)
struct ivi_shell_surface,
configured_listener);
- int32_t dest_width = 0;
- int32_t dest_height = 0;
-
- ivi_layout_surface_get_dimension(layout_surf,
-&dest_width, &dest_height);
+ int32_t dest_width = layout_surf->prop.dest_width;
+ int32_t dest_height = layout_surf->prop.dest_height;
if (shell_surf->resource)
ivi_surface_send_configure(shell_surf->resource,
diff --git a/tests/ivi_layout-internal-test.c b/tests/ivi_layout-internal-test.c
index 061a74a..3847c80 100644
--- a/tests/ivi_layout-internal-test.c
+++ b/tests/ivi_layout-internal-test.c
@@ -96,17 +96,10 @@ static void
test_surface_bad_dimension(struct test_context *ctx)
{
const struct ivi_layout_interface *lyt = ctx->layout_interface;
- struct ivi_layout_surface *ivisurf = NULL;
- int32_t dest_width;
- int32_t dest_height;
iassert(lyt->surface_set_dimension(NULL, 200, 300) == IVI_FAILED);
lyt->commit_changes();
-
- iassert(lyt->surface_get_dimension(NULL, &dest_width, &dest_height) ==
IVI_FA
[PATCH weston 10/14] ivi-shell: remove ivi_layout_layer_get_orientation API
Signed-off-by: Emre Ucan
---
ivi-shell/ivi-layout-export.h| 10 --
ivi-shell/ivi-layout.c | 12
tests/ivi_layout-internal-test.c | 10 +++---
3 files changed, 3 insertions(+), 29 deletions(-)
diff --git a/ivi-shell/ivi-layout-export.h b/ivi-shell/ivi-layout-export.h
index 21fe96c..86315f6 100644
--- a/ivi-shell/ivi-layout-export.h
+++ b/ivi-shell/ivi-layout-export.h
@@ -554,16 +554,6 @@ struct ivi_layout_interface {
enum wl_output_transform orientation);
/**
-* \brief Gets the orientation of a layer.
-*
-* \return (enum wl_output_transform)
-* if the method call was successful
-* \return WL_OUTPUT_TRANSFORM_NORMAL if the method call was failed
-*/
- enum wl_output_transform
- (*layer_get_orientation)(struct ivi_layout_layer *ivilayer);
-
- /**
* \brief Add a ivi_surface to a ivi_layer which is currently managed
by the service
*
* \return IVI_SUCCEEDED if the method call was successful
diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c
index fefdb7f..cc503e2 100644
--- a/ivi-shell/ivi-layout.c
+++ b/ivi-shell/ivi-layout.c
@@ -1936,17 +1936,6 @@ ivi_layout_layer_set_orientation(struct ivi_layout_layer
*ivilayer,
return IVI_SUCCEEDED;
}
-static enum wl_output_transform
-ivi_layout_layer_get_orientation(struct ivi_layout_layer *ivilayer)
-{
- if (ivilayer == NULL) {
- weston_log("ivi_layout_layer_get_orientation: invalid
argument\n");
- return 0;
- }
-
- return ivilayer->prop.orientation;
-}
-
int32_t
ivi_layout_layer_set_render_order(struct ivi_layout_layer *ivilayer,
struct ivi_layout_surface **pSurface,
@@ -2667,7 +2656,6 @@ static struct ivi_layout_interface ivi_layout_interface =
{
.layer_set_position = ivi_layout_layer_set_position,
.layer_set_dimension=
ivi_layout_layer_set_dimension,
.layer_set_orientation =
ivi_layout_layer_set_orientation,
- .layer_get_orientation =
ivi_layout_layer_get_orientation,
.layer_add_surface = ivi_layout_layer_add_surface,
.layer_remove_surface =
ivi_layout_layer_remove_surface,
.layer_set_render_order =
ivi_layout_layer_set_render_order,
diff --git a/tests/ivi_layout-internal-test.c b/tests/ivi_layout-internal-test.c
index de39403..cd71a16 100644
--- a/tests/ivi_layout-internal-test.c
+++ b/tests/ivi_layout-internal-test.c
@@ -211,18 +211,16 @@ test_layer_orientation(struct test_context *ctx)
ivilayer = lyt->layer_create_with_dimension(IVI_TEST_LAYER_ID(0), 200,
300);
iassert(ivilayer != NULL);
- iassert(lyt->layer_get_orientation(ivilayer) ==
WL_OUTPUT_TRANSFORM_NORMAL);
+ prop = lyt->get_properties_of_layer(ivilayer);
+ iassert(prop->orientation == WL_OUTPUT_TRANSFORM_NORMAL);
iassert(lyt->layer_set_orientation(
ivilayer, WL_OUTPUT_TRANSFORM_90) == IVI_SUCCEEDED);
- iassert(lyt->layer_get_orientation(ivilayer) ==
WL_OUTPUT_TRANSFORM_NORMAL);
+ iassert(prop->orientation == WL_OUTPUT_TRANSFORM_NORMAL);
lyt->commit_changes();
- iassert(lyt->layer_get_orientation(ivilayer) == WL_OUTPUT_TRANSFORM_90);
-
- prop = lyt->get_properties_of_layer(ivilayer);
iassert(prop->orientation == WL_OUTPUT_TRANSFORM_90);
lyt->layer_destroy(ivilayer);
@@ -430,8 +428,6 @@ test_layer_bad_orientation(struct test_context *ctx)
NULL, WL_OUTPUT_TRANSFORM_90) == IVI_FAILED);
lyt->commit_changes();
-
- iassert(lyt->layer_get_orientation(NULL) == WL_OUTPUT_TRANSFORM_NORMAL);
}
static void
--
1.7.9.5
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 12/14] ivi-shell: remove ivi_layout_layer_set_position API
Signed-off-by: Emre Ucan
---
ivi-shell/hmi-controller.c|6 +-
ivi-shell/ivi-layout-export.h |9 -
ivi-shell/ivi-layout-private.h|5 +++--
ivi-shell/ivi-layout-transition.c |3 ++-
ivi-shell/ivi-layout.c| 26 +-
tests/ivi_layout-internal-test.c | 35 ++-
6 files changed, 13 insertions(+), 71 deletions(-)
diff --git a/ivi-shell/hmi-controller.c b/ivi-shell/hmi-controller.c
index e97fa0a..915725e 100644
--- a/ivi-shell/hmi-controller.c
+++ b/ivi-shell/hmi-controller.c
@@ -1572,12 +1572,16 @@ static void
layer_set_pos(struct ivi_layout_layer *layer, wl_fixed_t pos_x,
wl_fixed_t pos_y)
{
+ const struct ivi_layout_layer_properties *prop;
int32_t layout_pos_x = 0;
int32_t layout_pos_y = 0;
+ prop = ivi_layout_interface->get_properties_of_layer(layer);
+
layout_pos_x = wl_fixed_to_int(pos_x);
layout_pos_y = wl_fixed_to_int(pos_y);
- ivi_layout_interface->layer_set_position(layer, layout_pos_x,
layout_pos_y);
+ ivi_layout_interface->layer_set_destination_rectangle(layer,
+ layout_pos_x, layout_pos_y, prop->dest_width,
prop->dest_height);
ivi_layout_interface->commit_changes();
}
diff --git a/ivi-shell/ivi-layout-export.h b/ivi-shell/ivi-layout-export.h
index 5e5b305..1a16b44 100644
--- a/ivi-shell/ivi-layout-export.h
+++ b/ivi-shell/ivi-layout-export.h
@@ -518,15 +518,6 @@ struct ivi_layout_interface {
int32_t width, int32_t
height);
/**
-* \brief Sets the horizontal and vertical position of the ivi_layer.
-*
-* \return IVI_SUCCEEDED if the method call was successful
-* \return IVI_FAILED if the method call was failed
-*/
- int32_t (*layer_set_position)(struct ivi_layout_layer *ivilayer,
- int32_t dest_x, int32_t dest_y);
-
- /**
* \brief Set the horizontal and vertical dimension of the layer.
*
* \return IVI_SUCCEEDED if the method call was successful
diff --git a/ivi-shell/ivi-layout-private.h b/ivi-shell/ivi-layout-private.h
index f0dba91..3a23ef4 100644
--- a/ivi-shell/ivi-layout-private.h
+++ b/ivi-shell/ivi-layout-private.h
@@ -200,8 +200,9 @@ int32_t
ivi_layout_layer_set_visibility(struct ivi_layout_layer *ivilayer,
bool newVisibility);
int32_t
-ivi_layout_layer_set_position(struct ivi_layout_layer *ivilayer,
- int32_t dest_x, int32_t dest_y);
+ivi_layout_layer_set_destination_rectangle(struct ivi_layout_layer *ivilayer,
+ int32_t x, int32_t y,
+ int32_t width, int32_t height);
int32_t
ivi_layout_layer_set_render_order(struct ivi_layout_layer *ivilayer,
struct ivi_layout_surface **pSurface,
diff --git a/ivi-shell/ivi-layout-transition.c
b/ivi-shell/ivi-layout-transition.c
index df6112d..3b613a5 100644
--- a/ivi-shell/ivi-layout-transition.c
+++ b/ivi-shell/ivi-layout-transition.c
@@ -664,7 +664,8 @@ transition_move_layer_user_frame(struct
ivi_layout_transition *transition)
const int32_t dest_y = data->start_y +
(data->end_y - data->start_y) * current;
- ivi_layout_layer_set_position(layer, dest_x, dest_y);
+ ivi_layout_layer_set_destination_rectangle(layer, dest_x, dest_y,
+ layer->prop.dest_width, layer->prop.dest_height);
}
static void
diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c
index 1b0b77f..9f4a0e2 100644
--- a/ivi-shell/ivi-layout.c
+++ b/ivi-shell/ivi-layout.c
@@ -1838,7 +1838,7 @@ ivi_layout_layer_set_source_rectangle(struct
ivi_layout_layer *ivilayer,
return IVI_SUCCEEDED;
}
-static int32_t
+int32_t
ivi_layout_layer_set_destination_rectangle(struct ivi_layout_layer *ivilayer,
int32_t x, int32_t y,
int32_t width, int32_t height)
@@ -1891,29 +1891,6 @@ ivi_layout_layer_set_dimension(struct ivi_layout_layer
*ivilayer,
return IVI_SUCCEEDED;
}
-int32_t
-ivi_layout_layer_set_position(struct ivi_layout_layer *ivilayer,
- int32_t dest_x, int32_t dest_y)
-{
- struct ivi_layout_layer_properties *prop = NULL;
-
- if (ivilayer == NULL) {
- weston_log("ivi_layout_layer_set_position: invalid argument\n");
- return IVI_FAILED;
- }
-
- prop = &ivilayer->pending.prop;
- prop->dest_x = dest_x;
- prop->dest_y = dest_y;
-
- if (ivilayer->prop.dest_x != dest_x || ivilayer->prop.dest_y != dest_y)
- ivilayer->event_mask |= IVI_NOTIFICATION_POSITION;
- else
- ivilayer->event_mask &= ~IVI_NOTIFICATION_POSITION;
-
- return I
[PATCH weston 06/14] ivi-shell: remove ivi_layout_layer_get_position API
Signed-off-by: Emre Ucan
---
ivi-shell/hmi-controller.c| 11 ---
ivi-shell/ivi-layout-export.h |9 -
ivi-shell/ivi-layout-private.h|3 ---
ivi-shell/ivi-layout-transition.c |6 ++
ivi-shell/ivi-layout.c| 16
tests/ivi_layout-internal-test.c | 33 +
6 files changed, 15 insertions(+), 63 deletions(-)
diff --git a/ivi-shell/hmi-controller.c b/ivi-shell/hmi-controller.c
index 8da3d3c..e97fa0a 100644
--- a/ivi-shell/hmi-controller.c
+++ b/ivi-shell/hmi-controller.c
@@ -1423,6 +1423,7 @@ move_workspace_grab_end(struct move_grab *move, struct
wl_resource* resource,
{
struct hmi_controller *hmi_ctrl = wl_resource_get_user_data(resource);
int32_t width = hmi_ctrl->workspace_background_layer.width;
+ const struct ivi_layout_layer_properties *prop;
struct timespec time = {0};
double grab_time = 0.0;
@@ -1449,8 +1450,9 @@ move_workspace_grab_end(struct move_grab *move, struct
wl_resource* resource,
if (200 < from_motion_time)
pointer_v = 0.0;
- ivi_layout_interface->layer_get_position(layer, &pos_x, &pos_y);
-
+ prop = ivi_layout_interface->get_properties_of_layer(layer);
+ pos_x = prop->dest_x;
+ pos_y = prop->dest_y;
if (is_flick) {
int orgx = wl_fixed_to_int(move->dst[0] + grab_x);
@@ -1739,6 +1741,7 @@ move_grab_init_workspace(struct move_grab* move,
{
struct hmi_controller *hmi_ctrl = wl_resource_get_user_data(resource);
struct ivi_layout_layer *layer = hmi_ctrl->workspace_layer.ivilayer;
+ const struct ivi_layout_layer_properties *prop;
int32_t workspace_count = hmi_ctrl->workspace_count;
int32_t workspace_width = hmi_ctrl->workspace_background_layer.width;
int32_t layer_pos_x = 0;
@@ -1747,7 +1750,9 @@ move_grab_init_workspace(struct move_grab* move,
wl_fixed_t rgn[2][2] = {{0}};
wl_fixed_t grab_pos[2] = { grab_x, grab_y };
- ivi_layout_interface->layer_get_position(layer, &layer_pos_x,
&layer_pos_y);
+ prop = ivi_layout_interface->get_properties_of_layer(layer);
+ layer_pos_x = prop->dest_x;
+ layer_pos_y = prop->dest_y;
start_pos[0] = wl_fixed_from_int(layer_pos_x);
start_pos[1] = wl_fixed_from_int(layer_pos_y);
diff --git a/ivi-shell/ivi-layout-export.h b/ivi-shell/ivi-layout-export.h
index 4dbac50..6428ef0 100644
--- a/ivi-shell/ivi-layout-export.h
+++ b/ivi-shell/ivi-layout-export.h
@@ -555,15 +555,6 @@ struct ivi_layout_interface {
int32_t dest_x, int32_t dest_y);
/**
-* \brief Get the horizontal and vertical position of the ivi_layer.
-*
-* \return IVI_SUCCEEDED if the method call was successful
-* \return IVI_FAILED if the method call was failed
-*/
- int32_t (*layer_get_position)(struct ivi_layout_layer *ivilayer,
- int32_t *dest_x, int32_t *dest_y);
-
- /**
* \brief Set the horizontal and vertical dimension of the layer.
*
* \return IVI_SUCCEEDED if the method call was successful
diff --git a/ivi-shell/ivi-layout-private.h b/ivi-shell/ivi-layout-private.h
index 9f0ca79..913bb0c 100644
--- a/ivi-shell/ivi-layout-private.h
+++ b/ivi-shell/ivi-layout-private.h
@@ -206,9 +206,6 @@ int32_t
ivi_layout_layer_set_position(struct ivi_layout_layer *ivilayer,
int32_t dest_x, int32_t dest_y);
int32_t
-ivi_layout_layer_get_position(struct ivi_layout_layer *ivilayer,
- int32_t *dest_x, int32_t *dest_y);
-int32_t
ivi_layout_layer_set_render_order(struct ivi_layout_layer *ivilayer,
struct ivi_layout_surface **pSurface,
int32_t number);
diff --git a/ivi-shell/ivi-layout-transition.c
b/ivi-shell/ivi-layout-transition.c
index 529bd56..df6112d 100644
--- a/ivi-shell/ivi-layout-transition.c
+++ b/ivi-shell/ivi-layout-transition.c
@@ -736,12 +736,10 @@ ivi_layout_transition_move_layer(struct ivi_layout_layer
*layer,
int32_t dest_x, int32_t dest_y,
uint32_t duration)
{
- int32_t start_pos_x = 0;
- int32_t start_pos_y = 0;
+ int32_t start_pos_x = layer->prop.dest_x;
+ int32_t start_pos_y = layer->prop.dest_y;
struct ivi_layout_transition *transition = NULL;
- ivi_layout_layer_get_position(layer, &start_pos_x, &start_pos_y);
-
transition = create_move_layer_transition(
layer,
start_pos_x, start_pos_y,
diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c
index 81fea95..8119af4 100644
--- a/ivi-shell/ivi-layout.c
+++ b/ivi-shell/ivi-layout.c
@@ -1907,21 +1907,6 @@ ivi_layout_layer_set_dimension(struct ivi_layout_layer
*ivilayer,
}
int32_t
-ivi
[PATCH weston 08/14] ivi-shell: remove ivi_layout_layer_get_dimension API
Signed-off-by: Emre Ucan
---
ivi-shell/ivi-layout-export.h|9 -
ivi-shell/ivi-layout.c | 16
tests/ivi_layout-internal-test.c | 38 +-
3 files changed, 5 insertions(+), 58 deletions(-)
diff --git a/ivi-shell/ivi-layout-export.h b/ivi-shell/ivi-layout-export.h
index b5c6627..4b2697c 100644
--- a/ivi-shell/ivi-layout-export.h
+++ b/ivi-shell/ivi-layout-export.h
@@ -555,15 +555,6 @@ struct ivi_layout_interface {
int32_t dest_width, int32_t dest_height);
/**
-* \brief Get the horizontal and vertical dimension of the layer.
-*
-* \return IVI_SUCCEEDED if the method call was successful
-* \return IVI_FAILED if the method call was failed
-*/
- int32_t (*layer_get_dimension)(struct ivi_layout_layer *ivilayer,
- int32_t *dest_width, int32_t
*dest_height);
-
- /**
* \brief Sets the orientation of a ivi_layer.
*
* \return IVI_SUCCEEDED if the method call was successful
diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c
index 5552f5d..d2598b3 100644
--- a/ivi-shell/ivi-layout.c
+++ b/ivi-shell/ivi-layout.c
@@ -1867,21 +1867,6 @@ ivi_layout_layer_set_destination_rectangle(struct
ivi_layout_layer *ivilayer,
}
static int32_t
-ivi_layout_layer_get_dimension(struct ivi_layout_layer *ivilayer,
- int32_t *dest_width, int32_t *dest_height)
-{
- if (ivilayer == NULL || dest_width == NULL || dest_height == NULL) {
- weston_log("ivi_layout_layer_get_dimension: invalid
argument\n");
- return IVI_FAILED;
- }
-
- *dest_width = ivilayer->prop.dest_width;
- *dest_height = ivilayer->prop.dest_height;
-
- return IVI_SUCCEEDED;
-}
-
-static int32_t
ivi_layout_layer_set_dimension(struct ivi_layout_layer *ivilayer,
int32_t dest_width, int32_t dest_height)
{
@@ -2693,7 +2678,6 @@ static struct ivi_layout_interface ivi_layout_interface =
{
.layer_set_destination_rectangle=
ivi_layout_layer_set_destination_rectangle,
.layer_set_position = ivi_layout_layer_set_position,
.layer_set_dimension=
ivi_layout_layer_set_dimension,
- .layer_get_dimension=
ivi_layout_layer_get_dimension,
.layer_set_orientation =
ivi_layout_layer_set_orientation,
.layer_get_orientation =
ivi_layout_layer_get_orientation,
.layer_add_surface = ivi_layout_layer_add_surface,
diff --git a/tests/ivi_layout-internal-test.c b/tests/ivi_layout-internal-test.c
index 3847c80..8b57fa0 100644
--- a/tests/ivi_layout-internal-test.c
+++ b/tests/ivi_layout-internal-test.c
@@ -236,32 +236,21 @@ test_layer_dimension(struct test_context *ctx)
const struct ivi_layout_interface *lyt = ctx->layout_interface;
struct ivi_layout_layer *ivilayer;
const struct ivi_layout_layer_properties *prop;
- int32_t dest_width;
- int32_t dest_height;
ivilayer = lyt->layer_create_with_dimension(IVI_TEST_LAYER_ID(0), 200,
300);
iassert(ivilayer != NULL);
- iassert(lyt->layer_get_dimension(
- ivilayer, &dest_width, &dest_height) == IVI_SUCCEEDED);
- iassert(dest_width == 200);
- iassert(dest_height == 300);
+ prop = lyt->get_properties_of_layer(ivilayer);
+ iassert(prop->dest_width == 200);
+ iassert(prop->dest_height == 300);
iassert(lyt->layer_set_dimension(ivilayer, 400, 600) == IVI_SUCCEEDED);
- iassert(lyt->layer_get_dimension(
- ivilayer, &dest_width, &dest_height) == IVI_SUCCEEDED);
- iassert(dest_width == 200);
- iassert(dest_height == 300);
+ iassert(prop->dest_width == 200);
+ iassert(prop->dest_height == 300);
lyt->commit_changes();
- iassert(IVI_SUCCEEDED == lyt->layer_get_dimension(
- ivilayer, &dest_width, &dest_height));
- iassert(dest_width == 400);
- iassert(dest_height == 600);
-
- prop = lyt->get_properties_of_layer(ivilayer);
iassert(prop->dest_width == 400);
iassert(prop->dest_height == 600);
@@ -302,8 +291,6 @@ test_layer_destination_rectangle(struct test_context *ctx)
const struct ivi_layout_interface *lyt = ctx->layout_interface;
struct ivi_layout_layer *ivilayer;
const struct ivi_layout_layer_properties *prop;
- int32_t dest_width;
- int32_t dest_height;
ivilayer = lyt->layer_create_with_dimension(IVI_TEST_LAYER_ID(0), 200,
300);
iassert(ivilayer != NULL);
@@ -325,12 +312,6 @@ test_layer_destination_rectangle(struct test_context *ctx)
lyt->commit_changes();
- iassert(lyt->layer_get_dimension(
- ivilayer, &dest_widt
[PATCH weston 11/14] ivi-shell: remove ivi_layout_surface_set_position API
Signed-off-by: Emre Ucan
---
ivi-shell/ivi-layout-export.h|9 -
ivi-shell/ivi-layout.c | 24
tests/ivi_layout-internal-test.c | 11 ---
tests/ivi_layout-test-plugin.c | 16 +++-
4 files changed, 3 insertions(+), 57 deletions(-)
diff --git a/ivi-shell/ivi-layout-export.h b/ivi-shell/ivi-layout-export.h
index 86315f6..5e5b305 100644
--- a/ivi-shell/ivi-layout-export.h
+++ b/ivi-shell/ivi-layout-export.h
@@ -312,15 +312,6 @@ struct ivi_layout_interface {
int32_t width, int32_t
height);
/**
-* \brief Sets the horizontal and vertical position of the surface.
-*
-* \return IVI_SUCCEEDED if the method call was successful
-* \return IVI_FAILED if the method call was failed
-*/
- int32_t (*surface_set_position)(struct ivi_layout_surface *ivisurf,
- int32_t dest_x, int32_t dest_y);
-
- /**
* \brief Set the horizontal and vertical dimension of the surface.
*
* \return IVI_SUCCEEDED if the method call was successful
diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c
index cc503e2..1b0b77f 100644
--- a/ivi-shell/ivi-layout.c
+++ b/ivi-shell/ivi-layout.c
@@ -2077,29 +2077,6 @@ ivi_layout_surface_set_dimension(struct
ivi_layout_surface *ivisurf,
}
static int32_t
-ivi_layout_surface_set_position(struct ivi_layout_surface *ivisurf,
- int32_t dest_x, int32_t dest_y)
-{
- struct ivi_layout_surface_properties *prop = NULL;
-
- if (ivisurf == NULL) {
- weston_log("ivi_layout_surface_set_position: invalid
argument\n");
- return IVI_FAILED;
- }
-
- prop = &ivisurf->pending.prop;
- prop->dest_x = dest_x;
- prop->dest_y = dest_y;
-
- if (ivisurf->prop.dest_x != dest_x || ivisurf->prop.dest_y != dest_y)
- ivisurf->event_mask |= IVI_NOTIFICATION_POSITION;
- else
- ivisurf->event_mask &= ~IVI_NOTIFICATION_POSITION;
-
- return IVI_SUCCEEDED;
-}
-
-static int32_t
ivi_layout_surface_set_orientation(struct ivi_layout_surface *ivisurf,
enum wl_output_transform orientation)
{
@@ -2624,7 +2601,6 @@ static struct ivi_layout_interface ivi_layout_interface =
{
.surface_set_opacity=
ivi_layout_surface_set_opacity,
.surface_set_source_rectangle =
ivi_layout_surface_set_source_rectangle,
.surface_set_destination_rectangle =
ivi_layout_surface_set_destination_rectangle,
- .surface_set_position =
ivi_layout_surface_set_position,
.surface_set_dimension =
ivi_layout_surface_set_dimension,
.surface_set_orientation=
ivi_layout_surface_set_orientation,
.surface_set_content_observer =
ivi_layout_surface_set_content_observer,
diff --git a/tests/ivi_layout-internal-test.c b/tests/ivi_layout-internal-test.c
index cd71a16..02e5524 100644
--- a/tests/ivi_layout-internal-test.c
+++ b/tests/ivi_layout-internal-test.c
@@ -101,16 +101,6 @@ test_surface_bad_dimension(struct test_context *ctx)
}
static void
-test_surface_bad_position(struct test_context *ctx)
-{
- const struct ivi_layout_interface *lyt = ctx->layout_interface;
-
- iassert(lyt->surface_set_position(NULL, 20, 30) == IVI_FAILED);
-
- lyt->commit_changes();
-}
-
-static void
test_surface_bad_source_rectangle(struct test_context *ctx)
{
const struct ivi_layout_interface *lyt = ctx->layout_interface;
@@ -1043,7 +1033,6 @@ run_internal_tests(void *data)
test_surface_bad_destination_rectangle(ctx);
test_surface_bad_orientation(ctx);
test_surface_bad_dimension(ctx);
- test_surface_bad_position(ctx);
test_surface_bad_source_rectangle(ctx);
test_surface_bad_properties(ctx);
diff --git a/tests/ivi_layout-test-plugin.c b/tests/ivi_layout-test-plugin.c
index b531db7..933ee2d 100644
--- a/tests/ivi_layout-test-plugin.c
+++ b/tests/ivi_layout-test-plugin.c
@@ -458,8 +458,9 @@ RUNNER_TEST(surface_position)
runner_assert(prop->dest_x == 20);
runner_assert(prop->dest_y == 30);
- runner_assert(lyt->surface_set_position(
- ivisurf, 20, 30) == IVI_SUCCEEDED);
+ runner_assert(lyt->surface_set_destination_rectangle(
+ ivisurf, 20, 30,
+ prop->dest_width, prop->dest_height) == IVI_SUCCEEDED);
runner_assert(prop->dest_x == 0);
runner_assert(prop->dest_y == 0);
@@ -631,17 +632,6 @@
RUNNER_TEST(commit_changes_after_dimension_set_surface_destroy)
ivisurf, 200, 300) == IVI_SUCCEEDED);
}
-RUNNER_TEST(commit_changes_after_position_set_surface_destroy)
-{
- const struct ivi_layout_interface *lyt = ctx->layout_int
[PATCH weston 13/14] ivi-shell: remove ivi_layout_surface_set_dimension API
Signed-off-by: Emre Ucan
---
ivi-shell/ivi-layout-export.h|9 -
ivi-shell/ivi-layout.c | 25 -
tests/ivi_layout-internal-test.c | 11 ---
tests/ivi_layout-test-plugin.c | 14 ++
4 files changed, 2 insertions(+), 57 deletions(-)
diff --git a/ivi-shell/ivi-layout-export.h b/ivi-shell/ivi-layout-export.h
index 1a16b44..f079931 100644
--- a/ivi-shell/ivi-layout-export.h
+++ b/ivi-shell/ivi-layout-export.h
@@ -312,15 +312,6 @@ struct ivi_layout_interface {
int32_t width, int32_t
height);
/**
-* \brief Set the horizontal and vertical dimension of the surface.
-*
-* \return IVI_SUCCEEDED if the method call was successful
-* \return IVI_FAILED if the method call was failed
-*/
- int32_t (*surface_set_dimension)(struct ivi_layout_surface *ivisurf,
-int32_t dest_width, int32_t
dest_height);
-
- /**
* \brief Sets the orientation of a ivi_surface.
*
* \return IVI_SUCCEEDED if the method call was successful
diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c
index 9f4a0e2..c3998e3 100644
--- a/ivi-shell/ivi-layout.c
+++ b/ivi-shell/ivi-layout.c
@@ -2030,30 +2030,6 @@ ivi_layout_surface_set_destination_rectangle(struct
ivi_layout_surface *ivisurf,
}
static int32_t
-ivi_layout_surface_set_dimension(struct ivi_layout_surface *ivisurf,
-int32_t dest_width, int32_t dest_height)
-{
- struct ivi_layout_surface_properties *prop = NULL;
-
- if (ivisurf == NULL) {
- weston_log("ivi_layout_surface_set_dimension: invalid
argument\n");
- return IVI_FAILED;
- }
-
- prop = &ivisurf->pending.prop;
- prop->dest_width = dest_width;
- prop->dest_height = dest_height;
-
- if (ivisurf->prop.dest_width != dest_width ||
- ivisurf->prop.dest_height != dest_height)
- ivisurf->event_mask |= IVI_NOTIFICATION_DIMENSION;
- else
- ivisurf->event_mask &= ~IVI_NOTIFICATION_DIMENSION;
-
- return IVI_SUCCEEDED;
-}
-
-static int32_t
ivi_layout_surface_set_orientation(struct ivi_layout_surface *ivisurf,
enum wl_output_transform orientation)
{
@@ -2578,7 +2554,6 @@ static struct ivi_layout_interface ivi_layout_interface =
{
.surface_set_opacity=
ivi_layout_surface_set_opacity,
.surface_set_source_rectangle =
ivi_layout_surface_set_source_rectangle,
.surface_set_destination_rectangle =
ivi_layout_surface_set_destination_rectangle,
- .surface_set_dimension =
ivi_layout_surface_set_dimension,
.surface_set_orientation=
ivi_layout_surface_set_orientation,
.surface_set_content_observer =
ivi_layout_surface_set_content_observer,
.surface_add_notification =
ivi_layout_surface_add_notification,
diff --git a/tests/ivi_layout-internal-test.c b/tests/ivi_layout-internal-test.c
index fd2f0db..14bdb9f 100644
--- a/tests/ivi_layout-internal-test.c
+++ b/tests/ivi_layout-internal-test.c
@@ -91,16 +91,6 @@ test_surface_bad_orientation(struct test_context *ctx)
}
static void
-test_surface_bad_dimension(struct test_context *ctx)
-{
- const struct ivi_layout_interface *lyt = ctx->layout_interface;
-
- iassert(lyt->surface_set_dimension(NULL, 200, 300) == IVI_FAILED);
-
- lyt->commit_changes();
-}
-
-static void
test_surface_bad_source_rectangle(struct test_context *ctx)
{
const struct ivi_layout_interface *lyt = ctx->layout_interface;
@@ -1003,7 +993,6 @@ run_internal_tests(void *data)
test_surface_bad_visibility(ctx);
test_surface_bad_destination_rectangle(ctx);
test_surface_bad_orientation(ctx);
- test_surface_bad_dimension(ctx);
test_surface_bad_source_rectangle(ctx);
test_surface_bad_properties(ctx);
diff --git a/tests/ivi_layout-test-plugin.c b/tests/ivi_layout-test-plugin.c
index 933ee2d..4ee36e9 100644
--- a/tests/ivi_layout-test-plugin.c
+++ b/tests/ivi_layout-test-plugin.c
@@ -431,7 +431,8 @@ RUNNER_TEST(surface_dimension)
runner_assert(prop->dest_height == 1);
runner_assert(IVI_SUCCEEDED ==
- lyt->surface_set_dimension(ivisurf, 200, 300));
+ lyt->surface_set_destination_rectangle(ivisurf,
prop->dest_x,
+ prop->dest_y, 200, 300));
runner_assert(prop->dest_width == 1);
runner_assert(prop->dest_height == 1);
@@ -621,17 +622,6 @@
RUNNER_TEST(commit_changes_after_orientation_set_surface_destroy)
ivisurf, WL_OUTPUT_TRANSFORM_90) == IVI_SUCCEEDED);
}
-RUNNER_TEST(commit_changes_after_dimension_set_surface_destroy)
-{
- const struct ivi_layout_interface *l
[PATCH weston 14/14] ivi-shell: remove ivi_layout_layer_set_dimension API
Signed-off-by: Emre Ucan
---
ivi-shell/ivi-layout-export.h|9 -
ivi-shell/ivi-layout.c | 26 --
tests/ivi_layout-internal-test.c | 35 ++-
3 files changed, 2 insertions(+), 68 deletions(-)
diff --git a/ivi-shell/ivi-layout-export.h b/ivi-shell/ivi-layout-export.h
index f079931..85bf91a 100644
--- a/ivi-shell/ivi-layout-export.h
+++ b/ivi-shell/ivi-layout-export.h
@@ -509,15 +509,6 @@ struct ivi_layout_interface {
int32_t width, int32_t
height);
/**
-* \brief Set the horizontal and vertical dimension of the layer.
-*
-* \return IVI_SUCCEEDED if the method call was successful
-* \return IVI_FAILED if the method call was failed
-*/
- int32_t (*layer_set_dimension)(struct ivi_layout_layer *ivilayer,
- int32_t dest_width, int32_t dest_height);
-
- /**
* \brief Sets the orientation of a ivi_layer.
*
* \return IVI_SUCCEEDED if the method call was successful
diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c
index c3998e3..b6a2bb1 100644
--- a/ivi-shell/ivi-layout.c
+++ b/ivi-shell/ivi-layout.c
@@ -1867,31 +1867,6 @@ ivi_layout_layer_set_destination_rectangle(struct
ivi_layout_layer *ivilayer,
}
static int32_t
-ivi_layout_layer_set_dimension(struct ivi_layout_layer *ivilayer,
- int32_t dest_width, int32_t dest_height)
-{
- struct ivi_layout_layer_properties *prop = NULL;
-
- if (ivilayer == NULL) {
- weston_log("ivi_layout_layer_set_dimension: invalid
argument\n");
- return IVI_FAILED;
- }
-
- prop = &ivilayer->pending.prop;
-
- prop->dest_width = dest_width;
- prop->dest_height = dest_height;
-
- if (ivilayer->prop.dest_width != dest_width ||
- ivilayer->prop.dest_height != dest_height)
- ivilayer->event_mask |= IVI_NOTIFICATION_DIMENSION;
- else
- ivilayer->event_mask &= ~IVI_NOTIFICATION_DIMENSION;
-
- return IVI_SUCCEEDED;
-}
-
-static int32_t
ivi_layout_layer_set_orientation(struct ivi_layout_layer *ivilayer,
enum wl_output_transform orientation)
{
@@ -2581,7 +2556,6 @@ static struct ivi_layout_interface ivi_layout_interface =
{
.layer_set_opacity = ivi_layout_layer_set_opacity,
.layer_set_source_rectangle =
ivi_layout_layer_set_source_rectangle,
.layer_set_destination_rectangle=
ivi_layout_layer_set_destination_rectangle,
- .layer_set_dimension=
ivi_layout_layer_set_dimension,
.layer_set_orientation =
ivi_layout_layer_set_orientation,
.layer_add_surface = ivi_layout_layer_add_surface,
.layer_remove_surface =
ivi_layout_layer_remove_surface,
diff --git a/tests/ivi_layout-internal-test.c b/tests/ivi_layout-internal-test.c
index 14bdb9f..40b3b48 100644
--- a/tests/ivi_layout-internal-test.c
+++ b/tests/ivi_layout-internal-test.c
@@ -220,7 +220,8 @@ test_layer_dimension(struct test_context *ctx)
iassert(prop->dest_width == 200);
iassert(prop->dest_height == 300);
- iassert(lyt->layer_set_dimension(ivilayer, 400, 600) == IVI_SUCCEEDED);
+ iassert(lyt->layer_set_destination_rectangle(ivilayer, prop->dest_x,
prop->dest_y,
+ 400, 600) == IVI_SUCCEEDED);
iassert(prop->dest_width == 200);
iassert(prop->dest_height == 300);
@@ -412,22 +413,6 @@ test_layer_bad_orientation(struct test_context *ctx)
}
static void
-test_layer_bad_dimension(struct test_context *ctx)
-{
- const struct ivi_layout_interface *lyt = ctx->layout_interface;
- struct ivi_layout_layer *ivilayer;
-
- ivilayer = lyt->layer_create_with_dimension(IVI_TEST_LAYER_ID(0), 200,
300);
- iassert(ivilayer != NULL);
-
- iassert(lyt->layer_set_dimension(NULL, 200, 300) == IVI_FAILED);
-
- lyt->commit_changes();
-
- lyt->layer_destroy(ivilayer);
-}
-
-static void
test_layer_bad_source_rectangle(struct test_context *ctx)
{
const struct ivi_layout_interface *lyt = ctx->layout_interface;
@@ -489,20 +474,6 @@
test_commit_changes_after_orientation_set_layer_destroy(struct test_context *ctx
}
static void
-test_commit_changes_after_dimension_set_layer_destroy(struct test_context *ctx)
-{
- const struct ivi_layout_interface *lyt = ctx->layout_interface;
- struct ivi_layout_layer *ivilayer;
-
- ivilayer = lyt->layer_create_with_dimension(IVI_TEST_LAYER_ID(0), 200,
300);
- iassert(ivilayer != NULL);
-
- iassert(lyt->layer_set_dimension(ivilayer, 200, 300) == IVI_SUCCEEDED);
- lyt->layer_destroy(ivilayer);
- lyt->commit_changes();
-}
-
-static void
test_commit_changes_
[PATCH weston 09/14] ivi-shell: remove ivi_layout_surface_get_orientation API
Signed-off-by: Emre Ucan
---
ivi-shell/ivi-layout-export.h| 10 --
ivi-shell/ivi-layout.c | 12
tests/ivi_layout-internal-test.c |2 --
tests/ivi_layout-test-plugin.c | 13 -
4 files changed, 4 insertions(+), 33 deletions(-)
diff --git a/ivi-shell/ivi-layout-export.h b/ivi-shell/ivi-layout-export.h
index 4b2697c..21fe96c 100644
--- a/ivi-shell/ivi-layout-export.h
+++ b/ivi-shell/ivi-layout-export.h
@@ -339,16 +339,6 @@ struct ivi_layout_interface {
enum wl_output_transform
orientation);
/**
-* \brief Gets the orientation of a surface.
-*
-* \return (enum wl_output_transform)
-* if the method call was successful
-* \return WL_OUTPUT_TRANSFORM_NORMAL if the method call was failed
-*/
- enum wl_output_transform
- (*surface_get_orientation)(struct ivi_layout_surface *ivisurf);
-
- /**
* \brief Set an observer callback for ivi_surface content status
change.
*
* \return IVI_SUCCEEDED if the method call was successful
diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c
index d2598b3..fefdb7f 100644
--- a/ivi-shell/ivi-layout.c
+++ b/ivi-shell/ivi-layout.c
@@ -2132,17 +2132,6 @@ ivi_layout_surface_set_orientation(struct
ivi_layout_surface *ivisurf,
return IVI_SUCCEEDED;
}
-static enum wl_output_transform
-ivi_layout_surface_get_orientation(struct ivi_layout_surface *ivisurf)
-{
- if (ivisurf == NULL) {
- weston_log("ivi_layout_surface_get_orientation: invalid
argument\n");
- return 0;
- }
-
- return ivisurf->prop.orientation;
-}
-
static int32_t
ivi_layout_screen_add_layer(struct ivi_layout_screen *iviscrn,
struct ivi_layout_layer *addlayer)
@@ -2649,7 +2638,6 @@ static struct ivi_layout_interface ivi_layout_interface =
{
.surface_set_position =
ivi_layout_surface_set_position,
.surface_set_dimension =
ivi_layout_surface_set_dimension,
.surface_set_orientation=
ivi_layout_surface_set_orientation,
- .surface_get_orientation=
ivi_layout_surface_get_orientation,
.surface_set_content_observer =
ivi_layout_surface_set_content_observer,
.surface_add_notification =
ivi_layout_surface_add_notification,
.surface_remove_notification=
ivi_layout_surface_remove_notification,
diff --git a/tests/ivi_layout-internal-test.c b/tests/ivi_layout-internal-test.c
index 8b57fa0..de39403 100644
--- a/tests/ivi_layout-internal-test.c
+++ b/tests/ivi_layout-internal-test.c
@@ -88,8 +88,6 @@ test_surface_bad_orientation(struct test_context *ctx)
const struct ivi_layout_interface *lyt = ctx->layout_interface;
iassert(lyt->surface_set_orientation(NULL, WL_OUTPUT_TRANSFORM_90) ==
IVI_FAILED);
-
- iassert(lyt->surface_get_orientation(NULL) ==
WL_OUTPUT_TRANSFORM_NORMAL);
}
static void
diff --git a/tests/ivi_layout-test-plugin.c b/tests/ivi_layout-test-plugin.c
index 95999f8..b531db7 100644
--- a/tests/ivi_layout-test-plugin.c
+++ b/tests/ivi_layout-test-plugin.c
@@ -402,22 +402,17 @@ RUNNER_TEST(surface_orientation)
ivisurf = lyt->get_surface_from_id(IVI_TEST_SURFACE_ID(0));
runner_assert(ivisurf != NULL);
- runner_assert(lyt->surface_get_orientation(ivisurf) ==
- WL_OUTPUT_TRANSFORM_NORMAL);
+ prop = lyt->get_properties_of_surface(ivisurf);
+ runner_assert_or_return(prop);
+ runner_assert(prop->orientation == WL_OUTPUT_TRANSFORM_NORMAL);
runner_assert(lyt->surface_set_orientation(
ivisurf, WL_OUTPUT_TRANSFORM_90) == IVI_SUCCEEDED);
- runner_assert(lyt->surface_get_orientation(ivisurf) ==
- WL_OUTPUT_TRANSFORM_NORMAL);
+ runner_assert(prop->orientation == WL_OUTPUT_TRANSFORM_NORMAL);
lyt->commit_changes();
- runner_assert(lyt->surface_get_orientation(
- ivisurf) == WL_OUTPUT_TRANSFORM_90);
-
- prop = lyt->get_properties_of_surface(ivisurf);
- runner_assert_or_return(prop);
runner_assert(prop->orientation == WL_OUTPUT_TRANSFORM_90);
}
--
1.7.9.5
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 03/14] ivi-shell: remove ivi_layout_surface_get_opacity API
Signed-off-by: Emre Ucan
---
ivi-shell/ivi-layout-export.h |8
ivi-shell/ivi-layout-private.h|2 --
ivi-shell/ivi-layout-transition.c |6 +++---
ivi-shell/ivi-layout.c| 12
tests/ivi_layout-test-plugin.c| 24 +++-
5 files changed, 10 insertions(+), 42 deletions(-)
diff --git a/ivi-shell/ivi-layout-export.h b/ivi-shell/ivi-layout-export.h
index fc0e050..c6dc09b 100644
--- a/ivi-shell/ivi-layout-export.h
+++ b/ivi-shell/ivi-layout-export.h
@@ -289,14 +289,6 @@ struct ivi_layout_interface {
wl_fixed_t opacity);
/**
-* \brief Get the opacity of a ivi_surface.
-*
-* \return opacity if the method call was successful
-* \return wl_fixed_from_double(0.0) if the method call was failed
-*/
- wl_fixed_t (*surface_get_opacity)(struct ivi_layout_surface *ivisurf);
-
- /**
* \brief Set the area of a ivi_surface which should be used for the
rendering.
*
* \return IVI_SUCCEEDED if the method call was successful
diff --git a/ivi-shell/ivi-layout-private.h b/ivi-shell/ivi-layout-private.h
index 342ef4a..0d126dd 100644
--- a/ivi-shell/ivi-layout-private.h
+++ b/ivi-shell/ivi-layout-private.h
@@ -191,8 +191,6 @@ ivi_layout_surface_set_destination_rectangle(struct
ivi_layout_surface *ivisurf,
int32_t
ivi_layout_surface_set_opacity(struct ivi_layout_surface *ivisurf,
wl_fixed_t opacity);
-wl_fixed_t
-ivi_layout_surface_get_opacity(struct ivi_layout_surface *ivisurf);
int32_t
ivi_layout_surface_set_visibility(struct ivi_layout_surface *ivisurf,
bool newVisibility);
diff --git a/ivi-shell/ivi-layout-transition.c
b/ivi-shell/ivi-layout-transition.c
index fd8ce85..3c994a4 100644
--- a/ivi-shell/ivi-layout-transition.c
+++ b/ivi-shell/ivi-layout-transition.c
@@ -540,7 +540,7 @@ ivi_layout_transition_visibility_on(struct
ivi_layout_surface *surface,
{
struct ivi_layout_transition *transition;
bool is_visible = surface->prop.visibility;
- wl_fixed_t dest_alpha = ivi_layout_surface_get_opacity(surface);
+ wl_fixed_t dest_alpha = surface->prop.opacity;
struct store_alpha *user_data = NULL;
wl_fixed_t start_alpha = 0.0;
struct fade_view_data *data = NULL;
@@ -549,7 +549,7 @@ ivi_layout_transition_visibility_on(struct
ivi_layout_surface *surface,
IVI_LAYOUT_TRANSITION_VIEW_FADE,
surface);
if (transition) {
- start_alpha = ivi_layout_surface_get_opacity(surface);
+ start_alpha = surface->prop.opacity;
user_data = transition->user_data;
data = transition->private_data;
@@ -604,7 +604,7 @@ ivi_layout_transition_visibility_off(struct
ivi_layout_surface *surface,
uint32_t duration)
{
struct ivi_layout_transition *transition;
- wl_fixed_t start_alpha = ivi_layout_surface_get_opacity(surface);
+ wl_fixed_t start_alpha = surface->prop.opacity;
struct store_alpha* user_data = NULL;
struct fade_view_data* data = NULL;
diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c
index 4fb3518..364df99 100644
--- a/ivi-shell/ivi-layout.c
+++ b/ivi-shell/ivi-layout.c
@@ -2072,17 +2072,6 @@ ivi_layout_surface_set_opacity(struct ivi_layout_surface
*ivisurf,
return IVI_SUCCEEDED;
}
-wl_fixed_t
-ivi_layout_surface_get_opacity(struct ivi_layout_surface *ivisurf)
-{
- if (ivisurf == NULL) {
- weston_log("ivi_layout_surface_get_opacity: invalid
argument\n");
- return wl_fixed_from_double(0.0);
- }
-
- return ivisurf->prop.opacity;
-}
-
int32_t
ivi_layout_surface_set_destination_rectangle(struct ivi_layout_surface
*ivisurf,
int32_t x, int32_t y,
@@ -2726,7 +2715,6 @@ static struct ivi_layout_interface ivi_layout_interface =
{
.get_surfaces_on_layer =
ivi_layout_get_surfaces_on_layer,
.surface_set_visibility =
ivi_layout_surface_set_visibility,
.surface_set_opacity=
ivi_layout_surface_set_opacity,
- .surface_get_opacity=
ivi_layout_surface_get_opacity,
.surface_set_source_rectangle =
ivi_layout_surface_set_source_rectangle,
.surface_set_destination_rectangle =
ivi_layout_surface_set_destination_rectangle,
.surface_set_position =
ivi_layout_surface_set_position,
diff --git a/tests/ivi_layout-test-plugin.c b/tests/ivi_layout-test-plugin.c
index e7bcee9..db6155b 100644
--- a/tests/ivi_layout-test-plugin.c
+++ b/tests/ivi_layout-test-plugin.c
@@ -375,27 +375,21 @@ RUNNER_TEST(surface_opacity)
const struct ivi_layout_interface *lyt
[PATCH weston 05/14] ivi-shell: remove ivi_layout_surface_get_position API
Signed-off-by: Emre Ucan
---
ivi-shell/ivi-layout-export.h|9 -
ivi-shell/ivi-layout.c | 16
tests/ivi_layout-internal-test.c |7 ---
tests/ivi_layout-test-plugin.c | 27 ++-
4 files changed, 6 insertions(+), 53 deletions(-)
diff --git a/ivi-shell/ivi-layout-export.h b/ivi-shell/ivi-layout-export.h
index f58f5af..4dbac50 100644
--- a/ivi-shell/ivi-layout-export.h
+++ b/ivi-shell/ivi-layout-export.h
@@ -321,15 +321,6 @@ struct ivi_layout_interface {
int32_t dest_x, int32_t dest_y);
/**
-* \brief Get the horizontal and vertical position of the surface.
-*
-* \return IVI_SUCCEEDED if the method call was successful
-* \return IVI_FAILED if the method call was failed
-*/
- int32_t (*surface_get_position)(struct ivi_layout_surface *ivisurf,
- int32_t *dest_x, int32_t *dest_y);
-
- /**
* \brief Set the horizontal and vertical dimension of the surface.
*
* \return IVI_SUCCEEDED if the method call was successful
diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c
index 4475c94..81fea95 100644
--- a/ivi-shell/ivi-layout.c
+++ b/ivi-shell/ivi-layout.c
@@ -2156,21 +2156,6 @@ ivi_layout_surface_set_position(struct
ivi_layout_surface *ivisurf,
}
static int32_t
-ivi_layout_surface_get_position(struct ivi_layout_surface *ivisurf,
- int32_t *dest_x, int32_t *dest_y)
-{
- if (ivisurf == NULL || dest_x == NULL || dest_y == NULL) {
- weston_log("ivi_layout_surface_get_position: invalid
argument\n");
- return IVI_FAILED;
- }
-
- *dest_x = ivisurf->prop.dest_x;
- *dest_y = ivisurf->prop.dest_y;
-
- return IVI_SUCCEEDED;
-}
-
-static int32_t
ivi_layout_surface_set_orientation(struct ivi_layout_surface *ivisurf,
enum wl_output_transform orientation)
{
@@ -2707,7 +2692,6 @@ static struct ivi_layout_interface ivi_layout_interface =
{
.surface_set_source_rectangle =
ivi_layout_surface_set_source_rectangle,
.surface_set_destination_rectangle =
ivi_layout_surface_set_destination_rectangle,
.surface_set_position =
ivi_layout_surface_set_position,
- .surface_get_position =
ivi_layout_surface_get_position,
.surface_set_dimension =
ivi_layout_surface_set_dimension,
.surface_get_dimension =
ivi_layout_surface_get_dimension,
.surface_set_orientation=
ivi_layout_surface_set_orientation,
diff --git a/tests/ivi_layout-internal-test.c b/tests/ivi_layout-internal-test.c
index 4e1df28..c8693c7 100644
--- a/tests/ivi_layout-internal-test.c
+++ b/tests/ivi_layout-internal-test.c
@@ -113,17 +113,10 @@ static void
test_surface_bad_position(struct test_context *ctx)
{
const struct ivi_layout_interface *lyt = ctx->layout_interface;
- struct ivi_layout_surface *ivisurf = NULL;
- int32_t dest_x;
- int32_t dest_y;
iassert(lyt->surface_set_position(NULL, 20, 30) == IVI_FAILED);
lyt->commit_changes();
-
- iassert(lyt->surface_get_position(NULL, &dest_x, &dest_y) ==
IVI_FAILED);
- iassert(lyt->surface_get_position(ivisurf, NULL, &dest_y) ==
IVI_FAILED);
- iassert(lyt->surface_get_position(ivisurf, &dest_x, NULL) ==
IVI_FAILED);
}
static void
diff --git a/tests/ivi_layout-test-plugin.c b/tests/ivi_layout-test-plugin.c
index db6155b..72a5363 100644
--- a/tests/ivi_layout-test-plugin.c
+++ b/tests/ivi_layout-test-plugin.c
@@ -463,32 +463,23 @@ RUNNER_TEST(surface_position)
const struct ivi_layout_interface *lyt = ctx->layout_interface;
struct ivi_layout_surface *ivisurf;
const struct ivi_layout_surface_properties *prop;
- int32_t dest_x;
- int32_t dest_y;
ivisurf = lyt->get_surface_from_id(IVI_TEST_SURFACE_ID(0));
runner_assert(ivisurf != NULL);
- runner_assert(lyt->surface_get_position(
- ivisurf, &dest_x, &dest_y) == IVI_SUCCEEDED);
- runner_assert(dest_x == 0);
- runner_assert(dest_y == 0);
+ prop = lyt->get_properties_of_surface(ivisurf);
+ runner_assert_or_return(prop);
+ runner_assert(prop->dest_x == 20);
+ runner_assert(prop->dest_y == 30);
runner_assert(lyt->surface_set_position(
ivisurf, 20, 30) == IVI_SUCCEEDED);
- runner_assert(lyt->surface_get_position(
- ivisurf, &dest_x, &dest_y) == IVI_SUCCEEDED);
- runner_assert(dest_x == 0);
- runner_assert(dest_y == 0);
+ runner_assert(prop->dest_x == 0);
+ runner_assert(prop->dest_y == 0);
lyt->commit_changes();
- runner_assert(lyt->surface_get_position(
- iv
[PATCH weston 04/14] ivi-shell: remove ivi_layout_layer_get_opacity API
Signed-off-by: Emre Ucan
---
ivi-shell/ivi-layout-export.h |8
ivi-shell/ivi-layout-private.h|2 --
ivi-shell/ivi-layout-transition.c |2 +-
ivi-shell/ivi-layout.c| 12
tests/ivi_layout-internal-test.c | 16
5 files changed, 9 insertions(+), 31 deletions(-)
diff --git a/ivi-shell/ivi-layout-export.h b/ivi-shell/ivi-layout-export.h
index c6dc09b..f58f5af 100644
--- a/ivi-shell/ivi-layout-export.h
+++ b/ivi-shell/ivi-layout-export.h
@@ -530,14 +530,6 @@ struct ivi_layout_interface {
wl_fixed_t opacity);
/**
-* \brief Get the opacity of a ivi_layer.
-*
-* \return opacity if the method call was successful
-* \return wl_fixed_from_double(0.0) if the method call was failed
-*/
- wl_fixed_t (*layer_get_opacity)(struct ivi_layout_layer *ivilayer);
-
- /**
* \brief Set the area of a ivi_layer which should be used for the
rendering.
*
* Only this part will be visible.
diff --git a/ivi-shell/ivi-layout-private.h b/ivi-shell/ivi-layout-private.h
index 0d126dd..9f0ca79 100644
--- a/ivi-shell/ivi-layout-private.h
+++ b/ivi-shell/ivi-layout-private.h
@@ -199,8 +199,6 @@ ivi_layout_get_surface_from_id(uint32_t id_surface);
int32_t
ivi_layout_layer_set_opacity(struct ivi_layout_layer *ivilayer,
wl_fixed_t opacity);
-wl_fixed_t
-ivi_layout_layer_get_opacity(struct ivi_layout_layer *ivilayer);
int32_t
ivi_layout_layer_set_visibility(struct ivi_layout_layer *ivilayer,
bool newVisibility);
diff --git a/ivi-shell/ivi-layout-transition.c
b/ivi-shell/ivi-layout-transition.c
index 3c994a4..529bd56 100644
--- a/ivi-shell/ivi-layout-transition.c
+++ b/ivi-shell/ivi-layout-transition.c
@@ -831,7 +831,7 @@ ivi_layout_transition_fade_layer(
data = transition->private_data;
/* FIXME */
- fixed_opacity = ivi_layout_layer_get_opacity(layer);
+ fixed_opacity = layer->prop.opacity;
now_opacity = wl_fixed_to_double(fixed_opacity);
remain = 0.0;
diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c
index 364df99..4475c94 100644
--- a/ivi-shell/ivi-layout.c
+++ b/ivi-shell/ivi-layout.c
@@ -1810,17 +1810,6 @@ ivi_layout_layer_set_opacity(struct ivi_layout_layer
*ivilayer,
return IVI_SUCCEEDED;
}
-wl_fixed_t
-ivi_layout_layer_get_opacity(struct ivi_layout_layer *ivilayer)
-{
- if (ivilayer == NULL) {
- weston_log("ivi_layout_layer_get_opacity: invalid argument\n");
- return wl_fixed_from_double(0.0);
- }
-
- return ivilayer->prop.opacity;
-}
-
static int32_t
ivi_layout_layer_set_source_rectangle(struct ivi_layout_layer *ivilayer,
int32_t x, int32_t y,
@@ -2747,7 +2736,6 @@ static struct ivi_layout_interface ivi_layout_interface =
{
.get_layers_on_screen =
ivi_layout_get_layers_on_screen,
.layer_set_visibility =
ivi_layout_layer_set_visibility,
.layer_set_opacity = ivi_layout_layer_set_opacity,
- .layer_get_opacity = ivi_layout_layer_get_opacity,
.layer_set_source_rectangle =
ivi_layout_layer_set_source_rectangle,
.layer_set_destination_rectangle=
ivi_layout_layer_set_destination_rectangle,
.layer_set_position = ivi_layout_layer_set_position,
diff --git a/tests/ivi_layout-internal-test.c b/tests/ivi_layout-internal-test.c
index 1ba53b7..4e1df28 100644
--- a/tests/ivi_layout-internal-test.c
+++ b/tests/ivi_layout-internal-test.c
@@ -202,18 +202,16 @@ test_layer_opacity(struct test_context *ctx)
ivilayer = lyt->layer_create_with_dimension(IVI_TEST_LAYER_ID(0), 200,
300);
iassert(ivilayer != NULL);
- iassert(lyt->layer_get_opacity(ivilayer) == wl_fixed_from_double(1.0));
+ prop = lyt->get_properties_of_layer(ivilayer);
+ iassert(prop->opacity == wl_fixed_from_double(1.0));
iassert(lyt->layer_set_opacity(
ivilayer, wl_fixed_from_double(0.5)) == IVI_SUCCEEDED);
- iassert(lyt->layer_get_opacity(ivilayer) == wl_fixed_from_double(1.0));
+ iassert(prop->opacity == wl_fixed_from_double(1.0));
lyt->commit_changes();
- iassert(lyt->layer_get_opacity(ivilayer) == wl_fixed_from_double(0.5));
-
- prop = lyt->get_properties_of_layer(ivilayer);
iassert(prop->opacity == wl_fixed_from_double(0.5));
lyt->layer_destroy(ivilayer);
@@ -430,6 +428,7 @@ test_layer_bad_opacity(struct test_context *ctx)
{
const struct ivi_layout_interface *lyt = ctx->layout_interface;
struct ivi_layout_layer *ivilayer;
+ const struct ivi_layout_layer_properties *prop;
ivilayer = lyt->layer_create_with_d
[PATCH weston 00/14] IVI Layout API Cleanup
I removed the get APIs, because the same information can be get from ivi_layout_get_properties_of_surface/layer APIs. Therefore, these APIs are redundant. Furthermore, I removed *_set_position/dimension APIs, because position and dimension can be set by ivi_layout_surface/layer_set_destination_rectangle APIs. I adjusted ivi-layout-transition.c, ivi shell test code and hmi-controller.c for these changes. Emre Ucan (14): ivi-shell: remove ivi_layout_surface_get_visibility API ivi-shell: remove ivi_layout_layer_get_visibility API ivi-shell: remove ivi_layout_surface_get_opacity API ivi-shell: remove ivi_layout_layer_get_opacity API ivi-shell: remove ivi_layout_surface_get_position API ivi-shell: remove ivi_layout_layer_get_position API ivi-shell: remove ivi_layout_surface_get_dimension API ivi-shell: remove ivi_layout_layer_get_dimension API ivi-shell: remove ivi_layout_surface_get_orientation API ivi-shell: remove ivi_layout_layer_get_orientation API ivi-shell: remove ivi_layout_surface_set_position API ivi-shell: remove ivi_layout_layer_set_position API ivi-shell: remove ivi_layout_surface_set_dimension API ivi-shell: remove ivi_layout_layer_set_dimension API ivi-shell/hmi-controller.c| 17 ++- ivi-shell/ivi-layout-export.h | 127 ivi-shell/ivi-layout-private.h| 17 +-- ivi-shell/ivi-layout-transition.c | 19 ++- ivi-shell/ivi-layout.c| 237 + ivi-shell/ivi-shell.c |7 +- tests/ivi_layout-internal-test.c | 220 +- tests/ivi_layout-test-plugin.c| 126 +--- 8 files changed, 85 insertions(+), 685 deletions(-) -- 1.7.9.5 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 02/14] ivi-shell: remove ivi_layout_layer_get_visibility API
Signed-off-by: Emre Ucan
---
ivi-shell/ivi-layout-export.h|9 -
ivi-shell/ivi-layout.c | 12
tests/ivi_layout-internal-test.c | 11 ---
3 files changed, 4 insertions(+), 28 deletions(-)
diff --git a/ivi-shell/ivi-layout-export.h b/ivi-shell/ivi-layout-export.h
index e36d2fc..fc0e050 100644
--- a/ivi-shell/ivi-layout-export.h
+++ b/ivi-shell/ivi-layout-export.h
@@ -529,15 +529,6 @@ struct ivi_layout_interface {
bool newVisibility);
/**
-* \brief Get the visibility of a layer. If a layer is not visible,
-* the layer and its surfaces will not be rendered.
-*
-* \return true if layer is visible
-* \return false if layer is invisible or the method call was failed
-*/
- bool (*layer_get_visibility)(struct ivi_layout_layer *ivilayer);
-
- /**
* \brief Set the opacity of a ivi_layer.
*
* \return IVI_SUCCEEDED if the method call was successful
diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c
index f15ee32..4fb3518 100644
--- a/ivi-shell/ivi-layout.c
+++ b/ivi-shell/ivi-layout.c
@@ -1786,17 +1786,6 @@ ivi_layout_layer_set_visibility(struct ivi_layout_layer
*ivilayer,
return IVI_SUCCEEDED;
}
-static bool
-ivi_layout_layer_get_visibility(struct ivi_layout_layer *ivilayer)
-{
- if (ivilayer == NULL) {
- weston_log("ivi_layout_layer_get_visibility: invalid
argument\n");
- return false;
- }
-
- return ivilayer->prop.visibility;
-}
-
int32_t
ivi_layout_layer_set_opacity(struct ivi_layout_layer *ivilayer,
wl_fixed_t opacity)
@@ -2769,7 +2758,6 @@ static struct ivi_layout_interface ivi_layout_interface =
{
.get_layers_under_surface =
ivi_layout_get_layers_under_surface,
.get_layers_on_screen =
ivi_layout_get_layers_on_screen,
.layer_set_visibility =
ivi_layout_layer_set_visibility,
- .layer_get_visibility =
ivi_layout_layer_get_visibility,
.layer_set_opacity = ivi_layout_layer_set_opacity,
.layer_get_opacity = ivi_layout_layer_get_opacity,
.layer_set_source_rectangle =
ivi_layout_layer_set_source_rectangle,
diff --git a/tests/ivi_layout-internal-test.c b/tests/ivi_layout-internal-test.c
index ec4c8d3..1ba53b7 100644
--- a/tests/ivi_layout-internal-test.c
+++ b/tests/ivi_layout-internal-test.c
@@ -177,17 +177,16 @@ test_layer_visibility(struct test_context *ctx)
ivilayer = lyt->layer_create_with_dimension(IVI_TEST_LAYER_ID(0), 200,
300);
iassert(ivilayer != NULL);
- iassert(lyt->layer_get_visibility(ivilayer) == false);
+ prop = lyt->get_properties_of_layer(ivilayer);
+
+ iassert(prop->visibility == false);
iassert(lyt->layer_set_visibility(ivilayer, true) == IVI_SUCCEEDED);
- iassert(lyt->layer_get_visibility(ivilayer) == false);
+ iassert(prop->visibility == false);
lyt->commit_changes();
- iassert(lyt->layer_get_visibility(ivilayer) == true);
-
- prop = lyt->get_properties_of_layer(ivilayer);
iassert(prop->visibility == true);
lyt->layer_destroy(ivilayer);
@@ -424,8 +423,6 @@ test_layer_bad_visibility(struct test_context *ctx)
iassert(lyt->layer_set_visibility(NULL, true) == IVI_FAILED);
lyt->commit_changes();
-
- iassert(lyt->layer_get_visibility(NULL) == false);
}
static void
--
1.7.9.5
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston 01/14] ivi-shell: remove ivi_layout_surface_get_visibility API
Signed-off-by: Emre Ucan
---
ivi-shell/ivi-layout-export.h | 10 --
ivi-shell/ivi-layout-private.h|2 --
ivi-shell/ivi-layout-transition.c |2 +-
ivi-shell/ivi-layout.c| 12
tests/ivi_layout-internal-test.c |4
tests/ivi_layout-test-plugin.c|4
6 files changed, 1 insertion(+), 33 deletions(-)
diff --git a/ivi-shell/ivi-layout-export.h b/ivi-shell/ivi-layout-export.h
index 54af286..e36d2fc 100644
--- a/ivi-shell/ivi-layout-export.h
+++ b/ivi-shell/ivi-layout-export.h
@@ -280,16 +280,6 @@ struct ivi_layout_interface {
bool newVisibility);
/**
-* \brief Get the visibility of a surface.
-*
-* If a surface is not visible it will not be rendered.
-*
-* \return true if surface is visible
-* \return false if surface is invisible or the method call was failed
-*/
- bool (*surface_get_visibility)(struct ivi_layout_surface *ivisurf);
-
- /**
* \brief Set the opacity of a surface.
*
* \return IVI_SUCCEEDED if the method call was successful
diff --git a/ivi-shell/ivi-layout-private.h b/ivi-shell/ivi-layout-private.h
index 3244390..342ef4a 100644
--- a/ivi-shell/ivi-layout-private.h
+++ b/ivi-shell/ivi-layout-private.h
@@ -196,8 +196,6 @@ ivi_layout_surface_get_opacity(struct ivi_layout_surface
*ivisurf);
int32_t
ivi_layout_surface_set_visibility(struct ivi_layout_surface *ivisurf,
bool newVisibility);
-bool
-ivi_layout_surface_get_visibility(struct ivi_layout_surface *ivisurf);
struct ivi_layout_surface *
ivi_layout_get_surface_from_id(uint32_t id_surface);
int32_t
diff --git a/ivi-shell/ivi-layout-transition.c
b/ivi-shell/ivi-layout-transition.c
index 483437d..fd8ce85 100644
--- a/ivi-shell/ivi-layout-transition.c
+++ b/ivi-shell/ivi-layout-transition.c
@@ -539,7 +539,7 @@ ivi_layout_transition_visibility_on(struct
ivi_layout_surface *surface,
uint32_t duration)
{
struct ivi_layout_transition *transition;
- bool is_visible = ivi_layout_surface_get_visibility(surface);
+ bool is_visible = surface->prop.visibility;
wl_fixed_t dest_alpha = ivi_layout_surface_get_opacity(surface);
struct store_alpha *user_data = NULL;
wl_fixed_t start_alpha = 0.0;
diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c
index f7c4f27..f15ee32 100644
--- a/ivi-shell/ivi-layout.c
+++ b/ivi-shell/ivi-layout.c
@@ -2059,17 +2059,6 @@ ivi_layout_surface_set_visibility(struct
ivi_layout_surface *ivisurf,
return IVI_SUCCEEDED;
}
-bool
-ivi_layout_surface_get_visibility(struct ivi_layout_surface *ivisurf)
-{
- if (ivisurf == NULL) {
- weston_log("ivi_layout_surface_get_visibility: invalid
argument\n");
- return false;
- }
-
- return ivisurf->prop.visibility;
-}
-
int32_t
ivi_layout_surface_set_opacity(struct ivi_layout_surface *ivisurf,
wl_fixed_t opacity)
@@ -2747,7 +2736,6 @@ static struct ivi_layout_interface ivi_layout_interface =
{
.get_properties_of_surface =
ivi_layout_get_properties_of_surface,
.get_surfaces_on_layer =
ivi_layout_get_surfaces_on_layer,
.surface_set_visibility =
ivi_layout_surface_set_visibility,
- .surface_get_visibility =
ivi_layout_surface_get_visibility,
.surface_set_opacity=
ivi_layout_surface_set_opacity,
.surface_get_opacity=
ivi_layout_surface_get_opacity,
.surface_set_source_rectangle =
ivi_layout_surface_set_source_rectangle,
diff --git a/tests/ivi_layout-internal-test.c b/tests/ivi_layout-internal-test.c
index d0c759b..ec4c8d3 100644
--- a/tests/ivi_layout-internal-test.c
+++ b/tests/ivi_layout-internal-test.c
@@ -68,14 +68,10 @@ static void
test_surface_bad_visibility(struct test_context *ctx)
{
const struct ivi_layout_interface *lyt = ctx->layout_interface;
- bool visibility;
iassert(lyt->surface_set_visibility(NULL, true) == IVI_FAILED);
lyt->commit_changes();
-
- visibility = lyt->surface_get_visibility(NULL);
- iassert(visibility == false);
}
static void
diff --git a/tests/ivi_layout-test-plugin.c b/tests/ivi_layout-test-plugin.c
index f7b6c3c..e7bcee9 100644
--- a/tests/ivi_layout-test-plugin.c
+++ b/tests/ivi_layout-test-plugin.c
@@ -356,7 +356,6 @@ RUNNER_TEST(surface_visibility)
const struct ivi_layout_interface *lyt = ctx->layout_interface;
struct ivi_layout_surface *ivisurf;
int32_t ret;
- bool visibility;
const struct ivi_layout_surface_properties *prop;
ivisurf = lyt->get_surface_from_id(IVI_TEST_SURFACE_ID(0));
@@ -367,9 +366,6 @@ RUNNER_TEST(surface_visibility)
lyt->commit_changes();
Re: [PATCH v3 wayland 3/3] doc: generate doxygen html output from the scanner
On Mon, 22 Feb 2016 08:57:47 +1000 Peter Hutterer wrote: > This switches the scanner to generate doxygen-compatible tags for the > generated protocol headers, and hooks up the doxygen build to generate server > and client-side API documentation. > > Each protocol is a separate doxygen @page, with each interface a @subpage. > Wayland only has one protocol, wayland-protocols will have these nested. > Each protocol page has a list of interfaces and the copyright and description > where available. > All interfaces are grouped by doxygen @defgroup and @ingroups and appear in > "Modules" in the generated output. Each interface subpage has the description > and a link to the actual API doc. > Function, struct and #defines are documented in doxygen style and associated > with the matching interface. > > Note that pages and groups have fixed HTML file names and are directly > linkable/bookmark-able. > > The @mainpage is a separate file that's included at build time. It doesn't > contain much other than links to where the interesting bits are. It's a static > file though that supports markdown, so we can extend it easily in the future. > > For doxygen we need the new options EXTRACT_ALL and OPTIMIZE_OUTPUT_FOR_C so > it scans C code properly. EXTRACT_STATIC is needed since most of the protocol > hooks are static. WARN_IF_DOC_ERROR is required to silence the warnings when > some parameters are documented but others aren't. > > Signed-off-by: Peter Hutterer Hi Peter, I started looking at the output of this, and it took a while to realize where all the output goes. Nothing much changes with the existing documentation we install, so I got puzzled. Then I looked at doc/doxygen/html/index.html and it didn't seem to have what you described. Then re-reading the Makefile.am and which directory it is in, I realized I need to look at doc/doxygen/html/Client/index.html and doc/doxygen/html/Server/index.html. These are the main outputs of this patch, right? I get the feeling the generated documentation is quite scattered and not easy to find, but you probably intend to fix that in future? I have quite forgot the earlier discussions, sorry. Looking at e.g. file:///home/pq/git/wayland/doc/doxygen/html/Client/group__iface__wl__surface.html it seems all the function documentations are missing. Well, they were missing also before. I have opposed silencing the warnings about undocumented parameters before. Is there something here that makes documenting all parameters not possible, rather than just not having written docs in XML? When I compare old and new wayland-client-protocol.h, I see that in the beginning with the list of extern const struct wl_interface wl_display_interface; etc. the documentation seems to be twice in the new comment. Is that necessary? Thanks, pq > --- > Changes to v3: > - rebased onto cde251a124d > > doc/doxygen/Makefile.am| 27 +- > doc/doxygen/mainpage.dox | 13 +++ > doc/doxygen/wayland.doxygen.in | 5 +- > src/scanner.c | 214 > ++--- > 4 files changed, 180 insertions(+), 79 deletions(-) > create mode 100644 doc/doxygen/mainpage.dox > > diff --git a/doc/doxygen/Makefile.am b/doc/doxygen/Makefile.am > index a8bb95f..e80c401 100644 > --- a/doc/doxygen/Makefile.am > +++ b/doc/doxygen/Makefile.am > @@ -1,7 +1,11 @@ > > .SUFFIXES = .gv .png .map > > -noinst_DATA = xml/Client/index.xml xml/Server/index.xml > +noinst_DATA = \ > + xml/Client/index.xml \ > + xml/Server/index.xml \ > + html/Client/index.html \ > + html/Server/index.html > dist_noinst_DATA = wayland.doxygen.in > > scanned_src_files_shared = \ > @@ -27,6 +31,17 @@ scanned_src_files_man = > \ > $(top_srcdir)/src/wayland-client.h \ > $(top_srcdir)/src/wayland-client-core.h > > +extra_doxygen = \ > + mainpage.dox > + > +extra_doxygen_Server = \ > + $(top_builddir)/protocol/wayland-server-protocol.h \ > + $(extra_doxygen) > + > +extra_doxygen_Client = \ > + $(top_builddir)/protocol/wayland-client-protocol.h \ > + $(extra_doxygen) > + > diagramsdir := $(srcdir)/dot > diagramssrc := $(wildcard $(diagramsdir)/*.gv) > diagrams := $(patsubst $(diagramsdir)/%,xml/%,$(diagramssrc:.gv=.png)) > @@ -38,7 +53,7 @@ diagram_maps := $(patsubst > $(diagramsdir)/%,xml/%,$(diagramssrc:.gv=.map)) > dist_man3_MANS = $(shell test -d man && find man/man3 -name "wl_*.3" -printf > "man/man3/%P\n") > > # Listing various directories that might need to be created. > -alldirs := xml xml/Client xml/Server man/man3 > +alldirs := xml xml/Client xml/Server man/man3 html/Client html/Server > > $(diagrams): $(diagramssrc) > > @@ -51,6 +66,13 @@ xml/%/index.xml: $(top_srcdir)/src/scanner.c > $(scanned_src_files_%) wayland.doxy >echo "INPUT= $(scanned_src_files_$*)"; \ >) | $(DOXYGEN) - > > +ht
Re: [PATCH wayland-protocols] xdg-shell: Add startup notification
Hey :), On Fri, Feb 26, 2016 at 1:54 PM, Pekka Paalanen wrote: > On Mon, 15 Feb 2016 16:59:02 +0800 > Jonas Ådahl wrote: > >> On Thu, Feb 11, 2016 at 01:52:36PM +0100, Carlos Garnacho wrote: >> > The xdg_launcher interface is added for the launcher, it's used >> > to notify of the startup ID to be transmitted to the launchee, >> > plus notifications about the startup success/failure. >> > >> > On the launchee side, we now have xdg_shell.set_startup_id, >> > which will notify the compositor of startup finalization. >> > >> > This has been made to be compatible with the XDG Startup >> > Notification spec available for X11, the startup ID is >> > transmitted from the launcher to the launchee in the same >> > ways, so we can launch x11 from wayland applications and >> > viceversa. The notable difference is that wayland launchers >> > receive startup IDs that are guaranteed to be unique, whereas >> > in X11 this is a best effort of the launcher client. >> > >> > Some notes have also been added about focus stealing prevention, >> > although that's mostly up for compositors to implement. >> > >> > Signed-off-by: Carlos Garnacho >> > --- >> > >> > I've got no full implementations yet, so this is mostly an RFC at the >> > moment. I mainly wonder, should we add a "serial" argument to the >> > create_launcher request? that'd at least ensure the launcher application >> > has some sort of focus, although nothing prevents an application from >> > being a fork bomb otherwise. >> >> Hey, >> >> I assume the compositor would have to limit to one startup per event >> or something like that if you add the serial? Doesn't seem to prevent >> fork bombs anyhow, the client can still fork as much as it wants. What >> it does limit is, say, opening an application as a response to not-user >> interaction. I don't have any reasonable use cases except remote >> controlled clients not being able to do this properly. Yeah, and the extent is kind of limited, the compositor should time out requests at some point anyway for legit reasons from its perspective (eg. the launched app crashing, a bogus file in execvpe(), ...). One might argue that this is a way to trigger extra activity in a compositor though... >> >> Overall, I'd like to see this being added as a separate extension. The >> reason is that I don't think this belongs in a "core" xdg shell >> interface, which we should try to keep as minimal as reasonable. It >> could for example be a "xdg_startup_notification" global (well, >> zxdg_startup_notification_v1 until later), which contains the requests >> you added to xdg_shell. >> >> > >> > unstable/xdg-shell/xdg-shell-unstable-v5.xml | 71 >> > +++- >> > 1 file changed, 70 insertions(+), 1 deletion(-) >> > >> > diff --git a/unstable/xdg-shell/xdg-shell-unstable-v5.xml >> > b/unstable/xdg-shell/xdg-shell-unstable-v5.xml >> > index 542491f..1c4ef54 100644 >> > --- a/unstable/xdg-shell/xdg-shell-unstable-v5.xml >> > +++ b/unstable/xdg-shell/xdg-shell-unstable-v5.xml >> > @@ -27,7 +27,7 @@ >> > DEALINGS IN THE SOFTWARE. >> > >> > >> > - >> > + >> > >> >xdg_shell allows clients to turn a wl_surface into a "real window" >> >which can be dragged, resized, stacked, and moved around by the >> > @@ -135,6 +135,35 @@ >> > >> > >> > >> > + >> > + >> > + >> > + >> > +Creates a new launcher context. >> > + >> > +The surface argument is the toplevel where the application >> > +was launched from, compositors may want to place the launched >> > +application relative to the launcher surface. >> > + >> > +Compositors that desire to implement focus stealing prevention >> > +can mark the time this request is received as the "startup" time. >> >> Not sure paragraph this belongs here. Compositors may do more things, >> and doesn't seem to be useful to list those things here. Maybe it would >> be good to add some high level blurb about how focus stealing prevention >> could be done in some generic place (for example in >> if it's its own extension protocol). >> >> > + >> > + >> > + >> > + >> > + >> > + >> > + >> > +Notifies the compositor of the startup ID of this launched >> > application. >> > +Applications will typically receive this through the >> > DESKTOP_STARTUP_ID >> > +environment variable as set by its launcher, and should unset the >> > +environment variable right after this request, in order to avoid >> > +propagating it to child processes. >> > + >> > +Compositors will ignore unknown startup IDs. >> > + >> > + >> > + >> >> How does this work when the application was already running? For example >> if the launcher opened gedit with a new file, but gedit was already, how >> is it communicated that the launched application was actually already >> launched? Does gedit need to communicate internally an
Re: [PATCH wayland-protocols] xdg-shell: Add startup notification
Hey :), On Mon, Feb 15, 2016 at 9:59 AM, Jonas Ådahl wrote: > On Thu, Feb 11, 2016 at 01:52:36PM +0100, Carlos Garnacho wrote: >> The xdg_launcher interface is added for the launcher, it's used >> to notify of the startup ID to be transmitted to the launchee, >> plus notifications about the startup success/failure. >> >> On the launchee side, we now have xdg_shell.set_startup_id, >> which will notify the compositor of startup finalization. >> >> This has been made to be compatible with the XDG Startup >> Notification spec available for X11, the startup ID is >> transmitted from the launcher to the launchee in the same >> ways, so we can launch x11 from wayland applications and >> viceversa. The notable difference is that wayland launchers >> receive startup IDs that are guaranteed to be unique, whereas >> in X11 this is a best effort of the launcher client. >> >> Some notes have also been added about focus stealing prevention, >> although that's mostly up for compositors to implement. >> >> Signed-off-by: Carlos Garnacho >> --- >> >> I've got no full implementations yet, so this is mostly an RFC at the >> moment. I mainly wonder, should we add a "serial" argument to the >> create_launcher request? that'd at least ensure the launcher application >> has some sort of focus, although nothing prevents an application from >> being a fork bomb otherwise. > > Hey, > > I assume the compositor would have to limit to one startup per event > or something like that if you add the serial? Doesn't seem to prevent > fork bombs anyhow, the client can still fork as much as it wants. What > it does limit is, say, opening an application as a response to not-user > interaction. I don't have any reasonable use cases except remote > controlled clients not being able to do this properly. Right, I guessed that'd prevent processes from being "fake startup" bombs, but that seems somewhat light compared to the real thing :). > > Overall, I'd like to see this being added as a separate extension. The > reason is that I don't think this belongs in a "core" xdg shell > interface, which we should try to keep as minimal as reasonable. It > could for example be a "xdg_startup_notification" global (well, > zxdg_startup_notification_v1 until later), which contains the requests > you added to xdg_shell. Yeah, probably makes sense to have this as a separate protocol. I guess it's safe to keep using xdg_surface in arguments, and rely that this protocol shall be used together with xdg-shell? > >> >> unstable/xdg-shell/xdg-shell-unstable-v5.xml | 71 >> +++- >> 1 file changed, 70 insertions(+), 1 deletion(-) >> >> diff --git a/unstable/xdg-shell/xdg-shell-unstable-v5.xml >> b/unstable/xdg-shell/xdg-shell-unstable-v5.xml >> index 542491f..1c4ef54 100644 >> --- a/unstable/xdg-shell/xdg-shell-unstable-v5.xml >> +++ b/unstable/xdg-shell/xdg-shell-unstable-v5.xml >> @@ -27,7 +27,7 @@ >> DEALINGS IN THE SOFTWARE. >> >> >> - >> + >> >>xdg_shell allows clients to turn a wl_surface into a "real window" >>which can be dragged, resized, stacked, and moved around by the >> @@ -135,6 +135,35 @@ >> >> >> >> + >> + >> + >> + >> +Creates a new launcher context. >> + >> +The surface argument is the toplevel where the application >> +was launched from, compositors may want to place the launched >> +application relative to the launcher surface. >> + >> +Compositors that desire to implement focus stealing prevention >> +can mark the time this request is received as the "startup" time. > > Not sure paragraph this belongs here. Compositors may do more things, > and doesn't seem to be useful to list those things here. Maybe it would > be good to add some high level blurb about how focus stealing prevention > could be done in some generic place (for example in > if it's its own extension protocol). Right. > >> + >> + >> + >> + >> + >> + >> + >> +Notifies the compositor of the startup ID of this launched >> application. >> +Applications will typically receive this through the >> DESKTOP_STARTUP_ID >> +environment variable as set by its launcher, and should unset the >> +environment variable right after this request, in order to avoid >> +propagating it to child processes. >> + >> +Compositors will ignore unknown startup IDs. >> + >> + >> + > > How does this work when the application was already running? For example > if the launcher opened gedit with a new file, but gedit was already, how > is it communicated that the launched application was actually already > launched? Does gedit need to communicate internally and call this > request again? Or does the "hey gedit, wake-up!" process need to make an > additional Wayland connection and call this? This is all implementation dependent. IIRC what happens in the gtk+ world is
Re: [PATCH wayland] server: add listener API for new clients
On Fri, 19 Feb 2016 19:03:29 -0800
Bryce Harrington wrote:
> On Thu, Feb 11, 2016 at 09:20:00PM +0900, nic...@nicesj.com wrote:
> > Using display object, Emit a signal if a new client is created.
> >
> > In the server-side, we can get the destroy event of a client,
> > But there is no way to get the created event of it.
> > Of course, we can get the client object from the global registry
> > binding callbacks.
> > But it can be called several times with same client object.
> > And even if A client creates display object,
> > (so there is a connection), The server could not know that.
> > There could be more use-cases not only for this.
> >
> > Signed-off-by: Sung-jae Park
>
> Hi Sung-jae,
>
> I'm a bit confused here; this patch appears to simply rename an existing
> API call and move its docs from the .h to the .c. Is there more code
> that's missing, or am I misunderstanding the purpose of this patch?
Hi,
perhaps Sung-jae sent an incremental patch, rather than re-sending a
modified patch.
Sung-jae, could you send the patch anew? When we revise patches, we
send a new version of the original patch, not another patch on top of
the original patch.
Thanks,
pq
> > diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
> > index cb72981..1bc4d6b 100644
> > --- a/src/wayland-server-core.h
> > +++ b/src/wayland-server-core.h
> > @@ -156,19 +156,8 @@ void
> > wl_display_add_destroy_listener(struct wl_display *display,
> > struct wl_listener *listener);
> >
> > -/** Add a listener for getting a notification of creation of clients.
> > - * If you add a listener, server will emits a signal if a new client
> > - * is created.
> > - *
> > - * \ref wl_client_create
> > - * \ref wl_display
> > - * \ref wl_listener
> > - *
> > - * \param display The display object
> > - * \param listener Signal handler object
> > - */
> > void
> > -wl_display_add_create_client_listener(struct wl_display *display,
> > +wl_display_add_client_created_listener(struct wl_display *display,
> > struct wl_listener *listener);
> >
> > struct wl_listener *
> > diff --git a/src/wayland-server.c b/src/wayland-server.c
> > index 0eff8f6..2857b1d 100644
> > --- a/src/wayland-server.c
> > +++ b/src/wayland-server.c
> > @@ -1357,8 +1357,19 @@ wl_display_add_destroy_listener(struct wl_display
> > *display,
> > wl_signal_add(&display->destroy_signal, listener);
> > }
> >
> > +/** Registers a listener for the client connection signal.
> > + * When a new client object is created, \a listener will be notified,
> > carring
> > + * a pointer to the new wl_client object.
> > + *
> > + * \ref wl_client_create
> > + * \ref wl_display
> > + * \ref wl_listener
> > + *
> > + * \param display The display object
> > + * \param listener Signal handler object
> > + */
> > WL_EXPORT void
> > -wl_display_add_create_client_listener(struct wl_display *display,
> > +wl_display_add_client_created_listener(struct wl_display *display,
> > struct wl_listener *listener)
> > {
> > wl_signal_add(&display->create_client_signal, listener);
pgp7mMZADcJ9Q.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] libinput-device: Remove unnecessary function call
On Mon, 22 Feb 2016 16:14:56 -0800 Bryce Harrington wrote: > On Mon, Feb 22, 2016 at 08:47:24AM -0500, Chris Michael wrote: > > When we handle keyboard key events, we already retrieve the key state > > at the top of this function, so there is no real need to call the same > > libinput function again as we can just reuse the 'key_state' variable > > that we have above. > > > > Signed-off-by: Chris Michael > > Reviewed-by: Bryce Harrington Pushed: c25f72d..7e7f793 master -> master Thanks, pq > > > --- > > src/libinput-device.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/src/libinput-device.c b/src/libinput-device.c > > index 78b0ac9..f8b614a 100644 > > --- a/src/libinput-device.c > > +++ b/src/libinput-device.c > > @@ -76,8 +76,7 @@ handle_keyboard_key(struct libinput_device > > *libinput_device, > > notify_key(device->seat, > >libinput_event_keyboard_get_time(keyboard_event), > >libinput_event_keyboard_get_key(keyboard_event), > > - libinput_event_keyboard_get_key_state(keyboard_event), > > - STATE_UPDATE_AUTOMATIC); > > + key_state, STATE_UPDATE_AUTOMATIC); > > } > > > > static bool > > -- > > 2.7.0 pgpIMblV1rhYg.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] server: Add get/set user data for wl_client
On Wed, 17 Feb 2016 18:47:44 +0900 Sung-Jin Park wrote: > Hi pq, > I'm sorry for replying late. > > The reasons why I tried to add are following. > - If a wl_client is able to has a user data, > a wayland server can use it for storing the client specific data such as > security privilege, the result of privilege check, and other things. > Those kinds of data are the client specific and need to be removed when > the client is gone. > In this case, it'll be better to use wl_client' user data as a data storage > other than storing those information in the server's data. > - There was a guy who wants to have tyis kind of APIs. (As I remember, he > is nicesj.) > - There was a reply for the above request and It seemed positive to me. > > If this can be merged into the stream, it'll be good for me and other guys > who wants to find this out. :) Hi, yes, you want to store data for each wl_client. Is it really so much better to add a single-use field in wl_client, when you can already add any number of user data records by using a wl_client destroy listener as the handle to the data? Do you need an example of how that works? Is your security thing an integral part of the compositor you are writing, or is it an add-on? If it is an add-on or its own sub-system, it probably should not use wl_client user data field, because the compositor core might want to use that for its own purposes. As I explained below, Sung-jae Park wants a notification when a new client is created, which is a good idea. How to store user data for wl_client is a separate issue. I'm still waiting for comments from other Wayland developers or compositor writers whether we really need two different ways to store user data for a wl_client. The way we already have is more versatile than the one you are proposing. Thanks, pq > 2016. 2. 11. 오후 7:08에 "Pekka Paalanen" 님이 작성: > > > Hi, > > > > I think we should think this a little more. > > > > There is no absolute requirement to add a user_data member to > > wl_client, because you can use a destroy listener to look up your user > > data struct. When you attach user data, you also want to always be > > notified about wl_client destruction, which means you always have a > > destroy listener. > > > > The downside of using a destroy listener to look up your user data > > struct is that it is an O(number of destroy listeners) operation rather > > than O(1), but usually that number should be 1 or 2 I think, your user > > data destructor included. > > > > So the first question is, is this really worthwhile? > > > > For comparison to another related patch, adding the client created > > signal is obviously worthwhile, since there is no other sure way to be > > notified about new wl_clients. > > > > Then let's assume user_data for wl_client is worthwhile. The closest > > example of a server-side structure with user data is wl_resource. > > wl_resource also has an explicit wl_resource_destroy_func_t member, and > > the rationale is clear: if you add user data, you want to be able to > > tear down the user data at the right time, so you always need a way to > > be notified about destruction. > > > > The second question is, should also wl_client have an explicit user > > data destructor function? If yes, you have to add API for it. > > > > The third question is: do all wl_client objects have the same single > > owner? > > > > The owner problem is particularly visible in the client side with > > wl_proxy, which also has a single user data member of type void > > pointer. This creates two problems: > > > > - If wl_proxy should have more than one user data attached, only one of > > them can use the user data member and the others must use the > > destructor trick anyway - oops, wl_proxy does not have a destroy > > signal, but at least wl_client does. > > > > - It is not always obvious who owns the user data. If there are more > > than one component that assume they own the wl_proxy and its user > > data, then passing the wl_proxy to another component that also > > assumes the same causes the other component to access not the data it > > thinks it gets. > > > > The latter problem is especially dangerous when an application is > > sharing the same connection with multiple toolkits. Both toolkits may > > create wl_surface objects and naturally put their own pointer in the > > wl_proxy user data field. They also listen for input events, and input > > events carry a reference to a wl_surface. Therefore, a toolkit may > > receive a wl_proxy with the other toolkit's user data, and crash or > > misbehave due to assuming it is its own. > > > > In the wl_client case, I think the owner issue should be negligible, > > there is always one obvious owner for the wl_client and its user data - > > or that is what I assume, as I haven't read any other compositor code > > than Weston's. > > > > In summary, while there is technically nothing wrong with this idea per > > se, I do wonder if it is a net win. >
Re: [PATCH wayland-protocols] xdg-shell: Add startup notification
On Mon, 15 Feb 2016 16:59:02 +0800 Jonas Ådahl wrote: > On Thu, Feb 11, 2016 at 01:52:36PM +0100, Carlos Garnacho wrote: > > The xdg_launcher interface is added for the launcher, it's used > > to notify of the startup ID to be transmitted to the launchee, > > plus notifications about the startup success/failure. > > > > On the launchee side, we now have xdg_shell.set_startup_id, > > which will notify the compositor of startup finalization. > > > > This has been made to be compatible with the XDG Startup > > Notification spec available for X11, the startup ID is > > transmitted from the launcher to the launchee in the same > > ways, so we can launch x11 from wayland applications and > > viceversa. The notable difference is that wayland launchers > > receive startup IDs that are guaranteed to be unique, whereas > > in X11 this is a best effort of the launcher client. > > > > Some notes have also been added about focus stealing prevention, > > although that's mostly up for compositors to implement. > > > > Signed-off-by: Carlos Garnacho > > --- > > > > I've got no full implementations yet, so this is mostly an RFC at the > > moment. I mainly wonder, should we add a "serial" argument to the > > create_launcher request? that'd at least ensure the launcher application > > has some sort of focus, although nothing prevents an application from > > being a fork bomb otherwise. > > Hey, > > I assume the compositor would have to limit to one startup per event > or something like that if you add the serial? Doesn't seem to prevent > fork bombs anyhow, the client can still fork as much as it wants. What > it does limit is, say, opening an application as a response to not-user > interaction. I don't have any reasonable use cases except remote > controlled clients not being able to do this properly. > > Overall, I'd like to see this being added as a separate extension. The > reason is that I don't think this belongs in a "core" xdg shell > interface, which we should try to keep as minimal as reasonable. It > could for example be a "xdg_startup_notification" global (well, > zxdg_startup_notification_v1 until later), which contains the requests > you added to xdg_shell. > > > > > unstable/xdg-shell/xdg-shell-unstable-v5.xml | 71 > > +++- > > 1 file changed, 70 insertions(+), 1 deletion(-) > > > > diff --git a/unstable/xdg-shell/xdg-shell-unstable-v5.xml > > b/unstable/xdg-shell/xdg-shell-unstable-v5.xml > > index 542491f..1c4ef54 100644 > > --- a/unstable/xdg-shell/xdg-shell-unstable-v5.xml > > +++ b/unstable/xdg-shell/xdg-shell-unstable-v5.xml > > @@ -27,7 +27,7 @@ > > DEALINGS IN THE SOFTWARE. > > > > > > - > > + > > > >xdg_shell allows clients to turn a wl_surface into a "real window" > >which can be dragged, resized, stacked, and moved around by the > > @@ -135,6 +135,35 @@ > > > > > > > > + > > + > > + > > + > > +Creates a new launcher context. > > + > > +The surface argument is the toplevel where the application > > +was launched from, compositors may want to place the launched > > +application relative to the launcher surface. > > + > > +Compositors that desire to implement focus stealing prevention > > +can mark the time this request is received as the "startup" time. > > Not sure paragraph this belongs here. Compositors may do more things, > and doesn't seem to be useful to list those things here. Maybe it would > be good to add some high level blurb about how focus stealing prevention > could be done in some generic place (for example in > if it's its own extension protocol). > > > + > > + > > + > > + > > + > > + > > + > > +Notifies the compositor of the startup ID of this launched > > application. > > +Applications will typically receive this through the > > DESKTOP_STARTUP_ID > > +environment variable as set by its launcher, and should unset the > > +environment variable right after this request, in order to avoid > > +propagating it to child processes. > > + > > +Compositors will ignore unknown startup IDs. > > + > > + > > + > > How does this work when the application was already running? For example > if the launcher opened gedit with a new file, but gedit was already, how > is it communicated that the launched application was actually already > launched? Does gedit need to communicate internally and call this > request again? Or does the "hey gedit, wake-up!" process need to make an > additional Wayland connection and call this? Hi, should the startup-id be used per-window instead? Wouldn't a user usually be expecting a new (or an old) window to pop up? Or should the launched application be potentially able to raise any number of independent windows from a single launch? > > > > > > > > @@ -622,4 +651,44 @@ > > > > > >
Re: [RFC wayland] scanner: Add support for dynamic symbol loading
On Thu, 18 Feb 2016 18:51:33 +0800 Jonas Ådahl wrote: > In certain configurations the wl_interface symbols up various > interfaces from wayland core (such as wl_surface_interface, > wl_pointer_interface etc) are not available at build time, for example > when the application will dlopen() the required symbols at run time. > > To make it possible to build files generated by wayland-scanner this > way, it needs to be possible to generate files that don't reference any > external symbols, while enabling the application to provide these > symbols at a later state. > > To provide this, two new commands are added to wayland-scanner: > 'dynamic-code' and 'dynamic-header'. > > The 'dynamic-code' command generates a C source code file similar to the > one generated by the code command, but instead of filling out the > 'types' wl_interface array with external symbols, it fills it with only > NULL pointers, while adding a function for providing all the necessary > wl_interface pointers. The application must call this function before > using any Wayland IPC functionality. > > The 'dynamic-header' command generates a C header file with the > declaration of the function the application must call to initialize the > wl_interface types array. > > Signed-off-by: Jonas Ådahl > --- > > Hi, > > While adding support for using wayland-protocol extensions in libSDL I came > across this issue with the generated -protocol.c file. The issue is that SDL > will not link to libwayland-client on build time but instead on-demand dlopen > all of the required symbols at run time. This makes all -protocol.c files > generated by wayland-scanner fail to build since they reference wl_interface > symbols for various core protocol interfaces. > > To make it possible to support building protocol extension files and then load > them at run time I had to add this feature to wayland-scanner to make it work. > > I imagine another potential solution would be to split up libwayland-client.so > and let libSDL run wayland-scanner on the installed wayland.xml and keep its > own copies of all the wl_interface symbols, but it at a glance adding this > feature to wayland-scanner seems like the better option. > > Let me know what you all think. Hi Jonas, TL;DR, it's a bit of an incoherent rant; I think we should make a dlopenable API a whole new set of Wayland C language bindings, and your RFC isn't going far enough. I think the previous times this came up, we have shrugged it off with "you should create your own library you dlopen() instead of libwayland-client". It's easy to say without knowing anything of the target software, and does have some perks like not always loading code you might not use and reducing the API you have to dlsym(), but I suppose it's also somewhat arrogant. Doesn't libSDL already have an abstraction towards window systems? How difficult would it be to write a SDL-internal library to be dlopen()'d instead of libwayland-client? Are there other reasons SDL does not use internal .so to dlopen()? Perhaps the desire to support several different major versions on a platform? It's not like you are switching between API implementations here like with GL, but switching a whole backend and backends have nothing in common in the system library APIs they dlopen(), so the whole dlsym()'ing is different in each backend. On a completely different matter, we don't have any wayland-scanner tests, which makes me uneasy to land any changes to wayland-scanner. OTOH, we have listed adding those tests as a part of potential GSoC tasks. > src/scanner.c | 268 > ++ > 1 file changed, 231 insertions(+), 37 deletions(-) I took a look at what this generates. I understand that to use this, you have to do the following during a build: 1. generate the .c file using "dynamic-code" instead of "code" 2. generate the usual headers using "client-header" 3. generate an extra header with "dynamic-header" Is that correct? This seems to take care of the data symbols partially, but I think there other things missing. The wl_interface data symbols that are defined in the generated .c file could be plugged in by the generator directly. The other interface symbols must be provided like you have done, so you are dlsym()'ing e.g. wl_surface_interface. Another problem is the generated headers, "client-header", which assumes it can statically call libwayland-client entry points, but you need to dlsym() them. You could probably hack your way around them [1], but it seems ugly without a generator mode to use function pointers instead to avoid name clashes. Even more so, as libwayland-client provides a pre-generated wayland-client-protocol.h, so you'd best avoid it and generate yourself. Is it really easier or somehow more desireable to do all that than write your own internal library to dlopen()? I also consider wayland.xml being exposed in libwayland-client to be the odd one out. Just like
Re: [PATCH] server: Fix shm_create_pool size fail path fd leak
On Mon, 22 Feb 2016 14:41:44 -0800
Bryce Harrington wrote:
> On Mon, Feb 22, 2016 at 01:23:00PM +0800, Jonas Ådahl wrote:
> > On Fri, Feb 19, 2016 at 11:13:25AM +0100, xerpi wrote:
> > > I was just reading the source when I found it (no valgrind involved). So
> > > as wl_connection_destroy() already takes care of that, my patch is
> > > pointless.
> >
> > I was wrong. Your patch is correct, and we do actually leak it. The fd
> > list that is closed automatically is just the fds that are collected
> > during reading but yet put in a wl_closure. I went a head and wrote a
> > test case for this, just to be sure; I'll send it as a reply to this
> > E-mail.
> >
> > So consider the patch Reviewed-by: Jonas Ådahl .
>
> Looks fine to me as well.
>
> Reviewed-by: Bryce Harrington
>
> I think this is fine to land as-is, but it'd be nice to land it along
> with the test case so will wait a bit for the compilation issue there to
> get squared away. Thanks for doing the test case btw!
Reviewed-by: Pekka Paalanen
Thanks,
pq
> > > My final idea would be to add support to fds that have the F_SEAL_SHRINK
> > > seal set (see memfd_create(2) and fcntl(2)), so if a client creates
> > > shm_pool with the shrink seal set, and the fd size (fstat(2)) is already
> > > >=
> > > size arg passed to wl_shm_create_pool, we no longer would need to set
> > > SIGBUS handlers when the compositor needs to access that shm memory.
> > > Does this make any sense/would be a good idea?
> >
> > Currently the server will close the fd immediately after mapping the
> > memory associated with it. How will F_SEAL_SHRINK protect from the
> > client unmapping the memory and closing the fd on its side, without the
> > server having its own fd un-closed?
> >
> >
> > Jonas
> >
> > >
> > > 2016-02-19 2:49 GMT+01:00 Jonas Ådahl :
> > >
> > > > On Thu, Feb 18, 2016 at 11:59:29PM +0100, Sergi Granell wrote:
> > > > > If the client passed a size <= 0 to shm_create_pool, it would
> > > > > go to err_free, which wouldn't close the fd, and thus leave it opened.
> > > > >
> > > > > We can also move the size check before the struct wl_shm_pool
> > > > > malloc, so in case the client passes a wrong size, it won't
> > > > > do an unnecessary malloc and then free.
> > > >
> > > > Did you detect this using valgrind or something like that? Because IIRC,
> > > > we collect all transmitted file descriptors and close them when the
> > > > client is destroyed (see wl_connection_destory()). Since we post an
> > > > error, the client will eventually be destroyed, so we shouldn't leak
> > > > the fd here already.
> > > >
> > > >
> > > > Jonas
> > > >
> > > > > ---
> > > > > src/wayland-shm.c | 20 ++--
> > > > > 1 file changed, 10 insertions(+), 10 deletions(-)
> > > > >
> > > > > diff --git a/src/wayland-shm.c b/src/wayland-shm.c
> > > > > index a4343a4..81bf657 100644
> > > > > --- a/src/wayland-shm.c
> > > > > +++ b/src/wayland-shm.c
> > > > > @@ -230,17 +230,17 @@ shm_create_pool(struct wl_client *client,
> > > > > struct
> > > > wl_resource *resource,
> > > > > {
> > > > > struct wl_shm_pool *pool;
> > > > >
> > > > > - pool = malloc(sizeof *pool);
> > > > > - if (pool == NULL) {
> > > > > - wl_client_post_no_memory(client);
> > > > > - goto err_close;
> > > > > - }
> > > > > -
> > > > > if (size <= 0) {
> > > > > wl_resource_post_error(resource,
> > > > > WL_SHM_ERROR_INVALID_STRIDE,
> > > > > "invalid size (%d)", size);
> > > > > - goto err_free;
> > > > > + goto err_close;
> > > > > + }
> > > > > +
> > > > > + pool = malloc(sizeof *pool);
> > > > > + if (pool == NULL) {
> > > > > + wl_client_post_no_memory(client);
> > > > > + goto err_close;
> > > > > }
> > > > >
> > > > > pool->refcount = 1;
> > > > > @@ -251,7 +251,7 @@ shm_create_pool(struct wl_client *client, struct
> > > > wl_resource *resource,
> > > > > wl_resource_post_error(resource,
> > > > > WL_SHM_ERROR_INVALID_FD,
> > > > > "failed mmap fd %d", fd);
> > > > > - goto err_close;
> > > > > + goto err_free;
> > > > > }
> > > > > close(fd);
> > > > >
> > > > > @@ -270,10 +270,10 @@ shm_create_pool(struct wl_client *client,
> > > > > struct
> > > > wl_resource *resource,
> > > > >
> > > > > return;
> > > > >
> > > > > -err_close:
> > > > > - close(fd);
> > > > > err_free:
> > > > > free(pool);
> > > > > +err_close:
> > > > > + close(fd);
> > > > > }
> > > > >
> > > > > static const struct wl_shm_interface shm_interface = {
> > > > > --
> > > > > 2.7.1
pgpQcDPCcxBPw.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-d
Re: [PATCH wayland] tests: Test creating a shm pool with 0 size
On Tue, 23 Feb 2016 08:25:23 +0800
Jonas Ådahl wrote:
> On Mon, Feb 22, 2016 at 02:35:42PM -0800, Bryce Harrington wrote:
> > On Mon, Feb 22, 2016 at 03:03:11PM +0100, Marek Chalupa wrote:
> > > Hi
> > >
> > > On 02/22/16 06:52, Jonas Ådahl wrote:
> > > >Add a test case that tests the servers behaviour when creating a pool
> > > >of size 0. The test suite will do the memory and fd leak check for us,
> > > >so the test case is only a triggerer.
> > > >
> > > >Signed-off-by: Jonas Ådahl
> > > >---
> > > > Makefile.am | 5 ++-
> > > > tests/shm-test.c | 108
> > > > +++
> > > > 2 files changed, 112 insertions(+), 1 deletion(-)
> > > > create mode 100644 tests/shm-test.c
> > > >+#include
> > > >+#include
> > > >+#include
> > > >+#include
> > > >+#include
> > > >+#include
> > > >+
> > > >+#include "wayland-client.h"
> > > >+#include "test-runner.h"
> > > >+#include "test-compositor.h"
> > > >+
> > > >+static void
> > > >+registry_handle_global(void *data, struct wl_registry *registry,
> > > >+ uint32_t id, const char *interface, uint32_t
> > > >version)
> > > >+{
> > > >+bool *tested = data;
> > > >+struct wl_shm *shm;
> > > >+int fd, ret;
> > > >+struct wl_shm_pool *pool;
> > > >+
> > > >+if (strcmp (interface, "wl_shm"))
> > > >+return;
> > > >+
> > > >+shm = wl_registry_bind(registry, id, &wl_shm_interface, 1);
> > > >+assert(shm);
> > > >+
> > > >+fd = syscall(__NR_memfd_create, "wayland-tests", MFD_CLOEXEC);
> > >
> > > memfd_create is rather new syscall, I think that this test won't
> > > compile on older systems. Shouldn't we guard it somehow? For
> > > example:
> > > https://github.com/systemd/kdbus/commit/392f91521592869d67d29a231211b93aa2069dc9
> > >
> >
> > Good point, on ubuntu 14.04 for example:
> >
> > CC tests/shm-test.o
> > tests/shm-test.c:27:25: fatal error: linux/memfd.h: No such file or
> > directory
> > #include
> > ^
> > compilation terminated.
> >
> >
> > I'd be okay with having this SKIP on older kernels; better to have the
> > test than not, and eventually distros will all catch up.
> >
> > But is there a different syscall or set of calls that would work on the
> > older kernels? Would be nice to get fuller coverage out of this test.
>
> The other way is to create anonymous files etc. I was lazy as this
> didn't need to copy that functio here (os_create_anonymous_file()) but
> if we want to support testing on older systems, I guess I'll just drop
> the memfd thing and go via tmpfs instead. I suspect BSD won't have memfd
> as well so it'd be more portable as well.
Yes, please, os_create_anonymous_file(). Memfd is a whole another story.
Thanks,
pq
pgpxN1plDCjVB.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland] client: Don't segfault when receiving error on destroyed object
On Fri, 26 Feb 2016 11:57:51 +0200
Pekka Paalanen wrote:
> On Mon, 22 Feb 2016 13:22:05 -0800
> Bryce Harrington wrote:
>
> > On Mon, Feb 22, 2016 at 02:34:57PM +0100, Marek Chalupa wrote:
> > > Hi,
> > >
> > > can confirm the segfault, tested it (will send the test I used for
> > > it as a follow-up). The only API change problem could be in
> > > returning
> > > NULL as the interface - if the user does not check for it, he/she
> > > dereferences NULL. But I don't think anybody (except us in tests) is
> > > using wl_display_get_protocol_error() for the error analysis, so IMO
> > > its OK break.
> > >
> > > Reviewed-by: Marek Chalupa
> >
> > I agree, given that any client in this particular situation would have
> > been crashing, I don't think anything would be dependent on interface
> > being non-NULL.
> >
> > Reviewed-by: Bryce Harrington
>
> And R-b me and pushed:
>cde251a..5646236 master -> master
>
> with a small change noted below...
>
> > > Cheers,
> > > Marek
> > >
> > > On 02/22/16 06:37, Jonas Ådahl wrote:
> > > >If an error is received on a destroyed object, we'd get NULL passed
> > > >to display_handle_error() instead of a pointer to a valid wl_proxy.
> > > >
> > > >The logging is changed to report [unknown interface] and [unknown id]
> > > >instead of the actual interface name and id.
> > > >
> > > >The wl_display_get_protocol_error() documentation is updated to handle
> > > >the situation. For when the proxy was NULL, the object id 0 and
> > > >interface NULL is written.
> > > >
> > > >Signed-off-by: Jonas Ådahl
> > > >---
> > > >
> > > >This is technically an API change, but I see no less breaking change.
> > > >Considering that clients would segfault before ever reaching here
> > > >without this
> > > >patch, maybe it's an Ok break.
> > > >
> > > >
> > > >Jonas
> > > >
> > > >
> > > > src/wayland-client.c | 32
> > > > 1 file changed, 24 insertions(+), 8 deletions(-)
> > > >
> > > >diff --git a/src/wayland-client.c b/src/wayland-client.c
> > > >index ef12bfc..87fc0e4 100644
> > > >--- a/src/wayland-client.c
> > > >+++ b/src/wayland-client.c
> > > >@@ -177,7 +177,7 @@ display_protocol_error(struct wl_display *display,
> > > >uint32_t code,
> > > > return;
> > > >
> > > > /* set correct errno */
> > > >-if (wl_interface_equal(intf, &wl_display_interface)) {
> > > >+if (intf && wl_interface_equal(intf, &wl_display_interface)) {
> > > > switch (code) {
> > > > case WL_DISPLAY_ERROR_INVALID_OBJECT:
> > > > case WL_DISPLAY_ERROR_INVALID_METHOD:
> > > >@@ -790,12 +790,26 @@ display_handle_error(void *data,
> > > > uint32_t code, const char *message)
> > > > {
> > > > struct wl_proxy *proxy = object;
> > > >+uint32_t object_id;
> > > >+const struct wl_interface *interface;
> > > >
> > > >-wl_log("%s@%u: error %d: %s\n",
> > > >- proxy->object.interface->name, proxy->object.id, code,
> > > >message);
> > > >+if (proxy) {
> > > >+wl_log("%s@%u: error %d: %s\n",
> > > >+ proxy->object.interface->name,
> > > >+ proxy->object.id,
> > > >+ code, message);
> > > >
> > > >-display_protocol_error(display, code, proxy->object.id,
> > > >- proxy->object.interface);
> > > >+object_id = proxy->object.id;
> > > >+interface = proxy->object.interface;
> > > >+} else {
> > > >+wl_log("[unknown interface]@[unknown id]: error %d:
> > > >%s\n",
>
> I changed "[unknown interface]@[unknown id]" to "[destroyed object]"
> after talking with Jason in irc. Seems like a more helpful message that
> is practically always correct.
*cough*
I mean Jonas, not Jason. Sorry :-p
Thanks,
pq
pgp5NSxThmkQD.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland] tests: add test for receiving an error on destroyed object
On Mon, 22 Feb 2016 13:25:33 -0800
Bryce Harrington wrote:
> On Mon, Feb 22, 2016 at 02:37:00PM +0100, Marek Chalupa wrote:
> > test if receiving an error on already destroyed object won't
> > do any harm
> >
> > Signed-off-by: Marek Chalupa
>
> Confirmed that this test catches the error that is fixed by
> https://patchwork.freedesktop.org/patch/74577/, and that the test passes
> when that patch is in place.
>
> Reviewed-by: Bryce Harrington
> Tested-by: Bryce Harrington
Nice! Pushed:
cde251a..5646236 master -> master
Thanks,
pq
> > ---
> > tests/display-test.c | 50
> > ++
> > 1 file changed, 50 insertions(+)
> >
> > diff --git a/tests/display-test.c b/tests/display-test.c
> > index 1a6c345..f9f8160 100644
> > --- a/tests/display-test.c
> > +++ b/tests/display-test.c
> > @@ -876,3 +876,53 @@ TEST(versions)
> >
> > display_destroy(d);
> > }
> > +
> > +static void
> > +check_error_on_destroyed_object(void *data)
> > +{
> > + struct client *c;
> > + struct wl_seat *seat;
> > + uint32_t id;
> > + const struct wl_interface *intf;
> > +
> > + c = client_connect();
> > + seat = client_get_seat(c);
> > +
> > + /* destroy the seat proxy. The display won't know
> > +* about it yet, so it will post the error as usual */
> > + wl_proxy_destroy((struct wl_proxy *) seat);
> > +
> > + /* let display post the error. The error will
> > +* be caught in stop_display while dispatching */
> > + assert(stop_display(c, 1) == -1);
> > +
> > + /* check the returned error. Since the object was destroyed,
> > +* we don't know the interface and id */
> > + assert(wl_display_get_error(c->wl_display) == EPROTO);
> > + assert(wl_display_get_protocol_error(c->wl_display, &intf, &id) == 23);
> > + assert(intf == NULL);
> > + assert(id == 0);
> > +
> > + client_disconnect_nocheck(c);
> > +}
> > +
> > +TEST(error_on_destroyed_object)
> > +{
> > + struct client_info *cl;
> > + struct display *d = display_create();
> > +
> > + wl_global_create(d->wl_display, &wl_seat_interface,
> > +1, d, bind_seat);
> > +
> > + cl = client_create_noarg(d, check_error_on_destroyed_object);
> > + display_run(d);
> > +
> > + /* did client bind to the seat? */
> > + assert(cl->data);
> > +
> > + /* post error on the destroyed object */
> > + wl_resource_post_error((struct wl_resource *) cl->data,
> > + 23, "Dummy error");
> > + display_resume(d);
> > + display_destroy(d);
> > +}
> > --
> > 2.5.0
> >
> > ___
> > wayland-devel mailing list
> > wayland-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
pgpqtAduhL8oI.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland] client: Don't segfault when receiving error on destroyed object
On Mon, 22 Feb 2016 13:22:05 -0800
Bryce Harrington wrote:
> On Mon, Feb 22, 2016 at 02:34:57PM +0100, Marek Chalupa wrote:
> > Hi,
> >
> > can confirm the segfault, tested it (will send the test I used for
> > it as a follow-up). The only API change problem could be in
> > returning
> > NULL as the interface - if the user does not check for it, he/she
> > dereferences NULL. But I don't think anybody (except us in tests) is
> > using wl_display_get_protocol_error() for the error analysis, so IMO
> > its OK break.
> >
> > Reviewed-by: Marek Chalupa
>
> I agree, given that any client in this particular situation would have
> been crashing, I don't think anything would be dependent on interface
> being non-NULL.
>
> Reviewed-by: Bryce Harrington
And R-b me and pushed:
cde251a..5646236 master -> master
with a small change noted below...
> > Cheers,
> > Marek
> >
> > On 02/22/16 06:37, Jonas Ådahl wrote:
> > >If an error is received on a destroyed object, we'd get NULL passed
> > >to display_handle_error() instead of a pointer to a valid wl_proxy.
> > >
> > >The logging is changed to report [unknown interface] and [unknown id]
> > >instead of the actual interface name and id.
> > >
> > >The wl_display_get_protocol_error() documentation is updated to handle
> > >the situation. For when the proxy was NULL, the object id 0 and
> > >interface NULL is written.
> > >
> > >Signed-off-by: Jonas Ådahl
> > >---
> > >
> > >This is technically an API change, but I see no less breaking change.
> > >Considering that clients would segfault before ever reaching here without
> > >this
> > >patch, maybe it's an Ok break.
> > >
> > >
> > >Jonas
> > >
> > >
> > > src/wayland-client.c | 32
> > > 1 file changed, 24 insertions(+), 8 deletions(-)
> > >
> > >diff --git a/src/wayland-client.c b/src/wayland-client.c
> > >index ef12bfc..87fc0e4 100644
> > >--- a/src/wayland-client.c
> > >+++ b/src/wayland-client.c
> > >@@ -177,7 +177,7 @@ display_protocol_error(struct wl_display *display,
> > >uint32_t code,
> > > return;
> > >
> > > /* set correct errno */
> > >- if (wl_interface_equal(intf, &wl_display_interface)) {
> > >+ if (intf && wl_interface_equal(intf, &wl_display_interface)) {
> > > switch (code) {
> > > case WL_DISPLAY_ERROR_INVALID_OBJECT:
> > > case WL_DISPLAY_ERROR_INVALID_METHOD:
> > >@@ -790,12 +790,26 @@ display_handle_error(void *data,
> > >uint32_t code, const char *message)
> > > {
> > > struct wl_proxy *proxy = object;
> > >+ uint32_t object_id;
> > >+ const struct wl_interface *interface;
> > >
> > >- wl_log("%s@%u: error %d: %s\n",
> > >- proxy->object.interface->name, proxy->object.id, code, message);
> > >+ if (proxy) {
> > >+ wl_log("%s@%u: error %d: %s\n",
> > >+ proxy->object.interface->name,
> > >+ proxy->object.id,
> > >+ code, message);
> > >
> > >- display_protocol_error(display, code, proxy->object.id,
> > >- proxy->object.interface);
> > >+ object_id = proxy->object.id;
> > >+ interface = proxy->object.interface;
> > >+ } else {
> > >+ wl_log("[unknown interface]@[unknown id]: error %d: %s\n",
I changed "[unknown interface]@[unknown id]" to "[destroyed object]"
after talking with Jason in irc. Seems like a more helpful message that
is practically always correct.
Thanks,
pq
> > >+ code, message);
> > >+
> > >+ object_id = 0;
> > >+ interface = NULL;
> > >+ }
> > >+
> > >+ display_protocol_error(display, code, object_id, interface);
> > > }
> > >
> > > static void
> > >@@ -1756,10 +1770,12 @@ wl_display_get_error(struct wl_display *display)
> > > /** Retrieves the information about a protocol error:
> > > *
> > > * \param displayThe Wayland display
> > >- * \param interface if not NULL, stores the interface where the error
> > >occurred
> > >+ * \param interface if not NULL, stores the interface where the error
> > >occurred,
> > >+ * or NULL, if unknown.
> > > * \param id if not NULL, stores the object id that generated
> > >- * the error. There's no guarantee the object is
> > >- * still valid; the client must know if it deleted the
> > >object.
> > >+ * the error, or 0, if the object id is unknown.
> > >There's no
> > >+ * guarantee the object is still valid; the client must
> > >know
> > >+ * if it deleted the object.
> > > * \return The error code as defined in the interface
> > > specification.
> > > *
> > > * \code
> > >
> > ___
> > wayland-devel mailing list
> > wayland-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> ___
> wayland-deve
