Page MenuHomePhabricator

evas filter: make curve work for every
ClosedPublic

Authored by kimcinoo on Wed, Jun 12, 2:07 AM.

Details

Summary

If an input buffer and an output buffer for the curve filter are same, it reads
and writes to the same texture which behavior is not defined. I could not find
good reference for this, but following could be a reference.

https://stackoverflow.com/questions/11410292/opengl-read-and-write-to-the-same-texture

The texture gets 0 color value as a result. So the curve filter does not work.
This patch makes the curve filter use different input and output buffer.

Test Plan

This attached file could explain what 'read and write to the same texture' is.

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.
kimcinoo created this revision.Wed, Jun 12, 2:07 AM
kimcinoo requested review of this revision.Wed, Jun 12, 2:07 AM
kimcinoo updated this revision to Diff 22701.Wed, Jun 12, 2:10 AM

Use correct name blendcmd instead of curvecmd

kimcinoo edited the test plan for this revision. (Show Details)Wed, Jun 12, 2:32 AM
kimcinoo edited the test plan for this revision. (Show Details)

While reading evas_filter_command_grow_add(), it could generate temporary buffer in a specific condition,

1288 if ((inbuf != outbuf) && out->dirty)
1289 {
1290 tmp = evas_filter_temporary_buffer_get(ctx, in->w, in->h, in->alpha_only, 1);
1291 EINA_SAFETY_ON_NULL_RETURN_VAL(tmp, NULL);
1292 growbuf = tmp->id;
1293 }

That case, it blends buffers twice. in curve_add() and grow_add().
tmpbuf (for blur) => tmp2buf (for curve) => blend (tmpbuf + tmp2buf) => blend(tmpbuf + outbuf);

What if you pass evas_filter_command_curve_add( tmpbuf, outbuf) ?
tmpbuf (for blur) => outbuf (for curve) => blend (tmpbuf + outbuf) => SAVE BLENDING!

Hermet requested changes to this revision.Wed, Jun 12, 9:34 PM

Please check my comment.

This revision now requires changes to proceed.Wed, Jun 12, 9:34 PM
Hermet added a comment.EditedWed, Jun 12, 9:37 PM

And It's very suspicious at this condition.

1288 if ((inbuf != outbuf) && out->dirty)

This condition might be related with this issue... Something make it out of the condition ...

kimcinoo updated this revision to Diff 22713.Wed, Jun 12, 11:28 PM
kimcinoo edited the test plan for this revision. (Show Details)

Use a temp buffer on grow_add side as well to reduce calling blend_add based on
valuable hermet's comment.

And It's very suspicious at this condition.

1288 if ((inbuf != outbuf) && out->dirty)

This condition might be related with this issue... Something make it out of the condition ...

I do not think so. Because the grow_add calls the curve_add using same buffer regardless of creating temp buffer.

kimcinoo updated this revision to Diff 22714.Wed, Jun 12, 11:41 PM

Update comment.

Hermet requested changes to this revision.Thu, Jun 13, 11:37 PM

Please check a comment.

src/lib/evas/filters/evas_filter.c
1288

Let's review this sequence and compare the current grow_add() logic.

If the output buffer is not dirty, (we can draw on output buffer directly?)
Input (In Buffer) -> [Grow] -> Output (Temp Buffer) -> [Curve] -> Output (Out Buffer)

If the output buffer is dirty,
Input (In Buffer) -> [Grow] -> Output (Temp Buffer) -> [Curve] -> Output (Temp Buffer2) -> [Blend] -> Output (Out Buffer)

Currently growbuf == outbuf is out of date. Logically, we can't use it anymore.

This revision now requires changes to proceed.Thu, Jun 13, 11:37 PM

Honestly I do not get what "Currently growbuf == outbuf is out of date." is. Do you mean that the outbuf is out of date, because it is same with growbuf? And is that for the first case that the outbuf is not dirty?

(1) If the condition "(inbuf != outbuf) && out->dirty" is false, then " growbuf == outbuf" and it will work as below.

blur_add(inbuf, growbuf(outbuf)) -> curve_add(growbuf(outbuf), tmp) -> blend(tmp, outbuf)
(i.e.) inbuf -> [Blur] -> growbuf(outbuf) -> [Curve] -> tmp -> [Blend] -> outbuf

(2) If the condition "(inbuf != outbuf) && out->dirty" is true, then " growbuf == tmp" and it will work as below.

blur_add(inbuf, growbuf(tmp)) -> curve_add(growbuf(tmp), tmp2) -> blend(tmp2, outbuf)
(i.e.) inbuf -> [Blur] -> growbuf(tmp) -> [Curve] -> tmp2 -> [Blend] -> outbuf

Without this patch each step could be following.

inbuf -> [Blur] -> growbuf(outbuf) -> [Curve] -> growbuf(outbuf) -> [Blend] -> outbuf
inbuf -> [Blur] -> growbuf(tmp) -> [Curve] ->growbuf(tmp) -> [Blend] -> outbuf

Then can we use the outbuf in these cases?

Hermet added a comment.EditedMon, Jun 17, 10:06 PM

Honestly I do not get what "Currently growbuf == outbuf is out of date." is. Do you mean that the outbuf is out of date, because it is same with growbuf? And is that for the first case that the outbuf is not dirty?

(1) If the condition "(inbuf != outbuf) && out->dirty" is false, then " growbuf == outbuf" and it will work as below.

blur_add(inbuf, growbuf(outbuf)) -> curve_add(growbuf(outbuf), tmp) -> blend(tmp, outbuf)
(i.e.) inbuf -> [Blur] -> growbuf(outbuf) -> [Curve] -> tmp -> [Blend] -> outbuf

(2) If the condition "(inbuf != outbuf) && out->dirty" is true, then " growbuf == tmp" and it will work as below.

blur_add(inbuf, growbuf(tmp)) -> curve_add(growbuf(tmp), tmp2) -> blend(tmp2, outbuf)
(i.e.) inbuf -> [Blur] -> growbuf(tmp) -> [Curve] -> tmp2 -> [Blend] -> outbuf

Without this patch each step could be following.

inbuf -> [Blur] -> growbuf(outbuf) -> [Curve] -> growbuf(outbuf) -> [Blend] -> outbuf
inbuf -> [Blur] -> growbuf(tmp) -> [Curve] ->growbuf(tmp) -> [Blend] -> outbuf

Then can we use the outbuf in these cases?

The point here is, logically, the outbuf should be used as outter, not temporary. That's why we are saying it outbuf.

Now it's turned out that, curve requires temporary buffer always, grow buffer shouldn't use outbufer as temporary since it must have a temporary buffer.

The point here is, logically, the outbuf should be used as outter, not temporary. That's why we are saying it outbuf.

Now it's turned out that, curve requires temporary buffer always, grow buffer shouldn't use outbufer as temporary since it must have a temporary buffer.

I am sorry. I do not get your point exactly yet. Are you saying about line 1332 below calling evas_filter_command_curve_add with growbuf as an input buffer?

[line 1332] threshcmd = evas_filter_command_curve_add(ctx, draw_context, growbuf, tmp->id,
Hermet accepted this revision.Tue, Jun 18, 6:27 PM
This revision is now accepted and ready to land.Tue, Jun 18, 6:27 PM

Maybe I could change furthermore.

This revision was automatically updated to reflect the committed changes.