Add missing integer formats

Add missing integer formats

BUG=612542
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/1592241702983c6e5c4778e6aefd2bd81a755b30
Cr-Commit-Position: refs/heads/master@{#428589}

[qiankun, 2016-10-25 03:20:43]

Description was changed from

==========
Add missing integer formats

BUG=612542
==========

to

==========
Add missing integer formats

BUG=612542
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[qiankun, 2016-10-25 03:23:42]

qiankun.miao@intel.com changed reviewers:
+ kbr@chromium.org, yunchao.he@intel.com, zmo@chromium.org

[qiankun, 2016-10-25 03:23:42]

Please take a look at this.
I found the issue when implementing CopyTextureCHROMIUM.

[Ken Russell (switch to Gerrit), 2016-10-25 05:33:56]

lgtm

[Zhenyao Mo, 2016-10-25 05:38:54]

On 2016/10/25 05:33:56, Ken Russell wrote:
> lgtm

I think I purposely left them out because they are not color renderable. So
there will be no reason to include them here.  Unless you find a use case to
include them, I suggest not to include them.

[Ken Russell (switch to Gerrit), 2016-10-25 05:43:15]

On 2016/10/25 05:38:54, Zhenyao Mo wrote:
> On 2016/10/25 05:33:56, Ken Russell wrote:
> > lgtm
> 
> I think I purposely left them out because they are not color renderable. So
> there will be no reason to include them here.  Unless you find a use case to
> include them, I suggest not to include them.

I see -- sorry if I misspoke on my review. Looking at texImage2D taking
TexImageSource in http://localhost:8000/specs/latest/2.0/#3.7.6 , it looks like
RGB8UI is in the table of supported formats, so even though it isn't
color-renderable, it is legal to upload a video element (for example) to it.
Perhaps that's what motivated this addition?

[Zhenyao Mo, 2016-10-25 05:48:02]

On 2016/10/25 05:43:15, Ken Russell wrote:
> On 2016/10/25 05:38:54, Zhenyao Mo wrote:
> > On 2016/10/25 05:33:56, Ken Russell wrote:
> > > lgtm
> > 
> > I think I purposely left them out because they are not color renderable. So
> > there will be no reason to include them here.  Unless you find a use case to
> > include them, I suggest not to include them.
> 
> I see -- sorry if I misspoke on my review. Looking at texImage2D taking
> TexImageSource in http://localhost:8000/specs/latest/2.0/#3.7.6 , it looks
like
> RGB8UI is in the table of supported formats, so even though it isn't
> color-renderable, it is legal to upload a video element (for example) to it.
> Perhaps that's what motivated this addition?

If there is a legitimate reason for these functions to include these formats, I
am not against that.

I just don't want them to be included without a reason.

[Zhenyao Mo, 2016-10-25 05:50:25]

On 2016/10/25 05:48:02, Zhenyao Mo wrote:
> On 2016/10/25 05:43:15, Ken Russell wrote:
> > On 2016/10/25 05:38:54, Zhenyao Mo wrote:
> > > On 2016/10/25 05:33:56, Ken Russell wrote:
> > > > lgtm
> > > 
> > > I think I purposely left them out because they are not color renderable.
So
> > > there will be no reason to include them here.  Unless you find a use case
to
> > > include them, I suggest not to include them.
> > 
> > I see -- sorry if I misspoke on my review. Looking at texImage2D taking
> > TexImageSource in http://localhost:8000/specs/latest/2.0/#3.7.6 , it looks
> like
> > RGB8UI is in the table of supported formats, so even though it isn't
> > color-renderable, it is legal to upload a video element (for example) to it.
> > Perhaps that's what motivated this addition?
> 
> If there is a legitimate reason for these functions to include these formats,
I
> am not against that.
> 
> I just don't want them to be included without a reason.

lgtm if this change is needed to move forward with the CopyTextureCHROMIUM
upgrade.

[qiankun, 2016-10-25 06:05:11]

On 2016/10/25 05:48:02, Zhenyao Mo wrote:
> On 2016/10/25 05:43:15, Ken Russell wrote:
> > On 2016/10/25 05:38:54, Zhenyao Mo wrote:
> > > On 2016/10/25 05:33:56, Ken Russell wrote:
> > > > lgtm
> > > 
> > > I think I purposely left them out because they are not color renderable.
So
> > > there will be no reason to include them here.  Unless you find a use case
to
> > > include them, I suggest not to include them.
> > 
> > I see -- sorry if I misspoke on my review. Looking at texImage2D taking
> > TexImageSource in http://localhost:8000/specs/latest/2.0/#3.7.6 , it looks
> like
> > RGB8UI is in the table of supported formats, so even though it isn't
> > color-renderable, it is legal to upload a video element (for example) to it.
> > Perhaps that's what motivated this addition?
> 
> If there is a legitimate reason for these functions to include these formats,
I
> am not against that.
> 
> I just don't want them to be included without a reason.

I saw only RGB8UI is the supported formats. Should I only add this format?
I also want you to clarify which formats should be supported by
CopyTextureCHROMIUM, both renderable and unrenderable formats?
glCopyTextureCHROMIUM is an extension of glCopyTexImage2D. glCopyTexImage2D only
support renderable formats in ES3. What do you think?

[Zhenyao Mo, 2016-10-25 06:10:56]

On 2016/10/25 06:05:11, qiankun wrote:
> On 2016/10/25 05:48:02, Zhenyao Mo wrote:
> > On 2016/10/25 05:43:15, Ken Russell wrote:
> > > On 2016/10/25 05:38:54, Zhenyao Mo wrote:
> > > > On 2016/10/25 05:33:56, Ken Russell wrote:
> > > > > lgtm
> > > > 
> > > > I think I purposely left them out because they are not color renderable.
> So
> > > > there will be no reason to include them here.  Unless you find a use
case
> to
> > > > include them, I suggest not to include them.
> > > 
> > > I see -- sorry if I misspoke on my review. Looking at texImage2D taking
> > > TexImageSource in http://localhost:8000/specs/latest/2.0/#3.7.6 , it looks
> > like
> > > RGB8UI is in the table of supported formats, so even though it isn't
> > > color-renderable, it is legal to upload a video element (for example) to
it.
> > > Perhaps that's what motivated this addition?
> > 
> > If there is a legitimate reason for these functions to include these
formats,
> I
> > am not against that.
> > 
> > I just don't want them to be included without a reason.
> 
> I saw only RGB8UI is the supported formats. Should I only add this format?
> I also want you to clarify which formats should be supported by
> CopyTextureCHROMIUM, both renderable and unrenderable formats?
> glCopyTextureCHROMIUM is an extension of glCopyTexImage2D. glCopyTexImage2D
only
> support renderable formats in ES3. What do you think?

Yes, that extension needs to support both renderable and unrenderable formats.
kbr describes paths for both cases in the crbug.

As for adding formats to these two functions, if you are adding some
non-color-renderable formats, it might be better to just include all - less
confusion.

[qiankun, 2016-10-25 06:34:54]

On 2016/10/25 06:10:56, Zhenyao Mo wrote:
> On 2016/10/25 06:05:11, qiankun wrote:
> > On 2016/10/25 05:48:02, Zhenyao Mo wrote:
> > > On 2016/10/25 05:43:15, Ken Russell wrote:
> > > > On 2016/10/25 05:38:54, Zhenyao Mo wrote:
> > > > > On 2016/10/25 05:33:56, Ken Russell wrote:
> > > > > > lgtm
> > > > > 
> > > > > I think I purposely left them out because they are not color
renderable.
> > So
> > > > > there will be no reason to include them here.  Unless you find a use
> case
> > to
> > > > > include them, I suggest not to include them.
> > > > 
> > > > I see -- sorry if I misspoke on my review. Looking at texImage2D taking
> > > > TexImageSource in http://localhost:8000/specs/latest/2.0/#3.7.6 , it
looks
> > > like
> > > > RGB8UI is in the table of supported formats, so even though it isn't
> > > > color-renderable, it is legal to upload a video element (for example) to
> it.
> > > > Perhaps that's what motivated this addition?
> > > 
> > > If there is a legitimate reason for these functions to include these
> formats,
> > I
> > > am not against that.
> > > 
> > > I just don't want them to be included without a reason.
> > 
> > I saw only RGB8UI is the supported formats. Should I only add this format?
> > I also want you to clarify which formats should be supported by
> > CopyTextureCHROMIUM, both renderable and unrenderable formats?
> > glCopyTextureCHROMIUM is an extension of glCopyTexImage2D. glCopyTexImage2D
> only
> > support renderable formats in ES3. What do you think?
> 
> Yes, that extension needs to support both renderable and unrenderable formats.
> kbr describes paths for both cases in the crbug.
> 
> As for adding formats to these two functions, if you are adding some
> non-color-renderable formats, it might be better to just include all - less
> confusion.

Thanks! I see. So any valid formats combination should be supported by
glCopyTextureCHROMIUM which is different than glCopyTexImage2D. glCopyTexImage2D
has many limitations.

[qiankun, 2016-10-29 01:05:57]

GLES2Util::IsSignedIntegerFormat and GLES2Util::IsUnsignedIntegerFormat are used
by CopyTexImage2D and ClearBuffer*.

This change fixes missing format for CopyTexImage2D and doesn't affect the
behavior of ClearBuffer*.

Landing now.

[qiankun, 2016-10-29 01:06:23]

The CQ bit was checked by qiankun.miao@intel.com

[commit-bot: I haz the power, 2016-10-29 01:06:42]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-29 02:03:18]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-10-29 02:12:45]

Description was changed from

==========
Add missing integer formats

BUG=612542
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Add missing integer formats

BUG=612542
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/1592241702983c6e5c4778e6aefd2bd81a755b30
Cr-Commit-Position: refs/heads/master@{#428589}
==========

[commit-bot: I haz the power, 2016-10-29 02:12:46]

Patchset 1 (id:??) landed as
https://crrev.com/1592241702983c6e5c4778e6aefd2bd81a755b30
Cr-Commit-Position: refs/heads/master@{#428589}