On Mon, Nov 26, 2012 at 4:35 AM, Gwanglim Lee <gwang...@gmail.com> wrote:
> 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.
>
Yes but as we discusses in person, "profile,changed" looks better.
>
> >> 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!
>
>
Thanks!
> >> 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);
>
>
>
I understand that. Let's keep 'const ***' then.
> >> 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.
>
>
Thanks muchly.
> >> 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? :)
>
>
I already did it. But I will still spank 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. :(
>
>
TIRED? That is a good reason.
Now for me it doesn't look like profiles_get is not necessary in the test
code.
Thanks.
Daniel Juyung Seo (SeoZ)
>
> 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
>
------------------------------------------------------------------------------
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
