On Fri, 03 Nov 2017 14:10:28 +0000 Mike Blumenkrantz
<michael.blumenkra...@gmail.com> said:

> On Fri, Nov 3, 2017 at 9:50 AM Carsten Haitzler <ras...@rasterman.com>
> wrote:
> 
> > On Fri, 03 Nov 2017 11:28:28 +0000 Mike Blumenkrantz
> > <michael.blumenkra...@gmail.com> said:
> >
> > > I see what you are going for here based on the theme that you have been
> > > working on, and I think it would be a nice improvement overall to have
> > this
> >
> > actually it applies to default too.. :)
> >
> 
> Nobody cares about the shadows in a theme which has been the default for
> over 5 years :) :) :) :) :)
> 
> 
> >
> > > sort of feature, but I think that this is not the best way to go about
> > > it--specifically adding it to the window menu since there are now two
> > > screenshot items, though the general implementation also seems to be
> > > something that would only be used by you since it either adds a large
> > > amount of padding by default or requires the user to trial+error in order
> > > to achieve the desired results.
> >
> > i just kind of thought i chose a pretty good default that would "do the
> > job 99%
> > of the time anyway"... if that's what you wanted. it does make for much
> > nicer
> > screenshots of specific windows. :)
> >
> > the problem here is we don't actually know the dimensions of the shadow.
> > it's
> > done outside the object bounds, thus just taking some value is about as
> > good as
> > it gets at this point. well in x11. in wayland we do. but if we calculated
> > it
> > given current shadows the top padding would be less, bottom more etc. so
> > you'd
> > want to take the max anyway.
> >
> 
> Using edje_object_parts_extends_calc() would yield correct dimensions for
> server-side shadows, and there are themes which have used this to have
> resize trigger based on pointer interaction with the shadow region in x11.

i didn't know that had been added.

> > > I think the better solution for reliably capturing shadows on windows
> > would
> > > be to do something like add a feature for snapshot objects where setting
> > > the clip to an object under it will restrict it to only being a snapshot
> > > from image data resulting from that object. Then all window-based
> > > screenshots could automatically be made to capture shadows and do it
> > > accurately.
> >
> > i did indeed try proxies... they ended up blank with no content...
> > (creates an
> > image set src to ec->frame etc. etc.) and that still has the above "shadow
> > is
> > done outside of object bounds" thing where you can't grab it via a proxy
> > this
> > way anyway... :(
> 
> Snapshot object, not proxy. I've created a task for it.
> https://phab.enlightenment.org/T6312

sure. but proxies would be the only way to capture a specific object and not
everything below it too at this point. :)

> > > Alternatively, dynamic cropping from e.g., emprint or ephoto could be
> > added
> > > to the base screenshot gui which would be a useful feature for many
> > cases.
> >
> > that was on a list of things to "add later" as it would take more work.
> > also
> > if it were just simply "draw a rect to crop" it's lead to inconsistent
> > padding
> > around windows. to do it well it'd be a bit like the screen select but you
> > can
> > click on windows, then click and drag handles outside of the window, maybe
> > with
> > a "add std padding" button with some field with the padding amount to add
> > around the bounds etc. - definitely more work by a good chunk.
> >
> > what i have done is a very quick and simple solution to having window shots
> > actually look "nice" with their surrounding decorations and shadow ... if
> > you
> > want. i kept the non-padded entry because sometimes you don't want other
> > windows
> > around your to leak into the shot... not the nicest of solutions, but it's
> > a
> > pretty quick and simple one for now.
> >
> 
> I understand why you did it as well as why you did it the way you did, but
> I disagree with the latter. Having more parameters to an action that only
> extreme power users will ever notice is whatever, but adding extra menu
> items which duplicate the functionality of existing menu items is not
> ideal. The toplevel menu should contain exactly one item for taking
> screenshots of windows, not one for taking screenshots and a second for
> taking slightly larger screenshots.

wile it doesnt look brilliant my choices are:

1. always take a padded screenshot - annoys lots of people who wanted the
window content and ONLY that and they have to now crop in gimp etc.
2. don't offer padded shots and it's only available to power users who mess with
bindings and params (effectively the feature may as well not exist then)
3. offer both options easily and let people choose.

i don't see an overwhelmingly strong case that people will only want one or the
other, thus i put in both. i totally see why 2 entries might not be great ...
but i see few alternatives at this point that are better.

> > > On Fri, Nov 3, 2017 at 3:18 AM Carsten Haitzler <ras...@rasterman.com>
> > > wrote:
> > >
> > > > raster pushed a commit to branch master.
> > > >
> > > >
> > > >
> > http://git.enlightenment.org/core/enlightenment.git/commit/?id=2be56cd8da7ae656f37f3894c1e5a6f66ff9f4b2
> > > >
> > > > commit 2be56cd8da7ae656f37f3894c1e5a6f66ff9f4b2
> > > > Author: Carsten Haitzler (Rasterman) <ras...@rasterman.com>
> > > > Date:   Fri Nov 3 16:16:45 2017 +0900
> > > >
> > > >     shot - add a padded screenshot so it can also grab
> > shadows/surrounds
> > > >
> > > >     nice to get a shot that also has the shadows etc. add as menu and
> > > >     param options
> > > >
> > > >     @feature
> > > > ---
> > > >  src/modules/shot/e_mod_main.c | 89
> > > > +++++++++++++++++++++++++++++++------------
> > > >  1 file changed, 65 insertions(+), 24 deletions(-)
> > > >
> > > > diff --git a/src/modules/shot/e_mod_main.c
> > b/src/modules/shot/e_mod_main.c
> > > > index 9929dd6e7..fd0599a25 100644
> > > > --- a/src/modules/shot/e_mod_main.c
> > > > +++ b/src/modules/shot/e_mod_main.c
> > > > @@ -605,6 +605,7 @@ _save_dialog_show(E_Zone *zone, E_Client *ec, const
> > > > char *params, void *dst, int
> > > >     Evas_Modifier_Mask mask;
> > > >     E_Radio_Group *rg;
> > > >     int w, h;
> > > > +   char smode[128], squal[128], sscreen[128];
> > > >
> > > >     win = elm_win_add(NULL, NULL, ELM_WIN_BASIC);
> > > >
> > > > @@ -768,27 +769,23 @@ _save_dialog_show(E_Zone *zone, E_Client *ec,
> > const
> > > > char *params, void *dst, int
> > > >     evas_object_size_hint_min_set(win, w, h);
> > > >     evas_object_size_hint_max_set(win, 99999, 99999);
> > > >
> > > > -   if (params)
> > > > +   if ((params) &&
> > > > +       (sscanf(params, "%100s %100s %100s", smode, squal, sscreen) ==
> > 3))
> > > >       {
> > > > -        char smode[128], squal[128], sscreen[128];
> > > > -
> > > > -        if (sscanf(params, "%100s %100s %100s", smode, squal,
> > sscreen) ==
> > > > 3)
> > > > -          {
> > > > -             screen = -1;
> > > > -             if ((zone) && (!strcmp(sscreen, "current"))) screen =
> > > > zone->num;
> > > > -             else if (!strcmp(sscreen, "all")) screen = -1;
> > > > -             else screen = atoi(sscreen);
> > > > -
> > > > -             quality = 90;
> > > > -             if (!strcmp(squal, "perfect")) quality = 100;
> > > > -             else if (!strcmp(squal, "high")) quality = 90;
> > > > -             else if (!strcmp(squal, "medium")) quality = 70;
> > > > -             else if (!strcmp(squal, "low")) quality = 50;
> > > > -             else quality = atoi(squal);
> > > > -
> > > > -             if (!strcmp(smode, "save")) _win_save_cb(NULL, NULL);
> > > > -             else if (!strcmp(smode, "share"))  _win_share_cb(NULL,
> > NULL);
> > > > -          }
> > > > +        screen = -1;
> > > > +        if ((zone) && (!strcmp(sscreen, "current"))) screen =
> > zone->num;
> > > > +        else if (!strcmp(sscreen, "all")) screen = -1;
> > > > +        else screen = atoi(sscreen);
> > > > +
> > > > +        quality = 90;
> > > > +        if (!strcmp(squal, "perfect")) quality = 100;
> > > > +        else if (!strcmp(squal, "high")) quality = 90;
> > > > +        else if (!strcmp(squal, "medium")) quality = 70;
> > > > +        else if (!strcmp(squal, "low")) quality = 50;
> > > > +        else quality = atoi(squal);
> > > > +
> > > > +        if (!strcmp(smode, "save")) _win_save_cb(NULL, NULL);
> > > > +        else if (!strcmp(smode, "share"))  _win_share_cb(NULL, NULL);
> > > >       }
> > > >     else
> > > >       {
> > > > @@ -832,7 +829,22 @@ _shot_now(E_Zone *zone, E_Client *ec, const char
> > > > *params)
> > > >       }
> > > >     else
> > > >       {
> > > > -        x = ec->x, y = ec->y, w = ec->w, h = ec->h;
> > > > +        int pad = 0;
> > > > +
> > > > +        if (params)
> > > > +          {
> > > > +             const char *p = strstr(params, "pad ");
> > > > +
> > > > +             if (p)
> > > > +               {
> > > > +                  pad = atoi(p + 4);
> > > > +                  if (pad < 0) pad = 0;
> > > > +               }
> > > > +          }
> > > > +        x = ec->x - pad;
> > > > +        y = ec->y - pad;
> > > > +        w = ec->w + (pad * 2);
> > > > +        h = ec->h + (pad * 2);
> > > >          x = E_CLAMP(x, 0, e_comp->w);
> > > >          y = E_CLAMP(y, 0, e_comp->h);
> > > >          w = E_CLAMP(w, 1, e_comp->w);
> > > > @@ -876,6 +888,18 @@ _shot_delay_border(void *data)
> > > >     return EINA_FALSE;
> > > >  }
> > > >
> > > > +static Eina_Bool
> > > > +_shot_delay_border_padded(void *data)
> > > > +{
> > > > +   char buf[128];
> > > > +
> > > > +   border_timer = NULL;
> > > > +   snprintf(buf, sizeof(buf), "pad %i", (int)(64 * e_scale));
> > > > +   _shot_now(NULL, data, buf);
> > > > +
> > > > +   return EINA_FALSE;
> > > > +}
> > > > +
> > > >  static void
> > > >  _shot_border(E_Client *ec)
> > > >  {
> > > > @@ -884,6 +908,13 @@ _shot_border(E_Client *ec)
> > > >  }
> > > >
> > > >  static void
> > > > +_shot_border_padded(E_Client *ec)
> > > > +{
> > > > +   if (border_timer) ecore_timer_del(border_timer);
> > > > +   border_timer = ecore_timer_loop_add(1.0, _shot_delay_border_padded,
> > > > ec);
> > > > +}
> > > > +
> > > > +static void
> > > >  _shot(E_Zone *zone)
> > > >  {
> > > >     if (timer) ecore_timer_del(timer);
> > > > @@ -897,13 +928,19 @@ _e_mod_menu_border_cb(void *data, E_Menu *m
> > > > EINA_UNUSED, E_Menu_Item *mi EINA_UN
> > > >  }
> > > >
> > > >  static void
> > > > +_e_mod_menu_border_padded_cb(void *data, E_Menu *m EINA_UNUSED,
> > > > E_Menu_Item *mi EINA_UNUSED)
> > > > +{
> > > > +   _shot_border_padded(data);
> > > > +}
> > > > +
> > > > +static void
> > > >  _e_mod_menu_cb(void *data EINA_UNUSED, E_Menu *m, E_Menu_Item *mi
> > > > EINA_UNUSED)
> > > >  {
> > > >     if (m->zone) _shot(m->zone);
> > > >  }
> > > >
> > > >  static void
> > > > -_e_mod_action_border_cb(E_Object *obj EINA_UNUSED, const char *params
> > > > EINA_UNUSED)
> > > > +_e_mod_action_border_cb(E_Object *obj EINA_UNUSED, const char *params)
> > > >  {
> > > >     E_Client *ec;
> > > >
> > > > @@ -914,7 +951,7 @@ _e_mod_action_border_cb(E_Object *obj EINA_UNUSED,
> > > > const char *params EINA_UNUSE
> > > >          ecore_timer_del(border_timer);
> > > >          border_timer = NULL;
> > > >       }
> > > > -   _shot_now(NULL, ec, NULL);
> > > > +   _shot_now(NULL, ec, params);
> > > >  }
> > > >
> > > >  typedef struct
> > > > @@ -985,6 +1022,10 @@ _bd_hook(void *d EINA_UNUSED, E_Client *ec)
> > > >     e_menu_item_label_set(mi, _("Take Shot"));
> > > >     e_util_menu_item_theme_icon_set(mi, "screenshot");
> > > >     e_menu_item_callback_set(mi, _e_mod_menu_border_cb, ec);
> > > > +   mi = e_menu_item_new_relative(m, mi);
> > > > +   e_menu_item_label_set(mi, _("Take Padded Shot"));
> > > > +   e_util_menu_item_theme_icon_set(mi, "screenshot");
> > > > +   e_menu_item_callback_set(mi, _e_mod_menu_border_padded_cb, ec);
> > > >  }
> > > >
> > > >  static void
> > > > @@ -1032,7 +1073,7 @@ e_modapi_init(E_Module *m)
> > > >          border_act->func.go = _e_mod_action_border_cb;
> > > >          e_action_predef_name_set(N_("Window : Actions"), N_("Take
> > Shot"),
> > > >                                   "border_shot", NULL,
> > > > -                                 "syntax: [share|save
> > > > perfect|high|medium|low|QUALITY all|current]", 1);
> > > > +                                 "syntax: [share|save
> > > > perfect|high|medium|low|QUALITY all|current] [pad N]", 1);
> > > >       }
> > > >     maug = e_int_menus_menu_augmentation_add_sorted
> > > >       ("main/2",  _("Take Screenshot"), _e_mod_menu_add, NULL, NULL,
> > NULL);
> > > >
> > > > --
> > > >
> > > >
> > > >
> > >
> > ------------------------------------------------------------------------------
> > > Check out the vibrant tech community on one of the world's most
> > > engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> > > _______________________________________________
> > > enlightenment-devel mailing list
> > > enlightenment-devel@lists.sourceforge.net
> > > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
> > >
> >
> >
> > --
> > ------------- Codito, ergo sum - "I code, therefore I am" --------------
> > Carsten Haitzler - ras...@rasterman.com
> >
> >


-- 
------------- Codito, ergo sum - "I code, therefore I am" --------------
Carsten Haitzler - ras...@rasterman.com


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to