Page MenuHomePhabricator

ecore_wl2: add new APIs
Needs RevisionPublic

Authored by eagleeye on Jul 3 2019, 11:49 PM.

Details

Reviewers
devilhorns
zmike
Summary

These APIs were used in Tizen, they are added to make management easier.

  • ecore_wl2_display
  • ecore_wl2_input
  • ecore_wl2_window

Diff Detail

Repository
rEFL core/efl
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 13342
Build 9419: arc lint + arc unit
eagleeye created this revision.Jul 3 2019, 11:49 PM

It seems that this patch has no reviewers specified. If you are unsure who can review your patch, please check this wiki page and see if anyone can be added: https://phab.enlightenment.org/w/maintainers_reviewers/

eagleeye requested review of this revision.Jul 3 2019, 11:49 PM
devilhorns requested changes to this revision.Jul 29 2019, 6:54 AM
devilhorns added a subscriber: devilhorns.

There are some minor things here that need corrections. Also, can you improve the commit Summary a little please. It makes no mention of adding new APIs

src/lib/ecore_wl2/Ecore_Wl2.h
688

@since 1.23

701

@since 1.23

741

@since 1.23

753

@since 1.23

763

@since 1.23

1599

@since 1.23

1610

@since 1.23

1621

@since 1.23

1631

@since 1.23

src/lib/ecore_wl2/ecore_wl2_display.c
701

These 2 can go on the same line:

int ret, last_dpy_err;

890

There should not be a space between * and ewd.

Ecore_Wl2_Display *ewd;

1139

This should return NULL, not 0.

EINA_SAFETY_ON_NULL_RETURN_VAL(display, NULL);

src/lib/ecore_wl2/ecore_wl2_input.c
1865

These should probably use EINA_SAFETY macros

1882

Use EINA_SAFETY macro here

1898

Use EINA_SAFETY macro here

This revision now requires changes to proceed.Jul 29 2019, 6:54 AM
devilhorns added a project: Restricted Project.Jul 29 2019, 6:58 AM
eagleeye updated this revision to Diff 24672.Sep 1 2019, 10:14 PM

fix version of efl and add ecore_wl2_window API

there is build failure now.

devilhorns requested changes to this revision.Sep 4 2019, 4:40 AM

To fix the build error, change the meson.build file in src/lib/ecore_wl2 so that it adds wayland-cursor as a dependency:

ecore_wl2_deps = [

dependency('wayland-client'), dependency('wayland-server'), dependency('xkbcommon'),
dependency('wayland-cursor'),
wayland_protocol, dl, m, ecore, ecore_input, libdrm, buildsystem

]

When the above is changed, then this patch will build.

The rest of the changes look good.

This revision now requires changes to proceed.Sep 4 2019, 4:40 AM
eagleeye updated this revision to Diff 25076.Mon, Sep 16, 10:55 PM

fix build error

eagleeye edited the summary of this revision. (Show Details)Mon, Sep 16, 10:59 PM
devilhorns accepted this revision.Tue, Sep 17, 6:11 AM

Great work !! :) We cannot push this upstream yet tho because of the freeze for 1.23 release. After the freeze is over, I will push these changes.

This revision is now accepted and ready to land.Tue, Sep 17, 6:11 AM
zmike requested changes to this revision.Tue, Sep 17, 6:59 AM
zmike added subscribers: raster, zmike.

Most of the proposed API here don't seem to be in line with how we're trying to use the Wayland protocol.

I'm not convinced we need many of these in upstream, particularly the ones which are stubs for code that can't/won't work correctly, or the ones which directly manipulate libwayland internals using an API which matches libwayland--thus forcing our "abstraction" layer to no longer be an abstraction.

src/lib/ecore_wl2/ecore_wl2_input.c
771

What?

1825

This function does nothing besides set an internal value that is never accessed. It's effectively dead code and should not be merged.

1896

I'm quite certain that we don't want to continue mimicking X-style cursors with this type of API in EFL. That applies to all "cursor name" type APIs, regardless of what may exist in higher level components.

1912

Can you provide a common use case for this?

src/lib/ecore_wl2/ecore_wl2_window.c
836

I don't see why this or the following two functions are necessary given that ecore_wl2_window_input_region_set exists and is superior based on it:

  1. being asynchronous
  2. handling rotation.
1120

This is effectively dead code since it's impossible to know whether a surface is actually minimized.

1134

See above.

1818

This really doesn't seem like a sane API in any sense.

This revision now requires changes to proceed.Tue, Sep 17, 6:59 AM
CHAN added a subscriber: CHAN.EditedTue, Oct 15, 3:09 AM

ecore_wl2_window_input_rect_set(Ecore_Wl2_Window *win, Eina_Rectangle *input_rect)
ecore_wl2_window_input_rect_add(Ecore_Wl2_Window *win, Eina_Rectangle *input_rect)
ecore_wl2_window_input_rect_subtract(Ecore_Wl2_Window *win, Eina_Rectangle *input_rect)

Those APIs created for two or more input areas.

Anyways, In contrast to ecore_wl2_window_input_rect_set(), the ecore_wl2_window_input_region_set() has additional features. (pending update.)
So now those two APIs has a bit different behavior.

We can use ecore_wl2_window_input_region_set() instead of ecore_wl2_window_input_rect_set(),
if there is no pending update feature in the ecore_wl2_window_input_region_set() API or if we support it.

@devilhorns Do we need to update it asynchronously(pending)? if so, we need a ecore_wl2_window_input_rect_set() API. and any opinion about it?