Dear Gwanglim Lee,
Thanks for the nice new feature patch.
I just have some comments.
1. "profile,set" -> "profile,changed"
For elm_win smart callbacks, "profile,changed" sounds better for
consistency.
Have a look at other elementary smart callbacks.
2. 2 spaces after EINA_LIST_FOREACH
Use 2 spaces after EINA_LIST_FOREACH.
We consider EINA_LIST_FOREACH as 'for'.
3. elm_win_available_profiles_set/get profiles
How about using Eina_List for the list of profiles instead of an array?
See ecore_evas_engines_get, ecore_evas_engines_free,
ecore_evas_ecore_evas_list_get, ecore_evas_ews_children_get, and etc.
4. author
How about adding yourself to the following codes.
elementary/AUTHORS
elementary/src/lib/elm_authors.h?
ecore/AUTHORS
ecore/src/lib/ecore/Ecore.h
You worth it :)
5. removing ecore_evas_profiles_set/get
Instead of marking ecore_evas_profiles_set/get deprecated in Ecore_Evas.h,
we need to remove them from header because there were no implementations at
all.
I give a big spank to Devilhorns with regarding to this. (r73206)
Thanks.
Daniel Juyung Seo (SeoZ)
On Sun, Nov 25, 2012 at 4:59 PM, Gwanglim Lee <gl77....@samsung.com> wrote:
> Hi Raster, many thanks for your kindly review. :)
>
> I've modified the code to according to your review comments.
>
> > 1. no xcb version of the code in ecore_x
> > 2. in ecore_evas i think you need to transport the profile across the
> > ecore_evas_extn ipc to the other end... so it can call the profile
> change stuff
> > since these are across process boundaires - we lose the information -
> similar
> > with ecore_evas_buffer - though this is just a direct call, no ipc. :)
> Thank you. I did'n know about this. I've added code to support the
> ecore_evas_extn
> and the ecore_evas_buffer.
>
> > 3. otherwise all the ecore stuff looks spot-on and good.
> I missed profile stuffs release code, I've added release code in
> _ecore_evas_free to fix it.
>
> > 4. elm stuff looks great.
> > 5. test code seems fine too
> Now, I've modified test code to check socket, plug and inlined image.
> Also it can check creation of the window with specified profile.
>
> > 6. you increase the config version number, but i see no changes to the
> version
> > number in e_config.h with major/minor version. no need to do this -
> leave the
> > config version alone unless you also bump the e_config.h versions too
> > E_CONFIG_FILE_EPOCH and E_CONFIG_FILE_GENERATION combined to become
> > E_CONFIG_FILE_VERSION).
> I've increased value of E_CONFIG_FILE_GENERATION to 0x0162.
>
> I'm attached 2nd patch and a file again, these are based on r79643.
>
> Let me know if you have questions.
>
> Thanks & Regards,
> Gwanglim
>
> ------- Original Message -------
> Sender : Carsten Haitzler<ras...@rasterman.com>
> Date : 2012-11-21 14:08 (GMT+09:00)
> Title : Re: [E-devel] [RFC] Virtual desktop window profile
>
> On Sun, 11 Nov 2012 08:11:43 +0000 (GMT) Gwanglim Lee <
> gl77....@samsung.com>
> said:
>
> review: :)
>
> 1. no xcb version of the code in ecore_x
> 2. in ecore_evas i think you need to transport the profile across the
> ecore_evas_extn ipc to the other end... so it can call the profile change
> stuff
> since these are across process boundaires - we lose the information -
> similar
> with ecore_evas_buffer - though this is just a direct call, no ipc. :)
> 3. otherwise all the ecore stuff looks spot-on and good.
> 4. elm stuff looks great.
> 5. test code seems fine too
> 6. you increase the config version number, but i see no changes to the
> version
> number in e_config.h with major/minor version. no need to do this - leave
> the
> config version alone unless you also bump the e_config.h versions too
> E_CONFIG_FILE_EPOCH and E_CONFIG_FILE_GENERATION combined to become
> E_CONFIG_FILE_VERSION).
>
> otherwise - looks fine to me - can you fix the above and re-send? and
> quickly
> too.. release is going out in a month... :) well on the e17 side anyway. :)
>
> > Dear EFL developers,
> >
> > I'd like to ask you for review on the virtual desktop window profile.
> > It is a new feature that allows the window manager (enlightenment) to
> > configure the elm profile of the application window (elm_win) according
> to
> > the different window profiles for each virtual desktop.
> >
> > [Virtual desktop window profile]
> > You can turn this feature on and also define a name of window profile
> for each
> > desktop in the setting dialog of the e as followings:
> >
> > Settings -> Settings Panel -> Screen -> Virtual Desktops -> Virtual
> Desktops
> > Settings -> Check 'Use desktop window profile'
> > -> Click desktop image to change name of window profile for each desktop
> >
> > Here's how it works.
> >
> > 1. Once you've enabled it, the WM is set the _E_WINDOW_PROFILE_SUPPORTED
> > property on the root window to indicate window profile protocol support.
> Let
> > me assume that you've defined names of window profile as follows:
> > desktop 0-0: "standard"
> > desktop 1-0: "mobile"
> > desktop 2-0: "default"
> >
> > 2. The application developer can specify a list of available profiles
> if he
> > wants. The WM will be able to use this list to decide where to place the
> > window. In other words, a list is a kind of hint for the WM. Let me
> assume
> > that a developer has specified a list as following: "mobile", "default"
> >
> > 3. When you run the app, it checks to see if the WM supports the profile
> > protocol at the initial time. If it is, the app sets the profile protocol
> > hint on its window and also sets a list of available profiles if exists.
> >
> > 4. The WM checks to see if app is using the profile protocol. If it is,
> the
> > WM also tries to get a list. In this case, the WM can get "mobile" and
> > "default" for the app's window. The WM chooses a "mobile" profile in that
> > list. And then the WM sends the 'profile change request' event to the
> app,
> > and waits for the 'finish' event.
> >
> > 5. Once the app receives the 'profile change request' event, the app
> > attempts to change the elm profile according to the requested profile
> > ("mobile") from the WM, and then the app sends back the 'finish' event
> to the
> > WM.
> >
> > 6. Upon receiving the 'finish' event, the WM shows app's window on the
> > desktop 1-0 "mobile" area.
> >
> > 7. The window may be moved by user to the desktop 2-0 default area from
> 1-0.
> > 8. When this happens, the WM requests that the window be changed with
> > "default" profile, and waits again.
> > 9. The app changes profile and then responds.
> > 10. Upon receiving the 'finish' event, the WM moves the window to the
> desktop
> > 2-0 "default" area.
> >
> > [Per-window elm profile]
> > The elementary is using a single global elm profile for the entire app
> right
> > now. Changing the elm profile affects all elm apps. In order to apply the
> > 'desktop window profile', a single global elm profile should be changed.
> Thus
> > I've changed elm profile to bind a profile to a specific window. But elm
> > still has a single global profile for the app, I hope it should be also
> fixed
> > later to bind a profile to a specific elm_win (2 windows with differing
> > profiles in a app).
> >
> >
> > I'm attaching patches that modifies the following files.
> > (These patchess are based on rev. 78536. I couldn't make patches with the
> > latest source because my linux host has a trouble to pull E svn today.)
> >
> > [ecore]
> > src/lib/ecore_x/ecore_x_atoms_decl.h
> > src/lib/ecore_x/Ecore_X.h
> > src/lib/ecore_x/xcb/ecore_xcb_e.c
> > src/lib/ecore_x/Ecore_X_Atoms.h
> > src/lib/ecore_x/xlib/ecore_x_e.c
> > src/lib/ecore_evas/ecore_evas_wayland_shm.c
> > src/lib/ecore_evas/ecore_evas_x.c
> > src/lib/ecore_evas/ecore_evas_buffer.c
> > src/lib/ecore_evas/ecore_evas_extn.c
> > src/lib/ecore_evas/ecore_evas_private.h
> > src/lib/ecore_evas/ecore_evas.c
> > src/lib/ecore_evas/ecore_evas_sdl.c
> > src/lib/ecore_evas/ecore_evas_psl1ght.c
> > src/lib/ecore_evas/ecore_evas_directfb.c
> > src/lib/ecore_evas/ecore_evas_cocoa.c
> > src/lib/ecore_evas/Ecore_Evas.h
> > src/lib/ecore_evas/ecore_evas_wayland_egl.c
> > src/lib/ecore_evas/ecore_evas_fb.c
> > src/lib/ecore_evas/ecore_evas_ews.c
> > src/lib/ecore_evas/ecore_evas_win32.c
> > NEWS
> > ChangeLog
> >
> > [elementary]
> > src/lib/elm_win.c
> > src/lib/elm_win.h
> > src/bin/test.c
> > src/bin/Makefile.am
> > ChangeLog
> > NEWS
> >
> > [e]: I know e17 is now in feature freeze. I can wait until doomsday. :)
> > src/bin/e_border.c
> > src/bin/e_border.h
> > src/bin/e_container.c
> > src/bin/e_container.h
> > src/bin/e_config.c
> > src/bin/e_config.h
> > src/bin/e_desk.c
> > src/bin/e_desk.h
> > src/bin/e_main.c
> > src/modules/conf_display/e_int_config_desks.c
> > src/modules/conf_display/e_int_config_desk.c
> > config/standard/e.src
> > config/default/e.src
> > config/mobile/e.src
> >
> > And I've added 1 new test file in elementary for the elementary_test.
> > elementary/src/bin/test_config.c
> >
> > I hope this is helpful to someone who wants to use different elm profiles
> > for each display.
> >
> > Any comments would be appreciated.
> >
> > Thanks & Regards,
> > Gwanglim
>
> --
> ------------- Codito, ergo sum - "I code, therefore I am" --------------
> The Rasterman (Carsten Haitzler) ras...@rasterman.com
>
>
> ------------------------------------------------------------------------------
> Monitor your physical, virtual and cloud infrastructure from a single
> web console. Get in-depth insight into apps, servers, databases, vmware,
> SAP, cloud infrastructure, etc. Download 30-day Free Trial.
> Pricing starts from $795 for 25 servers or applications!
> http://p.sf.net/sfu/zoho_dev2dev_nov
> _______________________________________________
> enlightenment-devel mailing list
> enlightenment-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
>
>
------------------------------------------------------------------------------
Monitor your physical, virtual and cloud infrastructure from a single
web console. Get in-depth insight into apps, servers, databases, vmware,
SAP, cloud infrastructure, etc. Download 30-day Free Trial.
Pricing starts from $795 for 25 servers or applications!
http://p.sf.net/sfu/zoho_dev2dev_nov
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
