Dear Daniel Juyung Seo, Thanks for the 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. Actually it was "profile,changed" in my 1st patch. But I found out that meaning and action of 'profile,changed' might be confusing application developer. For example, if the app only specifies available window profiles, but nevertheless the app can get 'profile,changed' signal by the window manager. Developers say that they've never set profile but their app get 'changed' signal at initial time. For this reason, I've changed name of signal to 'profile,set' from 'profile,changed' in my 2nd patch. >> 2. 2 spaces after EINA_LIST_FOREACH >> Use 2 spaces after EINA_LIST_FOREACH. >> We consider EINA_LIST_FOREACH as 'for'. Okay, I will fix it. Formatting is very important! >> 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. The available window profiles is not a list since it doesn't be changed again. Is it possible to make application to change 'tv' layout support from 'mobile' and 'tablet' layout support at run time? (It should specify 'tv', 'mobile' and 'tablet' layout support at initial time.) And it is more simple instead of using EINA_LIST. const char *supports[3] = { "tv", "mobile", "tablet" }; elm_win_available_profiles_set(w, supports, 3); >> 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 :) Thanks. I will add. >> 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) Okay. I will remove them. I know no implementations and no dependency. And could you please don't spank to Devilhorns? :) > 6. elm_win_available_profiles_get > Why don't you have a sample code of elm_win_available_profiles_get in > test_config.c? Because I'm very tired. :( BR, Gwanglim On Mon, Nov 26, 2012 at 2:10 AM, Daniel Juyung Seo <seojuyu...@gmail.com> wrote: > 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 ------------------------------------------------------------------------------ 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