Page MenuHomePhabricator

efl.ui.image
Closed, ResolvedPublic

Description

class Efl.Ui.Image @beta
├ (P) scalable
├ (P) align
├ (P) icon
├ (E) drop

Related Objects

StatusAssignedTask
OpenNone
ResolvedNone
ResolvedNone
ResolvedNone
Resolvedbu5hm4n
OpenNone
ResolvedNone
ResolvedNone
ResolvedNone
ResolvedNone
ResolvedNone
ResolvedNone
Resolvedcedric
OpenNone
OpenNone
ResolvedNone
ResolvedNone
Resolvedsegfaultxavi
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
ResolvedNone
OpenNone
bu5hm4n created this task.May 3 2019, 11:31 AM
bu5hm4n triaged this task as TODO priority.
zmike moved this task from Backlog to Evaluating on the efl: api board.Jun 12 2019, 9:43 AM

also includes:

struct @beta Efl.Ui.Image_Progress
{
   [[
     Structure associated with smart callback 'download,progress'.
   ]]
   now:   double; [[Current percentage]]
   total: double; [[Total percentage]]
}

struct @beta Efl.Ui.Image_Error
{
   [[
     Structure associated with smart callback 'download,progress'.
   ]]
   status:     int; [[Error status of the download]]
   open_error: bool; [[$true if the error happened when opening the file, $false otherwise]]
}
zmike added a comment.Jun 12 2019, 9:47 AM

The progress struct seems okay, the error struct is weird and I hate it; if there's no error then the status should be 0 since that's how Eina_Error works (and status is an Eina_Error), and there should be no bool.

Also worth noting that these structs are emitted from io copier events.

Sorry, but the concept of Total percentage confuses me. Isn't that always 100 by definition?

If status is an Eina_Error, shouldn't the eo file reflect it? If it cannot, then I think we have a critical dependency here which needs fixing before anything else.

I am wondering if we could not get rid of Efl.Ui.Image_Error and just use Eina_Error.

As for total percentage, if I remember correctly it is to reflect the multiple step of loading the image. You could be on a 50% of the downloading, but there is still decompressing, uploading texture and so on, so total percentage might be 25% with no indication on time that it would take to do the other percent.

I am wondering if we could not get rid of Efl.Ui.Image_Error and just use Eina_Error.

+1

As for total percentage, if I remember correctly it is to reflect the multiple step of loading the image. You could be on a 50% of the downloading, but there is still decompressing, uploading texture and so on, so total percentage might be 25% with no indication on time that it would take to do the other percent.

Then that needs a different name (and better docs). Even with your description it is still confusing :D

zmike added a comment.Jun 14 2019, 7:14 AM

I am wondering if we could not get rid of Efl.Ui.Image_Error and just use Eina_Error.

As for total percentage, if I remember correctly it is to reflect the multiple step of loading the image. You could be on a 50% of the downloading, but there is still decompressing, uploading texture and so on, so total percentage might be 25% with no indication on time that it would take to do the other percent.

According to efl io copier internals, the total value is always based on the size of the buffer, which in this case means that total should always be 100...

Diffusion closed subtask T7562: efl.input.interface as Resolved.
zmike added a comment.Aug 2 2019, 6:23 AM

Alright, looking at this:

scalable seems like it should be reduced to T7927?
align feels like we could replicate with Efl.Gfx.Arrangement (and thus remove the property here)
drop we can leave as beta (or comment out entirely) for now since I think this probably isn't supposed to be in this class.

In summary, I think this widget can be reduced to a single icon_name property (which should be renamed to avoid conflict).

  • scalable: hmmm.. there's a weird relationship there, since the docs say if $false, the internal image is not scaled up no matter what the scale type is.. So these two booleans seem to go further than the scale_type in Efl.Gfx.Image. Not sure if the implementation supports it, though.
  • align: yeah, Efl.Gfx.Arrangement controls the alignment of the content inside the container so it fits nicely here. It also has a padding property that wouldn't be used.
  • drop: it should go away, sure. We already have @beta events, but I am not sure how the mono docs react to them.

So this widget would be just a wrapper around Efl.Gfx.Image implementing a bunch of additional interfaces and providing an icon_name shortcut for file names? Seems about right to me.

Why do not we have image object? along side with image viewer/canvas
Almost all systems have image object, which is not directly related to viewing the image.

It contains:

  • image Content
  • image size
  • image type / format
  • load/save image

Then We can have ImageBitmap object, which contains bitmap of pixels when rendering image or manipulate or used for image processing

zmike added a comment.Aug 2 2019, 11:50 AM
  • scalable: hmmm.. there's a weird relationship there, since the docs say if $false, the internal image is not scaled up no matter what the scale type is.. So these two booleans seem to go further than the scale_type in Efl.Gfx.Image. Not sure if the implementation supports it, though.

I think this can be squashed into the 'none' scale hint.

  • align: yeah, Efl.Gfx.Arrangement controls the alignment of the content inside the container so it fits nicely here. It also has a padding property that wouldn't be used.

Padding could even be kept since it's pretty trivial to implement? Not high priority though.

  • drop: it should go away, sure. We already have @beta events, but I am not sure how the mono docs react to them.

    So this widget would be just a wrapper around Efl.Gfx.Image implementing a bunch of additional interfaces and providing an icon_name shortcut for file names? Seems about right to me.

That's always been the purpose, we just had too many duplicated things.

Why do not we have image object? along side with image viewer/canvas
Almost all systems have image object, which is not directly related to viewing the image.

It contains:

  • image Content
  • image size
  • image type / format
  • load/save image

    Then We can have ImageBitmap object, which contains bitmap of pixels when rendering image or manipulate or used for image processing

There is already efl.gfx.image (interface), efl.gfx.buffer (interface) and efl.canvas.image (class) for lower level image operations. This ticket is only for the higher level widget class.

cedric added a comment.Aug 2 2019, 1:04 PM

@zmike I mean, I can not see abstract class to present Image class, canvas is related to view image. but I think there should be Image class to manipulate and interact directly with image object.

EFL is a scenegraph UI system, not a direct rendering/paint oriented system. This lead to some different behavior. For example, having an image class that you give to an image painter make sense in paint oriented system, but in a scenegraph system the library itself will do all the work of opening the image (asynchronously if requested), deduplicating it and then drawing it when needed. This task is about the higher level of abstraction that Efl.Ui. namespace is about. We are not considering direct rendering API as part of the first release of EFL unified API.

ali.alzyod added a comment.EditedAug 2 2019, 1:36 PM

@cedric Thank you for your comments :) and explanations. And I am sorry if it bother you

I think Image object should not be part of rendering system (this is the idea), for Image properties (viewing is not part of image properties),properties should be like ImageType, ImageSize, ImageData,

For example if want to save jpg image as png image, So I think the part related to loading image into canvas then save image from canvas, has some waste process(user does not need it).

I think even for some other SceneGraph systems image is presented (I did quick search) like in https://codedocs.xyz/openscenegraph/OpenSceneGraph/a02446.html where It have image object.

I am used for look at things from user perspective (Like Tizen developer), he compare library with other similar framework like UIKit or Android

We are not considering direct rendering API as part of the first release of EFL unified API.
This make me excited a lot forward next releases

zmike added a comment.Aug 22 2019, 5:14 AM

I smashed align out. I wonder if we could do something with efl.file to replace icon in some way.

zmike added a comment.Aug 22 2019, 5:39 AM

scalable is gone, drop is @beta (and we can ignore it). All that's left is icon.

The icon property seems pretty related to images so I think it fits nicely here.
It could be moved to Efl.File if we added a mechanism similar to eina_vpath, but that looks more inconvenient for the user, no?

In efl_ui_image.eo there exist two structs (Efl.Ui.Image_Progress and Efl.Ui.Image_Error) that do not seem to be in use anywhere.

zmike added a comment.Aug 27 2019, 4:47 AM

Alright, we'll keep it here then.

The structs aren't currently used, so they can be marked explicitly as beta for now?

Why not remove those structs? I think errors were absorbed into Eina.Error but I dunno about the Progress one.

zmike added a comment.Aug 27 2019, 5:24 AM

The progress one is supposed to be for image downloading events I think. It's actually used internally iirc but isn't in eo.

I think this looks fine by now ?

woohyun added a comment.EditedAug 29 2019, 2:42 AM

@zmike @segfaultxavi @bu5hm4n

Hmm. I'm worrying of implementing "Efl.Player" interface (T7877) from this class.

It only implements "playable" and "play", and they are only small parts of the interface,
and I think this interface is something not for GIF player but for Media player.

Could we cut off the relation with "Efl.Player" by changing the property name (or creating new interface for these "playable" and "play") ?

bu5hm4n added a subscriber: Hermet.Aug 29 2019, 3:50 AM

Right now this only works for starting the playing of edje objects, where i do not know if this is even required, as the edje object will play by itself, and freezing the edje object will have the same effekt, as telling the object to pause

What do you think of keeping the efl_player interface beta for now, this would mean that the player interface is not visible to the outside world / c# bindings, but the code can stay as it is right now, and we still can improve that later on.

There was also more talk with @Hermet about the player interface for the animation player widget, which would also require a few adjustments, as of right now, noone implements the full interface, mute and volume is never implemented, so there is also more to check for efl_player.

zmike added a comment.Aug 29 2019, 7:46 AM

Right now this only works for starting the playing of edje objects, where i do not know if this is even required, as the edje object will play by itself, and freezing the edje object will have the same effekt, as telling the object to pause

What do you think of keeping the efl_player interface beta for now, this would mean that the player interface is not visible to the outside world / c# bindings, but the code can stay as it is right now, and we still can improve that later on.

There was also more talk with @Hermet about the player interface for the animation player widget, which would also require a few adjustments, as of right now, noone implements the full interface, mute and volume is never implemented, so there is also more to check for efl_player.

This is probably fine?

bu5hm4n moved this task from Evaluating to Stabilized on the efl: api board.Aug 29 2019, 8:54 AM
woohyun added a comment.EditedAug 29 2019, 3:30 PM

I agree to keep efl_player interface @beta.

efl_part ? I was talking about efl_player :-D

efl_part ? I was talking about efl_player :-D

Sorry! I just modified my comment :)

This needs implementation of Efl.Gfx.Image and Efl.Gfx.Image_Load_Controller.

zmike added a comment.Tue, Sep 24, 1:42 PM

Just for posterity, this has implementations of the above-mentioned interfaces.

cedric closed subtask T7877: efl.player as Resolved.Tue, Sep 24, 3:23 PM
zmike closed subtask T7876: efl.gfx.image_load_controller as Resolved.
zmike closed subtask T7578: efl.ui.view as Resolved.Thu, Sep 26, 8:12 AM