On Mon, Nov 26, 2012 at 1:59 AM, Daniel Juyung Seo <seojuyu...@gmail.com>wrote:
> 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)
>
I just removed them from trunk/branches.
And I have one more comments.
6. elm_win_available_profiles_get
Why don't you have a sample code of elm_win_available_profiles_get in
test_config.c?
Daniel Juyung Seo (SeoZ)
>
> 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
