Updating texture validation to account for sampler objects in ES3

gpu/command_buffer/service/gles2_cmd_decoder.cc

Index: gpu/command_buffer/service/gles2_cmd_decoder.cc
diff --git a/gpu/command_buffer/service/gles2_cmd_decoder.cc b/gpu/command_buffer/service/gles2_cmd_decoder.cc
index e8bbe5885459434f3dff96c195f981ae1b6ee30d..00f59bc33277fe346ddd64e8b2a5a1b8eb155cea 100644
--- a/gpu/command_buffer/service/gles2_cmd_decoder.cc
+++ b/gpu/command_buffer/service/gles2_cmd_decoder.cc
@@ -1910,6 +1910,8 @@ class GLES2DecoderImpl : public GLES2Decoder, public ErrorStateClient {
                                            GLint location,
                                            const std::string& name);
 
+  const SamplerState* GetSamplerStateForTextureUnit(GLenum target, GLuint unit);
+
   // Generate a member function prototype for each command in an automated and
   // typesafe way.
 #define GLES2_CMD_OP(name) \
@@ -7656,8 +7658,7 @@ void GLES2DecoderImpl::DoCopyTexImageIfNeeded(Texture* texture,
 
 bool GLES2DecoderImpl::PrepareTexturesForRender() {
   DCHECK(state_.current_program.get());
-  if (!texture_manager()->HaveUnrenderableTextures() &&

[piman, 2015/12/14 14:04:14]

This early exit is pretty important for performance, since this is called for
every single draw call - in the general case where the application correctly
defines its textures, this used to early exit. If not, we have to iterate over
every single texture unit, of which there can be a lot (e.g. NVIDIA).

How can we keep that property?

[bajones, 2015/12/14 17:40:38]

For ES3 we can't evaluate this ahead of time because it depends on both the
textures _and_ the samplers bound. We could try and update the value any time a
texture or sampler binding changes, but that would probably be nearly as much or
more work than simply checking it here at draw time. At least here we only have
to check the texture units used by the current shader program. (Not for every
unit supported by the hardware, as you feared.)

A previous iteration of this patch attempted to keep this check for ES2 contexts
and bypassed it for ES3, but that ended up tracking whether a texture was
created with an ES2 context or a ES3 context, which zmo@ pointed out was faulty
logic because said textures may be shared between contexts via mailbox. After
some discussion we felt that the additional validation overhead here was small
enough that it was worth the reduced code complexity. (We're also hoping to
simply defer this logic to ANGLE entirely in the near future.)

I'm happy to hear any alternative suggestions you have, though! I do believe
there's room for improvement here.

-      !texture_manager()->HaveImages()) {
+  if (!texture_manager()->HaveImages()) {
     return true;
   }
   bool textures_set = false;
@@ -7674,7 +7675,11 @@ bool GLES2DecoderImpl::PrepareTexturesForRender() {
         TextureRef* texture_ref =
             texture_unit.GetInfoForSamplerType(uniform_info->type).get();
         GLenum textarget = GetBindTargetForSamplerType(uniform_info->type);
-        if (!texture_ref || !texture_manager()->CanRender(texture_ref)) {
+        const SamplerState* sampler_state = GetSamplerStateForTextureUnit(
+            uniform_info->type, texture_unit_index);
+        if (!texture_ref ||
+            !texture_manager()->CanRenderWithSampler(
+                texture_ref, sampler_state)) {
           textures_set = true;
           glActiveTexture(GL_TEXTURE0 + texture_unit_index);
           glBindTexture(
@@ -7730,7 +7735,11 @@ void GLES2DecoderImpl::RestoreStateForTextures() {
         TextureUnit& texture_unit = state_.texture_units[texture_unit_index];
         TextureRef* texture_ref =
             texture_unit.GetInfoForSamplerType(uniform_info->type).get();
-        if (!texture_ref || !texture_manager()->CanRender(texture_ref)) {
+        const SamplerState* sampler_state = GetSamplerStateForTextureUnit(
+            uniform_info->type, texture_unit_index);
+        if (!texture_ref ||
+            !texture_manager()->CanRenderWithSampler(
+                texture_ref, sampler_state)) {
           glActiveTexture(GL_TEXTURE0 + texture_unit_index);
           // Get the texture_ref info that was previously bound here.
           texture_ref = texture_unit.bind_target == GL_TEXTURE_2D
@@ -15656,6 +15665,21 @@ void GLES2DecoderImpl::DoBindFragmentInputLocationCHROMIUM(
   program->SetFragmentInputLocationBinding(name, location);
 }
 
+const SamplerState* GLES2DecoderImpl::GetSamplerStateForTextureUnit(
+    GLenum target, GLuint unit) {
+  if (features().enable_samplers) {
+    Sampler* sampler = state_.sampler_units[unit].get();
+    if (sampler)
+      return sampler->sampler_state();
+  }
+  TextureUnit& texture_unit = state_.texture_units[unit];
+  TextureRef* texture_ref = texture_unit.GetInfoForSamplerType(target).get();
+  if (texture_ref)
+    return texture_ref->texture()->sampler_state();
+
+  return nullptr;
+}
+
 error::Error GLES2DecoderImpl::HandleBindFragmentInputLocationCHROMIUMBucket(
     uint32 immediate_data_size,
     const void* cmd_data) {