Add ColorBehavior to blink::Image draw methods

third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.cpp

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "modules/canvas2d/BaseRenderingContext2D.h"
 
 #include "bindings/core/v8/ExceptionMessages.h"
 #include "bindings/core/v8/ExceptionState.h"
 #include "bindings/core/v8/ExceptionStatePlaceholder.h"
 #include "core/css/cssom/CSSURLImageValue.h"
@@ skipping 1060 matching lines @@
     imagePaint.setImageFilter(nullptr);
   }
 
   if (!imageSmoothingEnabled() &&
       isDrawScalingDown(srcRect, dstRect, state().transform().xScaleSquared(),
                         state().transform().yScaleSquared()))
     imagePaint.setFilterQuality(kLow_SkFilterQuality);
 
   if (!imageSource->isVideoElement()) {
     imagePaint.setAntiAlias(shouldDrawImageAntialiased(dstRect));
+    // TODO(ccameron): Canvas should draw in sRGB by default.

[Justin Novosad, 2016/12/09 02:17:35]

Actually, we want to use whatever the canvas's colorspace is. You can assign
this to me.

+    // https://crbug.com/672299
     image->draw(c, imagePaint, dstRect, srcRect, DoNotRespectImageOrientation,
-                Image::DoNotClampImageToSourceRect);
+                Image::DoNotClampImageToSourceRect,
+                ColorBehavior::transformToGlobalTarget());

[Justin Novosad, 2016/12/09 02:17:35]

I am concerned about this approach, I understand that BitmapImage will end-up
caching the transformed copy of the source image to avoid re-tranforming it over
and over. Still, it means holding on to long-lived extra copy of the decoded
image in many cases.  Also, it means that the cached copy will thrash if the
image resource is requested in more that one colorspace at each animation frame.
Also, If the source image is another kind of Blink::Image, then what, do we want
to add caches in all subclasses? If we do that we may endup further multiplying
the redundant copies of images.

Transforming the images is not necessary.  The rasterization code only needs the
SkImages to be correctly tagged with colorspaces, and then skia will infer the
required transform and apply it on the fly at blit time. MY understanding is
that the required overhead for the on-the-fly transforms is not that high,
especially on the GPU.

Bottom line: There is an important compute vs. memory tradeoff here that needs
to be considered with caution.

[ccameron, 2016/12/09 03:30:56]

It is true, as you mention, that, if two different monitors are repeatedly
requesting the same image, we will thrash the decoder. This is a conscious
choice.
For other sub-classes, it may be feasible to do on-the-fly conversion, instead
of caching. Adding this interface doesn't force either behavior here.
This is not true. Skia will only transform images if you opt in to linear
blending, which is not permitted by the web. This is the reason that we're going
with this approach of pre-transforming all inputs, instead of using Skia's color
space support.

On-the-fly transform is efficient and precise with GPU rasterization (and
especially desirable for high-bit-depth rendering), but it's not clear if it is
efficient for CPU rasterization.
Either way that we make this trade-off, we will still need to use the interface
that I've added here. How we decide to do conversion (on the fly v not) will
only affect how each blink::Image sub-class behaves.

The default implementation that I've given (which thrashes the decoder in the
worst case) is the least-invasive initial behavior to add.

Also of note is that this is still all behind a flag (well, behind TODOs, which
will eventually become behind a flag), so we can perform measurements and
determine the best course action from that.

   } else {
     c->save();
     c->clipRect(dstRect);
     c->translate(dstRect.x(), dstRect.y());
     c->scale(dstRect.width() / srcRect.width(),
              dstRect.height() / srcRect.height());
     c->translate(-srcRect.x(), -srcRect.y());
     HTMLVideoElement* video = static_cast<HTMLVideoElement*>(imageSource);
     video->paintCurrentFrame(
         c,
@@ skipping 920 matching lines @@
       ExpensiveCanvasHeuristicParameters::ShadowFixedCost[index] *
           m_usageCounters.numBlurredShadows +
       ExpensiveCanvasHeuristicParameters::
               ShadowVariableCostPerAreaTimesShadowBlurSquared[index] *
           m_usageCounters.boundingBoxAreaTimesShadowBlurSquared;
 
   return basicCostOfDrawCalls + fillTypeAdjustment + shadowAdjustment;
 }
 
 }  // namespace blink