Use glUnpremultiplyAndDitherCopyCHROMIUM in GPU Raster

This change addresses banding issues by using the newly added
glUnpremultiplyAndDitherCopyCHROMIUM.

In cases where we previously rasterized into a RGBA4444 target, we
instead rasterize into a temporary RGBA8888 target, then do an
unpremultiply/dither copy into the RGBA4444 target.

This means that tiles may be either unpremultiplied or premultiplied, so
we need a way to pipe this information to the GLRenderer so it can
blend correctly. This change introduces a is_premultiplied property to
ContentDrawQuadBase to handle this.

Bug: 789153
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I8bb2af2a94934ca7fe79a180d65bff253163c98c
Reviewed-on: https://chromium-review.googlesource.com/899866
Reviewed-by: enne <enne@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Eric Karl <ericrk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538686}

cc/ipc/cc_param_traits_unittest.cc

@@ -405,10 +405,10 @@ TEST_F(CCParamTraitsTest, AllQuads) {
   pass_cmp->CopyFromAndAppendDrawQuad(texture_in);
 
   TileDrawQuad* tile_in = pass_in->CreateAndAppendDrawQuad<TileDrawQuad>();
-  tile_in->SetAll(shared_state3_in, arbitrary_rect2,
-                  arbitrary_rect1_inside_rect2, arbitrary_bool1,
-                  arbitrary_resourceid3, arbitrary_rectf1, arbitrary_size1,
-                  arbitrary_bool2, arbitrary_bool3, arbitrary_bool4);
+  tile_in->SetAll(
+      shared_state3_in, arbitrary_rect2, arbitrary_rect1_inside_rect2,
+      arbitrary_bool1, arbitrary_resourceid3, arbitrary_rectf1, arbitrary_size1,
+      arbitrary_bool2, arbitrary_bool3, arbitrary_bool4, arbitrary_bool5);
   pass_cmp->CopyFromAndAppendDrawQuad(tile_in);
 
   YUVVideoDrawQuad* yuvvideo_in =

cc/ipc/cc_serialization_perftest.cc

@@ -361,10 +361,11 @@ class CCSerializationPerfTest : public testing::Test {
           arbitrary_blend_mode2, arbitrary_context_id2);
       for (uint32_t j = 0; j < 6; ++j) {
         auto* tile_in = pass_in->CreateAndAppendDrawQuad<viz::TileDrawQuad>();
-        tile_in->SetAll(
-            shared_state2_in, arbitrary_rect2, arbitrary_rect1_inside_rect2,
-            arbitrary_bool1, arbitrary_resourceid3, arbitrary_rectf1,
-            arbitrary_size1, arbitrary_bool2, arbitrary_bool3, arbitrary_bool4);
+        tile_in->SetAll(shared_state2_in, arbitrary_rect2,
+                        arbitrary_rect1_inside_rect2, arbitrary_bool1,
+                        arbitrary_resourceid3, arbitrary_rectf1,
+                        arbitrary_size1, arbitrary_bool2, arbitrary_bool3,
+                        arbitrary_bool4, arbitrary_bool5);
       }
     }
 

cc/layers/picture_layer_impl.cc

@@ -470,7 +470,7 @@ void PictureLayerImpl::AppendQuads(viz::RenderPass* render_pass,
               offset_visible_geometry_rect, needs_blending,
               draw_info.resource_id_for_export(), texture_rect,
               draw_info.resource_size(), draw_info.contents_swizzled(),
-              nearest_neighbor_,
+              draw_info.is_premultiplied(), nearest_neighbor_,
               !layer_tree_impl()->settings().enable_edge_anti_aliasing);
           ValidateQuadResources(quad);
           has_draw_quad = true;

cc/layers/render_surface_unittest.cc

@@ -68,7 +68,7 @@ class FakePictureLayerImplForRenderSurfaceTest : public FakePictureLayerImpl {
     for (const auto& rect : quad_rects_) {
       auto* quad = render_pass->CreateAndAppendDrawQuad<viz::TileDrawQuad>();
       quad->SetNew(shared_quad_state, rect, rect, needs_blending, 0,
-                   gfx::RectF(rect), bounds(), false, false, false);
+                   gfx::RectF(rect), bounds(), false, false, false, false);
     }
   }
 

cc/raster/bitmap_raster_buffer_provider.cc

@@ -118,6 +118,11 @@ bool BitmapRasterBufferProvider::IsResourceSwizzleRequired(
   return ResourceFormatRequiresSwizzle(viz::RGBA_8888);
 }
 
+bool BitmapRasterBufferProvider::IsResourcePremultiplied(
+    bool must_support_alpha) const {
+  return true;
+}
+
 bool BitmapRasterBufferProvider::CanPartialRasterIntoProvidedResource() const {
   return true;
 }

cc/raster/bitmap_raster_buffer_provider.h

@@ -40,6 +40,7 @@ class CC_EXPORT BitmapRasterBufferProvider : public RasterBufferProvider {
   void Flush() override;
   viz::ResourceFormat GetResourceFormat(bool must_support_alpha) const override;
   bool IsResourceSwizzleRequired(bool must_support_alpha) const override;
+  bool IsResourcePremultiplied(bool must_support_alpha) const override;
   bool CanPartialRasterIntoProvidedResource() const override;
   bool IsResourceReadyToDraw(
       const ResourcePool::InUsePoolResource& resource) const override;

cc/raster/gpu_raster_buffer_provider.cc

@@ -28,6 +28,7 @@
 #include "gpu/command_buffer/client/gles2_interface.h"
 #include "gpu/command_buffer/client/raster_interface.h"
 #include "gpu/command_buffer/common/gpu_memory_buffer_support.h"
+#include "skia/ext/texture_handle.h"
 #include "third_party/skia/include/core/SkMultiPictureDraw.h"
 #include "third_party/skia/include/core/SkPictureRecorder.h"
 #include "third_party/skia/include/core/SkSurface.h"
@@ -38,6 +39,54 @@
 namespace cc {
 namespace {
 
+class ScopedSkSurfaceForUnpremultiplyAndDither {
+ public:
+  ScopedSkSurfaceForUnpremultiplyAndDither(
+      viz::RasterContextProvider* context_provider,
+      const gfx::Rect& playback_rect,
+      const gfx::Rect& raster_full_rect,
+      GLuint texture_id,
+      const gfx::Size& size,
+      bool use_distance_field_text,
+      bool can_use_lcd_text,
+      int msaa_sample_count)
+      : context_provider_(context_provider),
+        texture_id_(texture_id),
+        offset_(playback_rect.OffsetFromOrigin() -
+                raster_full_rect.OffsetFromOrigin()),
+        size_(playback_rect.size()) {
+    // Allocate a 32-bit surface for raster. We will copy from that into our
+    // actual surface in destruction.
+    SkImageInfo n32Info =
+        SkImageInfo::MakeN32Premul(size.width(), size.height());
+    SkSurfaceProps surface_props =
+        LayerTreeResourceProvider::ScopedSkSurface::ComputeSurfaceProps(
+            use_distance_field_text, can_use_lcd_text);
+    surface_ = SkSurface::MakeRenderTarget(
+        context_provider->GrContext(), SkBudgeted::kYes, n32Info,
+        msaa_sample_count, kTopLeft_GrSurfaceOrigin, &surface_props);
+  }
+
+  ~ScopedSkSurfaceForUnpremultiplyAndDither() {
+    GrBackendObject handle =
+        surface_->getTextureHandle(SkSurface::kFlushRead_BackendHandleAccess);
+    const GrGLTextureInfo* info =
+        skia::GrBackendObjectToGrGLTextureInfo(handle);
+    context_provider_->RasterInterface()->UnpremultiplyAndDitherCopyCHROMIUM(
+        info->fID, texture_id_, offset_.x(), offset_.y(), size_.width(),
+        size_.height());
+  }
+
+  SkSurface* surface() { return surface_.get(); }
+
+ private:
+  viz::RasterContextProvider* context_provider_;
+  GLuint texture_id_;
+  gfx::Vector2d offset_;
+  gfx::Size size_;
+  sk_sp<SkSurface> surface_;
+};
+
 static void RasterizeSourceOOP(
     const RasterSource* raster_source,
     bool resource_has_previous_content,
@@ -85,6 +134,9 @@ static void RasterizeSourceOOP(
                      raster_source->requires_clear());
   ri->EndRasterCHROMIUM();
 
+  // TODO(ericrk): Handle unpremultiply+dither for 4444 cases.
+  // https://crbug.com/789153
+
   ri->DeleteTextures(1, &texture_id);
 }
 
@@ -125,7 +177,8 @@ static void RasterizeSource(
     const RasterSource::PlaybackSettings& playback_settings,
     viz::RasterContextProvider* context_provider,
     bool use_distance_field_text,
-    int msaa_sample_count) {
+    int msaa_sample_count,
+    bool unpremultiply_and_dither) {
   ScopedGrContextAccess gr_context_access(context_provider);
 
   gpu::raster::RasterInterface* ri = context_provider->RasterInterface();
@@ -139,12 +192,23 @@ static void RasterizeSource(
   }
 
   {
-    LayerTreeResourceProvider::ScopedSkSurface scoped_surface(
-        context_provider->GrContext(), texture_id, texture_target,
-        resource_size, resource_format, use_distance_field_text,
-        playback_settings.use_lcd_text, msaa_sample_count);
-
-    SkSurface* surface = scoped_surface.surface();
+    base::Optional<LayerTreeResourceProvider::ScopedSkSurface> scoped_surface;
+    base::Optional<ScopedSkSurfaceForUnpremultiplyAndDither>
+        scoped_dither_surface;
+    SkSurface* surface;
+    if (!unpremultiply_and_dither) {
+      scoped_surface.emplace(context_provider->GrContext(), texture_id,
+                             texture_target, resource_size, resource_format,
+                             use_distance_field_text,
+                             playback_settings.use_lcd_text, msaa_sample_count);
+      surface = scoped_surface->surface();
+    } else {
+      scoped_dither_surface.emplace(
+          context_provider, playback_rect, raster_full_rect, texture_id,
+          resource_size, use_distance_field_text,
+          playback_settings.use_lcd_text, msaa_sample_count);
+      surface = scoped_dither_surface->surface();
+    }
 
     // Allocating an SkSurface will fail after a lost context.  Pretend we
     // rasterized, as the contents of the resource don't matter anymore.
@@ -168,6 +232,15 @@ static void RasterizeSource(
   ri->DeleteTextures(1, &texture_id);
 }
 
+bool ShouldUnpremultiplyAndDitherResource(viz::ResourceFormat format) {
+  switch (format) {
+    case viz::RGBA_4444:
+      return true;
+    default:
+      return false;
+  }
+}
+
 }  // namespace
 
 // Subclass for InUsePoolResource that holds ownership of a gpu-rastered backing
@@ -357,6 +430,12 @@ bool GpuRasterBufferProvider::IsResourceSwizzleRequired(
   return false;
 }
 
+bool GpuRasterBufferProvider::IsResourcePremultiplied(
+    bool must_support_alpha) const {
+  return !ShouldUnpremultiplyAndDitherResource(
+      GetResourceFormat(must_support_alpha));
+}
+
 bool GpuRasterBufferProvider::CanPartialRasterIntoProvidedResource() const {
   // Partial raster doesn't support MSAA, as the MSAA resolve is unaware of clip
   // rects.
@@ -467,7 +546,8 @@ gpu::SyncToken GpuRasterBufferProvider::PlaybackOnWorkerThread(
                     texture_storage_allocated, resource_size, resource_format,
                     color_space, raster_full_rect, playback_rect, transform,
                     playback_settings, worker_context_provider_,
-                    use_distance_field_text_, msaa_sample_count_);
+                    use_distance_field_text_, msaa_sample_count_,
+                    ShouldUnpremultiplyAndDitherResource(resource_format));
   }
 
   // Generate sync token for cross context synchronization.

cc/raster/gpu_raster_buffer_provider.h

@@ -40,6 +40,7 @@ class CC_EXPORT GpuRasterBufferProvider : public RasterBufferProvider {
   void Flush() override;
   viz::ResourceFormat GetResourceFormat(bool must_support_alpha) const override;
   bool IsResourceSwizzleRequired(bool must_support_alpha) const override;
+  bool IsResourcePremultiplied(bool must_support_alpha) const override;
   bool CanPartialRasterIntoProvidedResource() const override;
   bool IsResourceReadyToDraw(
       const ResourcePool::InUsePoolResource& resource) const override;

cc/raster/one_copy_raster_buffer_provider.cc

@@ -246,6 +246,13 @@ bool OneCopyRasterBufferProvider::IsResourceSwizzleRequired(
   return ResourceFormatRequiresSwizzle(GetResourceFormat(must_support_alpha));
 }
 
+bool OneCopyRasterBufferProvider::IsResourcePremultiplied(
+    bool must_support_alpha) const {
+  // TODO(ericrk): Handle unpremultiply/dither in one-copy case as well.
+  // https://crbug.com/789153
+  return true;
+}
+
 bool OneCopyRasterBufferProvider::CanPartialRasterIntoProvidedResource() const {
   // While OneCopyRasterBufferProvider has an internal partial raster
   // implementation, it cannot directly partial raster into the externally

cc/raster/one_copy_raster_buffer_provider.h

@@ -45,6 +45,7 @@ class CC_EXPORT OneCopyRasterBufferProvider : public RasterBufferProvider {
   void Flush() override;
   viz::ResourceFormat GetResourceFormat(bool must_support_alpha) const override;
   bool IsResourceSwizzleRequired(bool must_support_alpha) const override;
+  bool IsResourcePremultiplied(bool must_support_alpha) const override;
   bool CanPartialRasterIntoProvidedResource() const override;
   bool IsResourceReadyToDraw(
       const ResourcePool::InUsePoolResource& resource) const override;

cc/raster/raster_buffer_provider.h

@@ -69,6 +69,9 @@ class CC_EXPORT RasterBufferProvider {
   // Determine if the resource requires swizzling.
   virtual bool IsResourceSwizzleRequired(bool must_support_alpha) const = 0;
 
+  // Determines if the resource is premultiplied.
+  virtual bool IsResourcePremultiplied(bool must_support_alpha) const = 0;
+
   // Determine if the RasterBufferProvider can handle partial raster into
   // the Resource provided in AcquireBufferForRaster.
   virtual bool CanPartialRasterIntoProvidedResource() const = 0;

cc/raster/zero_copy_raster_buffer_provider.cc

@@ -254,6 +254,11 @@ bool ZeroCopyRasterBufferProvider::IsResourceSwizzleRequired(
   return ResourceFormatRequiresSwizzle(GetResourceFormat(must_support_alpha));
 }
 
+bool ZeroCopyRasterBufferProvider::IsResourcePremultiplied(
+    bool must_support_alpha) const {
+  return true;
+}
+
 bool ZeroCopyRasterBufferProvider::CanPartialRasterIntoProvidedResource()
     const {
   return false;

cc/raster/zero_copy_raster_buffer_provider.h

@@ -43,6 +43,7 @@ class CC_EXPORT ZeroCopyRasterBufferProvider : public RasterBufferProvider {
   void Flush() override;
   viz::ResourceFormat GetResourceFormat(bool must_support_alpha) const override;
   bool IsResourceSwizzleRequired(bool must_support_alpha) const override;
+  bool IsResourcePremultiplied(bool must_support_alpha) const override;
   bool CanPartialRasterIntoProvidedResource() const override;
   bool IsResourceReadyToDraw(
       const ResourcePool::InUsePoolResource& resource) const override;

cc/resources/layer_tree_resource_provider.cc

@@ -996,6 +996,21 @@ LayerTreeResourceProvider::ScopedSkSurface::ScopedSkSurface(
   texture_info.fFormat = TextureStorageFormat(format);
   GrBackendTexture backend_texture(size.width(), size.height(),
                                    GrMipMapped::kNo, texture_info);
+  SkSurfaceProps surface_props =
+      ComputeSurfaceProps(use_distance_field_text, can_use_lcd_text);
+  surface_ = SkSurface::MakeFromBackendTextureAsRenderTarget(
+      gr_context, backend_texture, kTopLeft_GrSurfaceOrigin, msaa_sample_count,
+      ResourceFormatToClosestSkColorType(format), nullptr, &surface_props);
+}
+
+LayerTreeResourceProvider::ScopedSkSurface::~ScopedSkSurface() {
+  if (surface_)
+    surface_->prepareForExternalIO();
+}
+
+SkSurfaceProps LayerTreeResourceProvider::ScopedSkSurface::ComputeSurfaceProps(
+    bool use_distance_field_text,
+    bool can_use_lcd_text) {
   uint32_t flags =
       use_distance_field_text ? SkSurfaceProps::kUseDistanceFieldFonts_Flag : 0;
   // Use unknown pixel geometry to disable LCD text.
@@ -1005,14 +1020,7 @@ LayerTreeResourceProvider::ScopedSkSurface::ScopedSkSurface(
     surface_props =
         SkSurfaceProps(flags, SkSurfaceProps::kLegacyFontHost_InitType);
   }
-  surface_ = SkSurface::MakeFromBackendTextureAsRenderTarget(
-      gr_context, backend_texture, kTopLeft_GrSurfaceOrigin, msaa_sample_count,
-      ResourceFormatToClosestSkColorType(format), nullptr, &surface_props);
-}
-
-LayerTreeResourceProvider::ScopedSkSurface::~ScopedSkSurface() {
-  if (surface_)
-    surface_->prepareForExternalIO();
+  return surface_props;
 }
 
 void LayerTreeResourceProvider::ValidateResource(viz::ResourceId id) const {

cc/resources/layer_tree_resource_provider.h

@@ -292,6 +292,9 @@ class CC_EXPORT LayerTreeResourceProvider : public ResourceProvider {
 
     SkSurface* surface() const { return surface_.get(); }
 
+    static SkSurfaceProps ComputeSurfaceProps(bool use_distance_field_text,
+                                              bool can_use_lcd_text);
+
    private:
     sk_sp<SkSurface> surface_;
 

cc/test/fake_raster_buffer_provider.cc

@@ -46,6 +46,11 @@ bool FakeRasterBufferProviderImpl::IsResourceSwizzleRequired(
   return ResourceFormatRequiresSwizzle(GetResourceFormat(must_support_alpha));
 }
 
+bool FakeRasterBufferProviderImpl::IsResourcePremultiplied(
+    bool must_support_alpha) const {
+  return true;
+}
+
 bool FakeRasterBufferProviderImpl::CanPartialRasterIntoProvidedResource()
     const {
   return true;

cc/test/fake_raster_buffer_provider.h

@@ -24,6 +24,7 @@ class FakeRasterBufferProviderImpl : public RasterBufferProvider {
   void Flush() override;
   viz::ResourceFormat GetResourceFormat(bool must_support_alpha) const override;
   bool IsResourceSwizzleRequired(bool must_support_alpha) const override;
+  bool IsResourcePremultiplied(bool must_support_alpha) const override;
   bool CanPartialRasterIntoProvidedResource() const override;
   bool IsResourceReadyToDraw(
       const ResourcePool::InUsePoolResource& resource) const override;