Page MenuHomePhabricator

Add zoomable property to Efl.Ui.Image
Closed, ResolvedPublic

Description

elm_photocam only provide a zoombale image (but only for jpeg) from a developer point of view. It would be easier for everyone if Efl.Ui.Image had a flag to turn on and off the ability to zoom in it using mouse and keyboard. The API for manually zooming in it from code should always be there. This should help simplify our API.

jpeg added a comment.Apr 5 2017, 10:30 PM

I'm not convinced it should be just a property as this will increase the code complexity. Basically we would have to rewrite all of photocam inside elm_image.
On the other hand I think a Efl.Ui.Image.Zoomable class would be fine, and it would inherit from Efl.Ui.Image and add this one property.

Anyway this is not a very simple task.

@cedric what do you think of this approach:
Photocam (renamed as Efl.Ui.Image.Zoomable) inherits from Efl.Ui.Image and overrides all the methods & properties of image.
This way we can keep the implementation as is. Photocam isn't perfect but fixing it is not part of the interfaces rework.
Then Efl.Ui.Image.Zoomable would have the following methods:

  1. zoomable may or may not be required. see also zoom_mode.
  2. zoom_animation (renamed from paused)
  3. zoom_gesture_enabled (probably moved to an interface with other gesture-enabled widgets)
  4. zoom
  5. zoom_mode could be removed and the fit inside/outside property of elm_image could be used
  6. image_region should be a set/get property (instead of show/get). it's not the same as the scrollable interface's region. it probably belongs to the Efl.Gfx.View interface instead.
  7. image_size needs to be an override of Efl.Gfx.View.view_size

I would recommend against trying to implement the zoomable feature as just a property of elm_image. The code bases have nothing in common, so merging them would be a much larger task than just cleaning up the interface.

singh.amitesh added a comment.EditedMay 29 2017, 2:56 AM

As discussed with @jpeg Image properties scale_type(), scalable() & align() conflict with Image.Zoomable zoom property.

As for now, We are not implementing these properties in Zoomable. @cedric
Please let us know your views about it. I think we could move these properties into some interface and inherit to Image.

jpeg added a comment.May 29 2017, 11:46 PM

A new thing came up today. Both icon and edje files have the same issue: they are backed by an edje object. This edje object is not "zoomable" per se. One idea is to create an edje object and a proxy to it, so we can exploit photocam's zooming code. The big question is how big should the edje object be created at? min size? current photocam size? efl.gfx.view.size.set?

cedric raised the priority of this task from TODO to High.Jul 10 2017, 2:26 PM

Wouldn't view.size.set make the most sense ?

In T5352#87905, @jpeg wrote:

A new thing came up today. Both icon and edje files have the same issue: they are backed by an edje object. This edje object is not "zoomable" per se. One idea is to create an edje object and a proxy to it, so we can exploit photocam's zooming code. The big question is how big should the edje object be created at? min size? current photocam size? efl.gfx.view.size.set?

currently, this is logic for calculating min size.

static void
_min_obj_size_get(Evas_Object *o, int *w, int *h)
{
   EFL_UI_IMAGE_ZOOMABLE_DATA_GET(o, sd);

   efl_gfx_view_size_get(o, w, h);

   if (*w == 0 || *h == 0)
     {
        evas_object_geometry_get(o, NULL, NULL, w, h);

        if (*w == 0 || *h == 0)
          {
             edje_object_size_min_get(sd->edje, w, h);
          }
     }
}

I think this task is completed. we can close this.

jpeg added a comment.Jul 11 2017, 10:47 PM

I think this task is completed. we can close this.

Yes I think so too. Image may still need some extra work, but Photocam looks done.

jpeg closed this task as Resolved.Jul 17 2017, 12:24 AM

I think any further work on Photocam / Image.Zoomable (eg. to rename to Image_Zoomable instead) can be done while reviewing the final EO APIs. To me this ticket can be closed now, the required features have been added.