Re: [E-devel] [EGIT] [core/efl] master 05/06: Evas GL:Bind texture to correct one.

[The Rasterman]

On Wed, 04 Jan 2017 06:53:20 + Andrew Williams  said:

> Could we look at this the other way around?
> Say you must add a test and the harnesses will get better out of necessity?

if i have to add a test to our harness for every bug i fix... you can bet you i
will stop fixing them. it will multiply the time it takes by several fold (at
least 2-3 if not more). we can't apply rules to people that we wouldnt be
willing to follow ourselves. i sure wouldn't be willing to given the current
test suite. and yes - i've added tests to it and frankly its as much work
adding tests as making an original feature. fixing a bug is mostly far simpler
than creating the feature ... and most of the test case work is just
boilerplate shuffling around, waiting for re-running autogen.sh to rebuild
makefiles etc. etc. :(

> It is said that legacy code is any that does not have automated test
> coverage ;)

yeah. we need to improve things. but our test suite setup really adds lots of
overhead to adding anything... and it's even worse when a test then fails to
figure out why ... :(

> Andy
> On Tue, 3 Jan 2017 at 23:29, Carsten Haitzler  wrote:
> 
> > On Tue, 03 Jan 2017 15:14:55 + Mike Blumenkrantz
> >  said:
> >
> > > To elaborate, my issue in this case is that the test plan is "Tizen 3.0"
> > as
> > > though that's something that can be tested. Yes, the issue is described,
> >
> > i know. it's "stupid". to the point where i ignore it. realistically just
> > leave
> > it blank instead of fill it in with a pointless test plan. a test plan
> > should
> > be "run app x, do action y, see if thing z happens or not" or "make check:
> > test
> > xyz fails or succeeds" which might be better as its automated. :) and
> > that's
> > where i'm picking this up from... :)
> >
> > > but for most bug fixes the test plan is something like "follow steps 1,
> > 2,
> > > 3 in app XYZ", where XYZ is a publicly available and easily executed app.
> >
> > dude. do you know how many "tizen custom" patches efl in tizen has? it's
> > 1000+.
> > it's also efl 1.16 plus 1000+ patches. it's missing lots of fixes and
> > improvements in 1.17, 1.18, so there is very little hope of reproducing
> > anything really equivalent...
> >
> > > In cases such as this one, the burden is now on upstream developers to
> > > create a new test case according to the listed steps (which may or may
> > not
> > > be accurately described) in order to verify that the issue has not
> > > regressed. This is not going to scale based on our available resources.
> >
> > i know it won't. we do need to reduce the work needed to add a test
> > regardless
> > so it's EASY to say "you must add a test". right now it's not easy to ask
> > that
> > of people.
> >
> > > On Mon, Jan 2, 2017 at 6:33 PM Carsten Haitzler 
> > > wrote:
> > >
> > > > On Mon, 02 Jan 2017 15:50:28 + Mike Blumenkrantz
> > > >  said:
> > > >
> > > > > Hi,
> > > > >
> > > > > I am not sure that "Tizen 3.0" is a valid test plan for our
> > purposes. If
> > > > a
> > > > > fix is submitted upstream, I think it makes sense to also submit a
> > > > > corresponding test to verify that the issue is solved and does not
> > recur
> > > > in
> > > > > the future.
> > > >
> > > > i'm not so sure that "test plan" is worth paying attention to...
> > > >
> > > > BUT... "for every bug you fix with an @fix as it was an existing bug
> > in a
> > > > release, provide a test case to check for the bug so it doesn't come
> > back"
> > > > is
> > > > what i think you are getting at.
> > > >
> > > > at this stage that'd raise the cost of doing bug fixes a LOT. i think
> > we
> > > > need
> > > > to improve our testing harness first - e.g. a thread a few weeks back
> > about
> > > > this. we need a far simpler way to put tests together. we also need
> > good
> > > > "template" harnesses - e.g. a raw evas canvas test that sets up a
> > canvas
> > > > with
> > > > some default state (a white, pink, blue, green rect background or
> > > > transparent)
> > > > then you can do things like:
> > > >
> > > > code1();
> > > > code2();
> > > > CHECK();
> > > > code3();
> > > > code4();
> > > > code5();
> > > > CHECK();
> > > > code6();
> > > > ...
> > > >
> > > > which would force manual renders of the state at the time when you
> > > > CHECK(); and
> > > > save an image and compare against a known good one (stored in tree)
> > etc.
> > > > etc.
> > > > as well as just drop that test into a dir without having to modify
> > > > makefiles
> > > > and thus basically do a full rebuild (autogen etc.) just to begin to do
> > > > testing.
> > > >
> > > > before we start making rules like this - we should get our testing
> > harness
> > > > into
> > > > an optimal state.
> > > >
> > > > > Thoughts?
> > > > >
> > > > > On Mon, Jan 2, 2017 at 2:44 AM Minkyoung Kim 
> > > > wrote:
> > > > >
> > > > > > 

Re: [E-devel] [EGIT] [core/efl] master 05/06: Evas GL:Bind texture to correct one.

[Andrew Williams]

Could we look at this the other way around?
Say you must add a test and the harnesses will get better out of necessity?

It is said that legacy code is any that does not have automated test
coverage ;)

Andy
On Tue, 3 Jan 2017 at 23:29, Carsten Haitzler  wrote:

> On Tue, 03 Jan 2017 15:14:55 + Mike Blumenkrantz
>  said:
>
> > To elaborate, my issue in this case is that the test plan is "Tizen 3.0"
> as
> > though that's something that can be tested. Yes, the issue is described,
>
> i know. it's "stupid". to the point where i ignore it. realistically just
> leave
> it blank instead of fill it in with a pointless test plan. a test plan
> should
> be "run app x, do action y, see if thing z happens or not" or "make check:
> test
> xyz fails or succeeds" which might be better as its automated. :) and
> that's
> where i'm picking this up from... :)
>
> > but for most bug fixes the test plan is something like "follow steps 1,
> 2,
> > 3 in app XYZ", where XYZ is a publicly available and easily executed app.
>
> dude. do you know how many "tizen custom" patches efl in tizen has? it's
> 1000+.
> it's also efl 1.16 plus 1000+ patches. it's missing lots of fixes and
> improvements in 1.17, 1.18, so there is very little hope of reproducing
> anything really equivalent...
>
> > In cases such as this one, the burden is now on upstream developers to
> > create a new test case according to the listed steps (which may or may
> not
> > be accurately described) in order to verify that the issue has not
> > regressed. This is not going to scale based on our available resources.
>
> i know it won't. we do need to reduce the work needed to add a test
> regardless
> so it's EASY to say "you must add a test". right now it's not easy to ask
> that
> of people.
>
> > On Mon, Jan 2, 2017 at 6:33 PM Carsten Haitzler 
> > wrote:
> >
> > > On Mon, 02 Jan 2017 15:50:28 + Mike Blumenkrantz
> > >  said:
> > >
> > > > Hi,
> > > >
> > > > I am not sure that "Tizen 3.0" is a valid test plan for our
> purposes. If
> > > a
> > > > fix is submitted upstream, I think it makes sense to also submit a
> > > > corresponding test to verify that the issue is solved and does not
> recur
> > > in
> > > > the future.
> > >
> > > i'm not so sure that "test plan" is worth paying attention to...
> > >
> > > BUT... "for every bug you fix with an @fix as it was an existing bug
> in a
> > > release, provide a test case to check for the bug so it doesn't come
> back"
> > > is
> > > what i think you are getting at.
> > >
> > > at this stage that'd raise the cost of doing bug fixes a LOT. i think
> we
> > > need
> > > to improve our testing harness first - e.g. a thread a few weeks back
> about
> > > this. we need a far simpler way to put tests together. we also need
> good
> > > "template" harnesses - e.g. a raw evas canvas test that sets up a
> canvas
> > > with
> > > some default state (a white, pink, blue, green rect background or
> > > transparent)
> > > then you can do things like:
> > >
> > > code1();
> > > code2();
> > > CHECK();
> > > code3();
> > > code4();
> > > code5();
> > > CHECK();
> > > code6();
> > > ...
> > >
> > > which would force manual renders of the state at the time when you
> > > CHECK(); and
> > > save an image and compare against a known good one (stored in tree)
> etc.
> > > etc.
> > > as well as just drop that test into a dir without having to modify
> > > makefiles
> > > and thus basically do a full rebuild (autogen etc.) just to begin to do
> > > testing.
> > >
> > > before we start making rules like this - we should get our testing
> harness
> > > into
> > > an optimal state.
> > >
> > > > Thoughts?
> > > >
> > > > On Mon, Jan 2, 2017 at 2:44 AM Minkyoung Kim 
> > > wrote:
> > > >
> > > > > jpeg pushed a commit to branch master.
> > > > >
> > > > >
> > > > >
> > >
> http://git.enlightenment.org/core/efl.git/commit/?id=40e9da0101e2c9afb6f823c7b21c02a011cc5300
> > > > >
> > > > > commit 40e9da0101e2c9afb6f823c7b21c02a011cc5300
> > > > > Author: Minkyoung Kim 
> > > > > Date:   Mon Jan 2 15:29:48 2017 +0900
> > > > >
> > > > > Evas GL:Bind texture to correct one.
> > > > >
> > > > > Summary:
> > > > > If user bind textureA and want to use it continuously, do not
> call
> > > > > glBindTexture(textureA) again.
> > > > > But expect that textureA will be binding.
> > > > > So EvasGL sould not change binded texture silently.
> > > > > Restore texture to previous bound one after allocating new
> texture.
> > > > > And when destroy texture, reset texture to 0 if it is current
> bound
> > > > > texture.
> > > > >
> > > > > Test Plan: Tizen 3.0
> > > > >
> > > > > Reviewers: wonsik, dkdk, cedric, jpeg
> > > > >
> > > > > Reviewed By: jpeg
> > > > >
> > > > > Differential Revision: https://phab.enlightenment.org/D4524
> > > > > ---
> > > > >  

Re: [E-devel] [EGIT] [core/efl] master 05/06: Evas GL:Bind texture to correct one.

[The Rasterman]

On Tue, 03 Jan 2017 15:14:55 + Mike Blumenkrantz
 said:

> To elaborate, my issue in this case is that the test plan is "Tizen 3.0" as
> though that's something that can be tested. Yes, the issue is described,

i know. it's "stupid". to the point where i ignore it. realistically just leave
it blank instead of fill it in with a pointless test plan. a test plan should
be "run app x, do action y, see if thing z happens or not" or "make check: test
xyz fails or succeeds" which might be better as its automated. :) and that's
where i'm picking this up from... :)

> but for most bug fixes the test plan is something like "follow steps 1, 2,
> 3 in app XYZ", where XYZ is a publicly available and easily executed app.

dude. do you know how many "tizen custom" patches efl in tizen has? it's 1000+.
it's also efl 1.16 plus 1000+ patches. it's missing lots of fixes and
improvements in 1.17, 1.18, so there is very little hope of reproducing
anything really equivalent...

> In cases such as this one, the burden is now on upstream developers to
> create a new test case according to the listed steps (which may or may not
> be accurately described) in order to verify that the issue has not
> regressed. This is not going to scale based on our available resources.

i know it won't. we do need to reduce the work needed to add a test regardless
so it's EASY to say "you must add a test". right now it's not easy to ask that
of people.

> On Mon, Jan 2, 2017 at 6:33 PM Carsten Haitzler 
> wrote:
> 
> > On Mon, 02 Jan 2017 15:50:28 + Mike Blumenkrantz
> >  said:
> >
> > > Hi,
> > >
> > > I am not sure that "Tizen 3.0" is a valid test plan for our purposes. If
> > a
> > > fix is submitted upstream, I think it makes sense to also submit a
> > > corresponding test to verify that the issue is solved and does not recur
> > in
> > > the future.
> >
> > i'm not so sure that "test plan" is worth paying attention to...
> >
> > BUT... "for every bug you fix with an @fix as it was an existing bug in a
> > release, provide a test case to check for the bug so it doesn't come back"
> > is
> > what i think you are getting at.
> >
> > at this stage that'd raise the cost of doing bug fixes a LOT. i think we
> > need
> > to improve our testing harness first - e.g. a thread a few weeks back about
> > this. we need a far simpler way to put tests together. we also need good
> > "template" harnesses - e.g. a raw evas canvas test that sets up a canvas
> > with
> > some default state (a white, pink, blue, green rect background or
> > transparent)
> > then you can do things like:
> >
> > code1();
> > code2();
> > CHECK();
> > code3();
> > code4();
> > code5();
> > CHECK();
> > code6();
> > ...
> >
> > which would force manual renders of the state at the time when you
> > CHECK(); and
> > save an image and compare against a known good one (stored in tree) etc.
> > etc.
> > as well as just drop that test into a dir without having to modify
> > makefiles
> > and thus basically do a full rebuild (autogen etc.) just to begin to do
> > testing.
> >
> > before we start making rules like this - we should get our testing harness
> > into
> > an optimal state.
> >
> > > Thoughts?
> > >
> > > On Mon, Jan 2, 2017 at 2:44 AM Minkyoung Kim 
> > wrote:
> > >
> > > > jpeg pushed a commit to branch master.
> > > >
> > > >
> > > >
> > http://git.enlightenment.org/core/efl.git/commit/?id=40e9da0101e2c9afb6f823c7b21c02a011cc5300
> > > >
> > > > commit 40e9da0101e2c9afb6f823c7b21c02a011cc5300
> > > > Author: Minkyoung Kim 
> > > > Date:   Mon Jan 2 15:29:48 2017 +0900
> > > >
> > > > Evas GL:Bind texture to correct one.
> > > >
> > > > Summary:
> > > > If user bind textureA and want to use it continuously, do not call
> > > > glBindTexture(textureA) again.
> > > > But expect that textureA will be binding.
> > > > So EvasGL sould not change binded texture silently.
> > > > Restore texture to previous bound one after allocating new texture.
> > > > And when destroy texture, reset texture to 0 if it is current bound
> > > > texture.
> > > >
> > > > Test Plan: Tizen 3.0
> > > >
> > > > Reviewers: wonsik, dkdk, cedric, jpeg
> > > >
> > > > Reviewed By: jpeg
> > > >
> > > > Differential Revision: https://phab.enlightenment.org/D4524
> > > > ---
> > > >  src/modules/evas/engines/gl_common/evas_gl_core.c | 9 -
> > > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/src/modules/evas/engines/gl_common/evas_gl_core.c
> > > > b/src/modules/evas/engines/gl_common/evas_gl_core.c
> > > > index 14d17f6..8292b5c 100644
> > > > --- a/src/modules/evas/engines/gl_common/evas_gl_core.c
> > > > +++ b/src/modules/evas/engines/gl_common/evas_gl_core.c
> > > > @@ -234,19 +234,26 @@ _texture_allocate_2d(GLuint tex, GLint ifmt,
> > GLenum
> > > > fmt, GLenum type, int w, int
> > > >  {
> > 

Re: [E-devel] [EGIT] [core/efl] master 05/06: Evas GL:Bind texture to correct one.

[Al Poole]

Outsource upscale?

On Tue, Jan 3, 2017 at 3:14 PM, Mike Blumenkrantz <
michael.blumenkra...@gmail.com> wrote:

> To elaborate, my issue in this case is that the test plan is "Tizen 3.0" as
> though that's something that can be tested. Yes, the issue is described,
> but for most bug fixes the test plan is something like "follow steps 1, 2,
> 3 in app XYZ", where XYZ is a publicly available and easily executed app.
>
> In cases such as this one, the burden is now on upstream developers to
> create a new test case according to the listed steps (which may or may not
> be accurately described) in order to verify that the issue has not
> regressed. This is not going to scale based on our available resources.
>
> On Mon, Jan 2, 2017 at 6:33 PM Carsten Haitzler 
> wrote:
>
> > On Mon, 02 Jan 2017 15:50:28 + Mike Blumenkrantz
> >  said:
> >
> > > Hi,
> > >
> > > I am not sure that "Tizen 3.0" is a valid test plan for our purposes.
> If
> > a
> > > fix is submitted upstream, I think it makes sense to also submit a
> > > corresponding test to verify that the issue is solved and does not
> recur
> > in
> > > the future.
> >
> > i'm not so sure that "test plan" is worth paying attention to...
> >
> > BUT... "for every bug you fix with an @fix as it was an existing bug in a
> > release, provide a test case to check for the bug so it doesn't come
> back"
> > is
> > what i think you are getting at.
> >
> > at this stage that'd raise the cost of doing bug fixes a LOT. i think we
> > need
> > to improve our testing harness first - e.g. a thread a few weeks back
> about
> > this. we need a far simpler way to put tests together. we also need good
> > "template" harnesses - e.g. a raw evas canvas test that sets up a canvas
> > with
> > some default state (a white, pink, blue, green rect background or
> > transparent)
> > then you can do things like:
> >
> > code1();
> > code2();
> > CHECK();
> > code3();
> > code4();
> > code5();
> > CHECK();
> > code6();
> > ...
> >
> > which would force manual renders of the state at the time when you
> > CHECK(); and
> > save an image and compare against a known good one (stored in tree) etc.
> > etc.
> > as well as just drop that test into a dir without having to modify
> > makefiles
> > and thus basically do a full rebuild (autogen etc.) just to begin to do
> > testing.
> >
> > before we start making rules like this - we should get our testing
> harness
> > into
> > an optimal state.
> >
> > > Thoughts?
> > >
> > > On Mon, Jan 2, 2017 at 2:44 AM Minkyoung Kim 
> > wrote:
> > >
> > > > jpeg pushed a commit to branch master.
> > > >
> > > >
> > > >
> > http://git.enlightenment.org/core/efl.git/commit/?id=
> 40e9da0101e2c9afb6f823c7b21c02a011cc5300
> > > >
> > > > commit 40e9da0101e2c9afb6f823c7b21c02a011cc5300
> > > > Author: Minkyoung Kim 
> > > > Date:   Mon Jan 2 15:29:48 2017 +0900
> > > >
> > > > Evas GL:Bind texture to correct one.
> > > >
> > > > Summary:
> > > > If user bind textureA and want to use it continuously, do not
> call
> > > > glBindTexture(textureA) again.
> > > > But expect that textureA will be binding.
> > > > So EvasGL sould not change binded texture silently.
> > > > Restore texture to previous bound one after allocating new
> texture.
> > > > And when destroy texture, reset texture to 0 if it is current
> bound
> > > > texture.
> > > >
> > > > Test Plan: Tizen 3.0
> > > >
> > > > Reviewers: wonsik, dkdk, cedric, jpeg
> > > >
> > > > Reviewed By: jpeg
> > > >
> > > > Differential Revision: https://phab.enlightenment.org/D4524
> > > > ---
> > > >  src/modules/evas/engines/gl_common/evas_gl_core.c | 9 -
> > > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/src/modules/evas/engines/gl_common/evas_gl_core.c
> > > > b/src/modules/evas/engines/gl_common/evas_gl_core.c
> > > > index 14d17f6..8292b5c 100644
> > > > --- a/src/modules/evas/engines/gl_common/evas_gl_core.c
> > > > +++ b/src/modules/evas/engines/gl_common/evas_gl_core.c
> > > > @@ -234,19 +234,26 @@ _texture_allocate_2d(GLuint tex, GLint ifmt,
> > GLenum
> > > > fmt, GLenum type, int w, int
> > > >  {
> > > > //if (!(*tex))
> > > > //   glGenTextures(1, tex);
> > > > +   GLint curr_tex = 0;
> > > > +   glGetIntegerv(GL_TEXTURE_BINDING_2D, _tex);
> > > > +
> > > > glBindTexture(GL_TEXTURE_2D, tex);
> > > > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S,
> > GL_CLAMP_TO_EDGE);
> > > > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T,
> > GL_CLAMP_TO_EDGE);
> > > > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER,
> GL_NEAREST);
> > > > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER,
> GL_NEAREST);
> > > > glTexImage2D(GL_TEXTURE_2D, 0, ifmt, w, h, 0, fmt, type, NULL);
> > > > -   glBindTexture(GL_TEXTURE_2D, 0);
> > > > +   glBindTexture(GL_TEXTURE_2D, (GLuint)curr_tex);
> > > >  }
> > > 

Re: [E-devel] [EGIT] [core/efl] master 05/06: Evas GL:Bind texture to correct one.

[Mike Blumenkrantz]

To elaborate, my issue in this case is that the test plan is "Tizen 3.0" as
though that's something that can be tested. Yes, the issue is described,
but for most bug fixes the test plan is something like "follow steps 1, 2,
3 in app XYZ", where XYZ is a publicly available and easily executed app.

In cases such as this one, the burden is now on upstream developers to
create a new test case according to the listed steps (which may or may not
be accurately described) in order to verify that the issue has not
regressed. This is not going to scale based on our available resources.

On Mon, Jan 2, 2017 at 6:33 PM Carsten Haitzler 
wrote:

> On Mon, 02 Jan 2017 15:50:28 + Mike Blumenkrantz
>  said:
>
> > Hi,
> >
> > I am not sure that "Tizen 3.0" is a valid test plan for our purposes. If
> a
> > fix is submitted upstream, I think it makes sense to also submit a
> > corresponding test to verify that the issue is solved and does not recur
> in
> > the future.
>
> i'm not so sure that "test plan" is worth paying attention to...
>
> BUT... "for every bug you fix with an @fix as it was an existing bug in a
> release, provide a test case to check for the bug so it doesn't come back"
> is
> what i think you are getting at.
>
> at this stage that'd raise the cost of doing bug fixes a LOT. i think we
> need
> to improve our testing harness first - e.g. a thread a few weeks back about
> this. we need a far simpler way to put tests together. we also need good
> "template" harnesses - e.g. a raw evas canvas test that sets up a canvas
> with
> some default state (a white, pink, blue, green rect background or
> transparent)
> then you can do things like:
>
> code1();
> code2();
> CHECK();
> code3();
> code4();
> code5();
> CHECK();
> code6();
> ...
>
> which would force manual renders of the state at the time when you
> CHECK(); and
> save an image and compare against a known good one (stored in tree) etc.
> etc.
> as well as just drop that test into a dir without having to modify
> makefiles
> and thus basically do a full rebuild (autogen etc.) just to begin to do
> testing.
>
> before we start making rules like this - we should get our testing harness
> into
> an optimal state.
>
> > Thoughts?
> >
> > On Mon, Jan 2, 2017 at 2:44 AM Minkyoung Kim 
> wrote:
> >
> > > jpeg pushed a commit to branch master.
> > >
> > >
> > >
> http://git.enlightenment.org/core/efl.git/commit/?id=40e9da0101e2c9afb6f823c7b21c02a011cc5300
> > >
> > > commit 40e9da0101e2c9afb6f823c7b21c02a011cc5300
> > > Author: Minkyoung Kim 
> > > Date:   Mon Jan 2 15:29:48 2017 +0900
> > >
> > > Evas GL:Bind texture to correct one.
> > >
> > > Summary:
> > > If user bind textureA and want to use it continuously, do not call
> > > glBindTexture(textureA) again.
> > > But expect that textureA will be binding.
> > > So EvasGL sould not change binded texture silently.
> > > Restore texture to previous bound one after allocating new texture.
> > > And when destroy texture, reset texture to 0 if it is current bound
> > > texture.
> > >
> > > Test Plan: Tizen 3.0
> > >
> > > Reviewers: wonsik, dkdk, cedric, jpeg
> > >
> > > Reviewed By: jpeg
> > >
> > > Differential Revision: https://phab.enlightenment.org/D4524
> > > ---
> > >  src/modules/evas/engines/gl_common/evas_gl_core.c | 9 -
> > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/modules/evas/engines/gl_common/evas_gl_core.c
> > > b/src/modules/evas/engines/gl_common/evas_gl_core.c
> > > index 14d17f6..8292b5c 100644
> > > --- a/src/modules/evas/engines/gl_common/evas_gl_core.c
> > > +++ b/src/modules/evas/engines/gl_common/evas_gl_core.c
> > > @@ -234,19 +234,26 @@ _texture_allocate_2d(GLuint tex, GLint ifmt,
> GLenum
> > > fmt, GLenum type, int w, int
> > >  {
> > > //if (!(*tex))
> > > //   glGenTextures(1, tex);
> > > +   GLint curr_tex = 0;
> > > +   glGetIntegerv(GL_TEXTURE_BINDING_2D, _tex);
> > > +
> > > glBindTexture(GL_TEXTURE_2D, tex);
> > > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S,
> GL_CLAMP_TO_EDGE);
> > > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T,
> GL_CLAMP_TO_EDGE);
> > > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
> > > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
> > > glTexImage2D(GL_TEXTURE_2D, 0, ifmt, w, h, 0, fmt, type, NULL);
> > > -   glBindTexture(GL_TEXTURE_2D, 0);
> > > +   glBindTexture(GL_TEXTURE_2D, (GLuint)curr_tex);
> > >  }
> > >
> > >  // Destroy Texture
> > >  static void
> > >  _texture_destroy(GLuint *tex)
> > >  {
> > > +   GLint curr_tex = 0;
> > > +   glGetIntegerv(GL_TEXTURE_BINDING_2D, _tex);
> > > +
> > > +   if ((GLuint)curr_tex == *tex) glBindTexture(GL_TEXTURE_2D, 0);
> > > if (*tex)
> > >   {
> > >  glDeleteTextures(1, tex);
> > >
> > > --
> > >
> > >
> > >
> >
> 

Re: [E-devel] [EGIT] [core/efl] master 05/06: Evas GL:Bind texture to correct one.

[The Rasterman]

On Mon, 02 Jan 2017 15:50:28 + Mike Blumenkrantz
 said:

> Hi,
> 
> I am not sure that "Tizen 3.0" is a valid test plan for our purposes. If a
> fix is submitted upstream, I think it makes sense to also submit a
> corresponding test to verify that the issue is solved and does not recur in
> the future.

i'm not so sure that "test plan" is worth paying attention to...

BUT... "for every bug you fix with an @fix as it was an existing bug in a
release, provide a test case to check for the bug so it doesn't come back" is
what i think you are getting at.

at this stage that'd raise the cost of doing bug fixes a LOT. i think we need
to improve our testing harness first - e.g. a thread a few weeks back about
this. we need a far simpler way to put tests together. we also need good
"template" harnesses - e.g. a raw evas canvas test that sets up a canvas with
some default state (a white, pink, blue, green rect background or transparent)
then you can do things like:

code1();
code2();
CHECK(); 
code3();
code4();
code5();
CHECK(); 
code6();
...

which would force manual renders of the state at the time when you CHECK(); and
save an image and compare against a known good one (stored in tree) etc. etc.
as well as just drop that test into a dir without having to modify makefiles
and thus basically do a full rebuild (autogen etc.) just to begin to do testing.

before we start making rules like this - we should get our testing harness into
an optimal state.

> Thoughts?
> 
> On Mon, Jan 2, 2017 at 2:44 AM Minkyoung Kim  wrote:
> 
> > jpeg pushed a commit to branch master.
> >
> >
> > http://git.enlightenment.org/core/efl.git/commit/?id=40e9da0101e2c9afb6f823c7b21c02a011cc5300
> >
> > commit 40e9da0101e2c9afb6f823c7b21c02a011cc5300
> > Author: Minkyoung Kim 
> > Date:   Mon Jan 2 15:29:48 2017 +0900
> >
> > Evas GL:Bind texture to correct one.
> >
> > Summary:
> > If user bind textureA and want to use it continuously, do not call
> > glBindTexture(textureA) again.
> > But expect that textureA will be binding.
> > So EvasGL sould not change binded texture silently.
> > Restore texture to previous bound one after allocating new texture.
> > And when destroy texture, reset texture to 0 if it is current bound
> > texture.
> >
> > Test Plan: Tizen 3.0
> >
> > Reviewers: wonsik, dkdk, cedric, jpeg
> >
> > Reviewed By: jpeg
> >
> > Differential Revision: https://phab.enlightenment.org/D4524
> > ---
> >  src/modules/evas/engines/gl_common/evas_gl_core.c | 9 -
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/modules/evas/engines/gl_common/evas_gl_core.c
> > b/src/modules/evas/engines/gl_common/evas_gl_core.c
> > index 14d17f6..8292b5c 100644
> > --- a/src/modules/evas/engines/gl_common/evas_gl_core.c
> > +++ b/src/modules/evas/engines/gl_common/evas_gl_core.c
> > @@ -234,19 +234,26 @@ _texture_allocate_2d(GLuint tex, GLint ifmt, GLenum
> > fmt, GLenum type, int w, int
> >  {
> > //if (!(*tex))
> > //   glGenTextures(1, tex);
> > +   GLint curr_tex = 0;
> > +   glGetIntegerv(GL_TEXTURE_BINDING_2D, _tex);
> > +
> > glBindTexture(GL_TEXTURE_2D, tex);
> > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
> > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
> > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
> > glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
> > glTexImage2D(GL_TEXTURE_2D, 0, ifmt, w, h, 0, fmt, type, NULL);
> > -   glBindTexture(GL_TEXTURE_2D, 0);
> > +   glBindTexture(GL_TEXTURE_2D, (GLuint)curr_tex);
> >  }
> >
> >  // Destroy Texture
> >  static void
> >  _texture_destroy(GLuint *tex)
> >  {
> > +   GLint curr_tex = 0;
> > +   glGetIntegerv(GL_TEXTURE_BINDING_2D, _tex);
> > +
> > +   if ((GLuint)curr_tex == *tex) glBindTexture(GL_TEXTURE_2D, 0);
> > if (*tex)
> >   {
> >  glDeleteTextures(1, tex);
> >
> > --
> >
> >
> >
> --
> Check out the vibrant tech community on one of the world's most 
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> ___
> enlightenment-devel mailing list
> enlightenment-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
> 


-- 
- Codito, ergo sum - "I code, therefore I am" --
The Rasterman (Carsten Haitzler)ras...@rasterman.com


--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net

Re: [E-devel] [EGIT] [core/efl] master 05/06: Evas GL:Bind texture to correct one.

[Mike Blumenkrantz]

Hi,

I am not sure that "Tizen 3.0" is a valid test plan for our purposes. If a
fix is submitted upstream, I think it makes sense to also submit a
corresponding test to verify that the issue is solved and does not recur in
the future.

Thoughts?

On Mon, Jan 2, 2017 at 2:44 AM Minkyoung Kim  wrote:

> jpeg pushed a commit to branch master.
>
>
> http://git.enlightenment.org/core/efl.git/commit/?id=40e9da0101e2c9afb6f823c7b21c02a011cc5300
>
> commit 40e9da0101e2c9afb6f823c7b21c02a011cc5300
> Author: Minkyoung Kim 
> Date:   Mon Jan 2 15:29:48 2017 +0900
>
> Evas GL:Bind texture to correct one.
>
> Summary:
> If user bind textureA and want to use it continuously, do not call
> glBindTexture(textureA) again.
> But expect that textureA will be binding.
> So EvasGL sould not change binded texture silently.
> Restore texture to previous bound one after allocating new texture.
> And when destroy texture, reset texture to 0 if it is current bound
> texture.
>
> Test Plan: Tizen 3.0
>
> Reviewers: wonsik, dkdk, cedric, jpeg
>
> Reviewed By: jpeg
>
> Differential Revision: https://phab.enlightenment.org/D4524
> ---
>  src/modules/evas/engines/gl_common/evas_gl_core.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/src/modules/evas/engines/gl_common/evas_gl_core.c
> b/src/modules/evas/engines/gl_common/evas_gl_core.c
> index 14d17f6..8292b5c 100644
> --- a/src/modules/evas/engines/gl_common/evas_gl_core.c
> +++ b/src/modules/evas/engines/gl_common/evas_gl_core.c
> @@ -234,19 +234,26 @@ _texture_allocate_2d(GLuint tex, GLint ifmt, GLenum
> fmt, GLenum type, int w, int
>  {
> //if (!(*tex))
> //   glGenTextures(1, tex);
> +   GLint curr_tex = 0;
> +   glGetIntegerv(GL_TEXTURE_BINDING_2D, _tex);
> +
> glBindTexture(GL_TEXTURE_2D, tex);
> glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
> glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
> glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
> glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
> glTexImage2D(GL_TEXTURE_2D, 0, ifmt, w, h, 0, fmt, type, NULL);
> -   glBindTexture(GL_TEXTURE_2D, 0);
> +   glBindTexture(GL_TEXTURE_2D, (GLuint)curr_tex);
>  }
>
>  // Destroy Texture
>  static void
>  _texture_destroy(GLuint *tex)
>  {
> +   GLint curr_tex = 0;
> +   glGetIntegerv(GL_TEXTURE_BINDING_2D, _tex);
> +
> +   if ((GLuint)curr_tex == *tex) glBindTexture(GL_TEXTURE_2D, 0);
> if (*tex)
>   {
>  glDeleteTextures(1, tex);
>
> --
>
>
>
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

[EGIT] [core/efl] master 05/06: Evas GL:Bind texture to correct one.

[Minkyoung Kim]

jpeg pushed a commit to branch master.

http://git.enlightenment.org/core/efl.git/commit/?id=40e9da0101e2c9afb6f823c7b21c02a011cc5300

commit 40e9da0101e2c9afb6f823c7b21c02a011cc5300
Author: Minkyoung Kim 
Date:   Mon Jan 2 15:29:48 2017 +0900

Evas GL:Bind texture to correct one.

Summary:
If user bind textureA and want to use it continuously, do not call 
glBindTexture(textureA) again.
But expect that textureA will be binding.
So EvasGL sould not change binded texture silently.
Restore texture to previous bound one after allocating new texture.
And when destroy texture, reset texture to 0 if it is current bound texture.

Test Plan: Tizen 3.0

Reviewers: wonsik, dkdk, cedric, jpeg

Reviewed By: jpeg

Differential Revision: https://phab.enlightenment.org/D4524
---
 src/modules/evas/engines/gl_common/evas_gl_core.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/modules/evas/engines/gl_common/evas_gl_core.c 
b/src/modules/evas/engines/gl_common/evas_gl_core.c
index 14d17f6..8292b5c 100644
--- a/src/modules/evas/engines/gl_common/evas_gl_core.c
+++ b/src/modules/evas/engines/gl_common/evas_gl_core.c
@@ -234,19 +234,26 @@ _texture_allocate_2d(GLuint tex, GLint ifmt, GLenum fmt, 
GLenum type, int w, int
 {
//if (!(*tex))
//   glGenTextures(1, tex);
+   GLint curr_tex = 0;
+   glGetIntegerv(GL_TEXTURE_BINDING_2D, _tex);
+
glBindTexture(GL_TEXTURE_2D, tex);
glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
glTexImage2D(GL_TEXTURE_2D, 0, ifmt, w, h, 0, fmt, type, NULL);
-   glBindTexture(GL_TEXTURE_2D, 0);
+   glBindTexture(GL_TEXTURE_2D, (GLuint)curr_tex);
 }
 
 // Destroy Texture
 static void
 _texture_destroy(GLuint *tex)
 {
+   GLint curr_tex = 0;
+   glGetIntegerv(GL_TEXTURE_BINDING_2D, _tex);
+
+   if ((GLuint)curr_tex == *tex) glBindTexture(GL_TEXTURE_2D, 0);
if (*tex)
  {
 glDeleteTextures(1, tex);

--