Page MenuHomePhabricator

evas gl: make 9 patch work
ClosedPublic

Authored by kimcinoo on Wed, Nov 18, 2:57 AM.

Details

Summary

The 9 patch is using image_stretch_region_get, but GL did not override it.
So the 9 patch did not work for GL engine at all.

Test Plan

Evas_Object*img = evas_object_image_filled_add(evas);
evas_object_image_file_set(img, "test.9.png", 0);
evas_object_show(img);

Diff Detail

Repository
rEFL core/efl
Branch
evas_png.make_9patch_work_for_gl
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 17403
Build 11665: arc lint + arc unit
kimcinoo created this revision.Wed, Nov 18, 2:57 AM
kimcinoo requested review of this revision.Wed, Nov 18, 2:57 AM
jsuya added inline comments.Wed, Nov 18, 4:56 PM
src/modules/evas/engines/gl_generic/evas_engine.c
2221

It seems we need null check for *horizontal and *vertical.
if (*horizontal) *horizontal = im->cache_entry.stretch.horizontal.region;
I think you should do this. What do you think?

kimcinoo updated this revision to Diff 31253.Wed, Nov 18, 5:41 PM

add NULL check of horizontal and vertical

kimcinoo marked an inline comment as done.Wed, Nov 18, 5:44 PM
kimcinoo added inline comments.
src/modules/evas/engines/gl_generic/evas_engine.c
2221

You probably mean the null check for horizontal and vertical.
So I have added line to check value of horizontal and vertical as below;

if (!horizontal || !vertical) return EINA_FALSE;

Hermet requested changes to this revision.Thu, Nov 19, 3:12 AM

Please check my comments.

src/modules/evas/engines/gl_generic/evas_engine.c
2209

In this case, what do we supposed to do? either way it breaks the behavior.
If we don't allow any excuse, this function should not allow null horizontal nor vertical,
so my opinion, this handling is unnecessary.

2211

you can make it simpler
if (!gim || !gimp->im) return EINA_FALSE;

This revision now requires changes to proceed.Thu, Nov 19, 3:12 AM
kimcinoo updated this revision to Diff 31254.Thu, Nov 19, 3:59 AM
kimcinoo marked an inline comment as done.

Update based on review comment.

kimcinoo marked 2 inline comments as done.Thu, Nov 19, 4:27 AM
kimcinoo added inline comments.
src/modules/evas/engines/gl_generic/evas_engine.c
2209

At the first time when I got a review comment from @jsuya
I though that the NULL check is not necessary, because this image_stretch_region_get is used by Evas internally only.
But with some consideration, and looking at following lines of image_max_size_get;

if (maxw) *maxw = gl_context->shared->info.max_texture_size;
if (maxh) *maxh = gl_context->shared->info.max_texture_size;

I finally added the line checking NULL.
But because it is internal API, so I would like to go without NULL check.
It is responsibility of caller.

Hermet accepted this revision.Thu, Nov 19, 6:21 PM
This revision is now accepted and ready to land.Thu, Nov 19, 6:21 PM
Hermet closed this revision.Thu, Nov 19, 6:25 PM