viz: Simplify ScopedRenderPassTexture interactions.

Pass in the ContextProvider instead of each capability individually. And in the
DirectRenderer classes, the FrameBuffer hint is always used so stop making it
conditional (this is a leftover from ResourceProvider code this was based on)
and just pass mipmap as a bool so that new bitflags can't be added without
child classes knowing about it. Last, pass RenderPassRequirements consistently
to both methods that receive them, instead of unpacking it for one of them, so
that the relationship is more clear in the APIs.

R=piman@chromium.org

Bug: 738190
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: Ie53c2041183207561efae7d10c0c32e42daf1bd2
Reviewed-on: https://chromium-review.googlesource.com/843162
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526059}

components/viz/service/display/direct_renderer.cc

@@ -194,7 +194,7 @@ void DirectRenderer::DecideRenderPassAllocationsForFrame(
       }
     }
     render_passes_in_frame[pass->id] = {RenderPassTextureSize(pass.get()),
-                                        RenderPassTextureHint(pass.get())};
+                                        pass->generate_mipmap};
   }
   UpdateRenderPassTextures(render_passes_in_draw_order, render_passes_in_frame);
 }
@@ -625,8 +625,8 @@ void DirectRenderer::UseRenderPass(const RenderPass* render_pass) {
   enlarged_size.Enlarge(enlarge_pass_texture_amount_.width(),
                         enlarge_pass_texture_amount_.height());
 
-  AllocateRenderPassResourceIfNeeded(render_pass->id, enlarged_size,
-                                     RenderPassTextureHint(render_pass));
+  AllocateRenderPassResourceIfNeeded(
+      render_pass->id, {enlarged_size, render_pass->generate_mipmap});
 
   // TODO(crbug.com/582554): This change applies only when Vulkan is enabled and
   // it will be removed once SkiaRenderer has complete support for Vulkan.
@@ -662,17 +662,6 @@ gfx::Size DirectRenderer::RenderPassTextureSize(const RenderPass* render_pass) {
   return render_pass->output_rect.size();
 }
 
-// static
-ResourceTextureHint DirectRenderer::RenderPassTextureHint(
-    const RenderPass* render_pass) {
-  // TODO(danakj): Pass these as 2 bools instead so subclasses don't have to
-  // worry about new hints being silently added to the field here.
-  ResourceTextureHint hint = ResourceTextureHint::kFramebuffer;
-  if (render_pass->generate_mipmap)
-    hint |= ResourceTextureHint::kMipmap;
-  return hint;
-}
-
 void DirectRenderer::SetCurrentFrameForTesting(const DrawingFrame& frame) {
   current_frame_valid_ = true;
   current_frame_ = frame;

components/viz/service/display/direct_renderer.h

@@ -107,7 +107,7 @@ class VIZ_SERVICE_EXPORT DirectRenderer {
 
   struct RenderPassRequirements {
     gfx::Size size;
-    ResourceTextureHint hint;
+    bool mipmap = false;
   };
 
   static gfx::RectF QuadVertexRect();
@@ -132,8 +132,6 @@ class VIZ_SERVICE_EXPORT DirectRenderer {
   void SetScissorTestRectInDrawSpace(const gfx::Rect& draw_space_rect);
 
   static gfx::Size RenderPassTextureSize(const RenderPass* render_pass);
-  static ResourceTextureHint RenderPassTextureHint(
-      const RenderPass* render_pass);
 
   void FlushPolygons(
       base::circular_deque<std::unique_ptr<DrawPolygon>>* poly_list,
@@ -165,8 +163,7 @@ class VIZ_SERVICE_EXPORT DirectRenderer {
           render_passes_in_frame) = 0;
   virtual void AllocateRenderPassResourceIfNeeded(
       const RenderPassId& render_pass_id,
-      const gfx::Size& enlarged_size,
-      ResourceTextureHint texturehint) = 0;
+      const RenderPassRequirements& requirements) = 0;
   virtual bool IsRenderPassResourceAllocated(
       const RenderPassId& render_pass_id) const = 0;
   virtual gfx::Size GetRenderPassTextureSize(

components/viz/service/display/gl_renderer.cc

@@ -3609,13 +3609,13 @@ void GLRenderer::UpdateRenderPassTextures(
       passes_to_delete.push_back(pair.first);
       continue;
     }
-    gfx::Size required_size = render_pass_it->second.size;
-    ResourceTextureHint required_hint = render_pass_it->second.hint;
+    const RenderPassRequirements& requirements = render_pass_it->second;
     const ScopedRenderPassTexture& texture = pair.second;
-    bool size_appropriate = texture.size().width() >= required_size.width() &&
-                            texture.size().height() >= required_size.height();
-    bool hint_appropriate = (texture.hint() & required_hint) == required_hint;
-    if (!size_appropriate || !hint_appropriate)
+    bool size_appropriate =
+        texture.size().width() >= requirements.size.width() &&
+        texture.size().height() >= requirements.size.height();
+    bool mipmap_appropriate = !requirements.mipmap || texture.mipmap();
+    if (!size_appropriate || !mipmap_appropriate)
       passes_to_delete.push_back(pair.first);
   }
   // Delete RenderPass textures from the previous frame that will not be used
@@ -3639,18 +3639,15 @@ ResourceFormat GLRenderer::BackbufferFormat() const {
 
 void GLRenderer::AllocateRenderPassResourceIfNeeded(
     const RenderPassId& render_pass_id,
-    const gfx::Size& enlarged_size,
-    ResourceTextureHint texturehint) {
-  const auto& caps = output_surface_->context_provider()->ContextCapabilities();
+    const RenderPassRequirements& requirements) {
   auto contents_texture_it = render_pass_textures_.find(render_pass_id);
   if (contents_texture_it != render_pass_textures_.end())
     return;
 
   ScopedRenderPassTexture contents_texture(
-      output_surface_->context_provider()->ContextGL(), enlarged_size,
-      texturehint, BackbufferFormat(),
-      current_frame()->current_render_pass->color_space, caps.texture_usage,
-      caps.texture_storage, caps.texture_npot);
+      output_surface_->context_provider(), requirements.size,
+      BackbufferFormat(), current_frame()->current_render_pass->color_space,
+      requirements.mipmap);
   render_pass_textures_[render_pass_id] = std::move(contents_texture);
 }
 

components/viz/service/display/gl_renderer.h

@@ -102,8 +102,7 @@ class VIZ_SERVICE_EXPORT GLRenderer : public DirectRenderer {
           render_passes_in_frame) override;
   void AllocateRenderPassResourceIfNeeded(
       const RenderPassId& render_pass_id,
-      const gfx::Size& enlarged_size,
-      ResourceTextureHint texturehint) override;
+      const RenderPassRequirements& requirements) override;
   bool IsRenderPassResourceAllocated(
       const RenderPassId& render_pass_id) const override;
   gfx::Size GetRenderPassTextureSize(

components/viz/service/display/scoped_render_pass_texture.cc

@@ -6,56 +6,65 @@
 
 #include "base/bits.h"
 #include "base/logging.h"
+#include "components/viz/common/gpu/context_provider.h"
 #include "components/viz/common/resources/resource_format_utils.h"
 #include "gpu/GLES2/gl2extchromium.h"
+#include "gpu/command_buffer/client/gles2_interface.h"
 
 namespace viz {
 
+ScopedRenderPassTexture::ScopedRenderPassTexture() = default;
+
 ScopedRenderPassTexture::ScopedRenderPassTexture(
-    gpu::gles2::GLES2Interface* gl,
+    ContextProvider* context_provider,
     const gfx::Size& size,
-    ResourceTextureHint hint,
     ResourceFormat format,
     const gfx::ColorSpace& color_space,
-    bool use_texture_usage_hint,
-    bool use_texture_storage,
-    bool use_texture_npot)
-    : gl_(gl), size_(size), hint_(hint), color_space_(color_space) {
-  DCHECK(gl_);
-  gl_->GenTextures(1, &gl_id_);
-
-  // Create and set texture properties.
-  gl_->BindTexture(GL_TEXTURE_2D, gl_id_);
-  gl_->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
-  gl_->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
-  gl_->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
-  gl_->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
-  if (use_texture_usage_hint && (hint_ & ResourceTextureHint::kFramebuffer)) {
-    gl_->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_USAGE_ANGLE,
-                       GL_FRAMEBUFFER_ATTACHMENT_ANGLE);
+    bool mipmap)
+    : context_provider_(context_provider),
+      size_(size),
+      mipmap_(mipmap),
+      color_space_(color_space) {
+  DCHECK(context_provider_);
+  gpu::gles2::GLES2Interface* gl = context_provider_->ContextGL();
+  const gpu::Capabilities& caps = context_provider_->ContextCapabilities();
+  gl->GenTextures(1, &gl_id_);
+
+  gl->BindTexture(GL_TEXTURE_2D, gl_id_);
+  gl->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
+  gl->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
+  gl->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
+  gl->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
+
+  // This texture will be bound as a framebuffer, so optimize for that.
+  if (caps.texture_usage) {
+    gl->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_USAGE_ANGLE,
+                      GL_FRAMEBUFFER_ATTACHMENT_ANGLE);
   }
 
-  if (use_texture_storage) {
+  if (caps.texture_storage) {
     GLint levels = 1;
-    if (use_texture_npot && (hint_ & ResourceTextureHint::kMipmap))
+    if (caps.texture_npot && mipmap_)
       levels += base::bits::Log2Floor(std::max(size_.width(), size_.height()));
 
-    gl_->TexStorage2DEXT(GL_TEXTURE_2D, levels, TextureStorageFormat(format),
-                         size_.width(), size_.height());
+    gl->TexStorage2DEXT(GL_TEXTURE_2D, levels, TextureStorageFormat(format),
+                        size_.width(), size_.height());
   } else {
-    gl_->TexImage2D(GL_TEXTURE_2D, 0, GLInternalFormat(format), size_.width(),
-                    size_.height(), 0, GLDataFormat(format), GLDataType(format),
-                    nullptr);
+    gl->TexImage2D(GL_TEXTURE_2D, 0, GLInternalFormat(format), size_.width(),
+                   size_.height(), 0, GLDataFormat(format), GLDataType(format),
+                   nullptr);
   }
 }
 
-ScopedRenderPassTexture::ScopedRenderPassTexture() = default;
+ScopedRenderPassTexture::~ScopedRenderPassTexture() {
+  Free();
+}
 
 ScopedRenderPassTexture::ScopedRenderPassTexture(
     ScopedRenderPassTexture&& other) {
-  gl_ = other.gl_;
+  context_provider_ = other.context_provider_;
   size_ = other.size_;
-  hint_ = other.hint_;
+  mipmap_ = other.mipmap_;
   color_space_ = other.color_space_;
   gl_id_ = other.gl_id_;
 
@@ -63,17 +72,13 @@ ScopedRenderPassTexture::ScopedRenderPassTexture(
   other.gl_id_ = 0;
 }
 
-ScopedRenderPassTexture::~ScopedRenderPassTexture() {
-  Free();
-}
-
 ScopedRenderPassTexture& ScopedRenderPassTexture::operator=(
     ScopedRenderPassTexture&& other) {
   if (this != &other) {
     Free();
-    gl_ = other.gl_;
+    context_provider_ = other.context_provider_;
     size_ = other.size_;
-    hint_ = other.hint_;
+    mipmap_ = other.mipmap_;
     color_space_ = other.color_space_;
     gl_id_ = other.gl_id_;
 
@@ -86,7 +91,8 @@ ScopedRenderPassTexture& ScopedRenderPassTexture::operator=(
 void ScopedRenderPassTexture::Free() {
   if (!gl_id_)
     return;
-  gl_->DeleteTextures(1, &gl_id_);
+  gpu::gles2::GLES2Interface* gl = context_provider_->ContextGL();
+  gl->DeleteTextures(1, &gl_id_);
   gl_id_ = 0;
 }
 

components/viz/service/display/scoped_render_pass_texture.h

@@ -9,48 +9,45 @@
 #include "components/viz/common/resources/resource_format.h"
 #include "components/viz/common/resources/resource_texture_hint.h"
 #include "components/viz/service/viz_service_export.h"
-#include "gpu/command_buffer/client/gles2_interface.h"
 #include "third_party/khronos/GLES2/gl2.h"
 #include "ui/gfx/color_space.h"
 #include "ui/gfx/geometry/size.h"
 
 namespace viz {
+class ContextProvider;
 
 // ScopedRenderPassTexture is resource used inside the same GL context and will
 // not being sent into another process. So no need to create fence and mailbox
 // for these resources.
 class VIZ_SERVICE_EXPORT ScopedRenderPassTexture {
  public:
-  explicit ScopedRenderPassTexture(gpu::gles2::GLES2Interface* gl,
-                                   const gfx::Size& size,
-                                   ResourceTextureHint hint,
-                                   ResourceFormat format,
-                                   const gfx::ColorSpace& color_space,
-                                   bool use_texture_usage_hint,
-                                   bool use_texture_storage,
-                                   bool use_texture_npot);
-
   ScopedRenderPassTexture();
+  ScopedRenderPassTexture(ContextProvider* context_provider,
+                          const gfx::Size& size,
+                          ResourceFormat format,
+                          const gfx::ColorSpace& color_space,
+                          bool mipmap);
+  ~ScopedRenderPassTexture();
+
   ScopedRenderPassTexture(ScopedRenderPassTexture&& other);
   ScopedRenderPassTexture& operator=(ScopedRenderPassTexture&& other);
-  ~ScopedRenderPassTexture();
 
   GLuint id() const { return gl_id_; }
   const gfx::Size& size() const { return size_; }
-  ResourceTextureHint hint() const { return hint_; }
+  bool mipmap() const { return mipmap_; }
   const gfx::ColorSpace& color_space() const { return color_space_; }
 
  private:
   void Free();
 
-  gpu::gles2::GLES2Interface* gl_;
+  ContextProvider* context_provider_ = nullptr;
   // The GL texture id.
   GLuint gl_id_ = 0;
   // Size of the resource in pixels.
   gfx::Size size_;
-  // A hint for texture-backed resources about how the resource will be used,
-  // that dictates how it should be allocated/used.
-  ResourceTextureHint hint_;
+  // When true, and immutable textures are used, this specifies to
+  // generate mipmaps at powers of 2.
+  bool mipmap_ = false;
   // TODO(xing.xu): Remove this and set the color space when we draw the
   // RenderPassDrawQuad.
   gfx::ColorSpace color_space_;

components/viz/service/display/skia_renderer.cc

@@ -767,15 +767,12 @@ void SkiaRenderer::UpdateRenderPassTextures(
       continue;
     }
 
-    gfx::Size required_size = render_pass_it->second.size;
-    ResourceTextureHint required_hint = render_pass_it->second.hint;
-
+    const RenderPassRequirements& requirements = render_pass_it->second;
     const RenderPassBacking& backing = pair.second;
-    bool size_appropriate = backing.size.width() >= required_size.width() &&
-                            backing.size.height() >= required_size.height();
-    bool hint_appropriate =
-        (backing.usage_hint & required_hint) == required_hint;
-    if (!size_appropriate || !hint_appropriate)
+    bool size_appropriate = backing.size.width() >= requirements.size.width() &&
+                            backing.size.height() >= requirements.size.height();
+    bool mipmap_appropriate = !requirements.mipmap || backing.mipmap;
+    if (!size_appropriate || !mipmap_appropriate)
       passes_to_delete.push_back(pair.first);
   }
 
@@ -794,8 +791,7 @@ void SkiaRenderer::UpdateRenderPassTextures(
 
 void SkiaRenderer::AllocateRenderPassResourceIfNeeded(
     const RenderPassId& render_pass_id,
-    const gfx::Size& enlarged_size,
-    ResourceTextureHint texture_hint) {
+    const RenderPassRequirements& requirements) {
 #if BUILDFLAG(ENABLE_VULKAN)
   NOTIMPLEMENTED();
   return;
@@ -816,11 +812,11 @@ void SkiaRenderer::AllocateRenderPassResourceIfNeeded(
   gl->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
   gl->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
   gl->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
+
+  // This texture will be bound as a framebuffer, so optimize for that.
   if (caps.texture_usage) {
-    if (texture_hint & ResourceTextureHint::kFramebuffer) {
-      gl->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_USAGE_ANGLE,
-                        GL_FRAMEBUFFER_ATTACHMENT_ANGLE);
-    }
+    gl->TexParameteri(GL_TEXTURE_2D, GL_TEXTURE_USAGE_ANGLE,
+                      GL_FRAMEBUFFER_ATTACHMENT_ANGLE);
   }
 
   ResourceFormat backbuffer_format;
@@ -843,26 +839,24 @@ void SkiaRenderer::AllocateRenderPassResourceIfNeeded(
     // If |texture_npot| is availble, and mipmaps are desired, we generate a
     // mipmap for each power of 2 size. This is only done when using
     // TexStorage2DEXT.
-    if (caps.texture_npot) {
-      if (texture_hint & ResourceTextureHint::kMipmap) {
-        levels += base::bits::Log2Floor(
-            std::max(enlarged_size.width(), enlarged_size.height()));
-      }
+    if (caps.texture_npot && requirements.mipmap) {
+      levels += base::bits::Log2Floor(
+          std::max(requirements.size.width(), requirements.size.height()));
     }
     gl->TexStorage2DEXT(GL_TEXTURE_2D, levels,
                         TextureStorageFormat(backbuffer_format),
-                        enlarged_size.width(), enlarged_size.height());
+                        requirements.size.width(), requirements.size.height());
   } else {
     gl->TexImage2D(GL_TEXTURE_2D, 0, GLInternalFormat(backbuffer_format),
-                   enlarged_size.width(), enlarged_size.height(), 0,
+                   requirements.size.width(), requirements.size.height(), 0,
                    GLDataFormat(backbuffer_format),
                    GLDataType(backbuffer_format), nullptr);
   }
 
   RenderPassBacking& backing = render_pass_backings_[render_pass_id];
   backing.gl_id = texture_id;
-  backing.size = enlarged_size;
-  backing.usage_hint = texture_hint;
+  backing.size = requirements.size;
+  backing.mipmap = requirements.mipmap;
   backing.format = backbuffer_format;
   backing.color_space = current_frame()->current_render_pass->color_space;
   gl->BindTexture(GL_TEXTURE_2D, 0);

components/viz/service/display/skia_renderer.h

@@ -48,8 +48,7 @@ class VIZ_SERVICE_EXPORT SkiaRenderer : public DirectRenderer {
           render_passes_in_frame) override;
   void AllocateRenderPassResourceIfNeeded(
       const RenderPassId& render_pass_id,
-      const gfx::Size& enlarged_size,
-      ResourceTextureHint texture_hint) override;
+      const RenderPassRequirements& requirements) override;
   bool IsRenderPassResourceAllocated(
       const RenderPassId& render_pass_id) const override;
   gfx::Size GetRenderPassTextureSize(
@@ -104,7 +103,7 @@ class VIZ_SERVICE_EXPORT SkiaRenderer : public DirectRenderer {
   struct RenderPassBacking {
     uint32_t gl_id;
     gfx::Size size;
-    ResourceTextureHint usage_hint;
+    bool mipmap;
     ResourceFormat format;
     gfx::ColorSpace color_space;
   };

components/viz/service/display/software_renderer.cc

@@ -813,21 +813,22 @@ void SoftwareRenderer::UpdateRenderPassTextures(
 
 void SoftwareRenderer::AllocateRenderPassResourceIfNeeded(
     const RenderPassId& render_pass_id,
-    const gfx::Size& enlarged_size,
-    ResourceTextureHint texture_hint) {
+    const RenderPassRequirements& requirements) {
   auto it = render_pass_bitmaps_.find(render_pass_id);
   if (it != render_pass_bitmaps_.end())
     return;
 
-  // The |texture_hint| is only used for gpu-based rendering, so not used here.
+  // The |requirements.mipmap| is only used for gpu-based rendering, so not used
+  // here.
   //
   // ColorSpace correctness for software compositing is a performance nightmare,
   // so we don't do it. If we did, then the color space of the current frame's
   // |current_render_pass| should be stored somewhere, but we should not set it
   // on the bitmap itself. Instead, we'd use it with a SkColorSpaceXformCanvas
   // that wraps the SkCanvas drawing into the bitmap.
-  SkImageInfo info = SkImageInfo::MakeN32(
-      enlarged_size.width(), enlarged_size.height(), kPremul_SkAlphaType);
+  SkImageInfo info =
+      SkImageInfo::MakeN32(requirements.size.width(),
+                           requirements.size.height(), kPremul_SkAlphaType);
   SkBitmap bitmap;
   bitmap.allocPixels(info);
   render_pass_bitmaps_.emplace(render_pass_id, std::move(bitmap));

components/viz/service/display/software_renderer.h

@@ -46,8 +46,7 @@ class VIZ_SERVICE_EXPORT SoftwareRenderer : public DirectRenderer {
           render_passes_in_frame) override;
   void AllocateRenderPassResourceIfNeeded(
       const RenderPassId& render_pass_id,
-      const gfx::Size& enlarged_size,
-      ResourceTextureHint texturehint) override;
+      const RenderPassRequirements& requirements) override;
   bool IsRenderPassResourceAllocated(
       const RenderPassId& render_pass_id) const override;
   gfx::Size GetRenderPassTextureSize(