Page MenuHomePhabricator

evas: add support for stretchable region.
ClosedPublic

Authored by cedric on Jun 14 2019, 5:02 PM.

Details

Summary

This is the first step into introducing support for Android 9 patch
format (extension: .9.png). The principle is to expose a new property
on image object that define a complete behavior incompatible with other
border and fill logic. The reason is that 9 patch allow for any number
of stretchable area inside an image, not just for each corner. The way
to define this is by giving a pointer to an array of the proper type
that define stretchable region relative to each other.

The logic being slightly more complex than the border and fill logic,
it is slightly slower. If you are just defining corner on your image
for something like a button, you would still get better performance
using border. I will try to make edje_cc detect those case and fallback
to border when possible.

Depends on D9095

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
cedric created this revision.Jun 14 2019, 5:02 PM
cedric updated this revision to Diff 22744.Jun 14 2019, 5:05 PM
cedric edited the summary of this revision. (Show Details)
zmike requested changes to this revision.Jun 17 2019, 9:45 AM
zmike added a subscriber: segfaultxavi.
zmike added inline comments.
src/lib/efl/interfaces/efl_gfx_image.eo
171

This feels like it needs separate set and get docs? @segfaultxavi

src/lib/evas/canvas/evas_object_image.c
597

param should be const

630

iterator params should be const

2268

This could probably be a while loop for clarity?

2405

?

This revision now requires changes to proceed.Jun 17 2019, 9:45 AM

Weird indentation suggests you mixed tabs and spaces... there's a special circle in hell for people of your kind.

src/lib/efl/interfaces/efl_gfx_image.eo
45
This struct holds the description of a stretchable zone in one dimension (vertical or horizontal). Used when scaling an image.

$offset + $length should be smaller than image size in that dimension.
49
offset: uint; [[First pixel of the stretchable zone, starting at 0.]]
length: uint; [[Length of the stretchable zone in pixels.]]
171

The iterator that self-destructs sounds weird yeah. I guess it only does that when the user provides it, right? (meaning, in the setter).
What happens with the getter?

If behaviors are not much different besides that, maybe we can get away by simply saying that

When the zones are set by the user, the method will walk the iterators once and then destroy them.
When the zones are retrieved by the user, it is his responsibility to destroy the iterators.

Also, use @ to mark other properties like @.border_scale.

182

Stop adding empty docs FFS!

cedric planned changes to this revision.Jun 19 2019, 11:11 AM
cedric added inline comments.
src/lib/efl/interfaces/efl_gfx_image.eo
171

Iterator can only be used once. They are the responsibility of whoever receive it. That is why they are not "self destructing", but they are being destroyed by the owner of it, which can only be the function that is using it.

From that angle, the behavior is the same in case of a getter and a setter. The setter being the one who walk the iterator and exhaust its content, it is the one that own it and should destroy it. The getter return an iterator that the caller will iterate over, exhaust its content and should destroy once done.

src/lib/evas/canvas/evas_object_image.c
597

No. An iterator can only be used one time. It doesn't make sense to give an iterator that can be used one time and not have the called function take full care of it aka destroy it. As such, it can not be const.

630

No. An iterator can only be used one time. It doesn't make sense to give an iterator that can be used one time and not have the called function take full care of it aka destroy it. As such, it can not be const.

cedric updated this revision to Diff 22839.Jun 19 2019, 12:13 PM

Rebase and take comment into account.

Docs look mostly fine now... remaining nuances should be taken care of in a general documentation sweep later on.

cedric updated this revision to Diff 22894.Thu, Jun 20, 4:59 PM
cedric retitled this revision from evas: add support for stretchable zone. to evas: add support for stretchable region..
cedric edited the summary of this revision. (Show Details)

Rebase and rename.

Do we really need to make it compatible with Android fully or just only for major usage?
Yet I've never seen practical images, that containing multiple stretchable regions, used.
And I'm worrying it just increase complexity in several points unnecessarily, maybe just 9 case is enough.

Do we really need to make it compatible with Android fully or just only for major usage?

For some user, a reduce support might be ok, but as we get more user, we will get bug reports and complaints. Having the right infrastructure in place from the beginning without shortcut avoid having to deal with the future pain here. Not having a hack to maintain, seems a safer long term bet.

Yet I've never seen practical images, that containing multiple stretchable regions, used.

On Android the classical example are the bubble used in instant messaging application where the side arrow is not stretchable and positioned somewhere else than in the 4 corners.

And I'm worrying it just increase complexity in several points unnecessarily, maybe just 9 case is enough.

I am afraid that corner only case isn't compliant with Android format and will lead to an hacked together solution that if exposed to more varied user will fall apart and impact our long term maintenance. Basically, I expect that we will have to bite the bullet at some point. We can delay it with short term small bit of code, or just do it right from the beginning.

Hermet accepted this revision.Mon, Jul 8, 10:48 PM

@cedric There were not any issues, Tizen has supported android 9 patch since 2012. (even we couldn't support margin area)

Now I'm wonder, Android is not the standard, do we really need to follow android spec fully?
Of course, It would be good if we can support a convenient method for android resource compatibility.

But i'm afraid of complexity and maintaining issues even these interfaces are not certain by other image types nor usages.

Now I'm wonder, Android is not the standard, do we really need to follow android spec fully?

It is a defacto standard in this case. Not following the specification mean that their will be incomprehension by users and complain of why did we do something different without a good reason. I don't think there is a good reason for not following something so trivial.

cedric updated this revision to Diff 23184.Wed, Jul 10, 10:09 AM

Rebase and make it more robust.

This revision was not accepted when it landed; it landed in state Needs Review.Fri, Jul 12, 10:02 AM
This revision was automatically updated to reflect the committed changes.